[1/3] aux revocation: add deterministic HTLCs - #11094
Conversation
🔴 PR Severity: CRITICAL
🔴 Critical (14 files)
🟢 Low (9 files, tests only)
AnalysisThis PR touches core To override, add a |
|
Added |
gijswijs
left a comment
There was a problem hiding this comment.
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?
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.
544139a to
74d5642
Compare
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.
74d5642 to
44e5a5e
Compare
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|ANYONECANPAYscheme 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 viaSIGHASH_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.