feat(sdk): expose OP_RETURN, output-order and change-to-VIN0 controls - #4286
feat(sdk): expose OP_RETURN, output-order and change-to-VIN0 controls#4286romchornyi wants to merge 3 commits into
Conversation
…trols MAYAChain requires a UTXO deposit shaped as VOUT0=vault, VOUT1=OP_RETURN memo, VOUT2=change paid back to the VIN0 address, with no output reordering, and it identifies the depositor by VIN0 for refunds. https://docs.mayaprotocol.com/mayachain-dev-docs/concepts/sending-transactions `CoreTransactionBuilder.buildSigned` builds and signs in one FFI call, so none of this can be applied after the fact — it has to be expressed on the builder. FFI (rs-platform-wallet-ffi): - core_wallet_tx_builder_add_op_return / _preserve_output_order / _change_to_first_input, mirroring the existing setter style - an over-long payload is rejected before take_builder() runs, so a refused memo cannot leave the slot holding a mem::take default and silently drop outputs the caller already configured - core_wallet_signed_transaction_v2_bytes: read the finalized transaction bytes without broadcasting, so the deposit shape can be asserted pre-broadcast Swift SDK: - addOpReturn / preserveOutputOrder / changeToFirstInput - FinalizedCoreTransaction.serializedData() Tests: MayaDepositVerificationIntegrationTests builds short- and long-memo deposits and asserts output count/order, the OP_RETURN payload, VOUT2 == VIN0 scriptPubKey, the memo ceiling, the dust floor and a >= 1 duff/byte fee, then checks fee parity for ordinary, multi-recipient, selected-input, drain and asset-lock shapes so the precise output sizing does not move existing fees. CI: fail the workspace workflow if the local rust-dashcore [patch] override is still present in Cargo.toml. Depends on key-wallet gaining add_op_return / preserve_output_order / change_to_first_input (dashpay/rust-dashcore, branch feat/tx-builder-op-return). Until that lands and the rev in Cargo.toml is bumped, building this needs a local [patch] override, which is deliberately NOT committed.
📝 WalkthroughWalkthroughThe PR adds finalized V2 transaction serialization, OP_RETURN and output-routing controls across the Rust FFI and Swift SDK, and opt-in Maya deposit and fee verification tests. The macOS workflow rejects local ChangesCore wallet transaction flow
Workspace validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MayaDepositVerificationIntegrationTests
participant CoreTransactionBuilder
participant CoreWalletFFI
participant SPVWallet
MayaDepositVerificationIntegrationTests->>SPVWallet: fund wallet and select UTXOs
MayaDepositVerificationIntegrationTests->>CoreTransactionBuilder: build deposit transaction
CoreTransactionBuilder->>CoreWalletFFI: add OP_RETURN and configure outputs
CoreWalletFFI-->>CoreTransactionBuilder: finalize transaction
CoreTransactionBuilder-->>MayaDepositVerificationIntegrationTests: return transaction
MayaDepositVerificationIntegrationTests->>SPVWallet: decode transaction and calculate fee
SPVWallet-->>MayaDepositVerificationIntegrationTests: transaction data and fee
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
ℹ️ Review skipped (commit c3ffb89) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4286 +/- ##
============================================
+ Coverage 87.54% 87.60% +0.06%
============================================
Files 2679 2679
Lines 341312 343263 +1951
============================================
+ Hits 298799 300718 +1919
- Misses 42513 42545 +32
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs (1)
311-321: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRestore the builder when
add_op_returnfails.
take_builder()replaces the storedTransactionBuilderwith themem::takedefault. Ifb.add_op_return(bytes)returnsErr,bis dropped and the slot keeps that default, so later builder operations or finalization are no longer based on previously configured inputs, outputs, and options.TransactionBuilderdoes not deriveClone, so the error path needs to avoid requiringb.clone()unless this dependency is changed to support it.🤖 Prompt for 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. In `@packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs` around lines 311 - 321, Update the add_op_return error path in the transaction-building method to restore the original TransactionBuilder into the shared builder slot before returning the error. Preserve b without cloning, store it through the existing store_builder mechanism on failure, and keep the current error result unchanged.
🤖 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/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift`:
- Around line 147-158: Guard all transaction collection accesses in the
verification flow before indexing: replace direct access to decoded.outputs[1],
decoded.outputs[0], and decoded.inputs[0] with safe first-element handling via
XCTUnwrap or equivalent count assertions. Ensure malformed transaction shapes
produce readable XCTest failures before evaluating opReturnPayload or
findMatchedUTXO, while preserving the existing outputTwoMatchesInputZeroScript
logic.
- Around line 55-57: Prevent testPrompt04StaticProofAndLegacyFeeParity from
hanging local Swift SDK CI by skipping it or splitting it so the long SPV
bootstrap and waitForSpendable flow is not run by the enabled run_tests.sh suite
until bootstrap stalls are resolved; do not add a local stopSpv call because
IntegrationTestCase.tearDown and suite cleanup already handle SPV teardown.
---
Nitpick comments:
In `@packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs`:
- Around line 311-321: Update the add_op_return error path in the
transaction-building method to restore the original TransactionBuilder into the
shared builder slot before returning the error. Preserve b without cloning,
store it through the existing store_builder mechanism on failure, and keep the
current error result unchanged.
🪄 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: b079eaf5-5ec2-4fda-b22a-a9b48a57753f
📒 Files selected for processing (5)
.github/workflows/tests-rs-workspace.ymlpackages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rspackages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rspackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/CoreTransactionBuilder.swiftpackages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The FFI allocation and Swift ownership plumbing follow existing repository conventions, but the exact PR head is not buildable because the pinned key-wallet revision lacks every newly referenced builder API and the imported constant has a different name upstream. The Maya integration test also does not require the expected VOUT2 change output and does not exercise the 80/81-byte OP_RETURN boundary or builder-state preservation after rejection.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking | 🟡 2 suggestion(s)
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `Cargo.toml`:
- [BLOCKING] Cargo.toml:55-62: Pin a key-wallet revision that provides the new builder API
The workspace still pins rust-dashcore and key-wallet to `70d4bf8e36057c58e02d56769a6e9760f701dd06`, which does not provide `TransactionBuilder::add_op_return`, `preserve_output_order`, or `change_to_first_input`, nor the imported OP_RETURN limit constant. At this exact head, `cargo check -p platform-wallet-ffi --locked` fails with E0432/E0599, so the native library and the new Swift API cannot be built. Upstream rust-dashcore PR #922 currently provides the methods at `eebacae3d1152609b04e9c9af05acc87ea9b32ad`, but exports the limit as `DEFAULT_MAX_OP_RETURN_BYTES`, not `MAX_STANDARD_OP_RETURN_BYTES`. After the upstream change is merged, pin all rust-dashcore workspace dependencies to a compatible revision, update `Cargo.lock`, and align the import with the exported name.
In `packages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift`:
- [SUGGESTION] packages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift:181-191: Require VOUT2 for fixtures that necessarily produce change
Both deposit fixtures leave millions of duffs after the vault payment and fee, so each transaction must contain exactly three outputs: vault, memo, and change. Allowing `outputCount == 2` and conditionally skipping the VIN0-script assertion lets a regression that suppresses change pass, even though change-to-VIN0 is the load-bearing behavior this test claims to verify. Require exactly three outputs and always validate VOUT2 against VIN0.
- [SUGGESTION] packages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift:13-17: Exercise the OP_RETURN 80/81-byte boundary
The long memo fixture is 79 UTF-8 bytes, and the later `memoBytes <= 80` assertion only verifies the fixture rather than the new API's boundary behavior. No test proves that an 80-byte payload succeeds, an 81-byte payload is rejected, or that a rejection preserves outputs and options already stored in the FFI builder. Add deterministic boundary coverage that rejects 81 bytes and then successfully finalizes the same builder with its earlier configuration intact, which directly tests the pre-`take_builder()` guarantee introduced by this PR.
| XCTAssertGreaterThanOrEqual(observation.outputCount, 2, "\(observation.name) output count below Maya minimum") | ||
| XCTAssertLessThanOrEqual(observation.outputCount, 3, "\(observation.name) output count above Maya maximum") | ||
| XCTAssertEqual(observation.actualVaultAmount, observation.depositAmount, "\(observation.name) VOUT0 amount mismatch") | ||
| XCTAssertTrue(observation.outputOneIsOpReturn, "\(observation.name) VOUT1 is not OP_RETURN") | ||
| XCTAssertEqual(observation.decodedMemo, observation.memo, "\(observation.name) memo payload mismatch") | ||
| XCTAssertGreaterThanOrEqual(observation.depositAmount, Constants.minimumDepositDuffs, "\(observation.name) deposit fell below Maya dust floor") | ||
| XCTAssertLessThanOrEqual(observation.memoBytes, Constants.maxMemoBytes, "\(observation.name) memo exceeded 80 bytes") | ||
| XCTAssertGreaterThanOrEqual(observation.feeDuffs, UInt64(observation.serializedSize), "\(observation.name) fee fell below 1 duff/byte") | ||
| if observation.outputCount == 3 { | ||
| XCTAssertTrue(observation.outputTwoMatchesInputZeroScript, "\(observation.name) VOUT2 does not return change to VIN0 scriptPubKey") | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: Require VOUT2 for fixtures that necessarily produce change
Both deposit fixtures leave millions of duffs after the vault payment and fee, so each transaction must contain exactly three outputs: vault, memo, and change. Allowing outputCount == 2 and conditionally skipping the VIN0-script assertion lets a regression that suppresses change pass, even though change-to-VIN0 is the load-bearing behavior this test claims to verify. Require exactly three outputs and always validate VOUT2 against VIN0.
| XCTAssertGreaterThanOrEqual(observation.outputCount, 2, "\(observation.name) output count below Maya minimum") | |
| XCTAssertLessThanOrEqual(observation.outputCount, 3, "\(observation.name) output count above Maya maximum") | |
| XCTAssertEqual(observation.actualVaultAmount, observation.depositAmount, "\(observation.name) VOUT0 amount mismatch") | |
| XCTAssertTrue(observation.outputOneIsOpReturn, "\(observation.name) VOUT1 is not OP_RETURN") | |
| XCTAssertEqual(observation.decodedMemo, observation.memo, "\(observation.name) memo payload mismatch") | |
| XCTAssertGreaterThanOrEqual(observation.depositAmount, Constants.minimumDepositDuffs, "\(observation.name) deposit fell below Maya dust floor") | |
| XCTAssertLessThanOrEqual(observation.memoBytes, Constants.maxMemoBytes, "\(observation.name) memo exceeded 80 bytes") | |
| XCTAssertGreaterThanOrEqual(observation.feeDuffs, UInt64(observation.serializedSize), "\(observation.name) fee fell below 1 duff/byte") | |
| if observation.outputCount == 3 { | |
| XCTAssertTrue(observation.outputTwoMatchesInputZeroScript, "\(observation.name) VOUT2 does not return change to VIN0 scriptPubKey") | |
| } | |
| XCTAssertEqual( | |
| observation.outputCount, | |
| 3, | |
| "\(observation.name) must contain vault, memo, and change outputs" | |
| ) | |
| XCTAssertEqual(observation.actualVaultAmount, observation.depositAmount, "\(observation.name) VOUT0 amount mismatch") | |
| XCTAssertTrue(observation.outputOneIsOpReturn, "\(observation.name) VOUT1 is not OP_RETURN") | |
| XCTAssertEqual(observation.decodedMemo, observation.memo, "\(observation.name) memo payload mismatch") | |
| XCTAssertGreaterThanOrEqual(observation.depositAmount, Constants.minimumDepositDuffs, "\(observation.name) deposit fell below Maya dust floor") | |
| XCTAssertLessThanOrEqual(observation.memoBytes, Constants.maxMemoBytes, "\(observation.name) memo exceeded 80 bytes") | |
| XCTAssertGreaterThanOrEqual(observation.feeDuffs, UInt64(observation.serializedSize), "\(observation.name) fee fell below 1 duff/byte") | |
| XCTAssertTrue(observation.outputTwoMatchesInputZeroScript, "\(observation.name) VOUT2 does not return change to VIN0 scriptPubKey") |
source: ['codex']
There was a problem hiding this comment.
Agreed, and this was the one worth catching. Fixed in 8370b6a2.
You are right that the fixtures make change mandatory: 0.5 DASH funding against a 200_000-duff deposit, and 0.4 DASH against 35_000_000, both leave a change output far above the 546-duff dust threshold. outputCount == 2 was unreachable for them, so the range check bought nothing and cost the assertion — a regression that dropped change entirely would have satisfied >= 2 && <= 3 and then skipped the VIN0 check under if observation.outputCount == 3, which is precisely the behaviour this test exists to prove.
Now:
XCTAssertEqual(observation.outputCount, 3, "\(observation.name) must be vault + memo + change")
...
XCTAssertTrue(observation.outputTwoMatchesInputZeroScript, "\(observation.name) VOUT2 does not return change to VIN0 scriptPubKey")with the VOUT2 assertion no longer conditional. I also moved the shape check ahead of the subscripting in buildDepositObservation, so a wrong shape fails readably instead of trapping on decoded.outputs[2].
One deliberate asymmetry worth recording, since it looks like the same check in two places: the app-side guard in dashpay/dashwallet-ios#916 (assertSwapDepositShape) still accepts 2 or 3 outputs. There the transaction is real, so change genuinely can fall below dust and be dropped by the builder — MAYAChain still identifies the depositor by VIN0 in that case, so a two-output deposit is valid. It is only here, where the fixture amounts rule that out, that three is the correct fixed expectation.
| static let maxMemoBytes = 80 | ||
| static let longMemo = | ||
| "=:ARB.GLD:0x51a1449b3B6D635EddeC781cD47a99221712De97:344233230e4/1/0:_/def:15/0" | ||
| static let shortMemo = | ||
| "=:r:thor166n4w5039meulfa3p6ydg60ve6ueac7tlt0jws:669458827/1/0:_/def:15/0" |
There was a problem hiding this comment.
🟡 Suggestion: Exercise the OP_RETURN 80/81-byte boundary
The long memo fixture is 79 UTF-8 bytes, and the later memoBytes <= 80 assertion only verifies the fixture rather than the new API's boundary behavior. No test proves that an 80-byte payload succeeds, an 81-byte payload is rejected, or that a rejection preserves outputs and options already stored in the FFI builder. Add deterministic boundary coverage that rejects 81 bytes and then successfully finalizes the same builder with its earlier configuration intact, which directly tests the pre-take_builder() guarantee introduced by this PR.
source: ['codex']
There was a problem hiding this comment.
Good catch on both halves — the fixture proved nothing about the boundary, and nothing at all covered the rejection path. Added testOpReturnCeilingBoundaryAndRejectionPreservesBuilder in 8370b6a2.
It does exactly the two things you asked for:
let builder = try CoreTransactionBuilder(network: .regtest)
try builder.addOutput(address: vaultAddress, amountDuffs: depositAmount)
try builder.preserveOutputOrder()
try builder.changeToFirstInput()
XCTAssertThrowsError(try builder.addOpReturn(overCeiling)) // 81 bytes
// Same builder instance: if the rejection had consumed it, this would build a
// transaction missing the vault output and the ordering flags.
try builder.addOpReturn(atCeiling) // exactly 80 bytes
let tx = try builder.finalizeAtomic(...)then asserts three outputs, that VOUT0 is still the vault payment for the original amount, and that the 80-byte payload round-trips verbatim out of VOUT1.
The reuse-after-rejection part is the one I care about most, because it pins a guarantee that is otherwise invisible: core_wallet_tx_builder_add_op_return takes the builder by value on the Rust side, so the naive implementation drops it on error and take_builder's mem::take leaves a default in the slot — the vault output and both ordering flags would vanish silently and the next finalize would produce a plain send. The FFI validates the payload before take_builder() specifically to avoid that, and until now nothing exercised it.
Two caveats I would rather state than have you discover: the new test is gated behind MAYA_DEPOSIT_VERIFICATION=1 along with the rest of the suite (CodeRabbit flagged that run_tests.sh runs this bundle and a bootstrap stall would hang the job), and it has not been run yet — the SDK does not currently build locally because platform-wallet-ffi needs the key-wallet API from dashpay/rust-dashcore#922, which is still open. So treat it as written-and-reviewed, not as passing.
Review follow-ups on the deposit verification test: - Require exactly three outputs and always assert VOUT2 against VIN0. Both fixtures leave millions of duffs after the vault payment and fee, so change is mandatory; accepting two outputs let a regression that suppresses change pass the very test that exists to prove change-to-VIN0. - Assert the output and input counts before indexing, so a wrong shape fails readably instead of trapping on an out-of-range subscript and taking the test process down. - Cover the 80/81-byte OP_RETURN boundary rather than just the fixture, and reuse the same builder after a rejected payload. That pins the FFI guarantee this branch adds: the size check runs before `take_builder()`, so a refused memo must leave already-configured outputs and options intact. - Gate the suite behind MAYA_DEPOSIT_VERIFICATION=1. `run_tests.sh` runs this bundle in CI, and these tests sit behind several 90-second waits on top of a full SPV bootstrap, so a bootstrap stall would hang the job rather than fail it. Also renames MAX_STANDARD_OP_RETURN_BYTES to DEFAULT_MAX_OP_RETURN_BYTES, following key-wallet making the ceiling configurable per builder.
|
Pushed Require exactly three outputs, always assert VOUT2 → VIN0 Agreed, and this was the important one. Both fixtures leave millions of duffs after the vault payment and fee, so change is mandatory; (For contrast, the app-side assertion in dashpay/dashwallet-ios#916 deliberately accepts 2 or 3, because a real deposit can drop change below the 546-duff dust threshold. Here the fixture amounts rule that out.) Guard counts before indexing Done. Shape assertions now run before any subscripting, so a wrong shape produces a readable failure instead of trapping and killing the test process. Exercise the 80/81-byte boundary Added Keep the suite from hanging local Swift SDK CI Gated behind On the red CI Every failing check has one root cause: Same failure in Rust workspace tests (macOS) and Kotlin SDK build; CodeRabbit or PastaClaw review failed only because no review had been submitted at that point and re-runs on its own. It clears when dashpay/rust-dashcore#922 merges and the rev here is bumped. One thing to flag before that bump, because it is not a one-liner: current So the rev bump wants either a companion change adapting Verification status, honestly: the Swift changes are not compile-verified locally — the SDK cannot be built here right now, since |
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/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift`:
- Around line 55-68: Reindent the changed Swift code in the Maya deposit
verification integration tests, including the isEnabled declaration,
skipUnlessEnabled(), and the additional referenced blocks, to use 2 spaces per
nesting level as required by the repository EditorConfig. Preserve all logic and
formatting content aside from indentation.
🪄 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: 8a1944b3-0642-4c4b-a2e7-2ef01a88468e
📒 Files selected for processing (2)
packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rspackages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs
| /// Opt-in gate. `run_tests.sh` (which CI runs for `swift-sdk-build`) executes this bundle, | ||
| /// and these tests sit behind several 90-second `waitForSpendable` windows on top of a full | ||
| /// SPV bootstrap. A bootstrap stall would hang the job rather than fail it, so they run only | ||
| /// when explicitly requested until the bootstrap path is reliable. | ||
| private static let isEnabled = | ||
| ProcessInfo.processInfo.environment["MAYA_DEPOSIT_VERIFICATION"] == "1" | ||
|
|
||
| private func skipUnlessEnabled() throws { | ||
| try XCTSkipUnless( | ||
| Self.isEnabled, | ||
| "Set MAYA_DEPOSIT_VERIFICATION=1 to run the Maya deposit verification suite " | ||
| + "(requires a local dashmate devnet and a completed SPV bootstrap)." | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use 2-space indentation in the changed Swift code.
The changed blocks use 4 spaces per nesting level. Reindent them to match the repository EditorConfig requirement.
As per coding guidelines, “Follow repository EditorConfig settings: 2-space indentation.” <coding_guidelines>
Also applies to: 70-71, 127-183, 226-241, 264-274
🤖 Prompt for 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.
In
`@packages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift`
around lines 55 - 68, Reindent the changed Swift code in the Maya deposit
verification integration tests, including the isEnabled declaration,
skipUnlessEnabled(), and the additional referenced blocks, to use 2 spaces per
nesting level as required by the repository EditorConfig. Preserve all logic and
formatting content aside from indentation.
Source: Coding guidelines
3 participants
Issue being fixed or feature implemented
The Dash iOS wallet is restoring MAYACHAIN swap routes, which requires the DASH
deposit to carry the swap memo in an
OP_RETURN.CoreTransactionBuildercouldnot express that, so Maya was disabled during the DashSync unlink.
MAYAChain's UTXO deposit contract (docs,
"UTXO Chains") demands a specific shape:
VOUT0= Asgard vault,VOUT1= thememo as a zero-value
OP_RETURN,VOUT2= change paid back to the VIN0address, and no output reordering. The change rule matters because MAYAChain
identifies the depositor by VIN0 and pays refunds there — routing change to a
fresh HD address fails silently, with only a later refund going astray.
Because
finalize/build_signedfund and sign inside a single FFI call, none ofthis can be applied after the fact. It has to be expressible on the builder.
What was done?
packages/rs-platform-wallet-ffi/src/core_wallet/transaction_builder.rs:core_wallet_tx_builder_add_op_return,..._preserve_output_orderand..._change_to_first_input, following the existing setter style. An over-longpayload is rejected before
take_builder()runs, so a refused memo cannotleave the slot holding a
mem::takedefault and silently drop outputs thecaller already configured.
packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs:core_wallet_signed_transaction_v2_bytes— read a finalized transaction'sbytes without broadcasting, so the deposit shape can be asserted pre-broadcast.
packages/swift-sdk/.../CoreTransactionBuilder.swift:addOpReturn(_:),preserveOutputOrder(),changeToFirstInput(), andFinalizedCoreTransaction.serializedData()..github/workflows/tests-rs-workspace.yml: fail the workflow if a local[patch."https://github.com/dashpay/rust-dashcore"]override is left inCargo.toml— that override is invisible in review and produces a build thatonly works on one machine.
Depends on dashpay/rust-dashcore#922, which adds the underlying
add_op_return,preserve_output_orderandchange_to_first_inputtokey-wallet. Until thatmerges and the
revinCargo.tomlis bumped, building this locally needs thepatch override — deliberately not committed, which is what the new CI guard
enforces.
How Has This Been Tested?
Added
packages/swift-sdk/SwiftTests/SwiftDashSDKIntegrationTests/Core/MayaDepositVerificationIntegrationTests.swift,which builds short- and long-memo deposits against a local dashmate devnet and
asserts output count and order, the
OP_RETURNpayload,VOUT2== VIN0scriptPubKey, the 80-byte memo ceiling, Maya's dust floor and a ≥ 1 duff/byte
fee — then checks fee parity for ordinary, multi-recipient, selected-input,
drain and asset-lock shapes so the precise output sizing does not move existing
fees.
Also verified:
cargo check -p platform-wallet-ffi,./build_ios.sh --target ios --target sim, and a greendashpaybuild of theconsuming wallet app.
Known gap: the integration test currently stalls in SPV bootstrap on a
22k-block devnet (compact filters lag past the 180 s wait) and has not yet run
its assertions end to end.
Breaking Changes
None. All three builder controls are opt-in and default behaviour is unchanged.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit