Skip to content

[1/3] aux revocation: add deterministic HTLCs - #11094

Open
GeorgeTsagk wants to merge 5 commits into
lightningnetwork:masterfrom
GeorgeTsagk:deterministic-htlcs
Open

[1/3] aux revocation: add deterministic HTLCs#11094
GeorgeTsagk wants to merge 5 commits into
lightningnetwork:masterfrom
GeorgeTsagk:deterministic-htlcs

Conversation

@GeorgeTsagk

Copy link
Copy Markdown
Collaborator

Description

Punishing a revoked commitment on an asset channel requires more than the bitcoin-level justice sweep: the asset side needs a valid asset witness for whatever transaction the cheater actually confirms, including second-level HTLC spends.

Under the normal SINGLE|ANYONECANPAY scheme those second-level transactions are malleable (the broadcaster can add fee inputs, batch, or RBF them), so their final form is unknowable in advance and no asset witness or proof chain could be prepared for them. With deterministic HTLCs both parties commit to the exact final second-level transaction via SIGHASH_DEFAULT, so the revoking party can pre-sign asset-level signatures for both spend paths of every HTLC and hand them over in RevokeAndAck (this will be the next part), giving the honest party everything needed to claim the assets no matter which path a breach takes.

@GeorgeTsagk GeorgeTsagk self-assigned this Aug 18, 2026
@github-actions github-actions Bot added the severity-critical Requires expert review - security/consensus critical label Aug 18, 2026
@github-actions

Copy link
Copy Markdown

🔴 PR Severity: CRITICAL

file classification | 23 files | 1205 lines changed (768 excluding tests)

🔴 Critical (14 files)
  • contractcourt/chain_arbitrator.go - on-chain dispute resolution coordination
  • contractcourt/chain_watcher.go - on-chain dispute resolution coordination
  • contractcourt/contract_resolver.go - on-chain dispute/breach resolution logic
  • contractcourt/htlc_success_resolver.go - HTLC breach/resolution handling
  • contractcourt/htlc_timeout_resolver.go - HTLC breach/resolution handling
  • htlcswitch/link.go - HTLC forwarding state machine
  • htlcswitch/mock.go - htlcswitch package support code
  • lnwallet/aux_signer.go - commitment/signing logic
  • lnwallet/chanfunding/coin_select.go - channel funding coin selection
  • lnwallet/channel.go - channel state, commitment transactions
  • lnwallet/commitment.go - commitment transaction construction
  • lnwallet/mock.go - lnwallet package support code
  • lnwallet/transactions.go - transaction construction/signing
  • server.go - core server coordination
🟢 Low (9 files, tests only)
  • contractcourt/breach_arbitrator_test.go
  • contractcourt/contract_resolver_test.go
  • htlcswitch/mailbox_test.go
  • input/size_test.go
  • lnrpc/walletrpc/walletkit_server_test.go
  • lnwallet/aux_signer_test.go
  • lnwallet/channel_test.go
  • lnwallet/chanfunding/coin_select_test.go
  • lnwallet/transactions_test.go

Analysis

This PR touches core contractcourt/*, htlcswitch/*, and lnwallet/* packages — on-chain breach/HTLC resolution, HTLC forwarding, and commitment transaction/signing logic — each of which is independently CRITICAL. The change spans multiple distinct critical packages (contractcourt, htlcswitch, lnwallet) and modifies ~768 non-test lines across 14 non-test files, well above the >500 line bump threshold, reinforcing the CRITICAL classification. Given the surface area (commitment transactions, HTLC resolvers, channel funding), this warrants expert review focused on correctness of on-chain resolution paths and signing logic.


To override, add a severity-override-{critical,high,medium,low} label.

@GeorgeTsagk

Copy link
Copy Markdown
Collaborator Author

Added no-changelog label, will include one with the 3rd and final part of this PR series.

@gijswijs gijswijs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are nearly there with this PR. I did a real human-powered, thorough review of the code and found some issues, that I commented on inline. The below comment is about a commit message.

The commit message on the first commit claims to replace CustomBlob.IsSome() but I don't see that specific method being replaced anywhere. Where does that come from?

Comment thread input/size_test.go Outdated
Comment thread lnwallet/aux_signer.go Outdated
Comment thread lnwallet/aux_signer_test.go
Comment thread lnwallet/channel.go Outdated
Comment thread contractcourt/contract_resolver.go Outdated
Comment thread contractcourt/contract_resolver.go
Comment thread contractcourt/contract_resolver.go
Comment thread contractcourt/htlc_timeout_resolver.go Outdated
Comment thread contractcourt/contract_resolver.go
Comment thread lnwallet/channel.go
Introduce a feature-bit negotiation approach for choosing the sighash
type of second-level HTLC transactions, via a new
AuxSigner.HtlcSigHashType hook.

Add a ResolveHtlcSigHashType helper that queries the aux signer for a
channel-specific sighash override based on negotiated features, falling
back to the default HtlcSigHashType when no aux signer is present or
the feature isn't negotiated. Thread the auxSigner through all HTLC
second-level transaction signing and validation call sites.

An earlier draft of this feature selected the sighash based on the
presence of the commitment's custom blob alone, which is backwards
incompatible: a peer that has not upgraded would disagree on the
sighash and force close the channel. Explicit feature negotiation
ensures both channel parties agree on the sighash type before it is
ever used.
When DeterministicHTLCs is negotiated (SigHashDefault second-level
HTLCs), the peer's signature commits to the entire transaction, so the
sweeper can neither add fee inputs nor RBF the pre-signed tx. Instead of
baking a large fixed fee into the transaction, give it a minimal
baked-in fee plus a CPFP anchor:

- The pre-signed second-level HTLC tx gets a second output: a taproot
  anchor (AnchorSize sats, keyed to the broadcaster's delay key) that
  the local party can sweep to CPFP-bump the package fee rate.
- The baked-in fee is computed at 1.1x the relay floor over the tx
  weight including the anchor output, just enough to clear min-relay
  even for nodes computing fee rate over raw serialized size.
- HtlcIsDust and the second-level fee/output-amount calculations are
  threaded with the sigHashDefault flag so dust decisions and the HTLC
  output value account for both the floor fee and the AnchorSize
  reduction.
- The sweep sign descriptor for the second-level output is reduced by
  AnchorSize accordingly, so the sweeper signs for the correct value.

Only aux/custom (taproot asset) channels can negotiate SigHashDefault
(see ResolveHtlcSigHashType); all other channel types keep their
existing fee and transaction form.
…ault

When a second-level HTLC transaction was signed with SigHashDefault, the
peer's signature commits to the entire transaction: the sweeper's usual
flow of rebuilding the tx with extra fee inputs or batched outputs would
invalidate it. Add isSigHashDefault() and publishTimeoutTx() /
publishSuccessTx() to the timeout and success resolvers, which broadcast
each pre-signed transaction directly and individually via PublishTx. The
second-level output itself is still swept through the sweeper after
confirmation, like today.

The gate lives in the shared isSecondLevelSigHashDefault helper and
requires the channel type to carry a tapscript root: SigHashDefault is
the zero value of SigHashType, so without the explicit channel-type
check any channel that never populates SignDetails.SigHashType would
false-positively match. Only aux/custom (taproot asset) channels carry
a tapscript root, so non-custom channels provably keep using the
sweeper flow.
After publishSuccessTx / publishTimeoutTx broadcasts the pre-signed
second-level HTLC tx, offer the anchor output at index 1 to the sweeper
so the local party can CPFP-bump the parent's effective fee rate.

The pre-signed tx itself cannot be RBF'd under SigHashDefault (the
peer's signature commits to the full tx), so CPFP via this anchor is the
only fee-bumping path. The sweeper handles fee estimation and package
math; we hand it the anchor outpoint, a key-path sign descriptor, and
the parent tx info needed for package fee-rate calculation: the exact
baked-in parent fee (spent commitment output value minus the parent's
outputs) and the parent weight.

The fee budget for the CPFP child is derived from the value under
protection (the second-level HTLC output) via the same
sweeper.budget.anchorcpfp(ratio) configuration used for commitment
anchor CPFP, plus the anchor value itself. The child is funded from
wallet inputs, so the budget can and usually must exceed the anchor's
own 330 sats. The sweep deadline is the incoming HTLC's expiry on the
timeout path (matching the other timeout-path sweeps) and the HTLC's
own expiry on the success path. The sweep result is consumed in a
tracked goroutine so terminal failures (budget exhausted, deadline
blown, persistent estimation failure) are logged rather than silently
dropped.

The anchor is keyed to the broadcaster's to-local delay key, which is
derived directly from the second-level output's sweep sign descriptor
(delay base point + single tweak), keeping this path self-contained. A
no-op when the parent tx has fewer than two outputs, i.e. the channel
did not use DeterministicHTLCs.

The publish itself happens asynchronously via publishPreSignedHtlcTx:
a pre-signed timeout tx carries an absolute locktime and is only final
once the chain reaches that height, so the broadcast waits on block
epochs for the locktime to become satisfiable and retries on every new
block until it succeeds. This also heals transient mempool rejections;
without it, a "non final" rejection at resolver launch would
permanently strand the resolver, since Launch() only runs once. The
anchor is offered to the sweeper only after the parent broadcast
succeeded, and all background goroutines are tracked by a wait group
the resolver's Stop drains.
Raise the upper bound on maxFeeRatio in sanityCheckFee from 1.0 to a
hard ceiling of 100.0. CPFP sweeps of second-level HTLC anchors
(AnchorSize sats) legitimately spend more in fees than the swept output
value: the anchor exists purely to pay fees for its parent package, so
a fee-to-output ratio above 100% is expected there rather than a bug.

Ratios above 1.0 only ever reach this code when an RPC caller
explicitly requests one via FundPsbt (every internal funding flow
passes DefaultMaxFeeRatio), and such requests are logged so the opt-in
is auditable. The ceiling still catches nonsensical values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

custom chans no-changelog severity-critical Requires expert review - security/consensus critical

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants