From fee16d967ca4f96ea2a1c728c25b6bec9d0fae13 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 27 Aug 2026 18:11:12 +0200 Subject: [PATCH 1/2] feat(key-wallet): payload-finalization seam for input-committing special transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A ProUpServTx's payload commits to the chosen inputs (inputs_hash) and is itself BLS-signed by the operator key, while each input's ECDSA sighash covers the finished payload. build_signed_reserved fuses input selection and input signing with no seam between them, so such a payload could never be finalized at the right moment. Add TransactionBuilder::build_signed_reserved_with_payload_finalizer: after selection reserves the chosen inputs, a finalizer closure receives the unsigned transaction (inputs chosen and BIP-69 sorted, placeholder payload attached) and returns the finalized payload; the builder installs it and only then signs the inputs. The placeholder is required so selection prices the payload bytes into the fee, and the finalized payload must keep the placeholder's variant and estimated size — the fee is fixed at selection time, so growth would underpay the configured rate. Every failure path (finalizer error, guards, signing) releases the reservation owner-guarded, exactly like a failed sign in build_signed_reserved. The fee-sizing payload match moves out of calculate_base_size into estimated_payload_size so the size guard and the fee estimate can never disagree. Co-Authored-By: Claude Fable 5 --- .../transaction_builder.rs | 510 +++++++++++++++--- 1 file changed, 435 insertions(+), 75 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 1ca3132f9..962b20356 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -347,81 +347,7 @@ impl TransactionBuilder { // Add special payload size if present // Based on dashsync payload size calculations if let Some(ref payload) = self.special_payload { - let payload_size = match payload { - TransactionPayload::CoinbasePayloadType(p) => { - // version (2) + height (4) + merkleRootMasternodeList (32) + merkleRootQuorums (32) - let mut size = 2 + 4 + 32 + 32; - // Optional fields for newer versions - if p.best_cl_height.is_some() { - size += 4; // best_cl_height - size += 96; // best_cl_signature (BLS) - } - if p.asset_locked_amount.is_some() { - size += 8; // asset_locked_amount - } - size - } - TransactionPayload::ProviderRegistrationPayloadType(p) => { - // Base payload + signature - // version (2) + type (2) + mode (2) + collateralHash (32) + collateralIndex (4) - // + ipAddress (16) + port (2) + KeyIDOwner (20) + KeyIDOperator (20) + KeyIDVoting (20) - // + operatorReward (2) + scriptPayoutSize + scriptPayout + inputsHash (32) - // + payloadSigSize (1-9) + payloadSig (up to 75) - let script_size = p.script_payout.len(); - let base = 2 - + 2 - + 2 - + 32 - + 4 - + 16 - + 2 - + 20 - + 20 - + 20 - + 2 - + varint_size(script_size) - + script_size - + 32; - base + varint_size(75) + 75 // MAX_ECDSA_SIGNATURE_SIZE = 75 - } - TransactionPayload::ProviderUpdateServicePayloadType(p) => { - // version (2) + optionally mn_type (2) + proTxHash (32) + ipAddress (16) + port (2) - // + scriptPayoutSize + scriptPayout + inputsHash (32) + payloadSig (96 for BLS) - let script_size = p.script_payout.len(); - let mut size = - 2 + 32 + 16 + 2 + varint_size(script_size) + script_size + 32 + 96; - if p.mn_type.is_some() { - size += 2; // mn_type for BasicBLS version - } - // Platform fields for Evo masternodes - if p.platform_node_id.is_some() { - size += 20; // platform_node_id - size += 2; // platform_p2p_port - size += 2; // platform_http_port - } - size - } - TransactionPayload::ProviderUpdateRegistrarPayloadType(p) => { - // version (2) + proTxHash (32) + mode (2) + PubKeyOperator (48) + KeyIDVoting (20) - // + scriptPayoutSize + scriptPayout + inputsHash (32) + payloadSig (up to 75) - let script_size = p.script_payout.len(); - 2 + 32 + 2 + 48 + 20 + varint_size(script_size) + script_size + 32 + 75 - } - TransactionPayload::ProviderUpdateRevocationPayloadType(_) => { - // version (2) + proTxHash (32) + reason (2) + inputsHash (32) + payloadSig (96 for BLS) - 2 + 32 + 2 + 32 + 96 - } - TransactionPayload::AssetLockPayloadType(p) => { - // version (1) + creditOutputsCount + creditOutputs - 1 + varint_size(p.credit_outputs.len()) - + p.credit_outputs.len() * TX_OUTPUT_SIZE - } - TransactionPayload::AssetUnlockPayloadType(_p) => { - // version (1) + index (8) + fee (4) + requestHeight (4) + quorumHash (32) + quorumSig (96) - 1 + 8 + 4 + 4 + 32 + 96 - } - _ => 100, // Default estimate for unknown types - }; + let payload_size = Self::estimated_payload_size(payload); // Add varint for payload length size += varint_size(payload_size) + payload_size; @@ -430,6 +356,87 @@ impl TransactionBuilder { size } + /// Estimated serialized size of a special payload, as priced into the fee by + /// [`Self::calculate_base_size`]. Also the yardstick + /// [`Self::build_signed_reserved_with_payload_finalizer`] holds a finalized + /// payload against: the fee is fixed at selection time from the placeholder's + /// estimate, so a finalized payload may not estimate larger. + fn estimated_payload_size(payload: &TransactionPayload) -> usize { + match payload { + TransactionPayload::CoinbasePayloadType(p) => { + // version (2) + height (4) + merkleRootMasternodeList (32) + merkleRootQuorums (32) + let mut size = 2 + 4 + 32 + 32; + // Optional fields for newer versions + if p.best_cl_height.is_some() { + size += 4; // best_cl_height + size += 96; // best_cl_signature (BLS) + } + if p.asset_locked_amount.is_some() { + size += 8; // asset_locked_amount + } + size + } + TransactionPayload::ProviderRegistrationPayloadType(p) => { + // Base payload + signature + // version (2) + type (2) + mode (2) + collateralHash (32) + collateralIndex (4) + // + ipAddress (16) + port (2) + KeyIDOwner (20) + KeyIDOperator (20) + KeyIDVoting (20) + // + operatorReward (2) + scriptPayoutSize + scriptPayout + inputsHash (32) + // + payloadSigSize (1-9) + payloadSig (up to 75) + let script_size = p.script_payout.len(); + let base = 2 + + 2 + + 2 + + 32 + + 4 + + 16 + + 2 + + 20 + + 20 + + 20 + + 2 + + varint_size(script_size) + + script_size + + 32; + base + varint_size(75) + 75 // MAX_ECDSA_SIGNATURE_SIZE = 75 + } + TransactionPayload::ProviderUpdateServicePayloadType(p) => { + // version (2) + optionally mn_type (2) + proTxHash (32) + ipAddress (16) + port (2) + // + scriptPayoutSize + scriptPayout + inputsHash (32) + payloadSig (96 for BLS) + let script_size = p.script_payout.len(); + let mut size = 2 + 32 + 16 + 2 + varint_size(script_size) + script_size + 32 + 96; + if p.mn_type.is_some() { + size += 2; // mn_type for BasicBLS version + } + // Platform fields for Evo masternodes + if p.platform_node_id.is_some() { + size += 20; // platform_node_id + size += 2; // platform_p2p_port + size += 2; // platform_http_port + } + size + } + TransactionPayload::ProviderUpdateRegistrarPayloadType(p) => { + // version (2) + proTxHash (32) + mode (2) + PubKeyOperator (48) + KeyIDVoting (20) + // + scriptPayoutSize + scriptPayout + inputsHash (32) + payloadSig (up to 75) + let script_size = p.script_payout.len(); + 2 + 32 + 2 + 48 + 20 + varint_size(script_size) + script_size + 32 + 75 + } + TransactionPayload::ProviderUpdateRevocationPayloadType(_) => { + // version (2) + proTxHash (32) + reason (2) + inputsHash (32) + payloadSig (96 for BLS) + 2 + 32 + 2 + 32 + 96 + } + TransactionPayload::AssetLockPayloadType(p) => { + // version (1) + creditOutputsCount + creditOutputs + 1 + varint_size(p.credit_outputs.len()) + p.credit_outputs.len() * TX_OUTPUT_SIZE + } + TransactionPayload::AssetUnlockPayloadType(_p) => { + // version (1) + index (8) + fee (4) + requestHeight (4) + quorumHash (32) + quorumSig (96) + 1 + 8 + 4 + 4 + 32 + 96 + } + _ => 100, // Default estimate for unknown types + } + } + /// Select inputs, build the unsigned transaction, and reserve the chosen /// inputs. The optional [`ReservationToken`] is `Some` exactly when a /// reservation set was attached (via [`add_funding`]) and identifies the @@ -780,6 +787,122 @@ impl TransactionBuilder { Ok((tx, total_input.saturating_sub(total_output), reservation)) } + + /// Like [`Self::build_signed_reserved`], but with a payload-finalization + /// seam between input selection and input signing, for special + /// transactions whose payload commits to the chosen inputs and is itself + /// signed — a ProUpServTx's `inputs_hash` + operator-BLS `payload_sig` + /// being the motivating case. + /// + /// The builder must be given a placeholder payload via + /// [`Self::set_special_payload`]: the same variant with every + /// selection-dependent field (inputs hash, payload signature) zeroed, so + /// coin selection prices the payload bytes into the fee. After selection + /// reserves the chosen inputs, `finalize_payload` receives the unsigned + /// transaction — inputs chosen and BIP-69 sorted, placeholder payload + /// still attached — and returns the finalized payload; the builder + /// installs it and only then computes each input's sighash, which covers + /// the finalized payload. Because that payload signature commits to the + /// input set, the input set is frozen from the finalizer on: there is no + /// fee-bump or input-substitution path for such a transaction. + /// + /// Fails without running the finalizer when no placeholder payload was + /// set, and fails after it when the finalized payload is a different + /// variant or estimates larger than the placeholder (the fee was fixed at + /// selection time, so growth would underpay the configured fee rate). On + /// any failure — finalizer error, guard, or signing — the reservation is + /// released owner-guarded, exactly as in [`Self::build_signed_reserved`]. + pub async fn build_signed_reserved_with_payload_finalizer( + self, + signer: &S, + path_resolver: P, + finalize_payload: F, + ) -> Result<(Transaction, u64, Option), BuilderError> + where + S: TransactionSigner + ?Sized + Sync, + P: Fn(Address) -> Option + Send, + F: FnOnce(&Transaction) -> Result + Send, + { + if self.special_payload.is_none() { + return Err(BuilderError::InvalidData( + "payload finalizer requires a placeholder payload: call set_special_payload with \ + the same variant (selection-dependent fields zeroed) so selection prices its \ + bytes into the fee" + .into(), + )); + } + + let funding = self.funding.clone(); + let (mut tx, inputs, reservation) = self.assemble_unsigned()?; + let total_input: u64 = inputs.iter().map(|utxo| utxo.value()).sum(); + // Same owner-guarded release discipline as `build_signed_reserved`: + // a failure past this point leaves the inputs spendable, so free them + // for the next build instead of stranding them until the TTL backstop. + let reserved: Vec = inputs.iter().map(|utxo| utxo.outpoint).collect(); + fn release( + funding: &[(ReservationSet, HashSet)], + reserved: &[OutPoint], + reservation: Option, + ) { + if let Some(token) = reservation { + for (reservations, _) in funding { + reservations.release_if_owner(reserved, token); + } + } + } + + let finalized = match finalize_payload(&tx) { + Ok(payload) => payload, + Err(err) => { + release(&funding, &reserved, reservation); + return Err(err); + } + }; + + // `assemble_unsigned` carries the placeholder into the transaction + // unchanged for every payload type (only an asset-lock drain rewrites + // one, and that variant has no selection-dependent fields to + // finalize). Guard rather than unwrap: this is library code. + let Some(placeholder) = tx.special_transaction_payload.as_ref() else { + release(&funding, &reserved, reservation); + return Err(BuilderError::InvalidData( + "placeholder payload missing from the assembled transaction".into(), + )); + }; + if core::mem::discriminant(&finalized) != core::mem::discriminant(placeholder) { + release(&funding, &reserved, reservation); + return Err(BuilderError::InvalidData( + "finalized payload is a different variant than the placeholder the fee was \ + selected for" + .into(), + )); + } + let placeholder_size = Self::estimated_payload_size(placeholder); + let finalized_size = Self::estimated_payload_size(&finalized); + if finalized_size > placeholder_size { + release(&funding, &reserved, reservation); + return Err(BuilderError::InvalidData(format!( + "finalized payload estimates {finalized_size} bytes, larger than the \ + {placeholder_size}-byte placeholder the fee was selected for" + ))); + } + tx.special_transaction_payload = Some(finalized); + + // Input sighashes cover the finalized payload (the legacy sighash + // consensus-encodes the whole transaction, payload included), so + // signing must come after the payload is installed. + let tx = match signer.sign_tx(tx, inputs, path_resolver).await { + Ok(tx) => tx, + Err(err) => { + release(&funding, &reserved, reservation); + return Err(err); + } + }; + + let total_output: u64 = tx.output.iter().map(|out| out.value).sum(); + + Ok((tx, total_input.saturating_sub(total_output), reservation)) + } } #[async_trait::async_trait] @@ -2127,4 +2250,241 @@ mod tests { other => panic!("expected the asset-lock payload, got {other:?}"), } } + + /// Placeholder ProUpServTx payload with every selection-dependent field + /// zeroed, as a finalizer-seam caller would construct it before selection. + fn pro_up_serv_placeholder(script_payout: ScriptBuf) -> TransactionPayload { + use dashcore::blockdata::transaction::special_transaction::provider_update_service::ProviderUpdateServicePayload; + use dashcore::bls_sig_utils::BLSSignature; + use dashcore::hash_types::InputsHash; + + TransactionPayload::ProviderUpdateServicePayloadType(ProviderUpdateServicePayload::new( + Some(0), + Txid::all_zeros(), + 0x00000000000000000000ffff7f000001, // 127.0.0.1 mapped + 19999, + script_payout, + InputsHash::all_zeros(), + None, + None, + None, + BLSSignature::from([0; 96]), + )) + } + + #[tokio::test] + async fn payload_finalizer_installs_payload_before_input_signing() { + use dashcore::bls_sig_utils::BLSSignature; + + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let a = Utxo::dummy(0x01, 600_000, 100, false, true); + let b = Utxo::dummy(0x02, 600_000, 100, false, true); + funds.utxos.insert(a.outpoint, a.clone()); + funds.utxos.insert(b.outpoint, b.clone()); + + let (tx, fee, token) = TransactionBuilder::new() + .set_current_height(200) + .set_fee_rate(FeeRate::normal()) + .add_funding(&mut funds, &account) + .set_change_address(Address::dummy(Network::Testnet, 1)) + .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) + .build_signed_reserved_with_payload_finalizer( + &ctx.wallet, + |_addr| Some(DerivationPath::master()), + |unsigned| { + // The finalizer sees the selected inputs (unsigned) and the + // placeholder payload, exactly what inputs_hash + a payload + // signature need. + assert!(!unsigned.input.is_empty(), "inputs are selected before finalizing"); + assert!( + unsigned.input.iter().all(|input| input.script_sig.is_empty()), + "inputs must not be signed before the payload is finalized" + ); + let Some(TransactionPayload::ProviderUpdateServicePayloadType(placeholder)) = + &unsigned.special_transaction_payload + else { + panic!("placeholder payload must still be attached"); + }; + let mut finalized = placeholder.clone(); + finalized.inputs_hash = unsigned.hash_inputs(); + finalized.payload_sig = BLSSignature::from([0xAB; 96]); + Ok(TransactionPayload::ProviderUpdateServicePayloadType(finalized)) + }, + ) + .await + .expect("finalized build signs"); + + assert!(fee > 0); + assert!(token.is_some(), "a funded build stamps a reservation token"); + let Some(TransactionPayload::ProviderUpdateServicePayloadType(payload)) = + &tx.special_transaction_payload + else { + panic!("finalized payload must ride the signed transaction"); + }; + assert_eq!(payload.inputs_hash, tx.hash_inputs(), "inputs_hash commits to the input set"); + assert_eq!(payload.payload_sig, BLSSignature::from([0xAB; 96])); + assert!( + tx.input.iter().all(|input| !input.script_sig.is_empty()), + "every input is ECDSA-signed after the payload landed" + ); + // The selected inputs stay reserved for the caller to broadcast. + let reserved = funds.reservations().reserved(200); + for input in &tx.input { + assert!(reserved.contains(&input.previous_output)); + } + } + + #[tokio::test] + async fn payload_finalizer_error_releases_reservation() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let utxo = Utxo::dummy(0x01, 1_000_000, 100, false, true); + funds.utxos.insert(utxo.outpoint, utxo.clone()); + + let result = TransactionBuilder::new() + .set_current_height(200) + .set_fee_rate(FeeRate::normal()) + .add_funding(&mut funds, &account) + .set_change_address(Address::dummy(Network::Testnet, 1)) + .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) + .build_signed_reserved_with_payload_finalizer( + &ctx.wallet, + |_addr| Some(DerivationPath::master()), + |_unsigned| Err(BuilderError::SigningFailed("operator key rejected".into())), + ) + .await; + + assert!(matches!(result, Err(BuilderError::SigningFailed(_)))); + assert!( + funds.reservations().reserved(200).is_empty(), + "a failed finalize must release the reserved inputs" + ); + } + + #[tokio::test] + async fn payload_finalizer_rejects_a_variant_change() { + use dashcore::blockdata::transaction::special_transaction::provider_update_revocation::ProviderUpdateRevocationPayload; + use dashcore::bls_sig_utils::BLSSignature; + + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let utxo = Utxo::dummy(0x01, 1_000_000, 100, false, true); + funds.utxos.insert(utxo.outpoint, utxo.clone()); + + let result = TransactionBuilder::new() + .set_current_height(200) + .set_fee_rate(FeeRate::normal()) + .add_funding(&mut funds, &account) + .set_change_address(Address::dummy(Network::Testnet, 1)) + .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) + .build_signed_reserved_with_payload_finalizer( + &ctx.wallet, + |_addr| Some(DerivationPath::master()), + |unsigned| { + Ok(TransactionPayload::ProviderUpdateRevocationPayloadType( + ProviderUpdateRevocationPayload { + version: ProviderUpdateRevocationPayload::CURRENT_VERSION, + pro_tx_hash: Txid::all_zeros(), + reason: 0, + inputs_hash: unsigned.hash_inputs(), + payload_sig: BLSSignature::from([0; 96]), + }, + )) + }, + ) + .await; + + assert!(matches!(result, Err(BuilderError::InvalidData(_)))); + assert!(funds.reservations().reserved(200).is_empty()); + } + + #[tokio::test] + async fn payload_finalizer_rejects_a_payload_that_outgrew_its_placeholder() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let utxo = Utxo::dummy(0x01, 1_000_000, 100, false, true); + funds.utxos.insert(utxo.outpoint, utxo.clone()); + + let result = TransactionBuilder::new() + .set_current_height(200) + .set_fee_rate(FeeRate::normal()) + .add_funding(&mut funds, &account) + .set_change_address(Address::dummy(Network::Testnet, 1)) + // Placeholder priced with an empty payout script... + .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) + .build_signed_reserved_with_payload_finalizer( + &ctx.wallet, + |_addr| Some(DerivationPath::master()), + // ...but finalized with a 25-byte P2PKH script the fee never + // paid for. + |_unsigned| { + Ok(pro_up_serv_placeholder(Address::dummy(Network::Testnet, 2).script_pubkey())) + }, + ) + .await; + + assert!(matches!(result, Err(BuilderError::InvalidData(_)))); + assert!(funds.reservations().reserved(200).is_empty()); + } + + #[tokio::test] + async fn payload_finalizer_requires_a_placeholder_payload() { + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let utxo = Utxo::dummy(0x01, 1_000_000, 100, false, true); + funds.utxos.insert(utxo.outpoint, utxo.clone()); + + let result = TransactionBuilder::new() + .set_current_height(200) + .set_fee_rate(FeeRate::normal()) + .add_funding(&mut funds, &account) + .set_change_address(Address::dummy(Network::Testnet, 1)) + .add_output(&Address::dummy(Network::Testnet, 0), 500_000) + .build_signed_reserved_with_payload_finalizer( + &ctx.wallet, + |_addr| Some(DerivationPath::master()), + |_unsigned| panic!("finalizer must not run without a placeholder"), + ) + .await; + + assert!(matches!(result, Err(BuilderError::InvalidData(_)))); + assert!( + funds.reservations().reserved(200).is_empty(), + "the refusal happens before anything is reserved" + ); + } + + /// The fee a finalizer-seam build pays must equal what the ordinary path + /// charges for the same placeholder: the finalized payload swaps in at + /// identical estimated size, so nothing about selection or change moves. + #[test] + fn payload_finalizer_size_guard_uses_the_fee_sizing_estimate() { + let placeholder = pro_up_serv_placeholder(ScriptBuf::new()); + let finalized = pro_up_serv_placeholder(ScriptBuf::new()); + assert_eq!( + TransactionBuilder::estimated_payload_size(&placeholder), + TransactionBuilder::estimated_payload_size(&finalized), + ); + let grown = pro_up_serv_placeholder(Address::dummy(Network::Testnet, 2).script_pubkey()); + assert!( + TransactionBuilder::estimated_payload_size(&grown) + > TransactionBuilder::estimated_payload_size(&placeholder) + ); + } } From 3f1bc69ef59b051d4a2a5eb687fc3957f0510a55 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 28 Aug 2026 12:34:23 +0200 Subject: [PATCH 2/2] refactor(key-wallet): make the payload finalizer builder state, not a build method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on the seam's shape: a third build entry point duplicated the signing path. The finalizer is now a set_payload_finalizer(..) field (boxed FnOnce, so captures must be owned) consumed by the existing build methods — build_signed_reserved runs it between selection and input signing, and build_unsigned_reserved applies it too, returning a finalized-but-unsigned transaction for external input signing rather than silently ignoring the configured field. Guards, placeholder requirement, and owner-guarded release on every failure path are unchanged, now shared via finalize_payload_in / release_reservation. Co-Authored-By: Claude Fable 5 --- .../transaction_builder.rs | 367 ++++++++++-------- 1 file changed, 214 insertions(+), 153 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 962b20356..3d145b3fa 100644 --- a/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs +++ b/key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs @@ -63,6 +63,13 @@ fn addresses_share_network(left: &Address, right: &Address) -> bool { ) } +/// A payload-finalization hook set via [`TransactionBuilder::set_payload_finalizer`]: it +/// receives the assembled unsigned transaction and returns the finalized special payload. +/// Boxed so it can live on the builder like the other `set_*` state; captures must be owned +/// (`'static`). +pub type PayloadFinalizer = + Box Result + Send>; + /// Transaction builder for creating Dash transactions /// /// This builder implements BIP-69 (Lexicographical Indexing of Transaction Inputs and Outputs) @@ -80,6 +87,9 @@ pub struct TransactionBuilder { change_to_first_input: bool, /// Special transaction payload for Dash-specific transactions special_payload: Option, + /// Finalizes `special_payload` after input selection, for payloads that + /// commit to the chosen inputs. See [`Self::set_payload_finalizer`]. + payload_finalizer: Option, /// Reservation set of each funding account paired with the outpoints that /// account contributed, captured by `add_funding`. Reservations live on the /// account that holds the UTXO, so each account reserves its own share of @@ -107,6 +117,7 @@ impl TransactionBuilder { preserve_output_order: false, change_to_first_input: false, special_payload: None, + payload_finalizer: None, funding: Vec::new(), } } @@ -256,6 +267,40 @@ impl TransactionBuilder { self } + /// Finalize the special payload after input selection, for special + /// transactions whose payload commits to the chosen inputs and is itself + /// signed — a ProUpServTx's `inputs_hash` + operator-BLS `payload_sig` + /// being the motivating case. + /// + /// The builder must also be given a placeholder payload via + /// [`Self::set_special_payload`]: the same variant with every + /// selection-dependent field (inputs hash, payload signature) zeroed, so + /// coin selection prices the payload bytes into the fee. After selection + /// reserves the chosen inputs, `finalize_payload` receives the unsigned + /// transaction — inputs chosen and BIP-69 sorted, placeholder payload + /// still attached — and returns the finalized payload, which replaces the + /// placeholder before any input is signed; each input's sighash covers the + /// finalized payload. Because that payload signature commits to the input + /// set, the input set is frozen from the finalizer on: there is no + /// fee-bump or input-substitution path for such a transaction. + /// + /// The build fails without running the finalizer when no placeholder + /// payload was set, and fails after it when the finalized payload is a + /// different variant or estimates larger than the placeholder (the fee was + /// fixed at selection time, so growth would underpay the configured fee + /// rate). On any failure — finalizer error, guard, or signing — the + /// reservation is released owner-guarded, exactly as for a failed sign in + /// [`Self::build_signed_reserved`]. + pub fn set_payload_finalizer( + mut self, + finalize_payload: impl FnOnce(&Transaction) -> Result + + Send + + 'static, + ) -> Self { + self.payload_finalizer = Some(Box::new(finalize_payload)); + self + } + /// Effective `tx.output` count: for AssetLock the only on-chain output is /// the OP_RETURN burn (credit outputs live in the payload), otherwise it's /// the user-provided outputs. @@ -357,10 +402,10 @@ impl TransactionBuilder { } /// Estimated serialized size of a special payload, as priced into the fee by - /// [`Self::calculate_base_size`]. Also the yardstick - /// [`Self::build_signed_reserved_with_payload_finalizer`] holds a finalized - /// payload against: the fee is fixed at selection time from the placeholder's - /// estimate, so a finalized payload may not estimate larger. + /// [`Self::calculate_base_size`]. Also the yardstick a + /// [`Self::set_payload_finalizer`] payload is held against: the fee is fixed + /// at selection time from the placeholder's estimate, so a finalized payload + /// may not estimate larger. fn estimated_payload_size(payload: &TransactionPayload) -> usize { match payload { TransactionPayload::CoinbasePayloadType(p) => { @@ -711,10 +756,25 @@ impl TransactionBuilder { /// only the inputs it contributed, in its own set. See /// `ReservationSet::release_if_owner` for why owner-guarded release is /// required (`dashpay/platform#4185`). + /// A payload finalizer set via [`Self::set_payload_finalizer`] runs here + /// too, right after selection: the returned unsigned transaction carries + /// the finalized payload, ready for external input signing. pub fn build_unsigned_reserved( - self, + mut self, ) -> Result<(Transaction, u64, Option), BuilderError> { - let (tx, inputs, reservation) = self.assemble_unsigned()?; + self.require_placeholder_for_finalizer()?; + let finalizer = self.payload_finalizer.take(); + let funding = self.funding.clone(); + + let (mut tx, inputs, reservation) = self.assemble_unsigned()?; + + if let Some(finalize_payload) = finalizer { + if let Err(err) = Self::finalize_payload_in(&mut tx, finalize_payload) { + let reserved: Vec = inputs.iter().map(|utxo| utxo.outpoint).collect(); + Self::release_reservation(&funding, &reserved, reservation); + return Err(err); + } + } let total_input: u64 = inputs.iter().map(|utxo| utxo.value()).sum(); let total_output: u64 = tx.output.iter().map(|out| out.value).sum(); @@ -746,8 +806,11 @@ impl TransactionBuilder { /// set is attached), for callers that may later abandon the transaction /// after awaiting a broadcast. See [`Self::build_unsigned_reserved`] for why /// the token is needed and how to release with it. + /// + /// A payload finalizer set via [`Self::set_payload_finalizer`] runs + /// between input selection and input signing. pub async fn build_signed_reserved( - self, + mut self, signer: &S, path_resolver: P, ) -> Result<(Transaction, u64, Option), BuilderError> @@ -755,14 +818,15 @@ impl TransactionBuilder { S: TransactionSigner + ?Sized + Sync, P: Fn(Address) -> Option + Send, { + self.require_placeholder_for_finalizer()?; + let finalizer = self.payload_finalizer.take(); let funding = self.funding.clone(); - let (tx, inputs, reservation) = self.assemble_unsigned()?; + let (mut tx, inputs, reservation) = self.assemble_unsigned()?; let total_input: u64 = inputs.iter().map(|utxo| utxo.value()).sum(); - // Signing never reaches the network for a local key, but an external - // signer can fail. A failed sign means the reserved inputs are still - // spendable, so release them now instead of stranding the funds until - // the TTL backstop reclaims them. + // A failure past this point — payload finalization or signing — leaves + // the reserved inputs spendable, so release them now instead of + // stranding the funds until the TTL backstop reclaims them. // // Release owner-guarded: `sign_tx` is an `.await`, and while it runs the // TTL sweep could reclaim this build's reservation and a concurrent @@ -771,14 +835,21 @@ impl TransactionBuilder { // inputs (the double-spend window of `dashpay/platform#4185`), so we // release only outpoints still owned by the token this build stamped. let reserved: Vec = inputs.iter().map(|utxo| utxo.outpoint).collect(); + + // Input sighashes cover the finalized payload (the legacy sighash + // consensus-encodes the whole transaction, payload included), so the + // finalizer must run before any input is signed. + if let Some(finalize_payload) = finalizer { + if let Err(err) = Self::finalize_payload_in(&mut tx, finalize_payload) { + Self::release_reservation(&funding, &reserved, reservation); + return Err(err); + } + } + let tx = match signer.sign_tx(tx, inputs, path_resolver).await { Ok(tx) => tx, Err(err) => { - if let Some(token) = reservation { - for (reservations, _) in &funding { - reservations.release_if_owner(&reserved, token); - } - } + Self::release_reservation(&funding, &reserved, reservation); return Err(err); } }; @@ -788,42 +859,10 @@ impl TransactionBuilder { Ok((tx, total_input.saturating_sub(total_output), reservation)) } - /// Like [`Self::build_signed_reserved`], but with a payload-finalization - /// seam between input selection and input signing, for special - /// transactions whose payload commits to the chosen inputs and is itself - /// signed — a ProUpServTx's `inputs_hash` + operator-BLS `payload_sig` - /// being the motivating case. - /// - /// The builder must be given a placeholder payload via - /// [`Self::set_special_payload`]: the same variant with every - /// selection-dependent field (inputs hash, payload signature) zeroed, so - /// coin selection prices the payload bytes into the fee. After selection - /// reserves the chosen inputs, `finalize_payload` receives the unsigned - /// transaction — inputs chosen and BIP-69 sorted, placeholder payload - /// still attached — and returns the finalized payload; the builder - /// installs it and only then computes each input's sighash, which covers - /// the finalized payload. Because that payload signature commits to the - /// input set, the input set is frozen from the finalizer on: there is no - /// fee-bump or input-substitution path for such a transaction. - /// - /// Fails without running the finalizer when no placeholder payload was - /// set, and fails after it when the finalized payload is a different - /// variant or estimates larger than the placeholder (the fee was fixed at - /// selection time, so growth would underpay the configured fee rate). On - /// any failure — finalizer error, guard, or signing — the reservation is - /// released owner-guarded, exactly as in [`Self::build_signed_reserved`]. - pub async fn build_signed_reserved_with_payload_finalizer( - self, - signer: &S, - path_resolver: P, - finalize_payload: F, - ) -> Result<(Transaction, u64, Option), BuilderError> - where - S: TransactionSigner + ?Sized + Sync, - P: Fn(Address) -> Option + Send, - F: FnOnce(&Transaction) -> Result + Send, - { - if self.special_payload.is_none() { + /// A payload finalizer without a placeholder payload cannot be priced into + /// the fee — refuse before selection reserves anything. + fn require_placeholder_for_finalizer(&self) -> Result<(), BuilderError> { + if self.payload_finalizer.is_some() && self.special_payload.is_none() { return Err(BuilderError::InvalidData( "payload finalizer requires a placeholder payload: call set_special_payload with \ the same variant (selection-dependent fields zeroed) so selection prices its \ @@ -831,46 +870,29 @@ impl TransactionBuilder { .into(), )); } + Ok(()) + } - let funding = self.funding.clone(); - let (mut tx, inputs, reservation) = self.assemble_unsigned()?; - let total_input: u64 = inputs.iter().map(|utxo| utxo.value()).sum(); - // Same owner-guarded release discipline as `build_signed_reserved`: - // a failure past this point leaves the inputs spendable, so free them - // for the next build instead of stranding them until the TTL backstop. - let reserved: Vec = inputs.iter().map(|utxo| utxo.outpoint).collect(); - fn release( - funding: &[(ReservationSet, HashSet)], - reserved: &[OutPoint], - reservation: Option, - ) { - if let Some(token) = reservation { - for (reservations, _) in funding { - reservations.release_if_owner(reserved, token); - } - } - } - - let finalized = match finalize_payload(&tx) { - Ok(payload) => payload, - Err(err) => { - release(&funding, &reserved, reservation); - return Err(err); - } - }; + /// Run the payload finalizer against the assembled unsigned transaction + /// and install the finalized payload, holding it to the placeholder's + /// variant and estimated size (the fee was fixed at selection time, so + /// growth would underpay the configured fee rate). + fn finalize_payload_in( + tx: &mut Transaction, + finalize_payload: PayloadFinalizer, + ) -> Result<(), BuilderError> { + let finalized = finalize_payload(tx)?; // `assemble_unsigned` carries the placeholder into the transaction // unchanged for every payload type (only an asset-lock drain rewrites // one, and that variant has no selection-dependent fields to // finalize). Guard rather than unwrap: this is library code. let Some(placeholder) = tx.special_transaction_payload.as_ref() else { - release(&funding, &reserved, reservation); return Err(BuilderError::InvalidData( "placeholder payload missing from the assembled transaction".into(), )); }; if core::mem::discriminant(&finalized) != core::mem::discriminant(placeholder) { - release(&funding, &reserved, reservation); return Err(BuilderError::InvalidData( "finalized payload is a different variant than the placeholder the fee was \ selected for" @@ -880,28 +902,29 @@ impl TransactionBuilder { let placeholder_size = Self::estimated_payload_size(placeholder); let finalized_size = Self::estimated_payload_size(&finalized); if finalized_size > placeholder_size { - release(&funding, &reserved, reservation); return Err(BuilderError::InvalidData(format!( "finalized payload estimates {finalized_size} bytes, larger than the \ {placeholder_size}-byte placeholder the fee was selected for" ))); } tx.special_transaction_payload = Some(finalized); - - // Input sighashes cover the finalized payload (the legacy sighash - // consensus-encodes the whole transaction, payload included), so - // signing must come after the payload is installed. - let tx = match signer.sign_tx(tx, inputs, path_resolver).await { - Ok(tx) => tx, - Err(err) => { - release(&funding, &reserved, reservation); - return Err(err); + Ok(()) + } + + /// Owner-guarded release of a build's reserved inputs, for every failure + /// path after `assemble_unsigned` has reserved them. See the comment in + /// [`Self::build_signed_reserved`] for why the release must be + /// owner-guarded (`dashpay/platform#4185`). + fn release_reservation( + funding: &[(ReservationSet, HashSet)], + reserved: &[OutPoint], + reservation: Option, + ) { + if let Some(token) = reservation { + for (reservations, _) in funding { + reservations.release_if_owner(reserved, token); } - }; - - let total_output: u64 = tx.output.iter().map(|out| out.value).sum(); - - Ok((tx, total_input.saturating_sub(total_output), reservation)) + } } } @@ -2292,29 +2315,26 @@ mod tests { .add_funding(&mut funds, &account) .set_change_address(Address::dummy(Network::Testnet, 1)) .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) - .build_signed_reserved_with_payload_finalizer( - &ctx.wallet, - |_addr| Some(DerivationPath::master()), - |unsigned| { - // The finalizer sees the selected inputs (unsigned) and the - // placeholder payload, exactly what inputs_hash + a payload - // signature need. - assert!(!unsigned.input.is_empty(), "inputs are selected before finalizing"); - assert!( - unsigned.input.iter().all(|input| input.script_sig.is_empty()), - "inputs must not be signed before the payload is finalized" - ); - let Some(TransactionPayload::ProviderUpdateServicePayloadType(placeholder)) = - &unsigned.special_transaction_payload - else { - panic!("placeholder payload must still be attached"); - }; - let mut finalized = placeholder.clone(); - finalized.inputs_hash = unsigned.hash_inputs(); - finalized.payload_sig = BLSSignature::from([0xAB; 96]); - Ok(TransactionPayload::ProviderUpdateServicePayloadType(finalized)) - }, - ) + .set_payload_finalizer(|unsigned| { + // The finalizer sees the selected inputs (unsigned) and the + // placeholder payload, exactly what inputs_hash + a payload + // signature need. + assert!(!unsigned.input.is_empty(), "inputs are selected before finalizing"); + assert!( + unsigned.input.iter().all(|input| input.script_sig.is_empty()), + "inputs must not be signed before the payload is finalized" + ); + let Some(TransactionPayload::ProviderUpdateServicePayloadType(placeholder)) = + &unsigned.special_transaction_payload + else { + panic!("placeholder payload must still be attached"); + }; + let mut finalized = placeholder.clone(); + finalized.inputs_hash = unsigned.hash_inputs(); + finalized.payload_sig = BLSSignature::from([0xAB; 96]); + Ok(TransactionPayload::ProviderUpdateServicePayloadType(finalized)) + }) + .build_signed_reserved(&ctx.wallet, |_addr| Some(DerivationPath::master())) .await .expect("finalized build signs"); @@ -2354,11 +2374,10 @@ mod tests { .add_funding(&mut funds, &account) .set_change_address(Address::dummy(Network::Testnet, 1)) .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) - .build_signed_reserved_with_payload_finalizer( - &ctx.wallet, - |_addr| Some(DerivationPath::master()), - |_unsigned| Err(BuilderError::SigningFailed("operator key rejected".into())), - ) + .set_payload_finalizer(|_unsigned| { + Err(BuilderError::SigningFailed("operator key rejected".into())) + }) + .build_signed_reserved(&ctx.wallet, |_addr| Some(DerivationPath::master())) .await; assert!(matches!(result, Err(BuilderError::SigningFailed(_)))); @@ -2387,21 +2406,18 @@ mod tests { .add_funding(&mut funds, &account) .set_change_address(Address::dummy(Network::Testnet, 1)) .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) - .build_signed_reserved_with_payload_finalizer( - &ctx.wallet, - |_addr| Some(DerivationPath::master()), - |unsigned| { - Ok(TransactionPayload::ProviderUpdateRevocationPayloadType( - ProviderUpdateRevocationPayload { - version: ProviderUpdateRevocationPayload::CURRENT_VERSION, - pro_tx_hash: Txid::all_zeros(), - reason: 0, - inputs_hash: unsigned.hash_inputs(), - payload_sig: BLSSignature::from([0; 96]), - }, - )) - }, - ) + .set_payload_finalizer(|unsigned| { + Ok(TransactionPayload::ProviderUpdateRevocationPayloadType( + ProviderUpdateRevocationPayload { + version: ProviderUpdateRevocationPayload::CURRENT_VERSION, + pro_tx_hash: Txid::all_zeros(), + reason: 0, + inputs_hash: unsigned.hash_inputs(), + payload_sig: BLSSignature::from([0; 96]), + }, + )) + }) + .build_signed_reserved(&ctx.wallet, |_addr| Some(DerivationPath::master())) .await; assert!(matches!(result, Err(BuilderError::InvalidData(_)))); @@ -2425,15 +2441,12 @@ mod tests { .set_change_address(Address::dummy(Network::Testnet, 1)) // Placeholder priced with an empty payout script... .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) - .build_signed_reserved_with_payload_finalizer( - &ctx.wallet, - |_addr| Some(DerivationPath::master()), - // ...but finalized with a 25-byte P2PKH script the fee never - // paid for. - |_unsigned| { - Ok(pro_up_serv_placeholder(Address::dummy(Network::Testnet, 2).script_pubkey())) - }, - ) + // ...but finalized with a 25-byte P2PKH script the fee never + // paid for. + .set_payload_finalizer(|_unsigned| { + Ok(pro_up_serv_placeholder(Address::dummy(Network::Testnet, 2).script_pubkey())) + }) + .build_signed_reserved(&ctx.wallet, |_addr| Some(DerivationPath::master())) .await; assert!(matches!(result, Err(BuilderError::InvalidData(_)))); @@ -2456,11 +2469,10 @@ mod tests { .add_funding(&mut funds, &account) .set_change_address(Address::dummy(Network::Testnet, 1)) .add_output(&Address::dummy(Network::Testnet, 0), 500_000) - .build_signed_reserved_with_payload_finalizer( - &ctx.wallet, - |_addr| Some(DerivationPath::master()), - |_unsigned| panic!("finalizer must not run without a placeholder"), - ) + .set_payload_finalizer(|_unsigned| { + panic!("finalizer must not run without a placeholder") + }) + .build_signed_reserved(&ctx.wallet, |_addr| Some(DerivationPath::master())) .await; assert!(matches!(result, Err(BuilderError::InvalidData(_)))); @@ -2470,6 +2482,55 @@ mod tests { ); } + /// The finalizer is builder state, not a build entry point: the unsigned + /// build applies it too, returning a finalized-but-unsigned transaction + /// ready for external input signing. + #[test] + fn payload_finalizer_runs_in_the_unsigned_build_too() { + use dashcore::bls_sig_utils::BLSSignature; + + let ctx = TestWalletContext::new_random(); + let account = + ctx.wallet.accounts.standard_bip44_accounts.get(&0).expect("BIP44 account").clone(); + + let mut funds = ManagedCoreFundsAccount::dummy_bip44(); + let utxo = Utxo::dummy(0x01, 1_000_000, 100, false, true); + funds.utxos.insert(utxo.outpoint, utxo.clone()); + + let (tx, _fee, token) = TransactionBuilder::new() + .set_current_height(200) + .set_fee_rate(FeeRate::normal()) + .add_funding(&mut funds, &account) + .set_change_address(Address::dummy(Network::Testnet, 1)) + .set_special_payload(pro_up_serv_placeholder(ScriptBuf::new())) + .set_payload_finalizer(|unsigned| { + let Some(TransactionPayload::ProviderUpdateServicePayloadType(placeholder)) = + &unsigned.special_transaction_payload + else { + panic!("placeholder payload must still be attached"); + }; + let mut finalized = placeholder.clone(); + finalized.inputs_hash = unsigned.hash_inputs(); + finalized.payload_sig = BLSSignature::from([0xCD; 96]); + Ok(TransactionPayload::ProviderUpdateServicePayloadType(finalized)) + }) + .build_unsigned_reserved() + .expect("unsigned finalized build"); + + assert!(token.is_some()); + let Some(TransactionPayload::ProviderUpdateServicePayloadType(payload)) = + &tx.special_transaction_payload + else { + panic!("finalized payload must ride the unsigned transaction"); + }; + assert_eq!(payload.inputs_hash, tx.hash_inputs()); + assert_eq!(payload.payload_sig, BLSSignature::from([0xCD; 96])); + assert!( + tx.input.iter().all(|input| input.script_sig.is_empty()), + "the unsigned build leaves input signing to the caller" + ); + } + /// The fee a finalizer-seam build pays must equal what the ordinary path /// charges for the same placeholder: the finalized payload swaps in at /// identical estimated size, so nothing about selection or change moves.