From 3c9188caec55dfe6b2dfd258183ab2545cf5b58e Mon Sep 17 00:00:00 2001 From: benny b Date: Thu, 15 Oct 2026 14:16:32 -0400 Subject: [PATCH 1/2] ddk-dlc: report funding shortfalls as a typed error and expose funding requirements A party whose inputs could not cover its collateral plus its fee share got an InvalidArgument string, so callers parsed `required_input_funds=` out of it, and coin selection called the public get_change_output_and_fees_with_counterparty with a fabricated input amount. ddk-dlc now returns Error::InsufficientFunds { required, available } and prices a party with PartyParams::funding_requirements, which the fund transaction constructor uses, so there is one fee formula. It replaces get_change_output_and_fees_with_counterparty and the private change_output_and_fees; get_change_output_and_fees stays as a wrapper over it. Who pays what is unchanged, including the zero-collateral party's zero fees and zero-value change. funding_requirements takes the payout script lengths rather than a counterparty script, because #224 prices the CET fee for the longest script a CET pays the accepting party, which a payout script override on the offer can make longer than its payout script. A requirement priced from the payout scripts alone would fall short for such an offer. The spliced constructor's input layout moves into PartyParams::spliced, so the constructor and the requirement for a splicing party lay out inputs in one place. Spliced contracts rebuild as before. #224 also moved the transaction builder into ddk-manager, so the offering party's price lives next to it: transactions::offer_funding_requirements shares the override fold with build_contract_transactions and lays out a splice offer as the builder funds it. ddk's contract::funding_requirements(offer, accept_payout_spk) wraps it, so it prices the offering party exactly as accept_offer will, override and splice inputs included. ddk maps the error to ContractError::InsufficientFunds (also when it arrives wrapped in a ddk-manager error). --- ddk-manager/src/contract/transactions.rs | 54 ++- ddk/src/contract/create.rs | 40 +- ddk/src/contract/error.rs | 27 +- ddk/src/contract/mod.rs | 14 +- ddk/tests/stateless.rs | 214 ++++++++- dlc/src/lib.rs | 549 +++++++++++++++++------ 6 files changed, 744 insertions(+), 154 deletions(-) diff --git a/ddk-manager/src/contract/transactions.rs b/ddk-manager/src/contract/transactions.rs index fb496cc3..c7d47809 100644 --- a/ddk-manager/src/contract/transactions.rs +++ b/ddk-manager/src/contract/transactions.rs @@ -12,10 +12,10 @@ use std::fmt; use std::ops::Range; use bitcoin::Amount; -use ddk_dlc::{DlcTransactions, FeeRule, PartyParams}; +use ddk_dlc::{DlcTransactions, FeeRule, FundingRequirements, PartyParams}; use ddk_messages::{OfferDlc, TlvStream}; -use super::cet_records::{cet_records, CetContext}; +use super::cet_records::{cet_records, CetContext, CetRecord}; use super::contract_info::ContractInfo; use super::offered_contract::OfferedContract; use crate::error::Error; @@ -172,12 +172,7 @@ pub fn build_contract_transactions( record.validate(contract_infos, terms.total_collateral)?; } - // A record may make a CET pay the accepting party's amount to a longer - // script than its own; the CET fee is reserved for the longest. - let accept_cet_script_len = records - .iter() - .map(|record| record.accept_payout_script_len()) - .fold(accept.payout_script_pubkey.len(), usize::max); + let accept_cet_script_len = accept_cet_script_len(&records, accept.payout_script_pubkey.len()); let payouts = contract_infos[0].get_payouts(terms.total_collateral)?; // A splice input carries a `dlc_input`; when present the funding @@ -257,3 +252,46 @@ pub fn build_contract_transactions( cet_ranges, }) } + +/// What the offering party's inputs must hold to fund `offer`'s side of a +/// contract with an accepting party paid at a script of +/// `accept_payout_script_len` bytes, and the fees that side pays. +/// +/// This prices the offering party as [`build_contract_transactions`] funds it +/// for a new contract, so a coin selector can be exact before any accept +/// exists: the CET records on the offer can make a CET pay the accepting +/// party a longer script, and a splice input changes the inputs the funding +/// transaction spends. An offer that funds the whole contract pays for both. +pub fn offer_funding_requirements( + offer: &PartyParams, + accept_payout_script_len: usize, + offer_tlvs: &TlvStream, + terms: &ContractTerms, +) -> Result { + let records = cet_records(offer_tlvs)?; + // An offer with a splice input is built by the spliced constructor, which + // lays out its inputs this way. + let offer = if offer.dlc_inputs.is_empty() { + offer.clone() + } else { + offer.spliced() + }; + Ok(offer.funding_requirements( + terms.total_collateral, + terms.fee_rate_per_vb, + Amount::ZERO, + offer.payout_script_pubkey.len(), + accept_cet_script_len(&records, accept_payout_script_len), + FeeRule::default(), + )?) +} + +/// The longest script a CET pays the accepting party: its payout script of +/// `accept_payout_script_len` bytes, or a longer one a record makes a CET pay +/// in its place. The CET fee is reserved for it. +fn accept_cet_script_len(records: &[Box], accept_payout_script_len: usize) -> usize { + records + .iter() + .map(|record| record.accept_payout_script_len()) + .fold(accept_payout_script_len, usize::max) +} diff --git a/ddk/src/contract/create.rs b/ddk/src/contract/create.rs index d5ab5452..a27ba695 100644 --- a/ddk/src/contract/create.rs +++ b/ddk/src/contract/create.rs @@ -1,10 +1,13 @@ //! Offer creation and validation. +use bitcoin::Script; use ddk_dlc::secp256k1_zkp::Secp256k1; +use ddk_dlc::FundingRequirements; use ddk_manager::contract::cet_records::validate_cet_records; +use ddk_manager::contract::transactions::offer_funding_requirements; use ddk_messages::OfferDlc; -use super::context::{ensure_protocol_version, validate_offer_funding_inputs}; +use super::context::{dlc_party_params, ensure_protocol_version, validate_offer_funding_inputs}; use super::error::ContractError; use super::types::{random_serial_id, random_temporary_contract_id, CreateOfferParams}; use super::PROTOCOL_VERSION; @@ -97,6 +100,41 @@ pub fn create_offer(params: CreateOfferParams) -> Result Result { + let params = dlc_party_params( + offer.funding_pubkey, + offer.payout_spk.clone(), + offer.payout_serial_id, + offer.change_spk.clone(), + offer.change_serial_id, + offer.offer_collateral, + &offer.funding_inputs, + )?; + Ok(offer_funding_requirements( + ¶ms, + accept_payout_spk.len(), + &offer.tlvs, + &offer.into(), + )?) +} + /// Validates an incoming offer's structure, oracle announcements, and timeout policy. /// /// `min_timeout_interval` and `max_timeout_interval` bound the distance between diff --git a/ddk/src/contract/error.rs b/ddk/src/contract/error.rs index 706dcfb0..3dc91218 100644 --- a/ddk/src/contract/error.rs +++ b/ddk/src/contract/error.rs @@ -1,5 +1,6 @@ //! Errors returned by the stateless contract API. +use bitcoin::Amount; use thiserror::Error; /// Errors returned by the stateless contract functions. @@ -52,6 +53,16 @@ pub enum ContractError { /// BIP32 key derivation failed. #[error("BIP32 error: {0}")] Bip32(String), + /// A party's funding inputs hold less than its collateral plus its share + /// of the fees. [`funding_requirements`](super::funding_requirements) + /// gives the offering party's requirement before the contract is built. + #[error("insufficient funds: the funding inputs hold {available} but {required} is required")] + InsufficientFunds { + /// The input total the party needs. + required: Amount, + /// The input total the party has. + available: Amount, + }, /// A DLC transaction or signature operation failed. #[error("DLC error: {0}")] Dlc(String), @@ -69,13 +80,25 @@ impl From for ContractError { impl From for ContractError { fn from(error: ddk_dlc::Error) -> Self { - ContractError::Dlc(error.to_string()) + match error { + ddk_dlc::Error::InsufficientFunds { + required, + available, + } => ContractError::InsufficientFunds { + required, + available, + }, + error => ContractError::Dlc(error.to_string()), + } } } impl From for ContractError { fn from(error: ddk_manager::error::Error) -> Self { - ContractError::Dlc(error.to_string()) + match error { + ddk_manager::error::Error::DlcError(error) => error.into(), + error => ContractError::Dlc(error.to_string()), + } } } diff --git a/ddk/src/contract/mod.rs b/ddk/src/contract/mod.rs index b19b2b4e..a5d300f0 100644 --- a/ddk/src/contract/mod.rs +++ b/ddk/src/contract/mod.rs @@ -82,6 +82,17 @@ //! signatures. External signers can fund with any script type they can //! finalize themselves. //! +//! # Funding requirements +//! +//! Each party's funding inputs must hold its collateral plus its share of the +//! funding and CET fees. [`funding_requirements`](crate::contract::funding_requirements) +//! prices the offering party's share from the offer and the accepting party's +//! payout script, with the same formula the transactions are built with, so +//! coin selection can be exact. A party whose inputs fall short makes +//! [`accept_offer`](crate::contract::accept_offer) fail with +//! [`ContractError::InsufficientFunds`](crate::contract::ContractError::InsufficientFunds), +//! which carries the amount required and the amount the inputs hold. +//! //! # Splicing //! //! A new contract can spend a previous contract's 2-of-2 funding output as an @@ -165,7 +176,8 @@ mod types; mod tests; pub use accept::{accept_offer, create_dlc_transactions, create_signed_dlc_transactions}; -pub use create::{create_offer, validate_offer}; +pub use create::{create_offer, funding_requirements, validate_offer}; +pub use ddk_dlc::FundingRequirements; pub use error::ContractError; pub use finalize::{finalize_sign, finalize_sign_spliced}; pub use keys::{ContractKeyProvider, KeyScheme}; diff --git a/ddk/tests/stateless.rs b/ddk/tests/stateless.rs index c53b86dc..21510c69 100644 --- a/ddk/tests/stateless.rs +++ b/ddk/tests/stateless.rs @@ -15,9 +15,10 @@ use bitcoin::{Amount, Network, OutPoint, ScriptBuf, Sequence, Transaction, TxIn, use ddk::contract::{ accept_offer, chain_hash_from_network, create_dlc_splice_input, create_dlc_transactions, create_funding_psbt, create_offer, create_signed_dlc_transactions, finalize_sign, - finalize_sign_spliced, funding_input, sign_accept, sign_accept_spliced, sign_cet, sign_refund, - signing, validate_offer, AcceptOfferParams, ContractError, CreateOfferParams, DescriptorInput, - DlcInputSigningKey, InputDerivation, Party, PartyParams, DLC_INPUT_MAX_WITNESS_LEN, + finalize_sign_spliced, funding_input, funding_requirements, sign_accept, sign_accept_spliced, + sign_cet, sign_refund, signing, validate_offer, AcceptOfferParams, ContractError, + CreateOfferParams, DescriptorInput, DlcInputSigningKey, InputDerivation, Party, PartyParams, + DLC_INPUT_MAX_WITNESS_LEN, }; use ddk_dlc::secp256k1_zkp::{All, Keypair, PublicKey, Secp256k1, SecretKey, XOnlyPublicKey}; use ddk_messages::contract_msgs::{ @@ -1800,6 +1801,177 @@ fn splice_out_completes_the_lifecycle() { ); } +/// Funds the single-funded offer that `offer_with_coin` builds with one wallet +/// coin on top of inputs worth `other_inputs_value`. A coin bringing the inputs +/// to exactly [`funding_requirements`] is accepted with no change left over; +/// one sat less fails with the requirement and what the inputs hold, so the +/// requirement is neither under nor over the real one. +fn assert_funding_requirement_is_exact( + secp: &Secp256k1, + accepter: &PartySetup, + other_inputs_value: Amount, + offer_with_coin: impl Fn(Amount) -> OfferDlc, +) { + let accept = |offer: &OfferDlc| { + accept_offer( + offer, + AcceptOfferParams { + party: accepter.party_params(secp, vec![]), + min_timeout_interval: MIN_TIMEOUT, + max_timeout_interval: MAX_TIMEOUT, + now_unix: NOW_UNIX, + }, + &accepter.funding_secret_key, + ) + }; + // The requirement depends on the coins' weights, not their values, so an + // offer over a one-sat coin prices the real one. + let required = funding_requirements(&offer_with_coin(Amount::ONE_SAT), &accepter.payout_spk) + .unwrap() + .required; + let exact_coin = required - other_inputs_value; + + let exact = accept(&offer_with_coin(exact_coin)).unwrap(); + assert_eq!( + exact.transactions.fund.output.len(), + 1, + "no change is left over" + ); + + let short = accept(&offer_with_coin(exact_coin - Amount::ONE_SAT)); + assert!( + matches!( + short, + Err(ContractError::InsufficientFunds { required: r, available }) + if r == required && available == required - Amount::ONE_SAT + ), + "expected the shortfall, got {:?}", + short.err() + ); +} + +#[test] +fn an_offer_funded_with_exactly_its_requirement_is_accepted() { + let secp = Secp256k1::new(); + let accepter = PartySetup::new(&secp, 34, NETWORK, Amount::from_sat(150_000), 2); + assert_funding_requirement_is_exact(&secp, &accepter, Amount::ZERO, |coin_value| { + let offerer = PartySetup::new(&secp, 33, NETWORK, coin_value, 1); + create_offer(offer_params( + &secp, + &offerer, + enum_contract_info(TOTAL_COLLATERAL), + TOTAL_COLLATERAL, + NETWORK, + vec![offerer.funding_input.clone()], + )) + .unwrap() + }); +} + +/// A splice input spending the funding output of a freshly signed contract, +/// and that output's value. +fn funded_splice_input(secp: &Secp256k1) -> (FundingInput, Amount) { + let (offerer, accepter, offer, accept) = enum_contract(secp, NETWORK); + let (sign, _) = fund_with_xpriv(secp, &offerer, &accepter, &offer, &accept); + let value = create_dlc_transactions(&offer, &accept) + .unwrap() + .get_fund_output() + .value; + let input = create_dlc_splice_input( + &offer, + &accept, + &sign, + Party::Offer, + Some(900), + DLC_INPUT_MAX_WITNESS_LEN, + ) + .unwrap(); + (input, value) +} + +#[test] +fn a_splice_offer_funded_with_exactly_its_requirement_is_accepted() { + let secp = Secp256k1::new(); + let (splice_input, splice_value) = funded_splice_input(&secp); + let collateral = splice_value + Amount::from_sat(40_000); + + let accepter = PartySetup::new(&secp, 6, NETWORK, Amount::from_sat(200_000), 11); + assert_funding_requirement_is_exact(&secp, &accepter, splice_value, |coin_value| { + let offerer = PartySetup::new(&secp, 5, NETWORK, coin_value, 10); + create_offer(offer_params( + &secp, + &offerer, + enum_contract_info(collateral), + collateral, + NETWORK, + vec![splice_input.clone(), offerer.funding_input.clone()], + )) + .unwrap() + }); +} + +/// The spliced constructor leaves out an ordinary input whose witness is +/// larger than a P2WPKH witness, and the requirement does too: an offer one +/// sat short of it is refused with exactly that requirement. +#[test] +fn a_splice_offer_is_priced_as_the_spliced_constructor_funds_it() { + let secp = Secp256k1::new(); + let (splice_input, splice_value) = funded_splice_input(&secp); + let collateral = splice_value + Amount::from_sat(40_000); + let wide_value = Amount::from_sat(30_000); + let wide_script = ScriptBuf::new_p2wsh(&bitcoin::WScriptHash::from_byte_array([3; 32])); + let wide_input = funding_input( + &previous_transaction(wide_value, wide_script), + 0, + Some(12), + u32::MAX, + 200, + ScriptBuf::new(), + ) + .unwrap(); + + let accepter = PartySetup::new(&secp, 6, NETWORK, Amount::from_sat(200_000), 11); + let offer_with_coin = |coin_value| { + let offerer = PartySetup::new(&secp, 5, NETWORK, coin_value, 10); + create_offer(offer_params( + &secp, + &offerer, + enum_contract_info(collateral), + collateral, + NETWORK, + vec![ + splice_input.clone(), + wide_input.clone(), + offerer.funding_input.clone(), + ], + )) + .unwrap() + }; + let required = funding_requirements(&offer_with_coin(Amount::ONE_SAT), &accepter.payout_spk) + .unwrap() + .required; + let one_sat_short = required - Amount::ONE_SAT; + let short = accept_offer( + &offer_with_coin(one_sat_short - splice_value - wide_value), + AcceptOfferParams { + party: accepter.party_params(&secp, vec![]), + min_timeout_interval: MIN_TIMEOUT, + max_timeout_interval: MAX_TIMEOUT, + now_unix: NOW_UNIX, + }, + &accepter.funding_secret_key, + ); + assert!( + matches!( + short, + Err(ContractError::InsufficientFunds { required: r, available }) + if r == required && available == one_sat_short + ), + "expected the shortfall, got {:?}", + short.err() + ); +} + #[test] fn finalize_sign_spliced_rejects_a_wrong_prior_key() { let prepared = prepare_splice(true); @@ -2585,6 +2757,42 @@ fn a_payout_script_override_pays_the_named_outcome_to_the_script() { } } +/// An offer that funds the whole contract pays the CET fee for the accepting +/// party's output, and an override can make that output a longer script. The +/// requirement prices the longer script: it is above the one without the +/// override, and an offer funded at exactly it is accepted while one sat less +/// is short. +#[test] +fn an_offer_with_a_longer_override_script_is_priced_for_it() { + let secp = Secp256k1::new(); + let accepter = PartySetup::new(&secp, 76, NETWORK, Amount::from_sat(150_000), 2); + let longer_script = ScriptBuf::new_p2wsh(&bitcoin::WScriptHash::from_byte_array([0x42; 32])); + assert!(longer_script.len() > accepter.payout_spk.len()); + let overrides = payout_overrides(&[(outcome("down"), longer_script)]); + let offer_with_coin = |coin_value| { + let offerer = PartySetup::new(&secp, 75, NETWORK, coin_value, 1); + offer_with_overrides( + &secp, + &offerer, + enum_contract_info(TOTAL_COLLATERAL), + TOTAL_COLLATERAL, + &overrides, + ) + }; + + let offer = offer_with_coin(Amount::ONE_SAT); + let mut without_overrides = offer.clone(); + without_overrides.tlvs = Default::default(); + let required = |offer: &OfferDlc| { + funding_requirements(offer, &accepter.payout_spk) + .unwrap() + .required + }; + assert!(required(&offer) > required(&without_overrides)); + + assert_funding_requirement_is_exact(&secp, &accepter, Amount::ZERO, offer_with_coin); +} + /// Both parties may pay out to the same script. The record replaces the /// accepting party's output, not whichever output happens to pay that script. #[test] diff --git a/dlc/src/lib.rs b/dlc/src/lib.rs index 55831f45..14df4579 100644 --- a/dlc/src/lib.rs +++ b/dlc/src/lib.rs @@ -231,6 +231,14 @@ pub enum Error { Miniscript(miniscript::Error), /// Error attempting to do an out of bounds access on the transaction inputs vector. InputsIndex(bitcoin::transaction::InputsIndexError), + /// A party's inputs hold less than its collateral plus its share of the + /// fees. See [`PartyParams::funding_requirements`]. + InsufficientFunds { + /// The input total the party needs. + required: Amount, + /// The input total the party has. + available: Amount, + }, } impl From for Error { @@ -271,6 +279,13 @@ impl fmt::Display for Error { Error::P2wpkh(ref e) => write!(f, "Error while computing p2wpkh sighash: {e}"), Error::InputsIndex(ref e) => write!(f, "Error ordering inputs: {e}"), Error::Miniscript(_) => write!(f, "Error within miniscript"), + Error::InsufficientFunds { + required, + available, + } => write!( + f, + "Insufficient funds: the inputs hold {available} but {required} is required" + ), } } } @@ -284,10 +299,40 @@ impl std::error::Error for Error { Error::InputsIndex(e) => Some(e), Error::InvalidArgument(_) => None, Error::Miniscript(e) => Some(e), + Error::InsufficientFunds { .. } => None, } } } +/// What one party's inputs must hold to fund its side of a contract, and the +/// fees that side pays. See [`PartyParams::funding_requirements`]. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub struct FundingRequirements { + /// The input total that exactly funds this party: its collateral plus + /// `fund_fee`, `cet_fee` and any extra fee. + pub required: Amount, + /// This party's share of the funding transaction fee. + pub fund_fee: Amount, + /// This party's share of the CET or refund transaction fee. The funding + /// output carries it until the contract settles. + pub cet_fee: Amount, +} + +impl FundingRequirements { + /// The change left from `available` input funds once this share is paid. + /// + /// Fails with [`Error::InsufficientFunds`] when `available` is below + /// [`FundingRequirements::required`]. + pub fn change(&self, available: Amount) -> Result { + available + .checked_sub(self.required) + .ok_or(Error::InsufficientFunds { + required: self.required, + available, + }) + } +} + /// Contains the parameters required for creating DLC transactions for a single /// party. Specifically these are the common fields between Offer and Accept /// messages. @@ -323,73 +368,75 @@ impl PartyParams { /// they are required to pay for the fund transaction and the cet or refund transaction. /// The change output value already accounts for the required fees. /// If input amount (sum of all input values) is lower than the sum of the collateral - /// plus the required fees, an error is returned. + /// plus the required fees, [`Error::InsufficientFunds`] is returned. + /// + /// The CET fee is priced as under [`FeeRule::OwnPayoutOnly`]; see + /// [`PartyParams::funding_requirements`] for the other rule. pub fn get_change_output_and_fees( &self, total_collateral: Amount, fee_rate_per_vb: u64, extra_fee: Amount, ) -> Result<(TxOut, Amount, Amount), Error> { - self.get_change_output_and_fees_with_counterparty( + let requirements = self.funding_requirements( total_collateral, fee_rate_per_vb, extra_fee, - None, - ) + self.payout_script_pubkey.len(), + 0, + FeeRule::OwnPayoutOnly, + )?; + Ok(( + self.change_output(&requirements)?, + requirements.fund_fee, + requirements.cet_fee, + )) } - /// Like [`PartyParams::get_change_output_and_fees`], but when this party - /// funds the whole contract its CET fee also prices the counterparty's - /// payout output. + /// What this party's inputs must hold to fund its side of a contract with + /// `total_collateral`, and the fees that side pays. + /// + /// This is the fee formula the transaction constructors use, so a coin + /// selector can be exact without building a transaction. The result + /// depends on the inputs' weights and the script lengths, never on + /// [`PartyParams::input_amount`]. A party with no collateral pays nothing, + /// and the constructors give it a zero-value change output whatever its + /// inputs hold. + /// + /// A splicing party is priced on [`PartyParams::spliced`], as the spliced + /// constructors fund it. + /// + /// The CET fee is priced from script lengths: `payout_script_len` is the + /// longest script a CET pays this party, and + /// `counterparty_payout_script_len` the longest a CET pays the + /// counterparty. Either is the party's payout script, or a longer one a + /// CET may pay in its place; see [`create_dlc_transactions_with_fee_rule`]. /// /// `CET_BASE_WEIGHT` covers everything in a CET except the payout script /// bytes. In a dual funded contract each party pays half the base weight /// plus its own payout script, which sums to the full CET. In a single - /// funded contract the zero-collateral party pays nothing, so the funding - /// party must also pay for the counterparty's payout script, otherwise the - /// pre-signed CETs carry a lower fee rate than the one negotiated. - /// [`create_dlc_transactions`] always passes the counterparty script. - pub fn get_change_output_and_fees_with_counterparty( - &self, - total_collateral: Amount, - fee_rate_per_vb: u64, - extra_fee: Amount, - counterparty_payout_script_pubkey: Option<&Script>, - ) -> Result<(TxOut, Amount, Amount), Error> { - self.change_output_and_fees( - total_collateral, - fee_rate_per_vb, - extra_fee, - self.payout_script_pubkey.len(), - counterparty_payout_script_pubkey.map(|script_pubkey| script_pubkey.len()), - ) - } - - /// [`PartyParams::get_change_output_and_fees_with_counterparty`] with the - /// CET fee priced from script lengths: `payout_script_len` is the longest - /// script a CET pays this party, and `counterparty_payout_script_len` the - /// longest a CET pays the counterparty. - fn change_output_and_fees( + /// funded contract the zero-collateral party pays nothing, so under + /// [`FeeRule::CounterpartyPayout`] the funding party also pays for the + /// counterparty's script, otherwise the pre-signed CETs carry a lower fee + /// rate than the one negotiated. [`FeeRule::OwnPayoutOnly`] ignores + /// `counterparty_payout_script_len`. + /// + /// `extra_fee` is added to the requirement of a party with collateral; + /// pass [`Amount::ZERO`] for none. + pub fn funding_requirements( &self, total_collateral: Amount, fee_rate_per_vb: u64, extra_fee: Amount, payout_script_len: usize, - counterparty_payout_script_len: Option, - ) -> Result<(TxOut, Amount, Amount), Error> { - let mut inputs_weight: usize = 0; - - // first check if a party does not need to fund the contract if so, then it is zero + counterparty_payout_script_len: usize, + fee_rule: FeeRule, + ) -> Result { if self.collateral == Amount::ZERO { - // We use a zero value output to indicate that the party does not need to fund the contract - let change_output = TxOut { - value: Amount::ZERO, - script_pubkey: self.change_script_pubkey.clone(), - }; - return Ok((change_output, Amount::ZERO, Amount::ZERO)); + return Ok(FundingRequirements::default()); } - inputs_weight += dlc_input::get_dlc_inputs_weight(&self.dlc_inputs); + let mut inputs_weight = dlc_input::get_dlc_inputs_weight(&self.dlc_inputs); for w in &self.inputs { let script_weight = util::redeem_script_to_script_sig(&w.redeem_script)? @@ -452,13 +499,15 @@ impl PartyParams { )))?; // A party funding the whole contract also pays for the counterparty's // payout output, which nobody else prices. - let counterparty_spk_weight = match counterparty_payout_script_len { - Some(script_len) if self.collateral == total_collateral => script_len - .checked_mul(4) - .ok_or(Error::InvalidArgument(format!( - "Counterparty output spk checked multiplication failed: {} * 4", - script_len - )))?, + let counterparty_spk_weight = match fee_rule { + FeeRule::CounterpartyPayout if self.collateral == total_collateral => { + counterparty_payout_script_len + .checked_mul(4) + .ok_or(Error::InvalidArgument(format!( + "Counterparty output spk checked multiplication failed: {} * 4", + counterparty_payout_script_len + )))? + } _ => 0, }; let total_cet_weight = checked_add!( @@ -466,23 +515,53 @@ impl PartyParams { output_spk_weight, counterparty_spk_weight )?; - let cet_or_refund_fee = util::weight_to_fee(total_cet_weight, fee_rate_per_vb)?; - - let required_input_funds = - checked_add!(self.collateral, fund_fee, cet_or_refund_fee, extra_fee)?; - if self.input_amount < required_input_funds { - return Err(Error::InvalidArgument(format!( - "Input amount is less than required input funds. input_amount={} required_input_funds={}", - self.input_amount, required_input_funds - ))); + let cet_fee = util::weight_to_fee(total_cet_weight, fee_rate_per_vb)?; + + Ok(FundingRequirements { + required: checked_add!(self.collateral, fund_fee, cet_fee, extra_fee)?, + fund_fee, + cet_fee, + }) + } + + /// These params with their inputs laid out as + /// [`create_spliced_dlc_transactions`] funds them, which is also how a + /// splicing party's [`PartyParams::funding_requirements`] must be priced. + /// + /// A splice input is listed both in `inputs` and in `dlc_inputs`. The + /// duplicate is removed by dropping every ordinary input whose witness is + /// larger than a P2WPKH witness, and each DLC input is then listed as an + /// ordinary input. An ordinary input with a larger witness is dropped too; + /// spliced contracts have always been built this way, so the rule stays + /// for them to rebuild identically. + pub fn spliced(&self) -> PartyParams { + let mut inputs = self + .inputs + .iter() + .filter(|input| input.max_witness_len <= P2WPKH_WITNESS_SIZE) + .cloned() + .collect::>(); + inputs.extend(self.dlc_inputs.iter().map(TxInputInfo::from)); + PartyParams { + inputs, + dlc_inputs: Vec::new(), + ..self.clone() } + } - let change_output = TxOut { - value: self.input_amount - required_input_funds, - script_pubkey: self.change_script_pubkey.clone(), + /// The change output this party's inputs leave once `requirements` are + /// paid. A party without collateral funds nothing and gets a zero-value + /// change output, which the funding transaction discards as dust. + fn change_output(&self, requirements: &FundingRequirements) -> Result { + let value = if self.collateral == Amount::ZERO { + Amount::ZERO + } else { + requirements.change(self.input_amount)? }; - - Ok((change_output, fund_fee, cet_or_refund_fee)) + Ok(TxOut { + value, + script_pubkey: self.change_script_pubkey.clone(), + }) } fn get_unsigned_tx_inputs_and_serial_ids( @@ -554,43 +633,9 @@ pub fn create_spliced_dlc_transactions_with_fee_rule( fee_rule: FeeRule, accept_cet_script_len: usize, ) -> Result { - // Create enhanced party parameters that include DLC inputs as regular inputs - let mut enhanced_offer_params = offer_params.clone(); - let mut enhanced_accept_params = accept_params.clone(); - - // Filter out DLC inputs from regular inputs to avoid duplicates - // DLC inputs are already included in the inputs[] array, so we need to remove them - // before adding them back from dlc_inputs[] to avoid double-counting - enhanced_offer_params - .inputs - .retain(|input| input.max_witness_len <= 108); - enhanced_accept_params - .inputs - .retain(|input| input.max_witness_len <= 108); - - let offer_dlc_tx_inputs = offer_params - .dlc_inputs - .iter() - .map(|input| input.into()) - .collect::>(); - - let accept_dlc_tx_inputs = accept_params - .dlc_inputs - .iter() - .map(|input| input.into()) - .collect::>(); - - // Add DLC inputs to regular inputs - enhanced_offer_params.inputs.extend(offer_dlc_tx_inputs); - enhanced_accept_params.inputs.extend(accept_dlc_tx_inputs); - - // Clear DLC inputs from enhanced params since they're now regular inputs - enhanced_offer_params.dlc_inputs.clear(); - enhanced_accept_params.dlc_inputs.clear(); - create_dlc_transactions_with_fee_rule( - &enhanced_offer_params, - &enhanced_accept_params, + &offer_params.spliced(), + &accept_params.spliced(), payouts, refund_lock_time, fee_rate_per_vb, @@ -727,36 +772,34 @@ pub fn create_fund_transaction_with_fees_and_rule( ) -> Result<(Transaction, ScriptBuf), Error> { let total_collateral = checked_add!(offer_params.collateral, accept_params.collateral)?; - let counterparty_payout = |script_len| match fee_rule { - FeeRule::CounterpartyPayout => Some(script_len), - FeeRule::OwnPayoutOnly => None, - }; - let (offer_change_output, offer_fund_fee, offer_cet_fee) = offer_params - .change_output_and_fees( - total_collateral, - fee_rate_per_vb, - extra_fee, - offer_params.payout_script_pubkey.len(), - counterparty_payout(accept_cet_script_len), - )?; - let (accept_change_output, accept_fund_fee, accept_cet_fee) = accept_params - .change_output_and_fees( - total_collateral, - fee_rate_per_vb, - extra_fee, - accept_cet_script_len, - counterparty_payout(offer_params.payout_script_pubkey.len()), - )?; + let offer_requirements = offer_params.funding_requirements( + total_collateral, + fee_rate_per_vb, + extra_fee, + offer_params.payout_script_pubkey.len(), + accept_cet_script_len, + fee_rule, + )?; + let offer_change_output = offer_params.change_output(&offer_requirements)?; + let accept_requirements = accept_params.funding_requirements( + total_collateral, + fee_rate_per_vb, + extra_fee, + accept_cet_script_len, + offer_params.payout_script_pubkey.len(), + fee_rule, + )?; + let accept_change_output = accept_params.change_output(&accept_requirements)?; let fund_output_value = checked_add!(offer_params.input_amount, accept_params.input_amount)? - offer_change_output.value - accept_change_output.value - - offer_fund_fee - - accept_fund_fee + - offer_requirements.fund_fee + - accept_requirements.fund_fee - extra_fee; assert_eq!( - total_collateral + offer_cet_fee + accept_cet_fee + extra_fee, + total_collateral + offer_requirements.cet_fee + accept_requirements.cet_fee + extra_fee, fund_output_value ); @@ -765,8 +808,8 @@ pub fn create_fund_transaction_with_fees_and_rule( fund_output_value + offer_change_output.value + accept_change_output.value - + offer_fund_fee - + accept_fund_fee + + offer_requirements.fund_fee + + accept_requirements.fund_fee + extra_fee ); @@ -1788,22 +1831,28 @@ mod tests { let (zero_collateral_party, _) = get_party_params(Amount::ZERO, Amount::ZERO, Some(2)); let total_collateral = Amount::ONE_BTC; - let (_, _, cet_fee) = funding_party - .get_change_output_and_fees_with_counterparty( + let cet_fee = funding_party + .funding_requirements( total_collateral, fee_rate, Amount::ZERO, - Some(&zero_collateral_party.payout_script_pubkey), + funding_party.payout_script_pubkey.len(), + zero_collateral_party.payout_script_pubkey.len(), + FeeRule::CounterpartyPayout, ) - .unwrap(); - let (_, _, counterparty_cet_fee) = zero_collateral_party - .get_change_output_and_fees_with_counterparty( + .unwrap() + .cet_fee; + let counterparty_cet_fee = zero_collateral_party + .funding_requirements( total_collateral, fee_rate, Amount::ZERO, - Some(&funding_party.payout_script_pubkey), + zero_collateral_party.payout_script_pubkey.len(), + funding_party.payout_script_pubkey.len(), + FeeRule::CounterpartyPayout, ) - .unwrap(); + .unwrap() + .cet_fee; // The whole CET, both payout scripts included, is paid by the one // party that funds it. @@ -1825,14 +1874,17 @@ mod tests { get_party_params(Amount::from_sat(150_000_000), Amount::ONE_BTC, Some(2)); let total_collateral = Amount::ONE_BTC + Amount::ONE_BTC; - let (_, _, cet_fee) = party - .get_change_output_and_fees_with_counterparty( + let cet_fee = party + .funding_requirements( total_collateral, fee_rate, Amount::ZERO, - Some(&counterparty.payout_script_pubkey), + party.payout_script_pubkey.len(), + counterparty.payout_script_pubkey.len(), + FeeRule::CounterpartyPayout, ) - .unwrap(); + .unwrap() + .cet_fee; let (_, _, legacy_cet_fee) = party .get_change_output_and_fees(total_collateral, fee_rate, Amount::ZERO) .unwrap(); @@ -1931,6 +1983,21 @@ mod tests { fee_rule: FeeRule, accept_cet_script_len: usize, ) -> DlcTransactions { + try_build_for_accept_cet_script( + offer_params, + accept_params, + fee_rule, + accept_cet_script_len, + ) + .unwrap() + } + + fn try_build_for_accept_cet_script( + offer_params: &PartyParams, + accept_params: &PartyParams, + fee_rule: FeeRule, + accept_cet_script_len: usize, + ) -> Result { let total = offer_params.collateral + accept_params.collateral; let payouts = vec![ Payout { @@ -1955,7 +2022,6 @@ mod tests { fee_rule, accept_cet_script_len, ) - .unwrap() } #[test] @@ -2049,6 +2115,211 @@ mod tests { } } + /// A party without collateral pays no fee and gets no change, whatever its + /// inputs hold: here they pay exactly the extra fee, and the funding + /// transaction keeps only the fund output and the funding party's change. + #[test] + fn a_zero_collateral_party_gets_no_change() { + let extra_fee = Amount::from_sat(10_000); + let (offer_params, _) = + get_party_params(Amount::from_sat(150_000_000), Amount::ONE_BTC, None); + let (accept_params, _) = get_party_params(extra_fee, Amount::ZERO, Some(2)); + let offer = offer_params + .funding_requirements( + Amount::ONE_BTC, + 4, + extra_fee, + offer_params.payout_script_pubkey.len(), + accept_params.payout_script_pubkey.len(), + FeeRule::CounterpartyPayout, + ) + .unwrap(); + + let (fund, funding_witness_script) = + create_fund_transaction_with_fees(&offer_params, &accept_params, 4, 0, 0, extra_fee) + .unwrap(); + + assert_eq!(fund.input.len(), 2); + let output = |script_pubkey: &Script| { + fund.output + .iter() + .find(|output| output.script_pubkey == *script_pubkey) + .map(|output| output.value) + }; + assert_eq!(fund.output.len(), 2); + assert_eq!( + output(&funding_witness_script.to_p2wsh()), + Some(Amount::ONE_BTC + offer.cet_fee + extra_fee) + ); + assert_eq!( + output(&offer_params.change_script_pubkey), + Some(offer_params.input_amount - offer.required) + ); + assert_eq!(output(&accept_params.change_script_pubkey), None); + } + + /// The spliced constructor funds the inputs `PartyParams::spliced` lays + /// out, so a splicing party priced on them and funded at exactly that + /// requirement builds with no change, and one sat less fails with it. Its + /// ordinary input with a witness above P2WPKH's is left out by both. + #[test] + fn a_splicing_party_is_priced_on_its_spliced_inputs() { + let (mut offer_params, _) = get_party_params(Amount::ZERO, Amount::ONE_BTC, None); + let (accept_params, _) = get_party_params(Amount::ZERO, Amount::ZERO, Some(2)); + let splice_amount = Amount::from_sat(50_000_000); + let splice = dlc_input::DlcInputInfo { + fund_tx: Transaction { + version: TX_VERSION, + lock_time: LockTime::ZERO, + input: vec![TxIn::default()], + output: vec![TxOut { + value: splice_amount, + script_pubkey: make_funding_redeemscript( + &offer_params.fund_pubkey, + &accept_params.fund_pubkey, + ) + .to_p2wsh(), + }], + }, + fund_vout: 0, + local_fund_pubkey: offer_params.fund_pubkey, + remote_fund_pubkey: accept_params.fund_pubkey, + fund_amount: splice_amount, + max_witness_len: 220, + input_serial_id: 9, + contract_id: [1; 32], + }; + // A splice input is listed both as an ordinary input and a DLC input. + offer_params.inputs.push(TxInputInfo::from(&splice)); + offer_params.dlc_inputs.push(splice); + offer_params.inputs.push(TxInputInfo { + outpoint: OutPoint { + txid: Txid::from_byte_array([7; 32]), + vout: 0, + }, + max_witness_len: 200, + redeem_script: ScriptBuf::new(), + serial_id: 7, + }); + + let spliced = offer_params.spliced(); + let serial_ids = spliced.inputs.iter().map(|input| input.serial_id); + assert_eq!(serial_ids.collect::>(), vec![1, 9]); + assert!(spliced.dlc_inputs.is_empty()); + let required = spliced + .funding_requirements( + Amount::ONE_BTC, + 4, + Amount::ZERO, + spliced.payout_script_pubkey.len(), + accept_params.payout_script_pubkey.len(), + FeeRule::CounterpartyPayout, + ) + .unwrap() + .required; + + let payouts = vec![ + Payout { + offer: Amount::ONE_BTC, + accept: Amount::ZERO, + }, + Payout { + offer: Amount::ZERO, + accept: Amount::ONE_BTC, + }, + ]; + let build = |input_amount| { + create_spliced_dlc_transactions( + &PartyParams { + input_amount, + ..offer_params.clone() + }, + &accept_params, + &payouts, + 100, + 4, + 0, + 10, + 0, + 0, + ) + }; + assert_eq!(build(required).unwrap().fund.output.len(), 1); + let one_sat_short = required - Amount::from_sat(1); + assert!(matches!( + build(one_sat_short), + Err(Error::InsufficientFunds { required: r, available }) + if r == required && available == one_sat_short + )); + } + + /// Inputs holding exactly what `funding_requirements` asks for build the + /// contract with nothing left over; one sat less on either side fails with + /// that side's requirement and what it had. + #[test] + fn funding_requirements_are_exactly_sufficient() { + let one_sat = Amount::from_sat(1); + for (offer_params, accept_params) in single_and_dual_funded_params() { + let total = offer_params.collateral + accept_params.collateral; + for fee_rule in [FeeRule::CounterpartyPayout, FeeRule::OwnPayoutOnly] { + let requirements = |party: &PartyParams, counterparty: &PartyParams| { + party + .funding_requirements( + total, + 4, + Amount::ZERO, + party.payout_script_pubkey.len(), + counterparty.payout_script_pubkey.len(), + fee_rule, + ) + .unwrap() + }; + let offer = requirements(&offer_params, &accept_params); + let accept = requirements(&accept_params, &offer_params); + let funded = |party: &PartyParams, input_amount| PartyParams { + input_amount, + ..party.clone() + }; + + let exact = try_build_for_accept_cet_script( + &funded(&offer_params, offer.required), + &funded(&accept_params, accept.required), + fee_rule, + accept_params.payout_script_pubkey.len(), + ) + .unwrap(); + assert_eq!(exact.fund.output.len(), 1, "no change is left over"); + + let short_offer = try_build_for_accept_cet_script( + &funded(&offer_params, offer.required - one_sat), + &funded(&accept_params, accept.required), + fee_rule, + accept_params.payout_script_pubkey.len(), + ); + assert!(matches!( + short_offer, + Err(Error::InsufficientFunds { required, available }) + if required == offer.required && available == offer.required - one_sat + )); + + if accept.required > Amount::ZERO { + let short_accept = try_build_for_accept_cet_script( + &funded(&offer_params, offer.required), + &funded(&accept_params, accept.required - one_sat), + fee_rule, + accept_params.payout_script_pubkey.len(), + ); + assert!(matches!( + short_accept, + Err(Error::InsufficientFunds { required, available }) + if required == accept.required + && available == accept.required - one_sat + )); + } + } + } + } + #[test] fn get_change_output_and_fees_enough_funds() { // Arrange From 5471cb17b2a233a3981570169222e6df1a6e329b Mon Sep 17 00:00:00 2001 From: benny b Date: Thu, 15 Oct 2026 14:20:28 -0400 Subject: [PATCH 2/2] ddk-dlc: lay out splice inputs by outpoint and keep the legacy layout for rebuilds PartyParams::spliced dropped every ordinary input whose witness is larger than a P2WPKH witness, not only the ordinary copy of a splice input. As public 2.0.0 API that would freeze a lossy rule: a new splice that also spends a P2WSH coin would leave the coin out of its funding transaction. spliced now removes the ordinary copy of each splice input by outpoint and keeps every other input, so new contracts spend and price all of their inputs. The old rule moves to PartyParams::legacy_spliced, documented as rebuild-only for contracts built by 2.0.0-rc.8 and earlier. For inputs whose witnesses fit the old rule the two layouts are identical, so every pinned contract shape, enum_spliced_in included, rebuilds to the same digests. A splice built by rc.8 with a small wide input has a valid funding transaction that the new layout no longer reproduces, so a signed contract is rebuilt under a BuildRule: a fee rule and a splice layout. #224 made ddk-manager's build_contract_transactions the one place transactions are built, so BuildRule lives next to it and the builder takes one in place of a FeeRule. BuildRule::CURRENT builds new contracts, in ddk-manager's accept path, ddk's accept_offer and offer_funding_requirements. ddk's signed_context tries BuildRule::SIGNED in order, each fee rule with the current layout and then with the legacy one, and accepts the first whose contract id matches the sign message. The builder lays out both parties under the rule and calls one constructor, so the spliced and plain constructor calls no longer repeat. ContractError is now #[non_exhaustive], so typed variants can be added after 2.0.0 without a major release. Every match on it in the workspace already has a wildcard arm. --- ddk-manager/src/contract/transactions.rs | 125 ++++++++++++++-------- ddk-manager/src/contract_updater.rs | 8 +- ddk/src/contract/accept.rs | 9 +- ddk/src/contract/context.rs | 56 +++++----- ddk/src/contract/error.rs | 4 +- ddk/src/contract/splice.rs | 9 +- ddk/src/contract/tests.rs | 126 ++++++++++++++++++++++- ddk/tests/stateless.rs | 7 +- dlc/src/lib.rs | 87 ++++++++++++---- 9 files changed, 326 insertions(+), 105 deletions(-) diff --git a/ddk-manager/src/contract/transactions.rs b/ddk-manager/src/contract/transactions.rs index c7d47809..d63e7369 100644 --- a/ddk-manager/src/contract/transactions.rs +++ b/ddk-manager/src/contract/transactions.rs @@ -65,6 +65,61 @@ impl From<&OfferDlc> for ContractTerms { } } +/// How a contract's transactions are built: how the CET fee is split, and how +/// a splicing party's inputs are laid out. +/// +/// A new contract is built under [`BuildRule::CURRENT`]. A signed contract is +/// rebuilt under whichever rule in [`BuildRule::SIGNED`] reproduces the +/// funding transaction both parties signed. +#[derive(Clone, Copy, Debug)] +pub struct BuildRule { + /// How the CET fee is split between the parties. + pub fee_rule: FeeRule, + /// How a party's inputs are laid out when the contract spends a splice + /// input: [`PartyParams::spliced`], or [`PartyParams::legacy_spliced`] for + /// a contract built by ddk-dlc 2.0.0-rc.8 and earlier. + pub splice_layout: fn(&PartyParams) -> PartyParams, +} + +impl BuildRule { + /// The rule new contracts are built under. + pub const CURRENT: BuildRule = BuildRule { + fee_rule: FeeRule::CounterpartyPayout, + splice_layout: PartyParams::spliced, + }; + + /// Every rule a signed contract can have been built under, the current + /// rule first. [`PartyParams::legacy_spliced`] differs from the current + /// layout only for a splice with an ordinary input whose witness is larger + /// than a P2WPKH witness, so both fee rules are tried with the current + /// layout before either is tried with the legacy one. + pub const SIGNED: [BuildRule; 4] = [ + BuildRule::CURRENT, + BuildRule { + fee_rule: FeeRule::OwnPayoutOnly, + splice_layout: PartyParams::spliced, + }, + BuildRule { + fee_rule: FeeRule::CounterpartyPayout, + splice_layout: PartyParams::legacy_spliced, + }, + BuildRule { + fee_rule: FeeRule::OwnPayoutOnly, + splice_layout: PartyParams::legacy_spliced, + }, + ]; + + /// `params` with their inputs laid out as this rule funds them, in a + /// contract that spends a splice input if `splicing`. + fn lay_out(&self, params: &PartyParams, splicing: bool) -> PartyParams { + if splicing { + (self.splice_layout)(params) + } else { + params.clone() + } + } +} + /// A contract's transactions, with the CETs of each contract info located. #[derive(Clone)] pub struct ContractTransactions { @@ -144,16 +199,16 @@ impl From for Error { /// /// The first contract info's payouts give the funding, CET and refund /// transactions; every further contract info adds its CETs after them. The -/// fee rule is the one the contract was funded under: [`FeeRule::default`] -/// for a new contract, and whichever rule reproduces a signed contract's -/// funding transaction for an existing one. +/// rule is the one the contract was built under: [`BuildRule::CURRENT`] for a +/// new contract, and whichever rule in [`BuildRule::SIGNED`] reproduces a +/// signed contract's funding transaction for an existing one. pub fn build_contract_transactions( offer: &PartyParams, accept: &PartyParams, contract_infos: &[ContractInfo], offer_tlvs: &TlvStream, terms: &ContractTerms, - fee_rule: FeeRule, + rule: BuildRule, ) -> Result { if contract_infos.is_empty() { return Err(BuildError::Offer( @@ -176,38 +231,23 @@ pub fn build_contract_transactions( let payouts = contract_infos[0].get_payouts(terms.total_collateral)?; // A splice input carries a `dlc_input`; when present the funding - // transaction spends the previous contract's 2-of-2 output and must be - // built through the spliced constructor. - let has_dlc_inputs = !offer.dlc_inputs.is_empty() || !accept.dlc_inputs.is_empty(); - let mut transactions = if has_dlc_inputs { - ddk_dlc::create_spliced_dlc_transactions_with_fee_rule( - offer, - accept, - &payouts, - terms.refund_locktime, - terms.fee_rate_per_vb, - 0, - terms.cet_locktime, - terms.fund_output_serial_id, - terms.contract_flags, - fee_rule, - accept_cet_script_len, - )? - } else { - ddk_dlc::create_dlc_transactions_with_fee_rule( - offer, - accept, - &payouts, - terms.refund_locktime, - terms.fee_rate_per_vb, - 0, - terms.cet_locktime, - terms.fund_output_serial_id, - terms.contract_flags, - fee_rule, - accept_cet_script_len, - )? - }; + // transaction spends the previous contract's 2-of-2 output, so both + // parties' inputs are laid out as the rule's spliced constructor lays them + // out. + let splicing = !offer.dlc_inputs.is_empty() || !accept.dlc_inputs.is_empty(); + let mut transactions = ddk_dlc::create_dlc_transactions_with_fee_rule( + &rule.lay_out(offer, splicing), + &rule.lay_out(accept, splicing), + &payouts, + terms.refund_locktime, + terms.fee_rate_per_vb, + 0, + terms.cet_locktime, + terms.fund_output_serial_id, + terms.contract_flags, + rule.fee_rule, + accept_cet_script_len, + )?; let mut cet_ranges = Vec::with_capacity(contract_infos.len()); cet_ranges.push(0..transactions.cets.len()); @@ -269,20 +309,17 @@ pub fn offer_funding_requirements( terms: &ContractTerms, ) -> Result { let records = cet_records(offer_tlvs)?; - // An offer with a splice input is built by the spliced constructor, which - // lays out its inputs this way. - let offer = if offer.dlc_inputs.is_empty() { - offer.clone() - } else { - offer.spliced() - }; + // Only the offering party may contribute a splice input, so the offer's + // own inputs say whether the contract splices. + let rule = BuildRule::CURRENT; + let offer = rule.lay_out(offer, !offer.dlc_inputs.is_empty()); Ok(offer.funding_requirements( terms.total_collateral, terms.fee_rate_per_vb, Amount::ZERO, offer.payout_script_pubkey.len(), accept_cet_script_len(&records, accept_payout_script_len), - FeeRule::default(), + rule.fee_rule, )?) } diff --git a/ddk-manager/src/contract_updater.rs b/ddk-manager/src/contract_updater.rs index ae1a4713..9caec94a 100644 --- a/ddk-manager/src/contract_updater.rs +++ b/ddk-manager/src/contract_updater.rs @@ -8,7 +8,7 @@ use bitcoin::script::PushBytesBuf; use bitcoin::Amount; use bitcoin::{consensus::Decodable, Script, ScriptBuf, Transaction, Witness}; use ddk_dlc::dlc_input::DlcInputInfo; -use ddk_dlc::{DlcTransactions, FeeRule, PartyParams}; +use ddk_dlc::{DlcTransactions, PartyParams}; use ddk_messages::{ oracle_msgs::{OracleAnnouncement, OracleAttestation}, AcceptDlc, FundingSignature, FundingSignatures, OfferDlc, SignDlc, WitnessElement, @@ -29,7 +29,7 @@ use crate::{ contract_input::ContractInput, offered_contract::OfferedContract, signed_contract::SignedContract, - transactions::{build_contract_transactions, ContractTransactions}, + transactions::{build_contract_transactions, BuildRule, ContractTransactions}, AdaptorInfo, }, conversion_utils::get_tx_input_infos, @@ -198,7 +198,7 @@ where &offered_contract.contract_info, &offered_contract.tlvs, &offered_contract.into(), - FeeRule::default(), + BuildRule::CURRENT, )?; log_created_transactions( logger, @@ -355,7 +355,7 @@ where &offered_contract.contract_info, &offered_contract.tlvs, &offered_contract.into(), - FeeRule::default(), + BuildRule::CURRENT, )?; log_created_transactions( logger, diff --git a/ddk/src/contract/accept.rs b/ddk/src/contract/accept.rs index 7aa8c7a4..439641cb 100644 --- a/ddk/src/contract/accept.rs +++ b/ddk/src/contract/accept.rs @@ -109,10 +109,13 @@ pub fn create_dlc_transactions( } /// Rebuilds a signed contract's transactions under the [`ddk_dlc::FeeRule`] -/// whose funding transaction matches `sign`'s contract id. +/// and splice input layout whose funding transaction matches `sign`'s +/// contract id. /// -/// Tries [`ddk_dlc::FeeRule::CounterpartyPayout`] first, then -/// [`ddk_dlc::FeeRule::OwnPayoutOnly`]. Fails when neither rule matches. +/// Tries the current rules first: [`ddk_dlc::FeeRule::CounterpartyPayout`] +/// and [`ddk_dlc::PartyParams::spliced`]. Then +/// [`ddk_dlc::FeeRule::OwnPayoutOnly`], and both fee rules again with +/// [`ddk_dlc::PartyParams::legacy_spliced`]. Fails when no rule matches. pub fn create_signed_dlc_transactions( offer: &OfferDlc, accept: &AcceptDlc, diff --git a/ddk/src/contract/context.rs b/ddk/src/contract/context.rs index 633ecbad..efbca703 100644 --- a/ddk/src/contract/context.rs +++ b/ddk/src/contract/context.rs @@ -7,9 +7,11 @@ use bitcoin::consensus::Decodable; use bitcoin::{Amount, ScriptBuf, Transaction, Witness}; use ddk_dlc::secp256k1_zkp::{All, EcdsaAdaptorSignature, PublicKey, Secp256k1, SecretKey}; -use ddk_dlc::{DlcTransactions, FeeRule, PartyParams as DlcPartyParams, TxInputInfo}; +use ddk_dlc::{DlcTransactions, PartyParams as DlcPartyParams, TxInputInfo}; use ddk_manager::contract::contract_info::ContractInfo as ExecutionContractInfo; -use ddk_manager::contract::transactions::{build_contract_transactions, ContractTransactions}; +use ddk_manager::contract::transactions::{ + build_contract_transactions, BuildRule, ContractTransactions, +}; use ddk_messages::{ AcceptDlc, CetAdaptorSignatures, FundingInput, FundingSignatures, OfferDlc, SignDlc, }; @@ -30,19 +32,20 @@ pub(crate) fn context_from_messages( offer: &OfferDlc, accept: &AcceptDlc, ) -> Result { - context_from_messages_with_fee_rule(offer, accept, FeeRule::default()) + context_from_messages_with_rule(offer, accept, BuildRule::CURRENT) } -/// Rebuilds a signed contract under the [`FeeRule`] whose funding transaction -/// matches the sign message's contract id. +/// Rebuilds a signed contract under the [`BuildRule`] whose funding +/// transaction matches the sign message's contract id. /// -/// The two rules give different funding transactions when one party funds the -/// whole contract. The contract id comes from the funding transaction that -/// both parties signed, so only the rule the contract was built with matches -/// it. [`FeeRule::CounterpartyPayout`] is tried first. If it cannot build the -/// transactions, or its contract id does not match, [`FeeRule::OwnPayoutOnly`] -/// is tried. If neither rule matches, the error from the first rule is -/// returned. +/// The [`ddk_dlc::FeeRule`]s give different funding transactions when one +/// party funds the whole contract, and the splice layouts do when a splice has +/// an ordinary input with a witness larger than a P2WPKH witness. The contract +/// id comes from the funding transaction that both parties signed, so only the +/// rule the contract was built with matches it. The rules in +/// [`BuildRule::SIGNED`] are tried in order: when one cannot build the +/// transactions, or its contract id does not match, the next is tried. If none +/// matches, the error from the current rule is returned. /// /// Use this only for a contract that is already signed. New contracts use /// [`context_from_messages`]. @@ -52,20 +55,22 @@ pub(crate) fn signed_context( sign: &SignDlc, ) -> Result { let rebuild = |rule| { - let context = context_from_messages_with_fee_rule(offer, accept, rule)?; + let context = context_from_messages_with_rule(offer, accept, rule)?; ensure_sign_message(offer, sign, &context)?; Ok(context) }; // The inputs of an OwnPayoutOnly contract can be too small for the // CounterpartyPayout fee, so a build error also moves to the next rule. - rebuild(FeeRule::default()) - .or_else(|current_error| rebuild(FeeRule::OwnPayoutOnly).map_err(|_| current_error)) + let [current, older @ ..] = BuildRule::SIGNED; + older.into_iter().fold(rebuild(current), |result, rule| { + result.or_else(|current_error| rebuild(rule).map_err(|_| current_error)) + }) } -fn context_from_messages_with_fee_rule( +fn context_from_messages_with_rule( offer: &OfferDlc, accept: &AcceptDlc, - fee_rule: FeeRule, + rule: BuildRule, ) -> Result { ensure_protocol_version(offer.protocol_version, ContractError::InvalidOffer)?; ensure_protocol_version(accept.protocol_version, ContractError::InvalidAccept)?; @@ -97,7 +102,7 @@ fn context_from_messages_with_fee_rule( accept.accept_collateral, &accept.funding_inputs, )?; - build_context_with_fee_rule(offer, &accept_params, fee_rule) + build_context_with_rule(offer, &accept_params, rule) } /// Rebuilds the contract transactions from an offer and the accepting party's @@ -107,13 +112,13 @@ pub(crate) fn build_context( offer: &OfferDlc, accept_params: &DlcPartyParams, ) -> Result { - build_context_with_fee_rule(offer, accept_params, FeeRule::default()) + build_context_with_rule(offer, accept_params, BuildRule::CURRENT) } -fn build_context_with_fee_rule( +fn build_context_with_rule( offer: &OfferDlc, accept_params: &DlcPartyParams, - fee_rule: FeeRule, + rule: BuildRule, ) -> Result { let offer_params = dlc_party_params( offer.funding_pubkey, @@ -137,7 +142,7 @@ fn build_context_with_fee_rule( &execution_infos, &offer.tlvs, &offer.into(), - fee_rule, + rule, )?; Ok(ContractContext { execution_infos, @@ -580,8 +585,11 @@ mod tests { previous.output[input.prev_tx_vout as usize].value = offer.offer_collateral + fund_fee + cet_fee; input.prev_tx = bitcoin::consensus::serialize(&previous); - let legacy = - context_from_messages_with_fee_rule(&offer, &accept, FeeRule::OwnPayoutOnly).unwrap(); + let legacy_rule = BuildRule { + fee_rule: ddk_dlc::FeeRule::OwnPayoutOnly, + ..BuildRule::CURRENT + }; + let legacy = context_from_messages_with_rule(&offer, &accept, legacy_rule).unwrap(); assert!(context_from_messages(&offer, &accept).is_err()); let secp = Secp256k1::new(); diff --git a/ddk/src/contract/error.rs b/ddk/src/contract/error.rs index 3dc91218..dd729cec 100644 --- a/ddk/src/contract/error.rs +++ b/ddk/src/contract/error.rs @@ -6,8 +6,10 @@ use thiserror::Error; /// Errors returned by the stateless contract functions. /// /// Variants carry plain strings so they can cross FFI and binding boundaries -/// without exposing internal library error types. +/// without exposing internal library error types. New variants may be added +/// in a minor release, so a `match` on it needs a wildcard arm. #[derive(Debug, Error)] +#[non_exhaustive] pub enum ContractError { /// The offer message, or data used to build one, is invalid. #[error("invalid offer: {0}")] diff --git a/ddk/src/contract/splice.rs b/ddk/src/contract/splice.rs index 8de9f074..1919e825 100644 --- a/ddk/src/contract/splice.rs +++ b/ddk/src/contract/splice.rs @@ -18,10 +18,11 @@ use super::types::{random_serial_id, Party}; /// Maximum witness length reported for a DLC (2-of-2 P2WSH) funding input. /// -/// Must exceed 108 so that [`ddk_dlc::create_spliced_dlc_transactions`] -/// separates DLC inputs from ordinary P2WPKH wallet inputs. The value matches -/// `ddk-manager`, so a stateful counterparty rebuilds an identical funding -/// transaction. +/// Must exceed 108, the P2WPKH witness size: spliced contracts built by +/// ddk-dlc 2.0.0-rc.8 and earlier told a DLC input from an ordinary wallet +/// input by that size (see [`ddk_dlc::PartyParams::legacy_spliced`]). The +/// value matches `ddk-manager`, so a stateful counterparty rebuilds an +/// identical funding transaction. pub const DLC_INPUT_MAX_WITNESS_LEN: u16 = 220; /// Builds a splice (DLC) funding input from a previous contract's messages. diff --git a/ddk/src/contract/tests.rs b/ddk/src/contract/tests.rs index 6d0a3c34..67af7b6b 100644 --- a/ddk/src/contract/tests.rs +++ b/ddk/src/contract/tests.rs @@ -15,9 +15,14 @@ use ddk_messages::oracle_msgs::{ tagged_announcement_msg, EnumEventDescriptor, EventDescriptor, OracleAnnouncement, OracleEvent, OracleInfo, SingleOracleInfo, }; -use ddk_messages::{AcceptDlc, CetAdaptorSignatures, DlcInput, FundingInput, OfferDlc}; +use ddk_messages::{ + AcceptDlc, CetAdaptorSignatures, DlcInput, FundingInput, FundingSignatures, OfferDlc, SignDlc, +}; -use super::context::{ensure_no_dlc_inputs, funding_input_index, validate_offer_funding_inputs}; +use super::context::{ + contract_id_from_transactions, dlc_party_params, ensure_no_dlc_inputs, funding_input_index, + validate_offer_funding_inputs, +}; use super::psbt::finalize_segwit_input; use super::types::{funding_input, network_from_chain_hash, random_serial_id}; use super::*; @@ -210,6 +215,123 @@ fn ensure_no_dlc_inputs_rejects_a_dlc_input() { assert!(ensure_no_dlc_inputs(&[dummy_funding_input(1)]).is_ok()); } +/// A single funded splice offer that also spends an ordinary P2WSH input of +/// 330 sats, whose witness is larger than a P2WPKH witness. Returns the +/// messages and the P2WSH input's outpoint. +fn splice_with_a_p2wsh_input() -> (OfferDlc, AcceptDlc, OutPoint) { + let (mut offer, mut accept) = messages_with_serial_ids(&[1], &[]); + offer.offer_collateral = Amount::from_sat(100_000); + accept.accept_collateral = Amount::ZERO; + let script = ScriptBuf::new_p2wsh(&bitcoin::WScriptHash::from_byte_array([3; 32])); + let previous_transaction = dummy_transaction(Amount::from_sat(330), script); + let p2wsh_outpoint = OutPoint::new(previous_transaction.compute_txid(), 0); + let p2wsh_input = funding_input( + &previous_transaction, + 0, + Some(7), + u32::MAX, + 200, + ScriptBuf::new(), + ) + .unwrap(); + offer + .funding_inputs + .extend([dlc_funding_input(220, ScriptBuf::new(), true), p2wsh_input]); + (offer, accept, p2wsh_outpoint) +} + +#[test] +fn a_new_splice_spends_its_p2wsh_input() { + let (offer, accept, p2wsh_outpoint) = splice_with_a_p2wsh_input(); + let fund = create_dlc_transactions(&offer, &accept).unwrap().fund; + let mut spent = fund.input.iter().map(|input| input.previous_output); + assert_eq!(fund.input.len(), 3, "the splice input is spent once"); + assert!(spent.any(|outpoint| outpoint == p2wsh_outpoint)); +} + +/// ddk-dlc 2.0.0-rc.8 and earlier left a splice's P2WSH input out of the +/// funding transaction. With 330 sats, less than the funding fee, that +/// transaction was still valid, so a signed contract built that way must +/// rebuild exactly. +#[test] +fn a_splice_built_with_the_legacy_layout_rebuilds_from_its_sign_message() { + let (offer, accept, p2wsh_outpoint) = splice_with_a_p2wsh_input(); + // What the spliced constructor of ddk-dlc 2.0.0-rc.8 built. + let offer_params = dlc_party_params( + offer.funding_pubkey, + offer.payout_spk.clone(), + offer.payout_serial_id, + offer.change_spk.clone(), + offer.change_serial_id, + offer.offer_collateral, + &offer.funding_inputs, + ) + .unwrap(); + let accept_params = dlc_party_params( + accept.funding_pubkey, + accept.payout_spk.clone(), + accept.payout_serial_id, + accept.change_spk.clone(), + accept.change_serial_id, + accept.accept_collateral, + &accept.funding_inputs, + ) + .unwrap(); + let total = offer.get_total_collateral(); + let legacy = ddk_dlc::create_dlc_transactions( + &offer_params.legacy_spliced(), + &accept_params.legacy_spliced(), + &[ + ddk_dlc::Payout { + offer: total, + accept: Amount::ZERO, + }, + ddk_dlc::Payout { + offer: Amount::ZERO, + accept: total, + }, + ], + offer.refund_locktime, + offer.fee_rate_per_vb, + 0, + offer.cet_locktime, + offer.fund_output_serial_id, + offer.contract_flags, + ) + .unwrap(); + let spent_value = offer_params.input_amount - Amount::from_sat(330); + let output_value = legacy.fund.output.iter().map(|output| output.value).sum(); + assert!(spent_value > output_value, "the legacy funding pays a fee"); + assert!(legacy + .fund + .input + .iter() + .all(|input| input.previous_output != p2wsh_outpoint)); + assert_ne!( + create_dlc_transactions(&offer, &accept).unwrap().fund, + legacy.fund + ); + + let secp = Secp256k1::new(); + let sign = SignDlc { + protocol_version: PROTOCOL_VERSION, + contract_id: contract_id_from_transactions(&legacy, &offer.temporary_contract_id), + cet_adaptor_signatures: CetAdaptorSignatures::from(&[][..]), + refund_signature: secp.sign_ecdsa( + &Message::from_digest([1; 32]), + &SecretKey::from_slice(&[1; 32]).unwrap(), + ), + funding_signatures: FundingSignatures { + funding_signatures: vec![], + }, + tlvs: Default::default(), + }; + let rebuilt = create_signed_dlc_transactions(&offer, &accept, &sign).unwrap(); + assert_eq!(rebuilt.fund, legacy.fund); + assert_eq!(rebuilt.refund, legacy.refund); + assert_eq!(rebuilt.cets, legacy.cets); +} + #[test] fn funding_input_index_orders_by_serial_id() { let (offer, accept) = messages_with_serial_ids(&[50, 3], &[12]); diff --git a/ddk/tests/stateless.rs b/ddk/tests/stateless.rs index 21510c69..de760b34 100644 --- a/ddk/tests/stateless.rs +++ b/ddk/tests/stateless.rs @@ -1910,9 +1910,10 @@ fn a_splice_offer_funded_with_exactly_its_requirement_is_accepted() { }); } -/// The spliced constructor leaves out an ordinary input whose witness is -/// larger than a P2WPKH witness, and the requirement does too: an offer one -/// sat short of it is refused with exactly that requirement. +/// The spliced constructor spends an ordinary input whose witness is larger +/// than a P2WPKH witness alongside the splice input, and the requirement +/// prices it too: an offer one sat short of it is refused with exactly that +/// requirement. #[test] fn a_splice_offer_is_priced_as_the_spliced_constructor_funds_it() { let secp = Secp256k1::new(); diff --git a/dlc/src/lib.rs b/dlc/src/lib.rs index 14df4579..fb39a750 100644 --- a/dlc/src/lib.rs +++ b/dlc/src/lib.rs @@ -528,20 +528,47 @@ impl PartyParams { /// [`create_spliced_dlc_transactions`] funds them, which is also how a /// splicing party's [`PartyParams::funding_requirements`] must be priced. /// - /// A splice input is listed both in `inputs` and in `dlc_inputs`. The - /// duplicate is removed by dropping every ordinary input whose witness is - /// larger than a P2WPKH witness, and each DLC input is then listed as an - /// ordinary input. An ordinary input with a larger witness is dropped too; - /// spliced contracts have always been built this way, so the rule stays - /// for them to rebuild identically. + /// A splice input is listed both in `inputs` and in `dlc_inputs`. Each DLC + /// input replaces the ordinary input with its outpoint and is listed after + /// the remaining ordinary inputs, whatever their witness size. pub fn spliced(&self) -> PartyParams { + self.with_dlc_inputs_listed(|input, dlc_inputs| { + dlc_inputs + .iter() + .all(|dlc_input| dlc_input.outpoint != input.outpoint) + }) + } + + /// These params with their inputs laid out as spliced contracts built by + /// ddk-dlc 2.0.0-rc.8 and earlier were funded. Use it only to rebuild such + /// a contract; new contracts use [`PartyParams::spliced`]. + /// + /// The duplicate of a splice input was removed by dropping every ordinary + /// input whose witness is larger than a P2WPKH witness, so an ordinary + /// input with a larger witness was dropped too. For any other inputs the + /// two layouts are identical. + pub fn legacy_spliced(&self) -> PartyParams { + self.with_dlc_inputs_listed(|input, _| input.max_witness_len <= P2WPKH_WITNESS_SIZE) + } + + /// These params with the ordinary inputs `keep` accepts, followed by the + /// DLC inputs listed as ordinary inputs. + fn with_dlc_inputs_listed( + &self, + keep: impl Fn(&TxInputInfo, &[TxInputInfo]) -> bool, + ) -> PartyParams { + let dlc_inputs = self + .dlc_inputs + .iter() + .map(TxInputInfo::from) + .collect::>(); let mut inputs = self .inputs .iter() - .filter(|input| input.max_witness_len <= P2WPKH_WITNESS_SIZE) + .filter(|input| keep(input, &dlc_inputs)) .cloned() .collect::>(); - inputs.extend(self.dlc_inputs.iter().map(TxInputInfo::from)); + inputs.extend(dlc_inputs); PartyParams { inputs, dlc_inputs: Vec::new(), @@ -2158,10 +2185,11 @@ mod tests { assert_eq!(output(&accept_params.change_script_pubkey), None); } - /// The spliced constructor funds the inputs `PartyParams::spliced` lays - /// out, so a splicing party priced on them and funded at exactly that - /// requirement builds with no change, and one sat less fails with it. Its - /// ordinary input with a witness above P2WPKH's is left out by both. + /// `PartyParams::spliced` drops the ordinary copy of a splice input by its + /// outpoint and keeps an ordinary input with a witness above P2WPKH's, + /// which the legacy layout drops. The spliced constructor funds those + /// inputs, so a splicing party priced on them and funded at exactly that + /// requirement builds with no change, and one sat less fails with it. #[test] fn a_splicing_party_is_priced_on_its_spliced_inputs() { let (mut offer_params, _) = get_party_params(Amount::ZERO, Amount::ONE_BTC, None); @@ -2192,20 +2220,28 @@ mod tests { // A splice input is listed both as an ordinary input and a DLC input. offer_params.inputs.push(TxInputInfo::from(&splice)); offer_params.dlc_inputs.push(splice); + let wide_outpoint = OutPoint { + txid: Txid::from_byte_array([7; 32]), + vout: 0, + }; offer_params.inputs.push(TxInputInfo { - outpoint: OutPoint { - txid: Txid::from_byte_array([7; 32]), - vout: 0, - }, + outpoint: wide_outpoint, max_witness_len: 200, redeem_script: ScriptBuf::new(), serial_id: 7, }); + let serial_ids = |params: PartyParams| { + assert!(params.dlc_inputs.is_empty()); + params + .inputs + .iter() + .map(|input| input.serial_id) + .collect::>() + }; + assert_eq!(serial_ids(offer_params.legacy_spliced()), vec![1, 9]); let spliced = offer_params.spliced(); - let serial_ids = spliced.inputs.iter().map(|input| input.serial_id); - assert_eq!(serial_ids.collect::>(), vec![1, 9]); - assert!(spliced.dlc_inputs.is_empty()); + assert_eq!(serial_ids(spliced.clone()), vec![1, 7, 9]); let required = spliced .funding_requirements( Amount::ONE_BTC, @@ -2244,7 +2280,18 @@ mod tests { 0, ) }; - assert_eq!(build(required).unwrap().fund.output.len(), 1); + let exact = build(required).unwrap(); + assert_eq!(exact.fund.output.len(), 1); + assert_eq!( + exact.fund.input.len(), + spliced.inputs.len() + accept_params.inputs.len(), + "the funding transaction spends the splice input once" + ); + assert!(exact + .fund + .input + .iter() + .any(|input| input.previous_output == wide_outpoint)); let one_sat_short = required - Amount::from_sat(1); assert!(matches!( build(one_sat_short),