Repository navigation
Conversation
Owner
Author
|
Note 🤖 Claude Opus 5.5 responding on behalf of bennyhodl Review against orange-grove's consumers ( Right owner for the fee math: orange-grove's
|
Owner
Author
|
Note 🤖 Claude Opus 5.5 responding on behalf of bennyhodl Follow-up on the review above:
|
…g 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).
… 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.
bennyhodl
force-pushed
the
benny/225-funding-requirements
branch
from
October 15, 2026 19:09
8d6f813 to
5471cb1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Item 6 of #225. Lygos's offer builder probes a fabricated accept and parses
required_input_funds=out of anInvalidArgumentstring to size its coin selection. This PR gives constructors a typed shortfall error, and gives coin selection a pure function that returns the exact requirement without building a transaction.API
ddk_dlc::Error::InsufficientFunds { required: Amount, available: Amount }replaces the string error from the fund-transaction constructor.ddk_dlc::FundingRequirements { required, fund_fee, cet_fee }withchange(available).PartyParams::funding_requirements(total_collateral, fee_rate_per_vb, extra_fee, counterparty_payout_spk, fee_rule)is now the single fee formula, and the constructor uses it.PartyParams::spliced()holds the spliced constructor's input layout (drop inputs whose witness is over 108 bytes, add DLC inputs as ordinary ones). Construction and pricing share it, so splice offers are priced the way they're built.ddk::contract::funding_requirements(offer, accept_payout_spk) -> FundingRequirementsis whataccept_offerwill require of the offering party, splice inputs included. It builds withoutmanager, andFundingRequirementsis re-exported fromddk::contract.ContractError::InsufficientFunds { required, available }.From<ddk_manager::error::Error>unwrapsDlcErrorthrough the same conversion, so the typed error survives a manager wrapper.Breaking changes
PartyParams::get_change_output_and_fees_with_counterpartyis removed. It was the workaround; usefunding_requirements.get_change_output_and_feesstays, as anOwnPayoutOnlywrapper.ContractErrormatch needs anInsufficientFundsarm. Lygos'srequired_fundscan callddk::contract::funding_requirementsand match the typed error.Compatibility
Known issue (predates this PR, not fixed)
A splice offer that also has an ordinary input declaring a witness over 108 bytes can't be accepted. The spliced constructor leaves that input out of the funding transaction,
build_funding_psbtthen fails to match it, and its value still counts toward the party's input total.funding_requirementsprices it the way the constructor does. The constructor behaviour itself needs its own fix.Not changed
ddk-manager/src/utils.rs::get_approximate_required_amountis still a separate, approximate pre-selection estimate. Moving it onto this formula changes the manager's coin-selection amounts.accept_offercaller can compareavailablewith its own inputs.Testing
funding_requirements_are_exactly_sufficient: covers both fee rules, single- and dual-funded contracts, and both sides being short. Inputs at exactlyrequiredbuild; one sat less fails with the exactInsufficientFunds { required, available }.a_zero_collateral_party_gets_no_change: a regression test for a zero-collateral party with an extra fee.a_splicing_party_is_priced_on_its_spliced_inputs.an_offer_funded_with_exactly_its_requirement_is_accepteda_splice_offer_funded_with_exactly_its_requirement_is_accepteda_splice_offer_is_priced_as_the_spliced_constructor_funds_itstateless_execution --ignoredon regtest: splice-in, single-funded and dual-funded closes pass.cargo fmt --check,cargo clippy -- -D warningsandcargo check -p ddk --no-default-featurespass.