Repository navigation
ddk-manager: let records on the offer shape the CETs, and settle every contract info - #224
Conversation
The transaction builder is about to move out of the stateless module and the manager into one place. These digests, replayed from pinned offer and accept messages, are the proof that the move changes no transaction of an existing contract: enum and numerical, dual and single funded, disjoint, spliced, and with the refund-to-accepter flag.
…wn CETs A contract with several contract infos holds their CETs one after another, and every contract info indexes its CETs from zero. The manager used those indexes against the whole list when the offering party signed its adaptor signatures, when the accepting party verified them, and when either party picked the CET to settle with. Both sides were wrong the same way, so the signatures verified, and a contract could only ever settle on its first contract info: the other side's CET did not match the decrypted signature, or, when the adaptor indexes happened to line up, the first contract info's CET was broadcast in its place. Each site now works on the contract info's own slice. The slices follow from each contract info's payout count and the adaptor signature offsets from its adaptor info, so stored contracts need no migration and nothing new is serialized. Finding the offsets from a contract info means comparing it with the contract's, hence PartialEq on the contract info types. MemoryOracle reported an event it had not signed as an attestation with no signatures, which validates and so stops the periodic check at the first contract info. It now reports it as an error, as a networked oracle does. The manager execution tests gain a disjoint contract whose first contract info never attests, settled from both sides, by hand and by the periodic check, with the second info enum or numerical. Every close path now also checks that the broadcast CET belongs to the attested contract info.
c852ff4 to
965586c
Compare
A TLV record for an offer naming the outcomes whose CETs pay a script in the accepting party's place, for products that settle an outcome to a third party: an outcome of an enum descriptor, or a range of the values of a numeric one. Only the wire form lives here; how it changes the CETs is the manager's concern. The type number is the one agreed with node-dlc, 65005. It is odd, so a peer that does not know the record carries it through and then fails signature verification, which is the safe failure. The record redirects what the accepter is paid, so an accepter has to check the overrides on an offer before accepting it. The encoding is pinned byte for byte, for node-dlc to match.
Accepting an offer, signing an accept and the stateless contract module each built the funding transaction, the CETs and the refund transaction themselves: the same choice of plain or spliced constructor for the first contract info, the same loop adding every further info's CETs after it. Three copies of what both parties have to agree on byte for byte. `contract::transactions::build_contract_transactions` is now the one builder. It takes both parties, the contract infos and the offer's terms, returns the transactions with each contract info's CET range, and owns the checks the copies made or skipped: no contract info, no CET, and collaterals that do not add up to the offer's total. Its `BuildError` keeps those apart from construction failures, so the stateless module still reports an invalid offer or an invalid accept rather than a generic DLC error. The CETs of every contract info after the first keep their locktime of zero and their copy of the first CET's input. That is how every stored disjoint contract was built, and the pinned transaction fixtures hold the builder to it.
d6a0688 to
24cff24
Compare
matthewjablack
left a comment
matthewjablack
left a comment
There was a problem hiding this comment.
This implementation makes a lot of sense.
One thing: the override script length check means a liquidator with a taproot address fails the accept when our payout script is P2WPKH. So either Taproot needs to be prevented application side, or we allow different lengths. What we could do is, at funding time, size the CET fee off the longest script among the accepter's payout script and the overrides, then drop the check.
Also, an override on a CET whose accept payout is under dust silently drops the liquidator output. We should probably fail this in the validate stage.
| // Funding priced the CET fee from the accepting party's payout | ||
| // script; a longer script in its place underpays the fee rate. | ||
| let accept_script_len = contract.accept.payout_script_pubkey.len(); | ||
| if override_.script_pubkey.len() > accept_script_len { |
There was a problem hiding this comment.
Should we size the CET fee off the longest script at funding time instead of rejecting here?
By having this check, it's possible for the third party's address type to fail the accept. If the accepter's payout script is P2WPKH (22 bytes) and an override script is taproot or P2WSH (34 bytes), the accept gets rejected, and the offerer doesn't always control what address the third party hands them. The fee only got reserved off the accepter's own script when the funding tx was built, here:
Lines 424 to 440 in 24cff24
So if the offer carries overrides, we could price the CET off the longest script among the accepter's payout script and every override script. Then this check can go, and the only cost is a few sats per CET when the accepter's own script is the shorter one.
There was a problem hiding this comment.
Note
🤖 Claude Opus 5.5 responding on behalf of bennyhodl
Done in dbecd57, the way you described. The length check is gone. A CetRecord now reports the longest script it pays in the accepter's place (accept_payout_script_len), and the builder has funding reserve the accepter's CET output fee at the longest of that and the accepter's own script. The extra goes to whoever already pays for the accepter's output: the accepter when both fund, the offerer when it funds everything. Offers without overrides fund exactly as before, and the pinned fixtures still match.
a_payout_script_override_longer_than_the_accept_script_keeps_the_cet_fee_rate signs a CET paying a taproot override for a P2WPKH accepter and checks it meets the offer's fee rate. Without the fee change it fails (a 180-vbyte CET paying 340 sats at 2 sat/vB). node-dlc has to price funding the same way; it's written up in the payout_overrides module docs.
One gap is left as is: coin selection still estimates with a P2WPKH script, so a party whose inputs only just cover the estimate can fail with "input amount is less than required" when a taproot override is present. That estimate already ignores the wallet's own script type.
| } | ||
| let mut covers_any = false; | ||
| for (index, contract_info) in contract_infos.iter().enumerate() { | ||
| for (position, _) in |
There was a problem hiding this comment.
Should we also be rejecting an override on a CET whose accept payout is under dust here?
covered_cets already hands back the payout, so it's for (position, payout) in ... and a check that payout.accept >= DUST_LIMIT. Right now the override is accepted, then create_cet drops the output as dust, so the third party silently ends up with nothing and nobody gets an error.
There was a problem hiding this comment.
Note
🤖 Claude Opus 5.5 responding on behalf of bennyhodl
Done in dbecd57. validate rejects an override covering any CET whose accept payout is under DUST_LIMIT (now public in ddk-dlc) as an invalid offer. Outcomes without an override are unaffected, so a loan whose borrower-favourable outcomes pay the lender nothing still works. A numeric range can't run into the part of the curve where the accepter gets under dust; it has to end before it. The invalid-offer test has an enum case and a numeric case, and each fails without the check.
A CET pays the two parties and nothing else, and a product settled by a DLC can need an outcome to pay someone else: a loan whose liquidation pays the liquidator in the lender's place. Both parties must build the same CETs or their adaptor signatures do not verify, so what changes a CET has to travel on the offer, and it is protocol, not a per-node option. `contract::cet_records` is the extension point. A `CetRecord` is a TLV record on the offer with a `validate` that runs when the offer arrives, before any accept exists, and an `apply` that rewrites the built CETs with both parties in hand. `CET_RECORDS` is the registry: the decoders of the record types the library knows, in the order the records apply. A record type that appears twice is rejected, since this crate reads the first copy and node-dlc the last. Adding a record is a type in dlc-messages, a `CetRecord` impl and an entry in the registry; the module documents what a record may change and the fee it has to stay within. The builder applies the records once for every path, the manager and the stateless module validate them when an offer arrives, and an offer without records builds exactly what it built before. The library applies every record it knows, whichever party wrote it, and a record can redirect what the accepting party is paid. So the manager takes TLV records on an offer only of the types the application allows with `Manager::with_allowed_offer_tlv_types`, or the builder's `set_allowed_offer_tlv_types`. None is allowed by default: an offer received or committed with a record of any other type is rejected. That includes the application records offers could carry since rc.4, so an application sending one now lists its type. Checking the records an offer carries before accepting it stays the application's job. `PayoutScriptOverrides` is the first record. For each CET an override covers, the output that would have paid the accepting party pays the override script, in every contract info the override matches. An enum override covers its outcome's CET; a numeric one covers every CET whose values lie in its range, and a range that covers part of a CET's values is an invalid offer, since that CET would have no single script. A CET's values are the ones it settles: when the oracles use different numbers of digits, the last CET also settles every value above the curve that the oracles with more digits can attest, so its range runs to the largest of those. The CET is rebuilt with `create_cet` rather than patched by matching scripts, which breaks when both parties pay out to the same one. An override that covers no CET, an empty script, a range that starts after it ends, two overrides covering one CET, and an override covering a CET that pays the accepting party under the dust limit, which would silently pay the script nothing, are invalid offers. An override script may be longer than the accepting party's payout script: a liquidator's taproot address with a P2WPKH lender. A `CetRecord` reports the longest script it pays in the accepting party's place, and the builder has funding reserve the CET fee for the accepting party's output at the longest of that and the party's own script. The `_with_fee_rule` constructors in ddk-dlc take that length. Whoever pays for the accepting party's output pays the difference: the accepting party when both fund, the offering party when it funds the whole contract. An offer without overrides funds exactly as before. ddk-dlc's `DUST_LIMIT` is public now, and the manager's contract input uses it in place of its own copy. Tests settle an overridden enum outcome from both sides through the stateless module and through the manager, and an overridden numeric range through the stateless module, keep the offering party's output when both parties share a script, keep the offer's CET fee rate with a taproot override for a P2WPKH accepting party, and cover each rejection.
The builder added the two parties' collateral with `Amount`'s `+`, which panics on overflow. The accepting party picks its own collateral, so an accept of `u64::MAX` against any offer took the node down instead of being rejected. The sum is now checked, and an overflow is the same invalid accept as a collateral that does not complete the total.
The override tests attested a random outcome, and only the outcomes that pay the accepting party are overridden, so half the runs settled on a CET the record never touched and passed with or without it. They now attest an overridden outcome, settle it from each side, and check the CET pays the whole collateral to the third party.
24cff24 to
8fad012
Compare
…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.
2 participants
Summary
Adds a way for TLV records on an offer to change the CETs a contract builds, with
PayoutScriptOverridesas the first record: the CETs of named outcomes, an enum outcome or a range of numeric values, pay a third-party script in place of the accepting party (e.g. a loan liquidation paying the liquidator). Along the way it fixes a pre-existing bug where a contract with several contract infos could only settle on its first one, and moves funding/CET/refund construction into one builder.Changes
contract::transactions::build_contract_transactionsreplaces three copies (manager accept, manager sign, stateless module). It owns the "no contract info", "no CET" and collateral-total checks. ItsBuildErrorkeeps the stateless API returningInvalidOffer/InvalidAccept. The collateral sum is checked, so an accept withu64::MAXcollateral is an invalid accept instead of a panic.contract::cet_recordsdefinesCetRecord:validateruns on the offer alone when it arrives, andapplyrewrites the built CETs once both parties are known. A record type that appears twice is rejected, because this crate reads the first copy and node-dlc the last. An offer without records builds exactly what it built before.PayoutScriptOverrides(TLV type 65005, agreed with node-dlc) indlc-messages. The type is odd, so a peer that doesn't know the record carries it through and then fails signature verification. node-dlc has to match the new encoding, which carries numeric ranges and is pinned byte for byte. An override names an enum outcome or a numeric range; a range covers every CET whose values lie inside it. A CET's values are the ones it settles: with oracles of different digit counts, the last CET also settles every value above the curve, so its range runs to the largest value an oracle can attest. Each overridden CET is rebuilt withcreate_cetrather than patched by script matching, which broke when both parties share a payout script. The following are rejected:CetRecord::accept_payout_script_lenreports the length, and the_with_fee_ruleconstructors inddk-dlctake it. Offers without overrides fund exactly as before. node-dlc has to price funding the same way.ddk_dlc::DUST_LIMITis now public.Manager::with_allowed_offer_tlv_typesor the builder'sset_allowed_offer_tlv_types. None is allowed by default: an offer received or committed with any other type is rejected. That includes the application records offers have been able to carry since rc.4, so an application that sends one must now list its type. Checking the records an offer carries before accepting it is still the application's job. The stateless module is unchanged, because its caller holds the offer.MemoryOraclereturns an error for an event it hasn't signed, as a networked oracle does, instead of an attestation with no signatures.Testing
ddk/tests/fixtures/contract_transactions/pins every existing contract shape (enum/numerical, dual/single funded, disjoint, spliced, refund-to-accepter): a wire-encoded offer and accept per shape (<shape>.offer.bin,<shape>.accept.bin) and the transaction digests indigests.json, all written by the builder before the move. The moved builder reproduces them byte for byte.cargo test --all-featurespasses.cargo fmt --checkandcargo clippy -- -D warningspass.--ignoredmanager_execution_testsrun was interrupted after 32 passing tests.enum_and_numerical_with_diff_3_of_5_testandenum_single_oracle_bad_accept_cet_sig_testfailed while a second bitcoind/electrs suite ran at the same time.enum_and_numerical_with_diff_3_of_5_testreruns. Rerun alone, it passed once. It failed twice with electrs dropping the connection ingenerate_blocks, but those reruns also overlapped the full suite. It needs a clean run, andenum_single_oracle_bad_accept_cet_sig_testhasn't been rerun.--ignoredstateless_executionpassed up to and including the slowsplice_out_numeric_with_difference_three_of_five_oracles_closes, which takes ~640s on both this branch and master. The tests after it haven't run on the final commit.