refactor(sdk): drop the vestigial v2 suffix from the finalized-transaction surface - #4325
Merged
Conversation
…ction surface With the v1 split build/sign path removed (#4323), the v2 markers on the finalized-transaction surface no longer disambiguate anything. Rename, with no behavior change: - C ABI: core_wallet_{broadcast,abandon}_signed_transaction_v2 -> core_wallet_{broadcast,abandon}_signed_transaction; core_wallet_signed_transaction_v2_{free,fee,bytes} -> core_wallet_signed_transaction_{free,fee,bytes} - Rust internals: FFICoreSignedTransactionV2 -> FFICoreSignedTransaction, CORE_SIGNED_TRANSACTION_V2_STORAGE -> CORE_SIGNED_TRANSACTION_STORAGE, publish_v2_handle -> publish_finalized_handle, test names and doc prose - JNI + Kotlin externals: coreWalletBroadcastSignedTransactionV2 -> coreWalletBroadcastSignedTransaction, coreSignedTransactionV2* -> coreSignedTransaction* - parity manifest entries and QA/parity doc references The public Kotlin/Swift API (FinalizedCoreTransaction, finalizeAtomic, broadcastTransactionWithOutcome, abandonTransaction) was already version-free and is untouched. The deferred core_wallet_signed_payment_* family is distinct and unchanged. Prebuilt-host artifacts (xcframework, Android JNI .so) pick up the renamed symbols on their next routine rebuild; both are generated out-of-tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
August 6, 2026 20:22
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR renames V2 signed-transaction storage and APIs to non-versioned names across Rust FFI, JNI, Kotlin, and Swift. Parity manifests, documentation, tests, and lifecycle assertions use the renamed symbols. ChangesSigned transaction API rename
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
|
🔍 Review in progress — actively reviewing now (commit 8b81b84) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/kotlin-sdk/PARITY.md`:
- Line 161: Update the parity record entry for SendTransaction to replace
“atomic V2 path” with “atomic finalized-transaction path,” while preserving the
referenced API chain and all other wording.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a2f124ff-dc67-47f6-bd87-30d141b5fefb
📒 Files selected for processing (18)
docs/sdk/sdk-parity-manifest.jsonpackages/kotlin-sdk/KotlinExampleApp/TEST_PLAN.mdpackages/kotlin-sdk/PARITY.mdpackages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/wallet/CoreTxBuilderOpReturnBindingTest.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/errors/DashSdkError.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/ffi/WalletManagerNative.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/CoreTransactionBuilder.ktpackages/kotlin-sdk/sdk/src/main/kotlin/org/dashfoundation/dashsdk/wallet/ManagedCoreWallet.ktpackages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rspackages/rs-platform-wallet-ffi/src/core_wallet/signed_payment.rspackages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rspackages/rs-platform-wallet-ffi/src/handle.rspackages/rs-platform-wallet-ffi/src/manager.rspackages/rs-unified-sdk-jni/src/wallet_manager.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/CoreTransactionBuilder.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletResult.swiftpackages/swift-sdk/SwiftExampleApp/TEST_PLAN.md
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
#4323 removed the v1 split build/sign path, so the
v2markers on the finalized-transaction surface no longer disambiguate anything — they were versioning against a counterpart that no longer exists, and they would otherwise fossilize into the released ABI.What was done?
Pure rename, no behavior change:
core_wallet_{broadcast,abandon}_signed_transaction_v2→core_wallet_{broadcast,abandon}_signed_transaction;core_wallet_signed_transaction_v2_{free,fee,bytes}→core_wallet_signed_transaction_{free,fee,bytes}FFICoreSignedTransactionV2→FFICoreSignedTransaction,CORE_SIGNED_TRANSACTION_V2_STORAGE→CORE_SIGNED_TRANSACTION_STORAGE,publish_v2_handle→publish_finalized_handle, plus test names and doc prosecoreWalletBroadcastSignedTransactionV2→coreWalletBroadcastSignedTransaction,coreSignedTransactionV2*→coreSignedTransaction*The public Kotlin/Swift API (
FinalizedCoreTransaction,finalizeAtomic,broadcastTransactionWithOutcome,abandonTransaction) was already version-free and is untouched, so app consumers (including dashwallet-ios post-#920) are unaffected. The deferredcore_wallet_signed_payment_*family is a distinct name family and unchanged. Unrelated v2s (Room migrations, KeychainSigner TODOs, rs-dpp/rs-drive version modules) are deliberately untouched. Prebuilt host artifacts (xcframework, Android JNI.so) are generated out-of-tree and pick up the renamed exports on their next routine rebuild.How Has This Been Tested?
cargo fmt/cargo clippy --all-targets(no new warnings) /cargo test -p platform-wallet-ffi(235 + 26 + 6 pass):sdk:compileDebugKotlin,:sdk:compileDebugAndroidTestKotlin,:sdk:testDebugUnitTestgreenswift build+SignedCoreTransactionTests(5 tests) greenBreaking Changes
None (pre-release symbol rename; no behavior change).
Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit