From 9ff48a9bb27331b525117a07d609c4c1beb4fba4 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 6 Aug 2026 07:55:13 -0700 Subject: [PATCH 1/4] feat(key-wallet): let a drain carry zero-value OP_RETURN data outputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SelectionStrategy::All required exactly one output, so a drain could not carry a memo. A MAYAChain deposit needs both: the vault output AND the memo as an OP_RETURN beside it, or the swap cannot be matched to its quote. That made "swap my whole balance" inexpressible — callers had to guess the fee, subtract it themselves and send an explicit amount, which under-pays the destination whenever the guess is low. Under-delivery is not a rounding annoyance: MAYAChain executes a swap for an amount the user never agreed to, and NEAR Intents refuses the deposit outright. Zero-value data carriers now ride along with a drain. They claim none of the drained balance, and effective_outputs_size already prices their bytes into the fee, so the drain arithmetic (total_input - estimated_fee to the single value carrier, no change) is unchanged. Guards kept tight: - an OP_RETURN with a non-zero value is refused: it would claim part of the drained balance - exactly one spendable output must remain; only zero-value OP_RETURN data outputs may accompany it - asset locks are unchanged: THEIR single output IS the OP_RETURN burn mirroring the payload credits, so it stays the value carrier - a drain whose inputs cannot cover the fee still returns typed InsufficientFunds Tests: 646 pass in key-wallet, including five new cases covering the data carrier beside the destination, the fee covering its bytes, two spendable outputs still refused, a value-bearing carrier refused, and the asset-lock carrier unchanged. Verified on testnet through the Android SDK and dash-wallet: a max Maya deposit builds as a drain carrying an 80-byte memo and reports 27442985 duffs deliverable with a measured 432-duff fee. The build is signed and released without broadcasting, so the path is proven without a transaction reaching the network. Co-Authored-By: Claude Opus 5 --- .../transaction_builder.rs | 172 +++++++++++++++++- 1 file changed, 168 insertions(+), 4 deletions(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 7a86d7f17..300310562 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -489,8 +489,8 @@ impl TransactionBuilder { }; if self.selection_strategy == SelectionStrategy::All { - // Drain: the single output takes the whole balance minus fee (the caller's amount is - // ignored); no change. + // Drain: the single VALUE-CARRYING output takes the whole balance minus fee (the + // caller's amount is ignored); no change. let drained = total_input.saturating_sub(selection.estimated_fee); if drained == 0 { return Err(BuilderError::InsufficientFunds { @@ -498,10 +498,46 @@ impl TransactionBuilder { required: selection.estimated_fee, }); } - let [out] = tx_outputs.as_mut_slice() else { + // Zero-value data carriers ride along with a drain. A MAYAChain deposit is the + // motivating case: the memo MUST be on-chain as an OP_RETURN beside the vault + // output, so refusing every second output made "swap my whole balance" impossible + // to express — callers had to guess the fee, subtract it themselves and send an + // explicit amount, which under-pays the destination whenever the guess is low. + // Data outputs claim none of the drained balance, and `effective_outputs_size` + // already prices their bytes into the fee, so the drain arithmetic is unchanged. + // + // An asset lock is the exception: ITS single output IS an OP_RETURN (the burn + // mirroring the payload credits), so it stays the value carrier. + let is_asset_lock = + matches!(self.special_payload, Some(TransactionPayload::AssetLockPayloadType(_))); + if !is_asset_lock + && tx_outputs.iter().any(|out| out.script_pubkey.is_op_return() && out.value != 0) + { return Err(BuilderError::InvalidData( - "SelectionStrategy::All requires exactly one output (the destination)".into(), + "SelectionStrategy::All requires OP_RETURN outputs to be zero-value: a data \ + carrier with a value would claim part of the drained balance" + .into(), )); + } + let value_carriers = if is_asset_lock { + tx_outputs.len() + } else { + tx_outputs.iter().filter(|out| !out.script_pubkey.is_op_return()).count() + }; + if value_carriers != 1 { + return Err(BuilderError::InvalidData( + "SelectionStrategy::All requires exactly one spendable output (the \ + destination); only zero-value OP_RETURN data outputs may accompany it" + .into(), + )); + } + let out = if is_asset_lock { + &mut tx_outputs[0] + } else { + tx_outputs + .iter_mut() + .find(|out| !out.script_pubkey.is_op_return()) + .expect("exactly one spendable output was just counted") }; out.value = drained; // An asset-lock drain must also rewrite the payload's credit @@ -1811,4 +1847,132 @@ mod tests { funds.release_reservation(&tx); assert!(funds.reservations().reserved(200).is_empty()); } + + /// A MAYACHAIN-style drain: sweep the wallet to the vault while the swap memo rides along + /// as a zero-value OP_RETURN. Before this was allowed, "swap my whole balance" could not be + /// expressed at all — the caller had to guess the fee and send an explicit amount. + #[test] + fn test_drain_allows_a_zero_value_op_return_beside_the_destination() { + let utxo = Utxo::dummy(0, 100_000, 100, false, true); + let vault = Address::dummy(Network::Testnet, 0); + let memo = b"=:ETH.ETH:0x1c7b17362c84287bd1184447e6dfeaf920c31bbe"; + + let (tx, fee, _reservation) = TransactionBuilder::new() + .set_current_height(200) + .add_inputs([utxo]) + .add_output(&vault, 1) // ignored by a drain + .add_op_return(memo) + .expect("memo within the OP_RETURN ceiling") + .preserve_output_order() + .set_selection_strategy(SelectionStrategy::All) + .build_unsigned_reserved() + .expect("a drain with a data carrier builds"); + + assert_eq!(tx.output.len(), 2, "vault + memo, no change"); + // The vault takes everything the fee does not. + assert_eq!(tx.output[0].value, 100_000 - fee); + assert_eq!(tx.output[0].script_pubkey, vault.script_pubkey()); + // The memo is on-chain, zero-value, and still at VOUT1. + assert_eq!(tx.output[1].value, 0); + assert_eq!(tx.output[1].script_pubkey, op_return_script(memo)); + // Nothing is left behind: inputs are fully accounted for by outputs + fee. + assert_eq!(tx.output.iter().map(|o| o.value).sum::() + fee, 100_000); + } + + /// The data carrier's bytes must be paid for. A drain whose fee ignored the OP_RETURN would + /// under-pay the miner and risk a stuck deposit. + #[test] + fn test_drain_fee_covers_the_data_carrier_bytes() { + let memo = vec![0x4d; 60]; + let build = |with_memo: bool| { + let mut b = TransactionBuilder::new() + .set_current_height(200) + .add_inputs([Utxo::dummy(0, 100_000, 100, false, true)]) + .add_output(&Address::dummy(Network::Testnet, 0), 1) + .set_selection_strategy(SelectionStrategy::All); + if with_memo { + b = b.add_op_return(&memo).expect("memo within the ceiling").preserve_output_order(); + } + b.build_unsigned_reserved().expect("drain builds").1 + }; + + let plain_fee = build(false); + let memo_fee = build(true); + // 60 payload bytes + OP_RETURN + push opcode + the 8-byte value and its script varint. + assert!( + memo_fee > plain_fee, + "a drain carrying a {}-byte memo must cost more than a bare drain ({memo_fee} vs {plain_fee})", + memo.len() + ); + } + + /// Two spendable outputs remain ambiguous: a drain has one balance to give away. + #[test] + fn test_drain_still_rejects_two_spendable_outputs() { + let result = TransactionBuilder::new() + .set_current_height(200) + .add_inputs([Utxo::dummy(0, 100_000, 100, false, true)]) + .add_output(&Address::dummy(Network::Testnet, 0), 1) + .add_output(&Address::dummy(Network::Testnet, 1), 1) + .set_selection_strategy(SelectionStrategy::All) + .build_unsigned_reserved(); + + match result { + Err(BuilderError::InvalidData(message)) => { + assert!(message.contains("exactly one spendable output"), "got: {message}"); + } + other => panic!("expected the two-destination drain to be rejected, got {other:?}"), + } + } + + /// A value-bearing OP_RETURN would claim part of the drained balance and overspend. + #[test] + fn test_drain_rejects_a_value_bearing_data_carrier() { + let mut builder = TransactionBuilder::new() + .set_current_height(200) + .add_inputs([Utxo::dummy(0, 100_000, 100, false, true)]) + .add_output(&Address::dummy(Network::Testnet, 0), 1) + .add_op_return(b"memo") + .expect("memo within the ceiling") + .set_selection_strategy(SelectionStrategy::All); + // Reach past the builder API, which cannot express this, to prove the guard holds. + builder.outputs.last_mut().expect("the data output").value = 5_000; + + match builder.build_unsigned_reserved() { + Err(BuilderError::InvalidData(message)) => { + assert!(message.contains("zero-value"), "got: {message}"); + } + other => panic!("expected a value-bearing data carrier to be rejected, got {other:?}"), + } + } + + /// An asset-lock drain is the one case whose value carrier IS an OP_RETURN (the burn + /// mirroring the payload credits) — it must keep working unchanged. + #[test] + fn test_asset_lock_drain_still_uses_its_burn_output_as_the_carrier() { + let credit_script = Address::dummy(Network::Testnet, 14).script_pubkey(); + let (tx, fee, _reservation) = TransactionBuilder::new() + .set_current_height(200) + .add_inputs([Utxo::dummy(0, 100_000, 100, false, true)]) + .set_special_payload(TransactionPayload::AssetLockPayloadType(AssetLockPayload { + version: 1, + credit_outputs: vec![TxOut { + value: 1, + script_pubkey: credit_script, + }], + })) + .set_selection_strategy(SelectionStrategy::All) + .build_unsigned_reserved() + .expect("asset-lock drain builds"); + + assert_eq!(tx.output.len(), 1); + assert_eq!(tx.output[0].value, 100_000 - fee); + assert!(tx.output[0].script_pubkey.is_op_return()); + match tx.special_transaction_payload { + Some(TransactionPayload::AssetLockPayloadType(p)) => { + assert_eq!(p.credit_outputs[0].value, 100_000 - fee, "credits mirror the burn"); + } + other => panic!("expected the asset-lock payload, got {other:?}"), + } + } } From 8209a11fc32ff7d95792c169a06285451af15c97 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 6 Aug 2026 13:43:27 -0700 Subject: [PATCH 2/4] test(key-wallet): price the drain's data carrier against serialized size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fee test asserted only that a memo-carrying drain costs more than a bare one, which passes even when the extra fee falls short of the bytes the carrier adds — the case that would let a drain broadcast below the relay rate. Return the transactions from the build closure and assert against their serialized sizes: the fee exceeds the serialized bytes by the per-input signature allowance, that allowance is unchanged by the carrier, and the 71 bytes the carrier adds are priced at the transaction's own rate (equality, so under-pricing and upward drift both fail). Also assert the deliverable output shrank by exactly that extra fee — the zero-value carrier takes nothing from the destination beyond its bytes. Co-Authored-By: Claude Opus 5 --- .../transaction_builder.rs | 59 +++++++++++++++++-- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 300310562..5cacc8fd7 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -1893,17 +1893,64 @@ mod tests { if with_memo { b = b.add_op_return(&memo).expect("memo within the ceiling").preserve_output_order(); } - b.build_unsigned_reserved().expect("drain builds").1 + let (tx, fee, _reservation) = b.build_unsigned_reserved().expect("drain builds"); + (tx, fee) }; - let plain_fee = build(false); - let memo_fee = build(true); - // 60 payload bytes + OP_RETURN + push opcode + the 8-byte value and its script varint. + let (plain_tx, plain_fee) = build(false); + let (memo_tx, memo_fee) = build(true); + + // "Costs more" is too weak on its own: it passes even when the extra fee + // falls short of the bytes the carrier actually adds. Price the fee + // against the SERIALIZED transactions instead, so an under-priced data + // carrier — the case that lets a drain broadcast below the relay rate — + // fails here. + let plain_size = dashcore::consensus::serialize(&plain_tx).len() as u64; + let memo_size = dashcore::consensus::serialize(&memo_tx).len() as u64; + + // These are unsigned transactions, so the fee must exceed the bytes on + // hand by the signature allowance the estimator adds per input. Both + // builds spend the same single input, so that allowance is identical: + // whatever the carrier costs shows up entirely as extra size. assert!( - memo_fee > plain_fee, - "a drain carrying a {}-byte memo must cost more than a bare drain ({memo_fee} vs {plain_fee})", + memo_fee > memo_size, + "the fee ({memo_fee}) must cover the {memo_size} serialized bytes plus signatures" + ); + assert_eq!( + memo_fee - memo_size, + plain_fee - plain_size, + "the carrier must not disturb the per-input signature allowance" + ); + + // The heart of it: every byte the carrier adds is paid for, at the same + // rate as the rest of the transaction. Equality (not >=) is deliberate — + // it catches under-pricing AND a fee that silently drifts upward. + let size_delta = memo_size - plain_size; + let fee_delta = memo_fee - plain_fee; + assert!( + size_delta >= memo.len() as u64, + "a {}-byte memo must add at least its payload to the serialized size (added {size_delta})", memo.len() ); + assert_eq!( + fee_delta, size_delta, + "the {size_delta} bytes the carrier adds must be priced at the transaction's own rate" + ); + + // And the deliverable output shrank by exactly that extra fee: the + // zero-value carrier takes nothing from the destination beyond its bytes. + let deliverable = |tx: &Transaction| { + tx.output + .iter() + .find(|o| !o.script_pubkey.is_op_return()) + .expect("one value carrier") + .value + }; + assert_eq!( + deliverable(&plain_tx) - deliverable(&memo_tx), + fee_delta, + "the zero-value carrier must cost the destination exactly the extra fee" + ); } /// Two spendable outputs remain ambiguous: a drain has one balance to give away. From 539358dc94c927e55f00c02f6eef91fb7ba9f935 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Fri, 7 Aug 2026 08:31:07 -0700 Subject: [PATCH 3/4] fix(key-wallet): resolve the drain's value carrier without expect() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The count check immediately above proves exactly one carrier exists, so neither arm can come back empty today — but this is library code, and a later change to that invariant would turn into a panic in a caller's process. Take `first_mut()` / `find(..)` as Options and return `BuilderError::InvalidData` when neither yields an output, so the invariant fails typed instead. Also drops the `tx_outputs[0]` index on the asset-lock arm, which could panic on an empty output set. Behavior is unchanged on every reachable path; 646 key-wallet tests pass. Co-Authored-By: Claude Opus 5 --- .../transaction_builder.rs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 5cacc8fd7..518b814fc 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -531,13 +531,21 @@ impl TransactionBuilder { .into(), )); } - let out = if is_asset_lock { - &mut tx_outputs[0] + // The count above already proves a carrier exists, so neither arm can + // come back empty today. Resolve it fallibly anyway: this is library + // code, and a later change to that invariant should surface as a typed + // error rather than a panic in a caller's process. + let carrier = if is_asset_lock { + tx_outputs.first_mut() } else { - tx_outputs - .iter_mut() - .find(|out| !out.script_pubkey.is_op_return()) - .expect("exactly one spendable output was just counted") + tx_outputs.iter_mut().find(|out| !out.script_pubkey.is_op_return()) + }; + let Some(out) = carrier else { + return Err(BuilderError::InvalidData( + "SelectionStrategy::All found no spendable output to receive the drained \ + balance" + .into(), + )); }; out.value = drained; // An asset-lock drain must also rewrite the payload's credit From e057d5a5b360a6cba79ae47d59cb86dcbd248f07 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sat, 8 Aug 2026 17:42:49 -0700 Subject: [PATCH 4/4] style(key-wallet): rustfmt the drain-with-memo test helper `cargo fmt` splits the builder chain in the drain test helper across lines. Formatting only, no behaviour change; clears the sole failing pre-commit hook. Co-Authored-By: Claude Opus 5 --- .../src/wallet/managed_wallet_info/transaction_builder.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs index 518b814fc..5063b28d0 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -1899,7 +1899,10 @@ mod tests { .add_output(&Address::dummy(Network::Testnet, 0), 1) .set_selection_strategy(SelectionStrategy::All); if with_memo { - b = b.add_op_return(&memo).expect("memo within the ceiling").preserve_output_order(); + b = b + .add_op_return(&memo) + .expect("memo within the ceiling") + .preserve_output_order(); } let (tx, fee, _reservation) = b.build_unsigned_reserved().expect("drain builds"); (tx, fee)