Skip to content

ddk: validate an offer's structure without a timeout policy - #233

Open
bennyhodl wants to merge 6 commits into
masterfrom
benny/225-validate-offer-structure
Open

bennyhodl wants to merge 6 commits into
masterfrom
benny/225-validate-offer-structure

Conversation

@bennyhodl

Copy link
Copy Markdown
Owner

Summary

Item 8 of #225. validate_offer(offer, min_timeout_interval, max_timeout_interval, now_unix) bundled structural checks with the accepting party's timeout policy and clock. Lygos re-implemented the structural half. This PR splits them, with one place per check.

API

  • ddk::contract::validate_offer_structure(offer) checks:
    • the oracle announcements;
    • the protocol version;
    • fee rate and collateral;
    • payout and change scripts against the spec's standard forms;
    • locktime units and order, and a CET locktime no later than maturity;
    • unique funding serial ids, and distinct change and fund output serial ids;
    • splice inputs against the 2-of-2 they name;
    • payouts that convert for every outcome the oracles can attest.
      It doesn't decode ordinary funding inputs, so it checks neither their values nor the spec's SegWit rule. The docs say so.
  • validate_offer = validate_offer_structure + the timeout and maturity policy. Its signature is unchanged.
  • dlc-messages: OfferDlc::validate is split into validate_announcements, validate_terms and validate_timing. validate runs all three and keeps its signature.
  • ddk_dlc::util::validate_collateral (offer ≤ total, total ≥ 1000 sats, offer collateral 0 or ≥ 1000 sats) is shared with ContractInput::validate in ddk-manager.
  • create_offer drops its own copies of these checks and runs the shared policy-free checks on the offer it built.

Creation doesn't validate announcements

create_offer, and the manager's own outgoing offers, skip announcement validation, as master did. The offering party chose its own oracles; validating announcements is the receiver's check. This also keeps Lygos's offers building: its oracle commits one nonce per enum outcome, which OracleEvent::validate rejects. Whether DDK supports that convention is item 7 of #225, which is undecided. A test pins the difference.

Fixes found along the way

  • ddk-manager/src/conversion_utils.rs subtracted an enum payout from total collateral unchecked, and expected a non-empty numeric payout function. A crafted offer panicked accept_offer on master; this PR would also have exposed that panic through validate_offer. Both cases now return InvalidParameters. The payout-function conversion is TryFrom instead of From.

Behaviour changes

  • Incoming offers, through accept_offer and the manager's on_offer_message, are now rejected for:
    • non-standard payout or change scripts;
    • mixed locktime units;
    • a change serial id equal to the fund output's;
    • non-zero offer collateral under 1000 sats.
      These are protocol requirements in the DLC spec.
  • Two deliberate exceptions, each documented where it's checked:
    • cet_locktime = 0 passes against either locktime unit (it locks nothing, and an existing test relies on it);
    • zero offer collateral is allowed, so the accepting party can fund the whole contract.
  • Duplicate funding serial ids from create_offer now return InvalidOffer; they used to return InvalidFundingInput.

Testing

  • New stateless tests:
    • offer_structure_is_validated_without_a_timeout_policy: a create_offer offer passes, and nine structural defects are each rejected.
    • validate_offer_adds_the_timeout_policy_to_the_structure
    • only_the_receiver_checks_the_announcements
    • unrepresentable_payouts_are_rejected: an enum payout above total and an empty numeric curve return errors from both validate_offer_structure and accept_offer, with no panic.
  • Unit suites: stateless 46, ddk contract lib 42, ddk-messages 73, ddk-dlc 22, ddk-manager lib 59, manager_tests 5 and contract_updater 1, all passing.
  • Before the review fixes, all 47 stateless_execution ignored tests and 5 of the 80 manager_execution tests passed on regtest. The fixes don't touch the funding or settlement path.
  • Doc tests weren't run locally. CI runs them.
  • cargo fmt --check, cargo clippy -- -D warnings and cargo check -p ddk --no-default-features pass.
  • Reviewed by GPT-6 Astra. Fixed from the review:
    • the Lygos creation regression;
    • panics on untrusted payouts;
    • docs that claimed more than the function checks.

@bennyhodl

Copy link
Copy Markdown
Owner Author

Note

🤖 Claude Opus 5.5 responding on behalf of bennyhodl

Review against orange-grove's consumers (lygos_utils::dlc, the wallet crate, the server's Fordefi signer) before 2.0.0. Change first.

The structure-versus-policy split is right, and it would replace about 70 lines of offer checks in lygos-utils/src/dlc/validate.rs. As it stands we can't use it:

  1. Creation and validation disagree. create_offer validates without the announcement check, so an offer with per-outcome enum nonces is created, then rejected by validate_offer_structure; only_the_receiver_checks_the_announcements records exactly this. Decide contract: the primitives the Lygos wallet had to write itself (external-signer split, keyless settle, tracker, verification) #225 item 7 (see the ddk: settle a contract from its messages without a key #228 comment) before 2.0.0, so the two sides agree.
  2. Behaviour differences to settle on purpose. It permits zero offer collateral and a zero CET locktime across units; orange-grove refuses both. If these are intended, say so in the docs; if not, refuse them here.
  3. A policy-free accept validator alongside this one would let consumers delete their accept checks too.

…info

`get_contract_info_and_announcements` subtracted each enum payout from
the total collateral unchecked, so a payout above the total panicked,
in release builds too. A numeric payout function with no pieces
converted fine and then hit an `expect` in `PayoutFunction::validate`.
Both come from the counterparty's offer. The subtraction is now
checked and the payout function conversion is a `TryFrom` that rejects
an empty function, both returning `InvalidParameters`.
…ming

`OfferDlc::validate` mixed the protocol rules with the receiver's
timeout bounds and clock, so a party with no timeout policy could not
check an offer at all. It is now three checks:

- `validate_announcements`: oracle signatures and events, the receiving
  party's check on oracles it did not choose;
- `validate_terms`: the message-only protocol rules;
- `validate_timing`: the refund window and maturity.

`validate_terms` also covers the offer_dlc MUSTs that went unchecked:
standard payout and change scripts, CET and refund locktimes in one
unit and in order, unique funding input serial ids, distinct change and
fund output serial ids, the fee rate, and the collateral. A zero CET
locktime passes against either unit: it does not lock the CETs.

The collateral rule moves to `ddk_dlc::util::validate_collateral` and
`ContractInput::validate` uses it instead of its own dust constant. It
keeps DDK's allowance for zero offer collateral. The manager checks its
own offers with `validate_terms`, where it only checked the CET
locktime before.
`validate_offer` needed timeout bounds and a clock, so a stateless
consumer that only stores or relays offers had to re-implement the
structural checks. `validate_offer_structure(offer)` runs the oracle
announcement checks, the protocol version, `OfferDlc::validate_terms`,
splice input consistency, and the execution information (payouts cover
every attestable outcome). It does not decode ordinary funding inputs,
and says so.

`validate_offer` is now that plus `OfferDlc::validate_timing`.
`create_offer` runs the same checks except the announcements, through
one private `validate_offer_terms`, instead of its own copies of the
fee rate, collateral, serial id, locktime and execution information
checks. The offering party chose its oracles; checking announcements is
the receiver's job, and creation never did it. Whether one-nonce-per-
outcome oracles are supported is still open (#225 item 7), and a test
pins that creation accepts them while validation rejects them.
`build_context` shares one `execution_infos` helper with validation.
Under fuzztarget every serial id and temporary id came from a freshly
seeded RNG, so each draw returned the same value. An offer built with
the feature then had equal change and fund output serial ids, which
offer validation now rejects. The generator keeps its fixed seed but
lives for the thread, so a run stays reproducible and its ids differ.
`create_offer` skipped the oracle announcement check, so it built offers
over an enum event with one nonce per outcome that `validate_offer_structure`
then rejected. An offer the counterparty refuses is never worth sending, so
per-outcome nonces are now refused on both sides: `create_offer` runs
`validate_offer_structure` itself, and the manager's `offer_contract` checks
each announcement with the same `OracleAnnouncement::validate` before it
reserves any wallet input. `OracleEvent::validate` stays the one owner of
the one-nonce-per-enum-event rule.

The test that recorded the old behaviour is replaced by one that asserts
creation refuses such an announcement, and the manager gains the matching
test.

Zero offer collateral and a zero CET locktime stay allowed. They are valid
DLCs (one-sided funding, CETs that are not time-locked); refusing them is
application policy. The docs of `validate_offer_structure` now say so, and
its test pins the zero CET locktime next to the zero collateral.
@bennyhodl
bennyhodl force-pushed the benny/225-validate-offer-structure branch from 3bcf799 to 4e8beaa Compare October 15, 2026 18:34
A party that stores, relays or audits an accept had no single call that
checked it; the rules lived inline in ddk's context building, and the
manager checked fewer of them. The accept now has one owner per rule,
split the way the offer's are.

`AcceptDlc::validate_terms` in dlc-messages holds the message-only rules:
the accept answers this offer (protocol version, temporary contract id),
carries no negotiation fields, has standard payout and change scripts,
uses funding input serial ids distinct across both parties, and has a
change serial id that differs from the fund output's. It shares the
script and serial-id helpers with `OfferDlc::validate_terms`.

`ddk::contract::validate_accept` runs `validate_offer_structure`, the
accept's terms, then rebuilds the contract and verifies the accepting
party's refund and CET adaptor signatures. `sign_accept` signs from that
same verified context, so the two cannot drift. ddk's context building
and `accept_offer` call `validate_terms` in place of their own checks,
and the manager's `verify_accepted_and_sign_contract` calls it too.

The collateral rule stays in `build_contract_transactions`: the builder
also runs on party parameters that never came in an accept message (the
accepting side, before its accept exists), and every path that builds a
contract passes through it.

The manager now rejects accepts it used to take: one whose protocol
version differs from the offer's, one with negotiation fields, one with a
non-standard payout or change script, one whose funding input serial id
repeats another input's, and one whose change serial id equals the fund
output serial id.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant