From 82d77bb47716314673255e82e68cbafa724f7607 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 20 Jun 2024 21:56:52 +0800 Subject: [PATCH 01/19] contractcourt: add verbose logging in resolvers We now put the outpoint in the resolvers's logging so it's easier to debug. --- contractcourt/anchor_resolver.go | 3 ++- contractcourt/breach_resolver.go | 5 +++-- contractcourt/commit_sweep_resolver.go | 4 ++-- contractcourt/contract_resolver.go | 6 ++++-- contractcourt/htlc_success_resolver.go | 25 +++++++++++++++---------- contractcourt/htlc_timeout_resolver.go | 24 ++++++++++++++---------- 6 files changed, 40 insertions(+), 27 deletions(-) diff --git a/contractcourt/anchor_resolver.go b/contractcourt/anchor_resolver.go index af7ac76462a..b50e061f447 100644 --- a/contractcourt/anchor_resolver.go +++ b/contractcourt/anchor_resolver.go @@ -2,6 +2,7 @@ package contractcourt import ( "errors" + "fmt" "io" "sync" @@ -71,7 +72,7 @@ func newAnchorResolver(anchorSignDescriptor input.SignDescriptor, currentReport: report, } - r.initLogger(r) + r.initLogger(fmt.Sprintf("%T(%v)", r, r.anchor)) return r } diff --git a/contractcourt/breach_resolver.go b/contractcourt/breach_resolver.go index 63395651cc3..9a5f4bbe08e 100644 --- a/contractcourt/breach_resolver.go +++ b/contractcourt/breach_resolver.go @@ -2,6 +2,7 @@ package contractcourt import ( "encoding/binary" + "fmt" "io" "github.com/lightningnetwork/lnd/channeldb" @@ -32,7 +33,7 @@ func newBreachResolver(resCfg ResolverConfig) *breachResolver { replyChan: make(chan struct{}), } - r.initLogger(r) + r.initLogger(fmt.Sprintf("%T(%v)", r, r.ChanPoint)) return r } @@ -114,7 +115,7 @@ func newBreachResolverFromReader(r io.Reader, resCfg ResolverConfig) ( return nil, err } - b.initLogger(b) + b.initLogger(fmt.Sprintf("%T(%v)", b, b.ChanPoint)) return b, nil } diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go index 423d235dbf7..7b101f80e33 100644 --- a/contractcourt/commit_sweep_resolver.go +++ b/contractcourt/commit_sweep_resolver.go @@ -88,7 +88,7 @@ func newCommitSweepResolver(res lnwallet.CommitOutputResolution, chanPoint: chanPoint, } - r.initLogger(r) + r.initLogger(fmt.Sprintf("%T(%v)", r, r.commitResolution.SelfOutPoint)) r.initReport() return r @@ -484,7 +484,7 @@ func newCommitSweepResolverFromReader(r io.Reader, resCfg ResolverConfig) ( // removed this, but keep in mind that this data may still be present in // the database. - c.initLogger(c) + c.initLogger(fmt.Sprintf("%T(%v)", c, c.commitResolution.SelfOutPoint)) c.initReport() return c, nil diff --git a/contractcourt/contract_resolver.go b/contractcourt/contract_resolver.go index 3629c1bc3c6..ff52ce9760d 100644 --- a/contractcourt/contract_resolver.go +++ b/contractcourt/contract_resolver.go @@ -120,8 +120,10 @@ func newContractResolverKit(cfg ResolverConfig) *contractResolverKit { } // initLogger initializes the resolver-specific logger. -func (r *contractResolverKit) initLogger(resolver ContractResolver) { - logPrefix := fmt.Sprintf("%T(%v):", resolver, r.ChanPoint) +func (r *contractResolverKit) initLogger(prefix string) { + logPrefix := fmt.Sprintf("ChannelArbitrator(%v): %s:", r.ChanPoint, + prefix) + r.log = log.WithPrefix(logPrefix) } diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index 06ebf4edc4a..363a5cc0444 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -2,6 +2,7 @@ package contractcourt import ( "encoding/binary" + "fmt" "io" "sync" @@ -81,27 +82,30 @@ func newSuccessResolver(res lnwallet.IncomingHtlcResolution, } h.initReport() + h.initLogger(fmt.Sprintf("%T(%v)", h, h.outpoint())) return h } -// ResolverKey returns an identifier which should be globally unique for this -// particular resolver within the chain the original contract resides within. -// -// NOTE: Part of the ContractResolver interface. -func (h *htlcSuccessResolver) ResolverKey() []byte { +// outpoint returns the outpoint of the HTLC output we're attempting to sweep. +func (h *htlcSuccessResolver) outpoint() wire.OutPoint { // The primary key for this resolver will be the outpoint of the HTLC // on the commitment transaction itself. If this is our commitment, // then the output can be found within the signed success tx, // otherwise, it's just the ClaimOutpoint. - var op wire.OutPoint if h.htlcResolution.SignedSuccessTx != nil { - op = h.htlcResolution.SignedSuccessTx.TxIn[0].PreviousOutPoint - } else { - op = h.htlcResolution.ClaimOutpoint + return h.htlcResolution.SignedSuccessTx.TxIn[0].PreviousOutPoint } - key := newResolverID(op) + return h.htlcResolution.ClaimOutpoint +} + +// ResolverKey returns an identifier which should be globally unique for this +// particular resolver within the chain the original contract resides within. +// +// NOTE: Part of the ContractResolver interface. +func (h *htlcSuccessResolver) ResolverKey() []byte { + key := newResolverID(h.outpoint()) return key[:] } @@ -679,6 +683,7 @@ func newSuccessResolverFromReader(r io.Reader, resCfg ResolverConfig) ( } h.initReport() + h.initLogger(fmt.Sprintf("%T(%v)", h, h.outpoint())) return h, nil } diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index 81d8e85d21c..ca456ec4c82 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -82,6 +82,7 @@ func newTimeoutResolver(res lnwallet.OutgoingHtlcResolution, } h.initReport() + h.initLogger(fmt.Sprintf("%T(%v)", h, h.outpoint())) return h } @@ -93,23 +94,25 @@ func (h *htlcTimeoutResolver) isTaproot() bool { ) } -// ResolverKey returns an identifier which should be globally unique for this -// particular resolver within the chain the original contract resides within. -// -// NOTE: Part of the ContractResolver interface. -func (h *htlcTimeoutResolver) ResolverKey() []byte { +// outpoint returns the outpoint of the HTLC output we're attempting to sweep. +func (h *htlcTimeoutResolver) outpoint() wire.OutPoint { // The primary key for this resolver will be the outpoint of the HTLC // on the commitment transaction itself. If this is our commitment, // then the output can be found within the signed timeout tx, // otherwise, it's just the ClaimOutpoint. - var op wire.OutPoint if h.htlcResolution.SignedTimeoutTx != nil { - op = h.htlcResolution.SignedTimeoutTx.TxIn[0].PreviousOutPoint - } else { - op = h.htlcResolution.ClaimOutpoint + return h.htlcResolution.SignedTimeoutTx.TxIn[0].PreviousOutPoint } - key := newResolverID(op) + return h.htlcResolution.ClaimOutpoint +} + +// ResolverKey returns an identifier which should be globally unique for this +// particular resolver within the chain the original contract resides within. +// +// NOTE: Part of the ContractResolver interface. +func (h *htlcTimeoutResolver) ResolverKey() []byte { + key := newResolverID(h.outpoint()) return key[:] } @@ -1038,6 +1041,7 @@ func newTimeoutResolverFromReader(r io.Reader, resCfg ResolverConfig) ( } h.initReport() + h.initLogger(fmt.Sprintf("%T(%v)", h, h.outpoint())) return h, nil } From 3cb48a01349ab2871eea040356e87134f91a0f44 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 14 Nov 2024 02:28:56 +0800 Subject: [PATCH 02/19] contractcourt: add spend path helpers in timeout/success resolver This commit adds a few helper methods to decide how the htlc output should be spent. --- contractcourt/htlc_success_resolver.go | 43 +++++++++++++++++++------- contractcourt/htlc_timeout_resolver.go | 40 ++++++++++++++++++++---- 2 files changed, 65 insertions(+), 18 deletions(-) diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index 363a5cc0444..2dc61b18a06 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -127,7 +127,7 @@ func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) { // If we don't have a success transaction, then this means that this is // an output on the remote party's commitment transaction. - if h.htlcResolution.SignedSuccessTx == nil { + if h.isRemoteCommitOutput() { return h.resolveRemoteCommitOutput() } @@ -176,7 +176,7 @@ func (h *htlcSuccessResolver) broadcastSuccessTx() ( // and attach fees at will. We let the sweeper handle this job. We use // the checkpointed outputIncubating field to determine if we already // swept the HTLC output into the second level transaction. - if h.htlcResolution.SignDetails != nil { + if h.isZeroFeeOutput() { return h.broadcastReSignedSuccessTx() } @@ -236,12 +236,9 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (*wire.OutPoint, // We will have to let the sweeper re-sign the success tx and wait for // it to confirm, if we haven't already. - isTaproot := txscript.IsPayToTaproot( - h.htlcResolution.SweepSignDesc.Output.PkScript, - ) if !h.outputIncubating { var secondLevelInput input.HtlcSecondLevelAnchorInput - if isTaproot { + if h.isTaproot() { //nolint:ll secondLevelInput = input.MakeHtlcSecondLevelSuccessTaprootInput( h.htlcResolution.SignedSuccessTx, @@ -371,7 +368,7 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (*wire.OutPoint, // Let the sweeper sweep the second-level output now that the // CSV/CLTV locks have expired. var witType input.StandardWitnessType - if isTaproot { + if h.isTaproot() { witType = input.TaprootHtlcAcceptedSuccessSecondLevel } else { witType = input.HtlcAcceptedSuccessSecondLevel @@ -421,16 +418,12 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (*wire.OutPoint, func (h *htlcSuccessResolver) resolveRemoteCommitOutput() ( ContractResolver, error) { - isTaproot := txscript.IsPayToTaproot( - h.htlcResolution.SweepSignDesc.Output.PkScript, - ) - // Before we can craft out sweeping transaction, we need to // create an input which contains all the items required to add // this input to a sweeping transaction, and generate a // witness. var inp input.Input - if isTaproot { + if h.isTaproot() { inp = lnutils.Ptr(input.MakeTaprootHtlcSucceedInput( &h.htlcResolution.ClaimOutpoint, &h.htlcResolution.SweepSignDesc, @@ -712,3 +705,29 @@ func (h *htlcSuccessResolver) SupplementDeadline(_ fn.Option[int32]) { // A compile time assertion to ensure htlcSuccessResolver meets the // ContractResolver interface. var _ htlcContractResolver = (*htlcSuccessResolver)(nil) + +// isRemoteCommitOutput returns a bool to indicate whether the htlc output is +// on the remote commitment. +func (h *htlcSuccessResolver) isRemoteCommitOutput() bool { + // If we don't have a success transaction, then this means that this is + // an output on the remote party's commitment transaction. + return h.htlcResolution.SignedSuccessTx == nil +} + +// isZeroFeeOutput returns a boolean indicating whether the htlc output is from +// a anchor-enabled channel, which uses the sighash SINGLE|ANYONECANPAY. +func (h *htlcSuccessResolver) isZeroFeeOutput() bool { + // If we have non-nil SignDetails, this means it has a 2nd level HTLC + // transaction that is signed using sighash SINGLE|ANYONECANPAY (the + // case for anchor type channels). In this case we can re-sign it and + // attach fees at will. + return h.htlcResolution.SignedSuccessTx != nil && + h.htlcResolution.SignDetails != nil +} + +// isTaproot returns true if the resolver is for a taproot output. +func (h *htlcSuccessResolver) isTaproot() bool { + return txscript.IsPayToTaproot( + h.htlcResolution.SweepSignDesc.Output.PkScript, + ) +} diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index ca456ec4c82..698b4ae21ec 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -634,7 +634,7 @@ func (h *htlcTimeoutResolver) spendHtlcOutput() ( // HTLC transaction that is signed using sighash SINGLE|ANYONECANPAY // (the case for anchor type channels). In this case we can re-sign it // and attach fees at will. We let the sweeper handle this job. - case h.htlcResolution.SignDetails != nil && !h.outputIncubating: + case h.isZeroFeeOutput() && !h.outputIncubating: if err := h.sweepSecondLevelTx(); err != nil { log.Errorf("Sending timeout tx to sweeper: %v", err) @@ -643,7 +643,7 @@ func (h *htlcTimeoutResolver) spendHtlcOutput() ( // If this is a remote commitment there's no second level timeout txn, // and we can just send this directly to the sweeper. - case h.htlcResolution.SignedTimeoutTx == nil && !h.outputIncubating: + case h.isRemoteCommitOutput() && !h.outputIncubating: if err := h.sweepDirectHtlcOutput(); err != nil { log.Errorf("Sending direct spend to sweeper: %v", err) @@ -653,7 +653,7 @@ func (h *htlcTimeoutResolver) spendHtlcOutput() ( // If we have a SignedTimeoutTx but no SignDetails, this is a local // commitment for a non-anchor channel, so we'll send it to the utxo // nursery. - case h.htlcResolution.SignDetails == nil && !h.outputIncubating: + case h.isLegacyOutput() && !h.outputIncubating: if err := h.sendSecondLevelTxLegacy(); err != nil { log.Errorf("Sending timeout tx to nursery: %v", err) @@ -769,7 +769,7 @@ func (h *htlcTimeoutResolver) handleCommitSpend( // If the sweeper is handling the second level transaction, wait for // the CSV and possible CLTV lock to expire, before sweeping the output // on the second-level. - case h.htlcResolution.SignDetails != nil: + case h.isZeroFeeOutput(): waitHeight := h.deriveWaitHeight( h.htlcResolution.CsvDelay, commitSpend, ) @@ -851,7 +851,7 @@ func (h *htlcTimeoutResolver) handleCommitSpend( // Finally, if this was an output on our commitment transaction, we'll // wait for the second-level HTLC output to be spent, and for that // transaction itself to confirm. - case h.htlcResolution.SignedTimeoutTx != nil: + case !h.isRemoteCommitOutput(): log.Infof("%T(%v): waiting for nursery/sweeper to spend CSV "+ "delayed output", h, claimOutpoint) @@ -1232,7 +1232,7 @@ func (h *htlcTimeoutResolver) consumeSpendEvents(resultChan chan *spendResult, // continue the loop. hasPreimage := isPreimageSpend( h.isTaproot(), spendDetail, - h.htlcResolution.SignedTimeoutTx != nil, + !h.isRemoteCommitOutput(), ) if !hasPreimage { log.Debugf("HTLC output %s spent doesn't "+ @@ -1260,3 +1260,31 @@ func (h *htlcTimeoutResolver) consumeSpendEvents(resultChan chan *spendResult, } } } + +// isRemoteCommitOutput returns a bool to indicate whether the htlc output is +// on the remote commitment. +func (h *htlcTimeoutResolver) isRemoteCommitOutput() bool { + // If we don't have a timeout transaction, then this means that this is + // an output on the remote party's commitment transaction. + return h.htlcResolution.SignedTimeoutTx == nil +} + +// isZeroFeeOutput returns a boolean indicating whether the htlc output is from +// a anchor-enabled channel, which uses the sighash SINGLE|ANYONECANPAY. +func (h *htlcTimeoutResolver) isZeroFeeOutput() bool { + // If we have non-nil SignDetails, this means it has a 2nd level HTLC + // transaction that is signed using sighash SINGLE|ANYONECANPAY (the + // case for anchor type channels). In this case we can re-sign it and + // attach fees at will. + return h.htlcResolution.SignedTimeoutTx != nil && + h.htlcResolution.SignDetails != nil +} + +// isLegacyOutput returns a boolean indicating whether the htlc output is from +// a non-anchor-enabled channel. +func (h *htlcTimeoutResolver) isLegacyOutput() bool { + // If we have a SignedTimeoutTx but no SignDetails, this is a local + // commitment for a non-anchor channel. + return h.htlcResolution.SignedTimeoutTx != nil && + h.htlcResolution.SignDetails == nil +} From 10d9544cb4fd99d5797f773ef20ed44f04ebd861 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 14 Nov 2024 21:59:16 +0800 Subject: [PATCH 03/19] contractcourt: add sweep senders in `htlcSuccessResolver` This commit is a pure refactor in which moves the sweep handling logic into the new methods. --- contractcourt/htlc_success_resolver.go | 381 +++++++++++++------------ 1 file changed, 199 insertions(+), 182 deletions(-) diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index 2dc61b18a06..3531d1fc92e 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -237,55 +237,7 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (*wire.OutPoint, // We will have to let the sweeper re-sign the success tx and wait for // it to confirm, if we haven't already. if !h.outputIncubating { - var secondLevelInput input.HtlcSecondLevelAnchorInput - if h.isTaproot() { - //nolint:ll - secondLevelInput = input.MakeHtlcSecondLevelSuccessTaprootInput( - h.htlcResolution.SignedSuccessTx, - h.htlcResolution.SignDetails, h.htlcResolution.Preimage, - h.broadcastHeight, - input.WithResolutionBlob( - h.htlcResolution.ResolutionBlob, - ), - ) - } else { - //nolint:ll - secondLevelInput = input.MakeHtlcSecondLevelSuccessAnchorInput( - h.htlcResolution.SignedSuccessTx, - h.htlcResolution.SignDetails, h.htlcResolution.Preimage, - h.broadcastHeight, - ) - } - - // Calculate the budget for this sweep. - value := btcutil.Amount( - secondLevelInput.SignDesc().Output.Value, - ) - budget := calculateBudget( - value, h.Budget.DeadlineHTLCRatio, - h.Budget.DeadlineHTLC, - ) - - // The deadline would be the CLTV in this HTLC output. If we - // are the initiator of this force close, with the default - // `IncomingBroadcastDelta`, it means we have 10 blocks left - // when going onchain. Given we need to mine one block to - // confirm the force close tx, and one more block to trigger - // the sweep, we have 8 blocks left to sweep the HTLC. - deadline := fn.Some(int32(h.htlc.RefundTimeout)) - - log.Infof("%T(%x): offering second-level HTLC success tx to "+ - "sweeper with deadline=%v, budget=%v", h, - h.htlc.RHash[:], h.htlc.RefundTimeout, budget) - - // We'll now offer the second-level transaction to the sweeper. - _, err := h.Sweeper.SweepInput( - &secondLevelInput, - sweep.Params{ - Budget: budget, - DeadlineHeight: deadline, - }, - ) + err := h.sweepSuccessTx() if err != nil { return nil, err } @@ -316,99 +268,18 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (*wire.OutPoint, "confirmed!", h, h.htlc.RHash[:]) } - // If we ended up here after a restart, we must again get the - // spend notification. - if commitSpend == nil { - var err error - commitSpend, err = waitForSpend( - &h.htlcResolution.SignedSuccessTx.TxIn[0].PreviousOutPoint, - h.htlcResolution.SignDetails.SignDesc.Output.PkScript, - h.broadcastHeight, h.Notifier, h.quit, - ) - if err != nil { - return nil, err - } - } - - // The HTLC success tx has a CSV lock that we must wait for, and if - // this is a lease enforced channel and we're the imitator, we may need - // to wait for longer. - waitHeight := h.deriveWaitHeight( - h.htlcResolution.CsvDelay, commitSpend, - ) - - // Now that the sweeper has broadcasted the second-level transaction, - // it has confirmed, and we have checkpointed our state, we'll sweep - // the second level output. We report the resolver has moved the next - // stage. - h.reportLock.Lock() - h.currentReport.Stage = 2 - h.currentReport.MaturityHeight = waitHeight - h.reportLock.Unlock() - - if h.hasCLTV() { - log.Infof("%T(%x): waiting for CSV and CLTV lock to "+ - "expire at height %v", h, h.htlc.RHash[:], - waitHeight) - } else { - log.Infof("%T(%x): waiting for CSV lock to expire at "+ - "height %v", h, h.htlc.RHash[:], waitHeight) + err := h.sweepSuccessTxOutput() + if err != nil { + return nil, err } - // We'll use this input index to determine the second-level output - // index on the transaction, as the signatures requires the indexes to - // be the same. We don't look for the second-level output script - // directly, as there might be more than one HTLC output to the same - // pkScript. + // Will return this outpoint, when this is spent the resolver is fully + // resolved. op := &wire.OutPoint{ Hash: *commitSpend.SpenderTxHash, Index: commitSpend.SpenderInputIndex, } - // Let the sweeper sweep the second-level output now that the - // CSV/CLTV locks have expired. - var witType input.StandardWitnessType - if h.isTaproot() { - witType = input.TaprootHtlcAcceptedSuccessSecondLevel - } else { - witType = input.HtlcAcceptedSuccessSecondLevel - } - inp := h.makeSweepInput( - op, witType, - input.LeaseHtlcAcceptedSuccessSecondLevel, - &h.htlcResolution.SweepSignDesc, - h.htlcResolution.CsvDelay, uint32(commitSpend.SpendingHeight), - h.htlc.RHash, h.htlcResolution.ResolutionBlob, - ) - - // Calculate the budget for this sweep. - budget := calculateBudget( - btcutil.Amount(inp.SignDesc().Output.Value), - h.Budget.NoDeadlineHTLCRatio, - h.Budget.NoDeadlineHTLC, - ) - - log.Infof("%T(%x): offering second-level success tx output to sweeper "+ - "with no deadline and budget=%v at height=%v", h, - h.htlc.RHash[:], budget, waitHeight) - - // TODO(roasbeef): need to update above for leased types - _, err := h.Sweeper.SweepInput( - inp, - sweep.Params{ - Budget: budget, - - // For second level success tx, there's no rush to get - // it confirmed, so we use a nil deadline. - DeadlineHeight: fn.None[int32](), - }, - ) - if err != nil { - return nil, err - } - - // Will return this outpoint, when this is spent the resolver is fully - // resolved. return op, nil } @@ -418,53 +289,7 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (*wire.OutPoint, func (h *htlcSuccessResolver) resolveRemoteCommitOutput() ( ContractResolver, error) { - // Before we can craft out sweeping transaction, we need to - // create an input which contains all the items required to add - // this input to a sweeping transaction, and generate a - // witness. - var inp input.Input - if h.isTaproot() { - inp = lnutils.Ptr(input.MakeTaprootHtlcSucceedInput( - &h.htlcResolution.ClaimOutpoint, - &h.htlcResolution.SweepSignDesc, - h.htlcResolution.Preimage[:], - h.broadcastHeight, - h.htlcResolution.CsvDelay, - input.WithResolutionBlob( - h.htlcResolution.ResolutionBlob, - ), - )) - } else { - inp = lnutils.Ptr(input.MakeHtlcSucceedInput( - &h.htlcResolution.ClaimOutpoint, - &h.htlcResolution.SweepSignDesc, - h.htlcResolution.Preimage[:], - h.broadcastHeight, - h.htlcResolution.CsvDelay, - )) - } - - // Calculate the budget for this sweep. - budget := calculateBudget( - btcutil.Amount(inp.SignDesc().Output.Value), - h.Budget.DeadlineHTLCRatio, - h.Budget.DeadlineHTLC, - ) - - deadline := fn.Some(int32(h.htlc.RefundTimeout)) - - log.Infof("%T(%x): offering direct-preimage HTLC output to sweeper "+ - "with deadline=%v, budget=%v", h, h.htlc.RHash[:], - h.htlc.RefundTimeout, budget) - - // We'll now offer the direct preimage HTLC to the sweeper. - _, err := h.Sweeper.SweepInput( - inp, - sweep.Params{ - Budget: budget, - DeadlineHeight: deadline, - }, - ) + err := h.sweepRemoteCommitOutput() if err != nil { return nil, err } @@ -731,3 +556,195 @@ func (h *htlcSuccessResolver) isTaproot() bool { h.htlcResolution.SweepSignDesc.Output.PkScript, ) } + +// sweepRemoteCommitOutput creates a sweep request to sweep the HTLC output on +// the remote commitment via the direct preimage-spend. +func (h *htlcSuccessResolver) sweepRemoteCommitOutput() error { + // Before we can craft out sweeping transaction, we need to create an + // input which contains all the items required to add this input to a + // sweeping transaction, and generate a witness. + var inp input.Input + + if h.isTaproot() { + inp = lnutils.Ptr(input.MakeTaprootHtlcSucceedInput( + &h.htlcResolution.ClaimOutpoint, + &h.htlcResolution.SweepSignDesc, + h.htlcResolution.Preimage[:], + h.broadcastHeight, + h.htlcResolution.CsvDelay, + input.WithResolutionBlob( + h.htlcResolution.ResolutionBlob, + ), + )) + } else { + inp = lnutils.Ptr(input.MakeHtlcSucceedInput( + &h.htlcResolution.ClaimOutpoint, + &h.htlcResolution.SweepSignDesc, + h.htlcResolution.Preimage[:], + h.broadcastHeight, + h.htlcResolution.CsvDelay, + )) + } + + // Calculate the budget for this sweep. + budget := calculateBudget( + btcutil.Amount(inp.SignDesc().Output.Value), + h.Budget.DeadlineHTLCRatio, + h.Budget.DeadlineHTLC, + ) + + deadline := fn.Some(int32(h.htlc.RefundTimeout)) + + log.Infof("%T(%x): offering direct-preimage HTLC output to sweeper "+ + "with deadline=%v, budget=%v", h, h.htlc.RHash[:], + h.htlc.RefundTimeout, budget) + + // We'll now offer the direct preimage HTLC to the sweeper. + _, err := h.Sweeper.SweepInput( + inp, + sweep.Params{ + Budget: budget, + DeadlineHeight: deadline, + }, + ) + + return err +} + +// sweepSuccessTx attempts to sweep the second level success tx. +func (h *htlcSuccessResolver) sweepSuccessTx() error { + var secondLevelInput input.HtlcSecondLevelAnchorInput + if h.isTaproot() { + secondLevelInput = input.MakeHtlcSecondLevelSuccessTaprootInput( + h.htlcResolution.SignedSuccessTx, + h.htlcResolution.SignDetails, h.htlcResolution.Preimage, + h.broadcastHeight, input.WithResolutionBlob( + h.htlcResolution.ResolutionBlob, + ), + ) + } else { + secondLevelInput = input.MakeHtlcSecondLevelSuccessAnchorInput( + h.htlcResolution.SignedSuccessTx, + h.htlcResolution.SignDetails, h.htlcResolution.Preimage, + h.broadcastHeight, + ) + } + + // Calculate the budget for this sweep. + value := btcutil.Amount(secondLevelInput.SignDesc().Output.Value) + budget := calculateBudget( + value, h.Budget.DeadlineHTLCRatio, h.Budget.DeadlineHTLC, + ) + + // The deadline would be the CLTV in this HTLC output. If we are the + // initiator of this force close, with the default + // `IncomingBroadcastDelta`, it means we have 10 blocks left when going + // onchain. + deadline := fn.Some(int32(h.htlc.RefundTimeout)) + + h.log.Infof("offering second-level HTLC success tx to sweeper with "+ + "deadline=%v, budget=%v", h.htlc.RefundTimeout, budget) + + // We'll now offer the second-level transaction to the sweeper. + _, err := h.Sweeper.SweepInput( + &secondLevelInput, + sweep.Params{ + Budget: budget, + DeadlineHeight: deadline, + }, + ) + + return err +} + +// sweepSuccessTxOutput attempts to sweep the output of the second level +// success tx. +func (h *htlcSuccessResolver) sweepSuccessTxOutput() error { + h.log.Debugf("sweeping output %v from 2nd-level HTLC success tx", + h.htlcResolution.ClaimOutpoint) + + // This should be non-blocking as we will only attempt to sweep the + // output when the second level tx has already been confirmed. In other + // words, waitForSpend will return immediately. + commitSpend, err := waitForSpend( + &h.htlcResolution.SignedSuccessTx.TxIn[0].PreviousOutPoint, + h.htlcResolution.SignDetails.SignDesc.Output.PkScript, + h.broadcastHeight, h.Notifier, h.quit, + ) + if err != nil { + return err + } + + // The HTLC success tx has a CSV lock that we must wait for, and if + // this is a lease enforced channel and we're the imitator, we may need + // to wait for longer. + waitHeight := h.deriveWaitHeight(h.htlcResolution.CsvDelay, commitSpend) + + // Now that the sweeper has broadcasted the second-level transaction, + // it has confirmed, and we have checkpointed our state, we'll sweep + // the second level output. We report the resolver has moved the next + // stage. + h.reportLock.Lock() + h.currentReport.Stage = 2 + h.currentReport.MaturityHeight = waitHeight + h.reportLock.Unlock() + + if h.hasCLTV() { + log.Infof("%T(%x): waiting for CSV and CLTV lock to expire at "+ + "height %v", h, h.htlc.RHash[:], waitHeight) + } else { + log.Infof("%T(%x): waiting for CSV lock to expire at height %v", + h, h.htlc.RHash[:], waitHeight) + } + + // We'll use this input index to determine the second-level output + // index on the transaction, as the signatures requires the indexes to + // be the same. We don't look for the second-level output script + // directly, as there might be more than one HTLC output to the same + // pkScript. + op := &wire.OutPoint{ + Hash: *commitSpend.SpenderTxHash, + Index: commitSpend.SpenderInputIndex, + } + + // Let the sweeper sweep the second-level output now that the + // CSV/CLTV locks have expired. + var witType input.StandardWitnessType + if h.isTaproot() { + witType = input.TaprootHtlcAcceptedSuccessSecondLevel + } else { + witType = input.HtlcAcceptedSuccessSecondLevel + } + inp := h.makeSweepInput( + op, witType, + input.LeaseHtlcAcceptedSuccessSecondLevel, + &h.htlcResolution.SweepSignDesc, + h.htlcResolution.CsvDelay, uint32(commitSpend.SpendingHeight), + h.htlc.RHash, h.htlcResolution.ResolutionBlob, + ) + + // Calculate the budget for this sweep. + budget := calculateBudget( + btcutil.Amount(inp.SignDesc().Output.Value), + h.Budget.NoDeadlineHTLCRatio, + h.Budget.NoDeadlineHTLC, + ) + + log.Infof("%T(%x): offering second-level success tx output to sweeper "+ + "with no deadline and budget=%v at height=%v", h, + h.htlc.RHash[:], budget, waitHeight) + + // TODO(yy): use the result chan returned from SweepInput. + _, err = h.Sweeper.SweepInput( + inp, + sweep.Params{ + Budget: budget, + + // For second level success tx, there's no rush to get + // it confirmed, so we use a nil deadline. + DeadlineHeight: fn.None[int32](), + }, + ) + + return err +} From 693287f745b258edf079eb59c4f15d1b8bf5fcca Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 14 Nov 2024 21:59:55 +0800 Subject: [PATCH 04/19] contractcourt: add resolver handlers in `htlcSuccessResolver` This commit refactors the `Resolve` method by adding two resolver handlers to handle waiting for spending confirmations. --- contractcourt/htlc_success_resolver.go | 229 ++++++++++++++++--------- contractcourt/htlc_timeout_resolver.go | 8 +- 2 files changed, 149 insertions(+), 88 deletions(-) diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index 3531d1fc92e..b436ccda89b 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -140,27 +140,7 @@ func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) { // To wrap this up, we'll wait until the second-level transaction has // been spent, then fully resolve the contract. - log.Infof("%T(%x): waiting for second-level HTLC output to be spent "+ - "after csv_delay=%v", h, h.htlc.RHash[:], h.htlcResolution.CsvDelay) - - spend, err := waitForSpend( - secondLevelOutpoint, - h.htlcResolution.SweepSignDesc.Output.PkScript, - h.broadcastHeight, h.Notifier, h.quit, - ) - if err != nil { - return nil, err - } - - h.reportLock.Lock() - h.currentReport.RecoveredBalance = h.currentReport.LimboBalance - h.currentReport.LimboBalance = 0 - h.reportLock.Unlock() - - h.resolved = true - return nil, h.checkpointClaim( - spend.SpenderTxHash, channeldb.ResolverOutcomeClaimed, - ) + return nil, h.resolveSuccessTxOutput(*secondLevelOutpoint) } // broadcastSuccessTx handles an HTLC output on our local commitment by @@ -187,40 +167,11 @@ func (h *htlcSuccessResolver) broadcastSuccessTx() ( // We'll now broadcast the second layer transaction so we can kick off // the claiming process. - // - // TODO(roasbeef): after changing sighashes send to tx bundler - label := labels.MakeLabel( - labels.LabelTypeChannelClose, &h.ShortChanID, - ) - err := h.PublishTx(h.htlcResolution.SignedSuccessTx, label) + err := h.resolveLegacySuccessTx() if err != nil { return nil, err } - // Otherwise, this is an output on our commitment transaction. In this - // case, we'll send it to the incubator, but only if we haven't already - // done so. - if !h.outputIncubating { - log.Infof("%T(%x): incubating incoming htlc output", - h, h.htlc.RHash[:]) - - err := h.IncubateOutputs( - h.ChanPoint, fn.None[lnwallet.OutgoingHtlcResolution](), - fn.Some(h.htlcResolution), - h.broadcastHeight, fn.Some(int32(h.htlc.RefundTimeout)), - ) - if err != nil { - return nil, err - } - - h.outputIncubating = true - - if err := h.Checkpoint(h); err != nil { - log.Errorf("unable to Checkpoint: %v", err) - return nil, err - } - } - return &h.htlcResolution.ClaimOutpoint, nil } @@ -242,33 +193,25 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (*wire.OutPoint, return nil, err } - log.Infof("%T(%x): waiting for second-level HTLC success "+ - "transaction to confirm", h, h.htlc.RHash[:]) - - // Wait for the second level transaction to confirm. - commitSpend, err = waitForSpend( - &h.htlcResolution.SignedSuccessTx.TxIn[0].PreviousOutPoint, - h.htlcResolution.SignDetails.SignDesc.Output.PkScript, - h.broadcastHeight, h.Notifier, h.quit, - ) + err = h.resolveSuccessTx() if err != nil { return nil, err } + } - // Now that the second-level transaction has confirmed, we - // checkpoint the state so we'll go to the next stage in case - // of restarts. - h.outputIncubating = true - if err := h.Checkpoint(h); err != nil { - log.Errorf("unable to Checkpoint: %v", err) - return nil, err - } - - log.Infof("%T(%x): second-level HTLC success transaction "+ - "confirmed!", h, h.htlc.RHash[:]) + // This should be non-blocking as we will only attempt to sweep the + // output when the second level tx has already been confirmed. In other + // words, waitForSpend will return immediately. + commitSpend, err := waitForSpend( + &h.htlcResolution.SignedSuccessTx.TxIn[0].PreviousOutPoint, + h.htlcResolution.SignDetails.SignDesc.Output.PkScript, + h.broadcastHeight, h.Notifier, h.quit, + ) + if err != nil { + return nil, err } - err := h.sweepSuccessTxOutput() + err = h.sweepSuccessTxOutput() if err != nil { return nil, err } @@ -304,23 +247,14 @@ func (h *htlcSuccessResolver) resolveRemoteCommitOutput() ( return nil, err } - // Once the transaction has received a sufficient number of - // confirmations, we'll mark ourselves as fully resolved and exit. - h.resolved = true - // Checkpoint the resolver, and write the outcome to disk. - return nil, h.checkpointClaim( - sweepTxDetails.SpenderTxHash, - channeldb.ResolverOutcomeClaimed, - ) + return nil, h.checkpointClaim(sweepTxDetails.SpenderTxHash) } // checkpointClaim checkpoints the success resolver with the reports it needs. // If this htlc was claimed two stages, it will write reports for both stages, // otherwise it will just write for the single htlc claim. -func (h *htlcSuccessResolver) checkpointClaim(spendTx *chainhash.Hash, - outcome channeldb.ResolverOutcome) error { - +func (h *htlcSuccessResolver) checkpointClaim(spendTx *chainhash.Hash) error { // Mark the htlc as final settled. err := h.ChainArbitratorConfig.PutFinalHtlcOutcome( h.ChannelArbitratorConfig.ShortChanID, h.htlc.HtlcIndex, true, @@ -348,7 +282,7 @@ func (h *htlcSuccessResolver) checkpointClaim(spendTx *chainhash.Hash, OutPoint: h.htlcResolution.ClaimOutpoint, Amount: amt, ResolverType: channeldb.ResolverTypeIncomingHtlc, - ResolverOutcome: outcome, + ResolverOutcome: channeldb.ResolverOutcomeClaimed, SpendTxID: spendTx, }, } @@ -373,6 +307,7 @@ func (h *htlcSuccessResolver) checkpointClaim(spendTx *chainhash.Hash, } // Finally, we checkpoint the resolver with our report(s). + h.resolved = true return h.Checkpoint(h, reports...) } @@ -748,3 +683,129 @@ func (h *htlcSuccessResolver) sweepSuccessTxOutput() error { return err } + +// resolveLegacySuccessTx handles an HTLC output from a pre-anchor type channel +// by broadcasting the second-level success transaction. +func (h *htlcSuccessResolver) resolveLegacySuccessTx() error { + // Otherwise we'll publish the second-level transaction directly and + // offer the resolution to the nursery to handle. + h.log.Infof("broadcasting legacy second-level success tx: %v", + h.htlcResolution.SignedSuccessTx.TxHash()) + + // We'll now broadcast the second layer transaction so we can kick off + // the claiming process. + // + // TODO(yy): offer it to the sweeper instead. + label := labels.MakeLabel( + labels.LabelTypeChannelClose, &h.ShortChanID, + ) + err := h.PublishTx(h.htlcResolution.SignedSuccessTx, label) + if err != nil { + return err + } + + // Fast-forward to resolve the output from the success tx if the it has + // already been sent to the UtxoNursery. + if h.outputIncubating { + return h.resolveSuccessTxOutput(h.htlcResolution.ClaimOutpoint) + } + + h.log.Infof("incubating incoming htlc output") + + // Send the output to the incubator. + err = h.IncubateOutputs( + h.ChanPoint, fn.None[lnwallet.OutgoingHtlcResolution](), + fn.Some(h.htlcResolution), + h.broadcastHeight, fn.Some(int32(h.htlc.RefundTimeout)), + ) + if err != nil { + return err + } + + // Mark the output as incubating and checkpoint it. + h.outputIncubating = true + if err := h.Checkpoint(h); err != nil { + return err + } + + // Move to resolve the output. + return h.resolveSuccessTxOutput(h.htlcResolution.ClaimOutpoint) +} + +// resolveSuccessTx waits for the sweeping tx of the second-level success tx to +// confirm and offers the output from the success tx to the sweeper. +func (h *htlcSuccessResolver) resolveSuccessTx() error { + h.log.Infof("waiting for 2nd-level HTLC success transaction to confirm") + + // Create aliases to make the code more readable. + outpoint := h.htlcResolution.SignedSuccessTx.TxIn[0].PreviousOutPoint + pkScript := h.htlcResolution.SignDetails.SignDesc.Output.PkScript + + // Wait for the second level transaction to confirm. + commitSpend, err := waitForSpend( + &outpoint, pkScript, h.broadcastHeight, h.Notifier, h.quit, + ) + if err != nil { + return err + } + + // We'll use this input index to determine the second-level output + // index on the transaction, as the signatures requires the indexes to + // be the same. We don't look for the second-level output script + // directly, as there might be more than one HTLC output to the same + // pkScript. + op := wire.OutPoint{ + Hash: *commitSpend.SpenderTxHash, + Index: commitSpend.SpenderInputIndex, + } + + // If the 2nd-stage sweeping has already been started, we can + // fast-forward to start the resolving process for the stage two + // output. + if h.outputIncubating { + return h.resolveSuccessTxOutput(op) + } + + // Now that the second-level transaction has confirmed, we checkpoint + // the state so we'll go to the next stage in case of restarts. + h.outputIncubating = true + if err := h.Checkpoint(h); err != nil { + log.Errorf("unable to Checkpoint: %v", err) + return err + } + + h.log.Infof("2nd-level HTLC success tx=%v confirmed", + commitSpend.SpenderTxHash) + + // Send the sweep request for the output from the success tx. + if err := h.sweepSuccessTxOutput(); err != nil { + return err + } + + return h.resolveSuccessTxOutput(op) +} + +// resolveSuccessTxOutput waits for the spend of the output from the 2nd-level +// success tx. +func (h *htlcSuccessResolver) resolveSuccessTxOutput(op wire.OutPoint) error { + // To wrap this up, we'll wait until the second-level transaction has + // been spent, then fully resolve the contract. + log.Infof("%T(%x): waiting for second-level HTLC output to be spent "+ + "after csv_delay=%v", h, h.htlc.RHash[:], + h.htlcResolution.CsvDelay) + + spend, err := waitForSpend( + &op, h.htlcResolution.SweepSignDesc.Output.PkScript, + h.broadcastHeight, h.Notifier, h.quit, + ) + if err != nil { + return err + } + + h.reportLock.Lock() + h.currentReport.RecoveredBalance = h.currentReport.LimboBalance + h.currentReport.LimboBalance = 0 + h.reportLock.Unlock() + + return h.checkpointClaim(spend.SpenderTxHash) +} diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index 698b4ae21ec..24167e70a46 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -548,9 +548,9 @@ func (h *htlcTimeoutResolver) sweepSecondLevelTx() error { return err } -// sendSecondLevelTxLegacy sends a second level timeout transaction to the utxo -// nursery. This transaction uses the legacy SIGHASH_ALL flag. -func (h *htlcTimeoutResolver) sendSecondLevelTxLegacy() error { +// resolveSecondLevelTxLegacy sends a second level timeout transaction to the +// utxo nursery. This transaction uses the legacy SIGHASH_ALL flag. +func (h *htlcTimeoutResolver) resolveSecondLevelTxLegacy() error { log.Debugf("%T(%v): incubating htlc output", h, h.htlcResolution.ClaimOutpoint) @@ -654,7 +654,7 @@ func (h *htlcTimeoutResolver) spendHtlcOutput() ( // commitment for a non-anchor channel, so we'll send it to the utxo // nursery. case h.isLegacyOutput() && !h.outputIncubating: - if err := h.sendSecondLevelTxLegacy(); err != nil { + if err := h.resolveSecondLevelTxLegacy(); err != nil { log.Errorf("Sending timeout tx to nursery: %v", err) return nil, err From fe29961c52875361a3eaad44e2d51aa8782ecabb Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 14 Nov 2024 22:07:32 +0800 Subject: [PATCH 05/19] contractcourt: remove redundant return value in `claimCleanUp` --- contractcourt/htlc_outgoing_contest_resolver.go | 4 ++-- contractcourt/htlc_timeout_resolver.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/contractcourt/htlc_outgoing_contest_resolver.go b/contractcourt/htlc_outgoing_contest_resolver.go index ec32ff7f172..7adce5a6893 100644 --- a/contractcourt/htlc_outgoing_contest_resolver.go +++ b/contractcourt/htlc_outgoing_contest_resolver.go @@ -87,7 +87,7 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { } // TODO(roasbeef): Checkpoint? - return h.claimCleanUp(commitSpend) + return nil, h.claimCleanUp(commitSpend) // If it hasn't, then we'll watch for both the expiration, and the // sweeping out this output. @@ -144,7 +144,7 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { // party is by revealing the preimage. So we'll perform // our duties to clean up the contract once it has been // claimed. - return h.claimCleanUp(commitSpend) + return nil, h.claimCleanUp(commitSpend) case <-h.quit: return nil, fmt.Errorf("resolver canceled") diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index 24167e70a46..b0a57352b8f 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -160,7 +160,7 @@ const ( // by the remote party. It'll extract the preimage, add it to the global cache, // and finally send the appropriate clean up message. func (h *htlcTimeoutResolver) claimCleanUp( - commitSpend *chainntnfs.SpendDetail) (ContractResolver, error) { + commitSpend *chainntnfs.SpendDetail) error { // Depending on if this is our commitment or not, then we'll be looking // for a different witness pattern. @@ -195,7 +195,7 @@ func (h *htlcTimeoutResolver) claimCleanUp( // element, then we're actually on the losing side of a breach // attempt... case h.isTaproot() && len(spendingInput.Witness) == 1: - return nil, fmt.Errorf("breach attempt failed") + return fmt.Errorf("breach attempt failed") // Otherwise, they'll be spending directly from our commitment output. // In which case the witness stack looks like: @@ -212,8 +212,8 @@ func (h *htlcTimeoutResolver) claimCleanUp( preimage, err := lntypes.MakePreimage(preimageBytes) if err != nil { - return nil, fmt.Errorf("unable to create pre-image from "+ - "witness: %v", err) + return fmt.Errorf("unable to create pre-image from witness: %w", + err) } log.Infof("%T(%v): extracting preimage=%v from on-chain "+ @@ -235,7 +235,7 @@ func (h *htlcTimeoutResolver) claimCleanUp( HtlcIndex: h.htlc.HtlcIndex, PreImage: &pre, }); err != nil { - return nil, err + return err } h.resolved = true @@ -250,7 +250,7 @@ func (h *htlcTimeoutResolver) claimCleanUp( SpendTxID: commitSpend.SpenderTxHash, } - return nil, h.Checkpoint(h, report) + return h.Checkpoint(h, report) } // chainDetailsToWatch returns the output and script which we use to watch for @@ -448,7 +448,7 @@ func (h *htlcTimeoutResolver) Resolve() (ContractResolver, error) { "witness cache", h, h.htlc.RHash[:], h.htlcResolution.ClaimOutpoint) - return h.claimCleanUp(commitSpend) + return nil, h.claimCleanUp(commitSpend) } // At this point, the second-level transaction is sufficiently From 7e0623c506a72e7908197a0f2e6ba6435bc2880b Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 14 Nov 2024 22:08:00 +0800 Subject: [PATCH 06/19] contractcourt: add sweep senders in `htlcTimeoutResolver` This commit adds new methods to handle making sweep requests based on the spending path used by the outgoing htlc output. --- contractcourt/htlc_timeout_resolver.go | 251 ++++++++++++++----------- 1 file changed, 140 insertions(+), 111 deletions(-) diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index b0a57352b8f..d17059364cf 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -476,13 +476,9 @@ func (h *htlcTimeoutResolver) Resolve() (ContractResolver, error) { return h.handleCommitSpend(commitSpend) } -// sweepSecondLevelTx sends a second level timeout transaction to the sweeper. +// sweepTimeoutTx sends a second level timeout transaction to the sweeper. // This transaction uses the SINLGE|ANYONECANPAY flag. -func (h *htlcTimeoutResolver) sweepSecondLevelTx() error { - log.Infof("%T(%x): offering second-layer timeout tx to sweeper: %v", - h, h.htlc.RHash[:], - spew.Sdump(h.htlcResolution.SignedTimeoutTx)) - +func (h *htlcTimeoutResolver) sweepTimeoutTx() error { var inp input.Input if h.isTaproot() { inp = lnutils.Ptr(input.MakeHtlcSecondLevelTimeoutTaprootInput( @@ -513,27 +509,12 @@ func (h *htlcTimeoutResolver) sweepSecondLevelTx() error { btcutil.Amount(inp.SignDesc().Output.Value), 2, 0, ) + h.log.Infof("offering 2nd-level HTLC timeout tx to sweeper "+ + "with deadline=%v, budget=%v", h.incomingHTLCExpiryHeight, + budget) + // For an outgoing HTLC, it must be swept before the RefundTimeout of // its incoming HTLC is reached. - // - // TODO(yy): we may end up mixing inputs with different time locks. - // Suppose we have two outgoing HTLCs, - // - HTLC1: nLocktime is 800000, CLTV delta is 80. - // - HTLC2: nLocktime is 800001, CLTV delta is 79. - // This means they would both have an incoming HTLC that expires at - // 800080, hence they share the same deadline but different locktimes. - // However, with current design, when we are at block 800000, HTLC1 is - // offered to the sweeper. When block 800001 is reached, HTLC1's - // sweeping process is already started, while HTLC2 is being offered to - // the sweeper, so they won't be mixed. This can become an issue tho, - // if we decide to sweep per X blocks. Or the contractcourt sees the - // block first while the sweeper is only aware of the last block. To - // properly fix it, we need `blockbeat` to make sure subsystems are in - // sync. - log.Infof("%T(%x): offering second-level HTLC timeout tx to sweeper "+ - "with deadline=%v, budget=%v", h, h.htlc.RHash[:], - h.incomingHTLCExpiryHeight, budget) - _, err := h.Sweeper.SweepInput( inp, sweep.Params{ @@ -551,21 +532,15 @@ func (h *htlcTimeoutResolver) sweepSecondLevelTx() error { // resolveSecondLevelTxLegacy sends a second level timeout transaction to the // utxo nursery. This transaction uses the legacy SIGHASH_ALL flag. func (h *htlcTimeoutResolver) resolveSecondLevelTxLegacy() error { - log.Debugf("%T(%v): incubating htlc output", h, - h.htlcResolution.ClaimOutpoint) + h.log.Debug("incubating htlc output") - err := h.IncubateOutputs( + // The utxo nursery will take care of broadcasting the second-level + // timeout tx and sweeping its output once it confirms. + return h.IncubateOutputs( h.ChanPoint, fn.Some(h.htlcResolution), fn.None[lnwallet.IncomingHtlcResolution](), h.broadcastHeight, h.incomingHTLCExpiryHeight, ) - if err != nil { - return err - } - - h.outputIncubating = true - - return h.Checkpoint(h) } // sweepDirectHtlcOutput sends the direct spend of the HTLC output to the @@ -635,7 +610,7 @@ func (h *htlcTimeoutResolver) spendHtlcOutput() ( // (the case for anchor type channels). In this case we can re-sign it // and attach fees at will. We let the sweeper handle this job. case h.isZeroFeeOutput() && !h.outputIncubating: - if err := h.sweepSecondLevelTx(); err != nil { + if err := h.sweepTimeoutTx(); err != nil { log.Errorf("Sending timeout tx to sweeper: %v", err) return nil, err @@ -696,9 +671,6 @@ func (h *htlcTimeoutResolver) watchHtlcSpend() (*chainntnfs.SpendDetail, func (h *htlcTimeoutResolver) waitForConfirmedSpend(op *wire.OutPoint, pkScript []byte) (*chainntnfs.SpendDetail, error) { - log.Infof("%T(%v): waiting for spent of HTLC output %v to be "+ - "fully confirmed", h, h.htlcResolution.ClaimOutpoint, op) - // We'll block here until either we exit, or the HTLC output on the // commitment transaction has been spent. spend, err := waitForSpend( @@ -770,82 +742,11 @@ func (h *htlcTimeoutResolver) handleCommitSpend( // the CSV and possible CLTV lock to expire, before sweeping the output // on the second-level. case h.isZeroFeeOutput(): - waitHeight := h.deriveWaitHeight( - h.htlcResolution.CsvDelay, commitSpend, - ) - - h.reportLock.Lock() - h.currentReport.Stage = 2 - h.currentReport.MaturityHeight = waitHeight - h.reportLock.Unlock() - - if h.hasCLTV() { - log.Infof("%T(%x): waiting for CSV and CLTV lock to "+ - "expire at height %v", h, h.htlc.RHash[:], - waitHeight) - } else { - log.Infof("%T(%x): waiting for CSV lock to expire at "+ - "height %v", h, h.htlc.RHash[:], waitHeight) - } - - // We'll use this input index to determine the second-level - // output index on the transaction, as the signatures requires - // the indexes to be the same. We don't look for the - // second-level output script directly, as there might be more - // than one HTLC output to the same pkScript. - op := &wire.OutPoint{ - Hash: *commitSpend.SpenderTxHash, - Index: commitSpend.SpenderInputIndex, - } - - var csvWitnessType input.StandardWitnessType - if h.isTaproot() { - //nolint:ll - csvWitnessType = input.TaprootHtlcOfferedTimeoutSecondLevel - } else { - csvWitnessType = input.HtlcOfferedTimeoutSecondLevel - } - - // Let the sweeper sweep the second-level output now that the - // CSV/CLTV locks have expired. - inp := h.makeSweepInput( - op, csvWitnessType, - input.LeaseHtlcOfferedTimeoutSecondLevel, - &h.htlcResolution.SweepSignDesc, - h.htlcResolution.CsvDelay, - uint32(commitSpend.SpendingHeight), h.htlc.RHash, - h.htlcResolution.ResolutionBlob, - ) - - // Calculate the budget for this sweep. - budget := calculateBudget( - btcutil.Amount(inp.SignDesc().Output.Value), - h.Budget.NoDeadlineHTLCRatio, - h.Budget.NoDeadlineHTLC, - ) - - log.Infof("%T(%x): offering second-level timeout tx output to "+ - "sweeper with no deadline and budget=%v at height=%v", - h, h.htlc.RHash[:], budget, waitHeight) - - _, err := h.Sweeper.SweepInput( - inp, - sweep.Params{ - Budget: budget, - - // For second level success tx, there's no rush - // to get it confirmed, so we use a nil - // deadline. - DeadlineHeight: fn.None[int32](), - }, - ) + err := h.sweepTimeoutTxOutput() if err != nil { return nil, err } - // Update the claim outpoint to point to the second-level - // transaction created by the sweeper. - claimOutpoint = *op fallthrough // Finally, if this was an output on our commitment transaction, we'll @@ -1288,3 +1189,131 @@ func (h *htlcTimeoutResolver) isLegacyOutput() bool { return h.htlcResolution.SignedTimeoutTx != nil && h.htlcResolution.SignDetails == nil } + +// waitHtlcSpendAndCheckPreimage waits for the htlc output to be spent and +// checks whether the spending reveals the preimage. If the preimage is found, +// it will be added to the preimage beacon to settle the incoming link, and a +// nil spend details will be returned. Otherwise, the spend details will be +// returned, indicating this is a non-preimage spend. +func (h *htlcTimeoutResolver) waitHtlcSpendAndCheckPreimage() ( + *chainntnfs.SpendDetail, error) { + + // Wait for the htlc output to be spent, which can happen in one of the + // paths, + // 1. The remote party spends the htlc output using the preimage. + // 2. The local party spends the htlc timeout tx from the local + // commitment. + // 3. The local party spends the htlc output directlt from the remote + // commitment. + spend, err := h.watchHtlcSpend() + if err != nil { + return nil, err + } + + // If the spend reveals the pre-image, then we'll enter the clean up + // workflow to pass the preimage back to the incoming link, add it to + // the witness cache, and exit. + if isPreimageSpend(h.isTaproot(), spend, !h.isRemoteCommitOutput()) { + return nil, h.claimCleanUp(spend) + } + + return spend, nil +} + +// sweepTimeoutTxOutput attempts to sweep the output of the second level +// timeout tx. +func (h *htlcTimeoutResolver) sweepTimeoutTxOutput() error { + h.log.Debugf("sweeping output %v from 2nd-level HTLC timeout tx", + h.htlcResolution.ClaimOutpoint) + + // This should be non-blocking as we will only attempt to sweep the + // output when the second level tx has already been confirmed. In other + // words, waitHtlcSpendAndCheckPreimage will return immediately. + commitSpend, err := h.waitHtlcSpendAndCheckPreimage() + if err != nil { + return err + } + + // Exit early if the spend is nil, as this means it's a remote spend + // using the preimage path, which is handled in claimCleanUp. + if commitSpend == nil { + h.log.Infof("preimage spend detected, skipping 2nd-level " + + "HTLC output sweep") + + return nil + } + + waitHeight := h.deriveWaitHeight(h.htlcResolution.CsvDelay, commitSpend) + + // Now that the sweeper has broadcasted the second-level transaction, + // it has confirmed, and we have checkpointed our state, we'll sweep + // the second level output. We report the resolver has moved the next + // stage. + h.reportLock.Lock() + h.currentReport.Stage = 2 + h.currentReport.MaturityHeight = waitHeight + h.reportLock.Unlock() + + if h.hasCLTV() { + h.log.Infof("waiting for CSV and CLTV lock to expire at "+ + "height %v", waitHeight) + } else { + h.log.Infof("waiting for CSV lock to expire at height %v", + waitHeight) + } + + // We'll use this input index to determine the second-level output + // index on the transaction, as the signatures requires the indexes to + // be the same. We don't look for the second-level output script + // directly, as there might be more than one HTLC output to the same + // pkScript. + op := &wire.OutPoint{ + Hash: *commitSpend.SpenderTxHash, + Index: commitSpend.SpenderInputIndex, + } + + var witType input.StandardWitnessType + if h.isTaproot() { + witType = input.TaprootHtlcOfferedTimeoutSecondLevel + } else { + witType = input.HtlcOfferedTimeoutSecondLevel + } + + // Let the sweeper sweep the second-level output now that the CSV/CLTV + // locks have expired. + inp := h.makeSweepInput( + op, witType, + input.LeaseHtlcOfferedTimeoutSecondLevel, + &h.htlcResolution.SweepSignDesc, + h.htlcResolution.CsvDelay, uint32(commitSpend.SpendingHeight), + h.htlc.RHash, h.htlcResolution.ResolutionBlob, + ) + + // Calculate the budget for this sweep. + budget := calculateBudget( + btcutil.Amount(inp.SignDesc().Output.Value), + h.Budget.NoDeadlineHTLCRatio, + h.Budget.NoDeadlineHTLC, + ) + + h.log.Infof("offering output from 2nd-level timeout tx to sweeper "+ + "with no deadline and budget=%v", budget) + + // TODO(yy): use the result chan returned from SweepInput to get the + // confirmation status of this sweeping tx so we don't need to make + // anothe subscription via `RegisterSpendNtfn` for this outpoint here + // in the resolver. + _, err = h.Sweeper.SweepInput( + inp, + sweep.Params{ + Budget: budget, + + // For second level success tx, there's no rush + // to get it confirmed, so we use a nil + // deadline. + DeadlineHeight: fn.None[int32](), + }, + ) + + return err +} From 066d351c32f5af8df89d33a21d5d8198fd40fe92 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 16 Jul 2024 08:44:53 +0800 Subject: [PATCH 07/19] contractcourt: add methods to checkpoint states This commit adds checkpoint methods in `htlcTimeoutResolver`, which are similar to those used in `htlcSuccessResolver`. --- contractcourt/htlc_timeout_resolver.go | 168 +++++++++++-------------- 1 file changed, 71 insertions(+), 97 deletions(-) diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index d17059364cf..f2f7e769983 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/davecgh/go-spew/spew" @@ -451,26 +452,6 @@ func (h *htlcTimeoutResolver) Resolve() (ContractResolver, error) { return nil, h.claimCleanUp(commitSpend) } - // At this point, the second-level transaction is sufficiently - // confirmed, or a transaction directly spending the output is. - // Therefore, we can now send back our clean up message, failing the - // HTLC on the incoming link. - // - // NOTE: This can be called twice if the outgoing resolver restarts - // before the second-stage timeout transaction is confirmed. - log.Infof("%T(%v): resolving htlc with incoming fail msg, "+ - "fully confirmed", h, h.htlcResolution.ClaimOutpoint) - - failureMsg := &lnwire.FailPermanentChannelFailure{} - err = h.DeliverResolutionMsg(ResolutionMsg{ - SourceChan: h.ShortChanID, - HtlcIndex: h.htlc.HtlcIndex, - Failure: failureMsg, - }) - if err != nil { - return nil, err - } - // Depending on whether this was a local or remote commit, we must // handle the spending transaction accordingly. return h.handleCommitSpend(commitSpend) @@ -680,30 +661,9 @@ func (h *htlcTimeoutResolver) waitForConfirmedSpend(op *wire.OutPoint, return nil, err } - // Once confirmed, persist the state on disk. - if err := h.checkPointSecondLevelTx(); err != nil { - return nil, err - } - return spend, err } -// checkPointSecondLevelTx persists the state of a second level HTLC tx to disk -// if it's published by the sweeper. -func (h *htlcTimeoutResolver) checkPointSecondLevelTx() error { - // If this was the second level transaction published by the sweeper, - // we can checkpoint the resolver now that it's confirmed. - if h.htlcResolution.SignDetails != nil && !h.outputIncubating { - h.outputIncubating = true - if err := h.Checkpoint(h); err != nil { - log.Errorf("unable to Checkpoint: %v", err) - return err - } - } - - return nil -} - // handleCommitSpend handles the spend of the HTLC output on the commitment // transaction. If this was our local commitment, the spend will be he // confirmed second-level timeout transaction, and we'll sweep that into our @@ -727,7 +687,8 @@ func (h *htlcTimeoutResolver) handleCommitSpend( // accordingly. spendTxID = commitSpend.SpenderTxHash - reports []*channeldb.ResolverReport + sweepTx *chainntnfs.SpendDetail + err error ) switch { @@ -756,7 +717,7 @@ func (h *htlcTimeoutResolver) handleCommitSpend( log.Infof("%T(%v): waiting for nursery/sweeper to spend CSV "+ "delayed output", h, claimOutpoint) - sweepTx, err := waitForSpend( + sweepTx, err = waitForSpend( &claimOutpoint, h.htlcResolution.SweepSignDesc.Output.PkScript, h.broadcastHeight, h.Notifier, h.quit, @@ -770,38 +731,16 @@ func (h *htlcTimeoutResolver) handleCommitSpend( // Once our sweep of the timeout tx has confirmed, we add a // resolution for our timeoutTx tx first stage transaction. - timeoutTx := commitSpend.SpendingTx - index := commitSpend.SpenderInputIndex - spendHash := commitSpend.SpenderTxHash - - reports = append(reports, &channeldb.ResolverReport{ - OutPoint: timeoutTx.TxIn[index].PreviousOutPoint, - Amount: h.htlc.Amt.ToSatoshis(), - ResolverType: channeldb.ResolverTypeOutgoingHtlc, - ResolverOutcome: channeldb.ResolverOutcomeFirstStage, - SpendTxID: spendHash, - }) + err = h.checkpointStageOne(*spendTxID) + if err != nil { + return nil, err + } } // With the clean up message sent, we'll now mark the contract // resolved, update the recovered balance, record the timeout and the // sweep txid on disk, and wait. - h.resolved = true - h.reportLock.Lock() - h.currentReport.RecoveredBalance = h.currentReport.LimboBalance - h.currentReport.LimboBalance = 0 - h.reportLock.Unlock() - - amt := btcutil.Amount(h.htlcResolution.SweepSignDesc.Output.Value) - reports = append(reports, &channeldb.ResolverReport{ - OutPoint: claimOutpoint, - Amount: amt, - ResolverType: channeldb.ResolverTypeOutgoingHtlc, - ResolverOutcome: channeldb.ResolverOutcomeTimeout, - SpendTxID: spendTxID, - }) - - return nil, h.Checkpoint(h, reports...) + return nil, h.checkpointClaim(sweepTx) } // Stop signals the resolver to cancel any current resolution processes, and @@ -1050,12 +989,6 @@ func (h *htlcTimeoutResolver) consumeSpendEvents(resultChan chan *spendResult, // Create a result chan to hold the results. result := &spendResult{} - // hasMempoolSpend is a flag that indicates whether we have found a - // preimage spend from the mempool. This is used to determine whether - // to checkpoint the resolver or not when later we found the - // corresponding block spend. - hasMempoolSpent := false - // Wait for a spend event to arrive. for { select { @@ -1083,23 +1016,6 @@ func (h *htlcTimeoutResolver) consumeSpendEvents(resultChan chan *spendResult, // Once confirmed, persist the state on disk if // we haven't seen the output's spending tx in // mempool before. - // - // NOTE: we don't checkpoint the resolver if - // it's spending tx has already been found in - // mempool - the resolver will take care of the - // checkpoint in its `claimCleanUp`. If we do - // checkpoint here, however, we'd create a new - // record in db for the same htlc resolver - // which won't be cleaned up later, resulting - // the channel to stay in unresolved state. - // - // TODO(yy): when fee bumper is implemented, we - // need to further check whether this is a - // preimage spend. Also need to refactor here - // to save us some indentation. - if !hasMempoolSpent { - result.err = h.checkPointSecondLevelTx() - } } // Send the result and exit the loop. @@ -1146,10 +1062,6 @@ func (h *htlcTimeoutResolver) consumeSpendEvents(resultChan chan *spendResult, result.spend = spendDetail resultChan <- result - // Set the hasMempoolSpent flag to true so we won't - // checkpoint the resolver again in db. - hasMempoolSpent = true - continue // If the resolver exits, we exit the goroutine. @@ -1317,3 +1229,65 @@ func (h *htlcTimeoutResolver) sweepTimeoutTxOutput() error { return err } + +// checkpointStageOne creates a checkpoint for the first stage of the htlc +// timeout transaction. This is used to ensure that the resolver can resume +// watching for the second stage spend in case of a restart. +func (h *htlcTimeoutResolver) checkpointStageOne( + spendTxid chainhash.Hash) error { + + h.log.Debugf("checkpoint stage one spend of HTLC output %v, spent "+ + "in tx %v", h.outpoint(), spendTxid) + + // Now that the second-level transaction has confirmed, we checkpoint + // the state so we'll go to the next stage in case of restarts. + h.outputIncubating = true + + // Create stage-one report. + report := &channeldb.ResolverReport{ + OutPoint: h.outpoint(), + Amount: h.htlc.Amt.ToSatoshis(), + ResolverType: channeldb.ResolverTypeOutgoingHtlc, + ResolverOutcome: channeldb.ResolverOutcomeFirstStage, + SpendTxID: &spendTxid, + } + + // At this point, the second-level transaction is sufficiently + // confirmed. We can now send back our clean up message, failing the + // HTLC on the incoming link. + failureMsg := &lnwire.FailPermanentChannelFailure{} + err := h.DeliverResolutionMsg(ResolutionMsg{ + SourceChan: h.ShortChanID, + HtlcIndex: h.htlc.HtlcIndex, + Failure: failureMsg, + }) + if err != nil { + return err + } + + return h.Checkpoint(h, report) +} + +// checkpointClaim checkpoints the timeout resolver with the reports it needs. +func (h *htlcTimeoutResolver) checkpointClaim( + spendDetail *chainntnfs.SpendDetail) error { + + h.log.Infof("resolving htlc with incoming fail msg, output=%v "+ + "confirmed in tx=%v", spendDetail.SpentOutPoint, + spendDetail.SpenderTxHash) + + // Create a resolver report for the claiming of the HTLC. + amt := btcutil.Amount(h.htlcResolution.SweepSignDesc.Output.Value) + report := &channeldb.ResolverReport{ + OutPoint: *spendDetail.SpentOutPoint, + Amount: amt, + ResolverType: channeldb.ResolverTypeOutgoingHtlc, + ResolverOutcome: channeldb.ResolverOutcomeTimeout, + SpendTxID: spendDetail.SpenderTxHash, + } + + // Finally, we checkpoint the resolver with our report(s). + h.resolved = true + + return h.Checkpoint(h, report) +} From c108487e70053faef727180f1223a095fe66d2a2 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 16 Jul 2024 08:53:00 +0800 Subject: [PATCH 08/19] contractcourt: add resolve handlers in `htlcTimeoutResolver` This commit adds more methods to handle resolving the spending of the output based on different spending paths. --- contractcourt/htlc_timeout_resolver.go | 201 +++++++++++++++---------- 1 file changed, 121 insertions(+), 80 deletions(-) diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index f2f7e769983..9904f37d3d7 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -454,7 +454,11 @@ func (h *htlcTimeoutResolver) Resolve() (ContractResolver, error) { // Depending on whether this was a local or remote commit, we must // handle the spending transaction accordingly. - return h.handleCommitSpend(commitSpend) + if h.isRemoteCommitOutput() { + return nil, h.resolveRemoteCommitOutput() + } + + return nil, h.resolveTimeoutTx() } // sweepTimeoutTx sends a second level timeout transaction to the sweeper. @@ -664,85 +668,6 @@ func (h *htlcTimeoutResolver) waitForConfirmedSpend(op *wire.OutPoint, return spend, err } -// handleCommitSpend handles the spend of the HTLC output on the commitment -// transaction. If this was our local commitment, the spend will be he -// confirmed second-level timeout transaction, and we'll sweep that into our -// wallet. If the was a remote commitment, the resolver will resolve -// immetiately. -func (h *htlcTimeoutResolver) handleCommitSpend( - commitSpend *chainntnfs.SpendDetail) (ContractResolver, error) { - - var ( - // claimOutpoint will be the outpoint of the second level - // transaction, or on the remote commitment directly. It will - // start out as set in the resolution, but we'll update it if - // the second-level goes through the sweeper and changes its - // txid. - claimOutpoint = h.htlcResolution.ClaimOutpoint - - // spendTxID will be the ultimate spend of the claimOutpoint. - // We set it to the commit spend for now, as this is the - // ultimate spend in case this is a remote commitment. If we go - // through the second-level transaction, we'll update this - // accordingly. - spendTxID = commitSpend.SpenderTxHash - - sweepTx *chainntnfs.SpendDetail - err error - ) - - switch { - - // If we swept an HTLC directly off the remote party's commitment - // transaction, then we can exit here as there's no second level sweep - // to do. - case h.htlcResolution.SignedTimeoutTx == nil: - break - - // If the sweeper is handling the second level transaction, wait for - // the CSV and possible CLTV lock to expire, before sweeping the output - // on the second-level. - case h.isZeroFeeOutput(): - err := h.sweepTimeoutTxOutput() - if err != nil { - return nil, err - } - - fallthrough - - // Finally, if this was an output on our commitment transaction, we'll - // wait for the second-level HTLC output to be spent, and for that - // transaction itself to confirm. - case !h.isRemoteCommitOutput(): - log.Infof("%T(%v): waiting for nursery/sweeper to spend CSV "+ - "delayed output", h, claimOutpoint) - - sweepTx, err = waitForSpend( - &claimOutpoint, - h.htlcResolution.SweepSignDesc.Output.PkScript, - h.broadcastHeight, h.Notifier, h.quit, - ) - if err != nil { - return nil, err - } - - // Update the spend txid to the hash of the sweep transaction. - spendTxID = sweepTx.SpenderTxHash - - // Once our sweep of the timeout tx has confirmed, we add a - // resolution for our timeoutTx tx first stage transaction. - err = h.checkpointStageOne(*spendTxID) - if err != nil { - return nil, err - } - } - - // With the clean up message sent, we'll now mark the contract - // resolved, update the recovered balance, record the timeout and the - // sweep txid on disk, and wait. - return nil, h.checkpointClaim(sweepTx) -} - // Stop signals the resolver to cancel any current resolution processes, and // suspend. // @@ -1291,3 +1216,119 @@ func (h *htlcTimeoutResolver) checkpointClaim( return h.Checkpoint(h, report) } + +// resolveRemoteCommitOutput handles sweeping an HTLC output on the remote +// commitment with via the timeout path. In this case we can sweep the output +// directly, and don't have to broadcast a second-level transaction. +func (h *htlcTimeoutResolver) resolveRemoteCommitOutput() error { + h.log.Debug("waiting for direct-timeout spend of the htlc to confirm") + + // Wait for the direct-timeout HTLC sweep tx to confirm. + spend, err := h.watchHtlcSpend() + if err != nil { + return err + } + + // If the spend reveals the preimage, then we'll enter the clean up + // workflow to pass the preimage back to the incoming link, add it to + // the witness cache, and exit. + if isPreimageSpend(h.isTaproot(), spend, !h.isRemoteCommitOutput()) { + return h.claimCleanUp(spend) + } + + // Send the clean up msg to fail the incoming HTLC. + failureMsg := &lnwire.FailPermanentChannelFailure{} + err = h.DeliverResolutionMsg(ResolutionMsg{ + SourceChan: h.ShortChanID, + HtlcIndex: h.htlc.HtlcIndex, + Failure: failureMsg, + }) + if err != nil { + return err + } + + // TODO(yy): should also update the `RecoveredBalance` and + // `LimboBalance` like other paths? + + // Checkpoint the resolver, and write the outcome to disk. + return h.checkpointClaim(spend) +} + +// resolveTimeoutTx waits for the sweeping tx of the second-level +// timeout tx to confirm and offers the output from the timeout tx to the +// sweeper. +func (h *htlcTimeoutResolver) resolveTimeoutTx() error { + h.log.Debug("waiting for first-stage 2nd-level HTLC timeout tx to " + + "confirm") + + // Wait for the second level transaction to confirm. + spend, err := h.watchHtlcSpend() + if err != nil { + return err + } + + // If the spend reveals the preimage, then we'll enter the clean up + // workflow to pass the preimage back to the incoming link, add it to + // the witness cache, and exit. + if isPreimageSpend(h.isTaproot(), spend, !h.isRemoteCommitOutput()) { + return h.claimCleanUp(spend) + } + + op := h.htlcResolution.ClaimOutpoint + spenderTxid := *spend.SpenderTxHash + + // If the timeout tx is a re-signed tx, we will need to find the actual + // spent outpoint from the spending tx. + if h.isZeroFeeOutput() { + op = wire.OutPoint{ + Hash: spenderTxid, + Index: spend.SpenderInputIndex, + } + } + + // If the 2nd-stage sweeping has already been started, we can + // fast-forward to start the resolving process for the stage two + // output. + if h.outputIncubating { + return h.resolveTimeoutTxOutput(op) + } + + h.log.Infof("2nd-level HTLC timeout tx=%v confirmed", spenderTxid) + + // Start the process to sweep the output from the timeout tx. + err = h.sweepTimeoutTxOutput() + if err != nil { + return err + } + + // Create a checkpoint since the timeout tx is confirmed and the sweep + // request has been made. + if err := h.checkpointStageOne(spenderTxid); err != nil { + return err + } + + // Start the resolving process for the stage two output. + return h.resolveTimeoutTxOutput(op) +} + +// resolveTimeoutTxOutput waits for the spend of the output from the 2nd-level +// timeout tx. +func (h *htlcTimeoutResolver) resolveTimeoutTxOutput(op wire.OutPoint) error { + h.log.Debugf("waiting for second-stage 2nd-level timeout tx output %v "+ + "to be spent after csv_delay=%v", op, h.htlcResolution.CsvDelay) + + spend, err := waitForSpend( + &op, h.htlcResolution.SweepSignDesc.Output.PkScript, + h.broadcastHeight, h.Notifier, h.quit, + ) + if err != nil { + return err + } + + h.reportLock.Lock() + h.currentReport.RecoveredBalance = h.currentReport.LimboBalance + h.currentReport.LimboBalance = 0 + h.reportLock.Unlock() + + return h.checkpointClaim(spend) +} From d78fd7adddd45596d1253a84cfdcd72317c09e52 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 24 Jun 2024 21:49:21 +0800 Subject: [PATCH 09/19] contractcourt: add `Launch` method to anchor/breach resolver We will use this and its following commits to break the original `Resolve` methods into two parts - the first part is moved to a new method `Launch`, which handles sending a sweep request to the sweeper. The second part remains in `Resolve`, which is mainly waiting for a spending tx. Breach resolver currently doesn't do anything in its `Launch` since the sweeping of justice outputs are not handled by the sweeper yet. --- contractcourt/anchor_resolver.go | 121 +++++++++++++++++++---------- contractcourt/breach_resolver.go | 21 +++++ contractcourt/contract_resolver.go | 10 +++ 3 files changed, 109 insertions(+), 43 deletions(-) diff --git a/contractcourt/anchor_resolver.go b/contractcourt/anchor_resolver.go index b50e061f447..ded3ebd83f2 100644 --- a/contractcourt/anchor_resolver.go +++ b/contractcourt/anchor_resolver.go @@ -84,49 +84,14 @@ func (c *anchorResolver) ResolverKey() []byte { return nil } -// Resolve offers the anchor output to the sweeper and waits for it to be swept. +// Resolve waits for the output to be swept. +// +// NOTE: Part of the ContractResolver interface. func (c *anchorResolver) Resolve() (ContractResolver, error) { - // Attempt to update the sweep parameters to the post-confirmation - // situation. We don't want to force sweep anymore, because the anchor - // lost its special purpose to get the commitment confirmed. It is just - // an output that we want to sweep only if it is economical to do so. - // - // An exclusive group is not necessary anymore, because we know that - // this is the only anchor that can be swept. - // - // We also clear the parent tx information for cpfp, because the - // commitment tx is confirmed. - // - // After a restart or when the remote force closes, the sweeper is not - // yet aware of the anchor. In that case, it will be added as new input - // to the sweeper. - witnessType := input.CommitmentAnchor - - // For taproot channels, we need to use the proper witness type. - if c.chanType.IsTaproot() { - witnessType = input.TaprootAnchorSweepSpend - } - - anchorInput := input.MakeBaseInput( - &c.anchor, witnessType, &c.anchorSignDescriptor, - c.broadcastHeight, nil, - ) - - resultChan, err := c.Sweeper.SweepInput( - &anchorInput, - sweep.Params{ - // For normal anchor sweeping, the budget is 330 sats. - Budget: btcutil.Amount( - anchorInput.SignDesc().Output.Value, - ), - - // There's no rush to sweep the anchor, so we use a nil - // deadline here. - DeadlineHeight: fn.None[int32](), - }, - ) - if err != nil { - return nil, err + // If we're already resolved, then we can exit early. + if c.resolved { + c.log.Errorf("already resolved") + return nil, nil } var ( @@ -135,7 +100,7 @@ func (c *anchorResolver) Resolve() (ContractResolver, error) { ) select { - case sweepRes := <-resultChan: + case sweepRes := <-c.sweepResultChan: switch sweepRes.Err { // Anchor was swept successfully. case nil: @@ -161,6 +126,8 @@ func (c *anchorResolver) Resolve() (ContractResolver, error) { return nil, errResolverShuttingDown } + c.log.Infof("resolved in tx %v", spendTx) + // Update report to reflect that funds are no longer in limbo. c.reportLock.Lock() if outcome == channeldb.ResolverOutcomeClaimed { @@ -181,6 +148,9 @@ func (c *anchorResolver) Resolve() (ContractResolver, error) { // // NOTE: Part of the ContractResolver interface. func (c *anchorResolver) Stop() { + c.log.Debugf("stopping...") + defer c.log.Debugf("stopped") + close(c.quit) } @@ -216,3 +186,68 @@ func (c *anchorResolver) Encode(w io.Writer) error { // A compile time assertion to ensure anchorResolver meets the // ContractResolver interface. var _ ContractResolver = (*anchorResolver)(nil) + +// Launch offers the anchor output to the sweeper. +func (c *anchorResolver) Launch() error { + if c.launched { + c.log.Tracef("already launched") + return nil + } + + c.log.Debugf("launching resolver...") + c.launched = true + + // If we're already resolved, then we can exit early. + if c.resolved { + c.log.Errorf("already resolved") + return nil + } + + // Attempt to update the sweep parameters to the post-confirmation + // situation. We don't want to force sweep anymore, because the anchor + // lost its special purpose to get the commitment confirmed. It is just + // an output that we want to sweep only if it is economical to do so. + // + // An exclusive group is not necessary anymore, because we know that + // this is the only anchor that can be swept. + // + // We also clear the parent tx information for cpfp, because the + // commitment tx is confirmed. + // + // After a restart or when the remote force closes, the sweeper is not + // yet aware of the anchor. In that case, it will be added as new input + // to the sweeper. + witnessType := input.CommitmentAnchor + + // For taproot channels, we need to use the proper witness type. + if c.chanType.IsTaproot() { + witnessType = input.TaprootAnchorSweepSpend + } + + anchorInput := input.MakeBaseInput( + &c.anchor, witnessType, &c.anchorSignDescriptor, + c.broadcastHeight, nil, + ) + + resultChan, err := c.Sweeper.SweepInput( + &anchorInput, + sweep.Params{ + // For normal anchor sweeping, the budget is 330 sats. + Budget: btcutil.Amount( + anchorInput.SignDesc().Output.Value, + ), + + // There's no rush to sweep the anchor, so we use a nil + // deadline here. + DeadlineHeight: fn.None[int32](), + }, + ) + + if err != nil { + return err + } + + c.sweepResultChan = resultChan + + return nil +} diff --git a/contractcourt/breach_resolver.go b/contractcourt/breach_resolver.go index 9a5f4bbe08e..1f74924d397 100644 --- a/contractcourt/breach_resolver.go +++ b/contractcourt/breach_resolver.go @@ -47,6 +47,8 @@ func (b *breachResolver) ResolverKey() []byte { // Resolve queries the BreachArbitrator to see if the justice transaction has // been broadcast. // +// NOTE: Part of the ContractResolver interface. +// // TODO(yy): let sweeper handle the breach inputs. func (b *breachResolver) Resolve() (ContractResolver, error) { if !b.subscribed { @@ -83,6 +85,7 @@ func (b *breachResolver) Resolve() (ContractResolver, error) { // Stop signals the breachResolver to stop. func (b *breachResolver) Stop() { + b.log.Debugf("stopping...") close(b.quit) } @@ -123,3 +126,21 @@ func newBreachResolverFromReader(r io.Reader, resCfg ResolverConfig) ( // A compile time assertion to ensure breachResolver meets the ContractResolver // interface. var _ ContractResolver = (*breachResolver)(nil) + +// Launch offers the breach outputs to the sweeper - currently it's a NOOP as +// the outputs here are not offered to the sweeper. +// +// NOTE: Part of the ContractResolver interface. +// +// TODO(yy): implement it once the outputs are offered to the sweeper. +func (b *breachResolver) Launch() error { + if b.launched { + b.log.Tracef("already launched") + return nil + } + + b.log.Debugf("launching resolver...") + b.launched = true + + return nil +} diff --git a/contractcourt/contract_resolver.go b/contractcourt/contract_resolver.go index ff52ce9760d..814c02ff564 100644 --- a/contractcourt/contract_resolver.go +++ b/contractcourt/contract_resolver.go @@ -10,6 +10,7 @@ import ( "github.com/btcsuite/btclog/v2" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/fn/v2" + "github.com/lightningnetwork/lnd/sweep" ) var ( @@ -109,6 +110,15 @@ type contractResolverKit struct { log btclog.Logger quit chan struct{} + + // sweepResultChan is the result chan returned from calling + // `SweepInput`. It should be mounted to the specific resolver once the + // input has been offered to the sweeper. + sweepResultChan chan sweep.Result + + // launched specifies whether the resolver has been launched. Calling + // `Launch` will be a no-op if this is true. + launched bool } // newContractResolverKit instantiates the mix-in struct. From 629c03702a1abf933744f06e9e8defbc2e04736d Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 20 Jun 2024 22:05:33 +0800 Subject: [PATCH 10/19] contractcourt: add `Launch` method to commit resolver --- contractcourt/commit_sweep_resolver.go | 339 +++++++++++--------- contractcourt/commit_sweep_resolver_test.go | 4 + 2 files changed, 186 insertions(+), 157 deletions(-) diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go index 7b101f80e33..b3323158db0 100644 --- a/contractcourt/commit_sweep_resolver.go +++ b/contractcourt/commit_sweep_resolver.go @@ -172,165 +172,10 @@ func (c *commitSweepResolver) getCommitTxConfHeight() (uint32, error) { func (c *commitSweepResolver) Resolve() (ContractResolver, error) { // If we're already resolved, then we can exit early. if c.resolved { + c.log.Errorf("already resolved") return nil, nil } - confHeight, err := c.getCommitTxConfHeight() - if err != nil { - return nil, err - } - - // Wait up until the CSV expires, unless we also have a CLTV that - // expires after. - unlockHeight := confHeight + c.commitResolution.MaturityDelay - if c.hasCLTV() { - unlockHeight = uint32(math.Max( - float64(unlockHeight), float64(c.leaseExpiry), - )) - } - - c.log.Debugf("commit conf_height=%v, unlock_height=%v", - confHeight, unlockHeight) - - // Update report now that we learned the confirmation height. - c.reportLock.Lock() - c.currentReport.MaturityHeight = unlockHeight - c.reportLock.Unlock() - - var ( - isLocalCommitTx bool - - signDesc = c.commitResolution.SelfOutputSignDesc - ) - - switch { - // For taproot channels, we'll know if this is the local commit based - // on the timelock value. For remote commitment transactions, the - // witness script has a timelock of 1. - case c.chanType.IsTaproot(): - delayKey := c.localChanCfg.DelayBasePoint.PubKey - nonDelayKey := c.localChanCfg.PaymentBasePoint.PubKey - - signKey := c.commitResolution.SelfOutputSignDesc.KeyDesc.PubKey - - // If the key in the script is neither of these, we shouldn't - // proceed. This should be impossible. - if !signKey.IsEqual(delayKey) && !signKey.IsEqual(nonDelayKey) { - return nil, fmt.Errorf("unknown sign key %v", signKey) - } - - // The commitment transaction is ours iff the signing key is - // the delay key. - isLocalCommitTx = signKey.IsEqual(delayKey) - - // The output is on our local commitment if the script starts with - // OP_IF for the revocation clause. On the remote commitment it will - // either be a regular P2WKH or a simple sig spend with a CSV delay. - default: - isLocalCommitTx = signDesc.WitnessScript[0] == txscript.OP_IF - } - isDelayedOutput := c.commitResolution.MaturityDelay != 0 - - c.log.Debugf("isDelayedOutput=%v, isLocalCommitTx=%v", isDelayedOutput, - isLocalCommitTx) - - // There're three types of commitments, those that have tweaks for the - // remote key (us in this case), those that don't, and a third where - // there is no tweak and the output is delayed. On the local commitment - // our output will always be delayed. We'll rely on the presence of the - // commitment tweak to discern which type of commitment this is. - var witnessType input.WitnessType - switch { - // The local delayed output for a taproot channel. - case isLocalCommitTx && c.chanType.IsTaproot(): - witnessType = input.TaprootLocalCommitSpend - - // The CSV 1 delayed output for a taproot channel. - case !isLocalCommitTx && c.chanType.IsTaproot(): - witnessType = input.TaprootRemoteCommitSpend - - // Delayed output to us on our local commitment for a channel lease in - // which we are the initiator. - case isLocalCommitTx && c.hasCLTV(): - witnessType = input.LeaseCommitmentTimeLock - - // Delayed output to us on our local commitment. - case isLocalCommitTx: - witnessType = input.CommitmentTimeLock - - // A confirmed output to us on the remote commitment for a channel lease - // in which we are the initiator. - case isDelayedOutput && c.hasCLTV(): - witnessType = input.LeaseCommitmentToRemoteConfirmed - - // A confirmed output to us on the remote commitment. - case isDelayedOutput: - witnessType = input.CommitmentToRemoteConfirmed - - // A non-delayed output on the remote commitment where the key is - // tweakless. - case c.commitResolution.SelfOutputSignDesc.SingleTweak == nil: - witnessType = input.CommitSpendNoDelayTweakless - - // A non-delayed output on the remote commitment where the key is - // tweaked. - default: - witnessType = input.CommitmentNoDelay - } - - c.log.Infof("Sweeping with witness type: %v", witnessType) - - // We'll craft an input with all the information required for the - // sweeper to create a fully valid sweeping transaction to recover - // these coins. - var inp *input.BaseInput - if c.hasCLTV() { - inp = input.NewCsvInputWithCltv( - &c.commitResolution.SelfOutPoint, witnessType, - &c.commitResolution.SelfOutputSignDesc, - c.broadcastHeight, c.commitResolution.MaturityDelay, - c.leaseExpiry, - input.WithResolutionBlob( - c.commitResolution.ResolutionBlob, - ), - ) - } else { - inp = input.NewCsvInput( - &c.commitResolution.SelfOutPoint, witnessType, - &c.commitResolution.SelfOutputSignDesc, - c.broadcastHeight, c.commitResolution.MaturityDelay, - input.WithResolutionBlob( - c.commitResolution.ResolutionBlob, - ), - ) - } - - // TODO(roasbeef): instead of ading ctrl block to the sign desc, make - // new input type, have sweeper set it? - - // Calculate the budget for the sweeping this input. - budget := calculateBudget( - btcutil.Amount(inp.SignDesc().Output.Value), - c.Budget.ToLocalRatio, c.Budget.ToLocal, - ) - c.log.Infof("Sweeping commit output using budget=%v", budget) - - // With our input constructed, we'll now offer it to the sweeper. - resultChan, err := c.Sweeper.SweepInput( - inp, sweep.Params{ - Budget: budget, - - // Specify a nil deadline here as there's no time - // pressure. - DeadlineHeight: fn.None[int32](), - }, - ) - if err != nil { - c.log.Errorf("unable to sweep input: %v", err) - - return nil, err - } - var sweepTxID chainhash.Hash // Sweeper is going to join this input with other inputs if possible @@ -339,7 +184,7 @@ func (c *commitSweepResolver) Resolve() (ContractResolver, error) { // happen. outcome := channeldb.ResolverOutcomeClaimed select { - case sweepResult := <-resultChan: + case sweepResult := <-c.sweepResultChan: switch sweepResult.Err { // If the remote party was able to sweep this output it's // likely what we sent was actually a revoked commitment. @@ -391,6 +236,8 @@ func (c *commitSweepResolver) Resolve() (ContractResolver, error) { // // NOTE: Part of the ContractResolver interface. func (c *commitSweepResolver) Stop() { + c.log.Debugf("stopping...") + defer c.log.Debugf("stopped") close(c.quit) } @@ -524,3 +371,181 @@ func (c *commitSweepResolver) initReport() { // A compile time assertion to ensure commitSweepResolver meets the // ContractResolver interface. var _ reportingContractResolver = (*commitSweepResolver)(nil) + +// Launch constructs a commit input and offers it to the sweeper. +func (c *commitSweepResolver) Launch() error { + if c.launched { + c.log.Tracef("already launched") + return nil + } + + c.log.Debugf("launching resolver...") + c.launched = true + + // If we're already resolved, then we can exit early. + if c.resolved { + c.log.Errorf("already resolved") + return nil + } + + confHeight, err := c.getCommitTxConfHeight() + if err != nil { + return err + } + + // Wait up until the CSV expires, unless we also have a CLTV that + // expires after. + unlockHeight := confHeight + c.commitResolution.MaturityDelay + if c.hasCLTV() { + unlockHeight = uint32(math.Max( + float64(unlockHeight), float64(c.leaseExpiry), + )) + } + + // Update report now that we learned the confirmation height. + c.reportLock.Lock() + c.currentReport.MaturityHeight = unlockHeight + c.reportLock.Unlock() + + // Derive the witness type for this input. + witnessType, err := c.decideWitnessType() + if err != nil { + return err + } + + // We'll craft an input with all the information required for the + // sweeper to create a fully valid sweeping transaction to recover + // these coins. + var inp *input.BaseInput + if c.hasCLTV() { + inp = input.NewCsvInputWithCltv( + &c.commitResolution.SelfOutPoint, witnessType, + &c.commitResolution.SelfOutputSignDesc, + c.broadcastHeight, c.commitResolution.MaturityDelay, + c.leaseExpiry, + ) + } else { + inp = input.NewCsvInput( + &c.commitResolution.SelfOutPoint, witnessType, + &c.commitResolution.SelfOutputSignDesc, + c.broadcastHeight, c.commitResolution.MaturityDelay, + ) + } + + // TODO(roasbeef): instead of ading ctrl block to the sign desc, make + // new input type, have sweeper set it? + + // Calculate the budget for the sweeping this input. + budget := calculateBudget( + btcutil.Amount(inp.SignDesc().Output.Value), + c.Budget.ToLocalRatio, c.Budget.ToLocal, + ) + c.log.Infof("sweeping commit output %v using budget=%v", witnessType, + budget) + + // With our input constructed, we'll now offer it to the sweeper. + resultChan, err := c.Sweeper.SweepInput( + inp, sweep.Params{ + Budget: budget, + + // Specify a nil deadline here as there's no time + // pressure. + DeadlineHeight: fn.None[int32](), + }, + ) + if err != nil { + c.log.Errorf("unable to sweep input: %v", err) + + return err + } + + c.sweepResultChan = resultChan + + return nil +} + +// decideWitnessType returns the witness type for the input. +func (c *commitSweepResolver) decideWitnessType() (input.WitnessType, error) { + var ( + isLocalCommitTx bool + signDesc = c.commitResolution.SelfOutputSignDesc + ) + + switch { + // For taproot channels, we'll know if this is the local commit based + // on the timelock value. For remote commitment transactions, the + // witness script has a timelock of 1. + case c.chanType.IsTaproot(): + delayKey := c.localChanCfg.DelayBasePoint.PubKey + nonDelayKey := c.localChanCfg.PaymentBasePoint.PubKey + + signKey := c.commitResolution.SelfOutputSignDesc.KeyDesc.PubKey + + // If the key in the script is neither of these, we shouldn't + // proceed. This should be impossible. + if !signKey.IsEqual(delayKey) && !signKey.IsEqual(nonDelayKey) { + return nil, fmt.Errorf("unknown sign key %v", signKey) + } + + // The commitment transaction is ours iff the signing key is + // the delay key. + isLocalCommitTx = signKey.IsEqual(delayKey) + + // The output is on our local commitment if the script starts with + // OP_IF for the revocation clause. On the remote commitment it will + // either be a regular P2WKH or a simple sig spend with a CSV delay. + default: + isLocalCommitTx = signDesc.WitnessScript[0] == txscript.OP_IF + } + + isDelayedOutput := c.commitResolution.MaturityDelay != 0 + + c.log.Debugf("isDelayedOutput=%v, isLocalCommitTx=%v", isDelayedOutput, + isLocalCommitTx) + + // There're three types of commitments, those that have tweaks for the + // remote key (us in this case), those that don't, and a third where + // there is no tweak and the output is delayed. On the local commitment + // our output will always be delayed. We'll rely on the presence of the + // commitment tweak to discern which type of commitment this is. + var witnessType input.WitnessType + switch { + // The local delayed output for a taproot channel. + case isLocalCommitTx && c.chanType.IsTaproot(): + witnessType = input.TaprootLocalCommitSpend + + // The CSV 1 delayed output for a taproot channel. + case !isLocalCommitTx && c.chanType.IsTaproot(): + witnessType = input.TaprootRemoteCommitSpend + + // Delayed output to us on our local commitment for a channel lease in + // which we are the initiator. + case isLocalCommitTx && c.hasCLTV(): + witnessType = input.LeaseCommitmentTimeLock + + // Delayed output to us on our local commitment. + case isLocalCommitTx: + witnessType = input.CommitmentTimeLock + + // A confirmed output to us on the remote commitment for a channel lease + // in which we are the initiator. + case isDelayedOutput && c.hasCLTV(): + witnessType = input.LeaseCommitmentToRemoteConfirmed + + // A confirmed output to us on the remote commitment. + case isDelayedOutput: + witnessType = input.CommitmentToRemoteConfirmed + + // A non-delayed output on the remote commitment where the key is + // tweakless. + case c.commitResolution.SelfOutputSignDesc.SingleTweak == nil: + witnessType = input.CommitSpendNoDelayTweakless + + // A non-delayed output on the remote commitment where the key is + // tweaked. + default: + witnessType = input.CommitmentNoDelay + } + + return witnessType, nil +} diff --git a/contractcourt/commit_sweep_resolver_test.go b/contractcourt/commit_sweep_resolver_test.go index 2195e33779f..6855fddcd32 100644 --- a/contractcourt/commit_sweep_resolver_test.go +++ b/contractcourt/commit_sweep_resolver_test.go @@ -15,6 +15,7 @@ import ( "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/sweep" + "github.com/stretchr/testify/require" ) type commitSweepResolverTestContext struct { @@ -82,6 +83,9 @@ func (i *commitSweepResolverTestContext) resolve() { // Start resolver. i.resolverResultChan = make(chan resolveResult, 1) go func() { + err := i.resolver.Launch() + require.NoError(i.t, err) + nextResolver, err := i.resolver.Resolve() i.resolverResultChan <- resolveResult{ nextResolver: nextResolver, From 406f443e30fa2ed67ed2a5d1551aa1937372daf1 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 16 Jul 2024 07:24:45 +0800 Subject: [PATCH 11/19] contractcourt: add `Launch` method to htlc success resolver This commit breaks the `Resolve` into two parts - the first part is moved into a `Launch` method that handles sending sweep requests, and the second part remains in `Resolve` which handles waiting for the spend. Since we are using both utxo nursery and sweeper at the same time, to make sure this change doesn't break the existing behavior, we implement the `Launch` as following, - zero-fee htlc - handled by the sweeper - direct output from the remote commit - handled by the sweeper - legacy htlc - handled by the utxo nursery --- contractcourt/htlc_success_resolver.go | 186 ++++++++------------ contractcourt/htlc_success_resolver_test.go | 87 +++++++-- 2 files changed, 152 insertions(+), 121 deletions(-) diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index b436ccda89b..20b45166343 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -10,8 +10,6 @@ import ( "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" - "github.com/davecgh/go-spew/spew" - "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/graph/db/models" @@ -116,139 +114,60 @@ func (h *htlcSuccessResolver) ResolverKey() []byte { // anymore. Every HTLC has already passed through the incoming contest resolver // and in there the invoice was already marked as settled. // -// TODO(roasbeef): create multi to batch -// // NOTE: Part of the ContractResolver interface. +// +// TODO(yy): refactor the interface method to return an error only. func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) { - // If we're already resolved, then we can exit early. - if h.resolved { - return nil, nil - } - - // If we don't have a success transaction, then this means that this is - // an output on the remote party's commitment transaction. - if h.isRemoteCommitOutput() { - return h.resolveRemoteCommitOutput() - } - - // Otherwise this an output on our own commitment, and we must start by - // broadcasting the second-level success transaction. - secondLevelOutpoint, err := h.broadcastSuccessTx() - if err != nil { - return nil, err - } - - // To wrap this up, we'll wait until the second-level transaction has - // been spent, then fully resolve the contract. - return nil, h.resolveSuccessTxOutput(*secondLevelOutpoint) -} - -// broadcastSuccessTx handles an HTLC output on our local commitment by -// broadcasting the second-level success transaction. It returns the ultimate -// outpoint of the second-level tx, that we must wait to be spent for the -// resolver to be fully resolved. -func (h *htlcSuccessResolver) broadcastSuccessTx() ( - *wire.OutPoint, error) { - - // If we have non-nil SignDetails, this means that have a 2nd level - // HTLC transaction that is signed using sighash SINGLE|ANYONECANPAY - // (the case for anchor type channels). In this case we can re-sign it - // and attach fees at will. We let the sweeper handle this job. We use - // the checkpointed outputIncubating field to determine if we already - // swept the HTLC output into the second level transaction. - if h.isZeroFeeOutput() { - return h.broadcastReSignedSuccessTx() - } + var err error - // Otherwise we'll publish the second-level transaction directly and - // offer the resolution to the nursery to handle. - log.Infof("%T(%x): broadcasting second-layer transition tx: %v", - h, h.htlc.RHash[:], spew.Sdump(h.htlcResolution.SignedSuccessTx)) - - // We'll now broadcast the second layer transaction so we can kick off - // the claiming process. - err := h.resolveLegacySuccessTx() - if err != nil { - return nil, err - } - - return &h.htlcResolution.ClaimOutpoint, nil -} + switch { + // If we're already resolved, then we can exit early. + case h.resolved: + h.log.Errorf("already resolved") -// broadcastReSignedSuccessTx handles the case where we have non-nil -// SignDetails, and offers the second level transaction to the Sweeper, that -// will re-sign it and attach fees at will. -func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (*wire.OutPoint, - error) { - - // Keep track of the tx spending the HTLC output on the commitment, as - // this will be the confirmed second-level tx we'll ultimately sweep. - var commitSpend *chainntnfs.SpendDetail - - // We will have to let the sweeper re-sign the success tx and wait for - // it to confirm, if we haven't already. - if !h.outputIncubating { - err := h.sweepSuccessTx() - if err != nil { - return nil, err - } + // If this is an output on the remote party's commitment transaction, + // use the direct-spend path to sweep the htlc. + case h.isRemoteCommitOutput(): + err = h.resolveRemoteCommitOutput() + // If this is an output on our commitment transaction using post-anchor + // channel type, it will be handled by the sweeper. + case h.isZeroFeeOutput(): err = h.resolveSuccessTx() - if err != nil { - return nil, err - } - } - - // This should be non-blocking as we will only attempt to sweep the - // output when the second level tx has already been confirmed. In other - // words, waitForSpend will return immediately. - commitSpend, err := waitForSpend( - &h.htlcResolution.SignedSuccessTx.TxIn[0].PreviousOutPoint, - h.htlcResolution.SignDetails.SignDesc.Output.PkScript, - h.broadcastHeight, h.Notifier, h.quit, - ) - if err != nil { - return nil, err - } - err = h.sweepSuccessTxOutput() - if err != nil { - return nil, err + // If this is an output on our own commitment using pre-anchor channel + // type, we will publish the success tx and offer the output to the + // nursery. + default: + err = h.resolveLegacySuccessTx() } - // Will return this outpoint, when this is spent the resolver is fully - // resolved. - op := &wire.OutPoint{ - Hash: *commitSpend.SpenderTxHash, - Index: commitSpend.SpenderInputIndex, - } - - return op, nil + return nil, err } // resolveRemoteCommitOutput handles sweeping an HTLC output on the remote // commitment with the preimage. In this case we can sweep the output directly, // and don't have to broadcast a second-level transaction. -func (h *htlcSuccessResolver) resolveRemoteCommitOutput() ( - ContractResolver, error) { - - err := h.sweepRemoteCommitOutput() - if err != nil { - return nil, err - } +func (h *htlcSuccessResolver) resolveRemoteCommitOutput() error { + h.log.Info("waiting for direct-preimage spend of the htlc to confirm") // Wait for the direct-preimage HTLC sweep tx to confirm. + // + // TODO(yy): use the result chan returned from `SweepInput`. sweepTxDetails, err := waitForSpend( &h.htlcResolution.ClaimOutpoint, h.htlcResolution.SweepSignDesc.Output.PkScript, h.broadcastHeight, h.Notifier, h.quit, ) if err != nil { - return nil, err + return err } + // TODO(yy): should also update the `RecoveredBalance` and + // `LimboBalance` like other paths? + // Checkpoint the resolver, and write the outcome to disk. - return nil, h.checkpointClaim(sweepTxDetails.SpenderTxHash) + return h.checkpointClaim(sweepTxDetails.SpenderTxHash) } // checkpointClaim checkpoints the success resolver with the reports it needs. @@ -316,6 +235,9 @@ func (h *htlcSuccessResolver) checkpointClaim(spendTx *chainhash.Hash) error { // // NOTE: Part of the ContractResolver interface. func (h *htlcSuccessResolver) Stop() { + h.log.Debugf("stopping...") + defer h.log.Debugf("stopped") + close(h.quit) } @@ -809,3 +731,47 @@ func (h *htlcSuccessResolver) resolveSuccessTxOutput(op wire.OutPoint) error { return h.checkpointClaim(spend.SpenderTxHash) } + +// Launch creates an input based on the details of the incoming htlc resolution +// and offers it to the sweeper. +func (h *htlcSuccessResolver) Launch() error { + if h.launched { + h.log.Tracef("already launched") + return nil + } + + h.log.Debugf("launching resolver...") + h.launched = true + + switch { + // If we're already resolved, then we can exit early. + case h.resolved: + h.log.Errorf("already resolved") + return nil + + // If this is an output on the remote party's commitment transaction, + // use the direct-spend path. + case h.isRemoteCommitOutput(): + return h.sweepRemoteCommitOutput() + + // If this is an anchor type channel, we now sweep either the + // second-level success tx or the output from the second-level success + // tx. + case h.isZeroFeeOutput(): + // If the second-level success tx has already been swept, we + // can go ahead and sweep its output. + if h.outputIncubating { + return h.sweepSuccessTxOutput() + } + + // Otherwise, sweep the second level tx. + return h.sweepSuccessTx() + + // If this is a legacy channel type, the output is handled by the + // nursery via the Resolve so we do nothing here. + // + // TODO(yy): handle the legacy output by offering it to the sweeper. + default: + return nil + } +} diff --git a/contractcourt/htlc_success_resolver_test.go b/contractcourt/htlc_success_resolver_test.go index 75c733638fe..f395d67215a 100644 --- a/contractcourt/htlc_success_resolver_test.go +++ b/contractcourt/htlc_success_resolver_test.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "testing" + "time" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -20,6 +21,7 @@ import ( "github.com/lightningnetwork/lnd/lntest/mock" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwire" + "github.com/stretchr/testify/require" ) var testHtlcAmt = lnwire.MilliSatoshi(200000) @@ -39,6 +41,15 @@ type htlcResolverTestContext struct { t *testing.T } +func newHtlcResolverTestContextFromReader(t *testing.T, + newResolver func(htlc channeldb.HTLC, + cfg ResolverConfig) ContractResolver) *htlcResolverTestContext { + + ctx := newHtlcResolverTestContext(t, newResolver) + + return ctx +} + func newHtlcResolverTestContext(t *testing.T, newResolver func(htlc channeldb.HTLC, cfg ResolverConfig) ContractResolver) *htlcResolverTestContext { @@ -133,6 +144,7 @@ func newHtlcResolverTestContext(t *testing.T, func (i *htlcResolverTestContext) resolve() { // Start resolver. i.resolverResultChan = make(chan resolveResult, 1) + go func() { nextResolver, err := i.resolver.Resolve() i.resolverResultChan <- resolveResult{ @@ -192,6 +204,7 @@ func TestHtlcSuccessSingleStage(t *testing.T) { // sweeper. details := &chainntnfs.SpendDetail{ SpendingTx: sweepTx, + SpentOutPoint: &htlcOutpoint, SpenderTxHash: &sweepTxid, } ctx.notifier.SpendChan <- details @@ -215,8 +228,8 @@ func TestHtlcSuccessSingleStage(t *testing.T) { ) } -// TestSecondStageResolution tests successful sweep of a second stage htlc -// claim, going through the Nursery. +// TestHtlcSuccessSecondStageResolution tests successful sweep of a second +// stage htlc claim, going through the Nursery. func TestHtlcSuccessSecondStageResolution(t *testing.T) { commitOutpoint := wire.OutPoint{Index: 2} htlcOutpoint := wire.OutPoint{Index: 3} @@ -279,6 +292,7 @@ func TestHtlcSuccessSecondStageResolution(t *testing.T) { ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ SpendingTx: sweepTx, + SpentOutPoint: &htlcOutpoint, SpenderTxHash: &sweepHash, } @@ -302,6 +316,8 @@ func TestHtlcSuccessSecondStageResolution(t *testing.T) { // TestHtlcSuccessSecondStageResolutionSweeper test that a resolver with // non-nil SignDetails will offer the second-level transaction to the sweeper // for re-signing. +// +//nolint:ll func TestHtlcSuccessSecondStageResolutionSweeper(t *testing.T) { commitOutpoint := wire.OutPoint{Index: 2} htlcOutpoint := wire.OutPoint{Index: 3} @@ -399,7 +415,20 @@ func TestHtlcSuccessSecondStageResolutionSweeper(t *testing.T) { _ bool) error { resolver := ctx.resolver.(*htlcSuccessResolver) - inp := <-resolver.Sweeper.(*mockSweeper).sweptInputs + + var ( + inp input.Input + ok bool + ) + + select { + case inp, ok = <-resolver.Sweeper.(*mockSweeper).sweptInputs: + require.True(t, ok) + + case <-time.After(1 * time.Second): + t.Fatal("expected input to be swept") + } + op := inp.OutPoint() if op != commitOutpoint { return fmt.Errorf("outpoint %v swept, "+ @@ -412,6 +441,7 @@ func TestHtlcSuccessSecondStageResolutionSweeper(t *testing.T) { SpenderTxHash: &reSignedHash, SpenderInputIndex: 1, SpendingHeight: 10, + SpentOutPoint: &commitOutpoint, } return nil }, @@ -434,13 +464,37 @@ func TestHtlcSuccessSecondStageResolutionSweeper(t *testing.T) { SpenderTxHash: &reSignedHash, SpenderInputIndex: 1, SpendingHeight: 10, + SpentOutPoint: &commitOutpoint, } } // We expect it to sweep the second-level // transaction we notfied about above. resolver := ctx.resolver.(*htlcSuccessResolver) - inp := <-resolver.Sweeper.(*mockSweeper).sweptInputs + + // Mock `waitForSpend` to return the commit + // spend. + ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ + SpendingTx: reSignedSuccessTx, + SpenderTxHash: &reSignedHash, + SpenderInputIndex: 1, + SpendingHeight: 10, + SpentOutPoint: &commitOutpoint, + } + + var ( + inp input.Input + ok bool + ) + + select { + case inp, ok = <-resolver.Sweeper.(*mockSweeper).sweptInputs: + require.True(t, ok) + + case <-time.After(1 * time.Second): + t.Fatal("expected input to be swept") + } + op := inp.OutPoint() exp := wire.OutPoint{ Hash: reSignedHash, @@ -457,6 +511,7 @@ func TestHtlcSuccessSecondStageResolutionSweeper(t *testing.T) { SpendingTx: sweepTx, SpenderTxHash: &sweepHash, SpendingHeight: 14, + SpentOutPoint: &op, } return nil @@ -504,11 +559,14 @@ func testHtlcSuccess(t *testing.T, resolution lnwallet.IncomingHtlcResolution, // for the next portion of the test. ctx := newHtlcResolverTestContext(t, func(htlc channeldb.HTLC, cfg ResolverConfig) ContractResolver { - return &htlcSuccessResolver{ + r := &htlcSuccessResolver{ contractResolverKit: *newContractResolverKit(cfg), htlc: htlc, htlcResolution: resolution, } + r.initLogger("htlcSuccessResolver") + + return r }, ) @@ -606,7 +664,12 @@ func runFromCheckpoint(t *testing.T, ctx *htlcResolverTestContext, checkpointedState = append(checkpointedState, b.Bytes()) nextCheckpoint++ - checkpointChan <- struct{}{} + select { + case checkpointChan <- struct{}{}: + case <-time.After(1 * time.Second): + t.Fatal("checkpoint timeout") + } + return nil } @@ -617,6 +680,8 @@ func runFromCheckpoint(t *testing.T, ctx *htlcResolverTestContext, // preCheckpoint logic if needed. resumed := true for i, cp := range expectedCheckpoints { + t.Logf("Running checkpoint %d", i) + if cp.preCheckpoint != nil { if err := cp.preCheckpoint(ctx, resumed); err != nil { t.Fatalf("failure at stage %d: %v", i, err) @@ -625,15 +690,15 @@ func runFromCheckpoint(t *testing.T, ctx *htlcResolverTestContext, resumed = false // Wait for the resolver to have checkpointed its state. - <-checkpointChan + select { + case <-checkpointChan: + case <-time.After(1 * time.Second): + t.Fatalf("resolver did not checkpoint at stage %d", i) + } } // Wait for the resolver to fully complete. ctx.waitForResult() - if nextCheckpoint < len(expectedCheckpoints) { - t.Fatalf("not all checkpoints hit") - } - return checkpointedState } From dcf7144c99725059510ead738c4cbeadb13702df Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 16 Jul 2024 09:05:47 +0800 Subject: [PATCH 12/19] contractcourt: add `Launch` method to htlc timeout resolver This commit breaks the `Resolve` into two parts - the first part is moved into a `Launch` method that handles sending sweep requests, and the second part remains in `Resolve` which handles waiting for the spend. Since we are using both utxo nursery and sweeper at the same time, to make sure this change doesn't break the existing behavior, we implement the `Launch` as following, - zero-fee htlc - handled by the sweeper - direct output from the remote commit - handled by the sweeper - legacy htlc - handled by the utxo nursery --- contractcourt/channel_arbitrator_test.go | 7 +- contractcourt/htlc_timeout_resolver.go | 162 ++++++------ contractcourt/htlc_timeout_resolver_test.go | 272 +++++++++++++------- 3 files changed, 254 insertions(+), 187 deletions(-) diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index a3bdf6ba3b9..d1ef2993d50 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -982,6 +982,7 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { }, }, } + closeTxid := closeTx.TxHash() htlcOp := wire.OutPoint{ Hash: closeTx.TxHash(), @@ -1117,7 +1118,11 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { // Notify resolver that the HTLC output of the commitment has been // spent. - oldNotifier.SpendChan <- &chainntnfs.SpendDetail{SpendingTx: closeTx} + oldNotifier.SpendChan <- &chainntnfs.SpendDetail{ + SpendingTx: closeTx, + SpentOutPoint: &wire.OutPoint{}, + SpenderTxHash: &closeTxid, + } // Finally, we should also receive a resolution message instructing the // switch to cancel back the HTLC. diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index 9904f37d3d7..eae52255f2b 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -425,40 +425,25 @@ func checkSizeAndIndex(witness wire.TxWitness, size, index int) bool { func (h *htlcTimeoutResolver) Resolve() (ContractResolver, error) { // If we're already resolved, then we can exit early. if h.resolved { + h.log.Errorf("already resolved") return nil, nil } - // Start by spending the HTLC output, either by broadcasting the - // second-level timeout transaction, or directly if this is the remote - // commitment. - commitSpend, err := h.spendHtlcOutput() - if err != nil { - return nil, err - } - - // If the spend reveals the pre-image, then we'll enter the clean up - // workflow to pass the pre-image back to the incoming link, add it to - // the witness cache, and exit. - if isPreimageSpend( - h.isTaproot(), commitSpend, - h.htlcResolution.SignedTimeoutTx != nil, - ) { - - log.Infof("%T(%v): HTLC has been swept with pre-image by "+ - "remote party during timeout flow! Adding pre-image to "+ - "witness cache", h, h.htlc.RHash[:], - h.htlcResolution.ClaimOutpoint) - - return nil, h.claimCleanUp(commitSpend) - } - - // Depending on whether this was a local or remote commit, we must - // handle the spending transaction accordingly. + // If this is an output on the remote party's commitment transaction, + // use the direct-spend path to sweep the htlc. if h.isRemoteCommitOutput() { return nil, h.resolveRemoteCommitOutput() } - return nil, h.resolveTimeoutTx() + // If this is a zero-fee HTLC, we now handle the spend from our + // commitment transaction. + if h.isZeroFeeOutput() { + return nil, h.resolveTimeoutTx() + } + + // If this is an output on our own commitment using pre-anchor channel + // type, we will let the utxo nursery handle it. + return nil, h.resolveSecondLevelTxLegacy() } // sweepTimeoutTx sends a second level timeout transaction to the sweeper. @@ -521,11 +506,16 @@ func (h *htlcTimeoutResolver) resolveSecondLevelTxLegacy() error { // The utxo nursery will take care of broadcasting the second-level // timeout tx and sweeping its output once it confirms. - return h.IncubateOutputs( + err := h.IncubateOutputs( h.ChanPoint, fn.Some(h.htlcResolution), fn.None[lnwallet.IncomingHtlcResolution](), h.broadcastHeight, h.incomingHTLCExpiryHeight, ) + if err != nil { + return err + } + + return h.resolveTimeoutTx() } // sweepDirectHtlcOutput sends the direct spend of the HTLC output to the @@ -581,53 +571,6 @@ func (h *htlcTimeoutResolver) sweepDirectHtlcOutput() error { return nil } -// spendHtlcOutput handles the initial spend of an HTLC output via the timeout -// clause. If this is our local commitment, the second-level timeout TX will be -// used to spend the output into the next stage. If this is the remote -// commitment, the output will be swept directly without the timeout -// transaction. -func (h *htlcTimeoutResolver) spendHtlcOutput() ( - *chainntnfs.SpendDetail, error) { - - switch { - // If we have non-nil SignDetails, this means that have a 2nd level - // HTLC transaction that is signed using sighash SINGLE|ANYONECANPAY - // (the case for anchor type channels). In this case we can re-sign it - // and attach fees at will. We let the sweeper handle this job. - case h.isZeroFeeOutput() && !h.outputIncubating: - if err := h.sweepTimeoutTx(); err != nil { - log.Errorf("Sending timeout tx to sweeper: %v", err) - - return nil, err - } - - // If this is a remote commitment there's no second level timeout txn, - // and we can just send this directly to the sweeper. - case h.isRemoteCommitOutput() && !h.outputIncubating: - if err := h.sweepDirectHtlcOutput(); err != nil { - log.Errorf("Sending direct spend to sweeper: %v", err) - - return nil, err - } - - // If we have a SignedTimeoutTx but no SignDetails, this is a local - // commitment for a non-anchor channel, so we'll send it to the utxo - // nursery. - case h.isLegacyOutput() && !h.outputIncubating: - if err := h.resolveSecondLevelTxLegacy(); err != nil { - log.Errorf("Sending timeout tx to nursery: %v", err) - - return nil, err - } - } - - // Now that we've handed off the HTLC to the nursery or sweeper, we'll - // watch for a spend of the output, and make our next move off of that. - // Depending on if this is our commitment, or the remote party's - // commitment, we'll be watching a different outpoint and script. - return h.watchHtlcSpend() -} - // watchHtlcSpend watches for a spend of the HTLC output. For neutrino backend, // it will check blocks for the confirmed spend. For btcd and bitcoind, it will // check both the mempool and the blocks. @@ -673,6 +616,9 @@ func (h *htlcTimeoutResolver) waitForConfirmedSpend(op *wire.OutPoint, // // NOTE: Part of the ContractResolver interface. func (h *htlcTimeoutResolver) Stop() { + h.log.Debugf("stopping...") + defer h.log.Debugf("stopped") + close(h.quit) } @@ -1018,15 +964,6 @@ func (h *htlcTimeoutResolver) isZeroFeeOutput() bool { h.htlcResolution.SignDetails != nil } -// isLegacyOutput returns a boolean indicating whether the htlc output is from -// a non-anchor-enabled channel. -func (h *htlcTimeoutResolver) isLegacyOutput() bool { - // If we have a SignedTimeoutTx but no SignDetails, this is a local - // commitment for a non-anchor channel. - return h.htlcResolution.SignedTimeoutTx != nil && - h.htlcResolution.SignDetails == nil -} - // waitHtlcSpendAndCheckPreimage waits for the htlc output to be spent and // checks whether the spending reveals the preimage. If the preimage is found, // it will be added to the preimage beacon to settle the incoming link, and a @@ -1296,9 +1233,11 @@ func (h *htlcTimeoutResolver) resolveTimeoutTx() error { h.log.Infof("2nd-level HTLC timeout tx=%v confirmed", spenderTxid) // Start the process to sweep the output from the timeout tx. - err = h.sweepTimeoutTxOutput() - if err != nil { - return err + if h.isZeroFeeOutput() { + err = h.sweepTimeoutTxOutput() + if err != nil { + return err + } } // Create a checkpoint since the timeout tx is confirmed and the sweep @@ -1332,3 +1271,52 @@ func (h *htlcTimeoutResolver) resolveTimeoutTxOutput(op wire.OutPoint) error { return h.checkpointClaim(spend) } + +// Launch creates an input based on the details of the outgoing htlc resolution +// and offers it to the sweeper. +func (h *htlcTimeoutResolver) Launch() error { + if h.launched { + h.log.Tracef("already launched") + return nil + } + + h.log.Debugf("launching resolver...") + h.launched = true + + switch { + // If we're already resolved, then we can exit early. + case h.resolved: + h.log.Errorf("already resolved") + return nil + + // If this is an output on the remote party's commitment transaction, + // use the direct timeout spend path. + // + // NOTE: When the outputIncubating is false, it means that the output + // has been offered to the utxo nursery as starting in 0.18.4, we + // stopped marking this flag for direct timeout spends (#9062). In that + // case, we will do nothing and let the utxo nursery handle it. + case h.isRemoteCommitOutput() && !h.outputIncubating: + return h.sweepDirectHtlcOutput() + + // If this is an anchor type channel, we now sweep either the + // second-level timeout tx or the output from the second-level timeout + // tx. + case h.isZeroFeeOutput(): + // If the second-level timeout tx has already been swept, we + // can go ahead and sweep its output. + if h.outputIncubating { + return h.sweepTimeoutTxOutput() + } + + // Otherwise, sweep the second level tx. + return h.sweepTimeoutTx() + + // If this is an output on our own commitment using pre-anchor channel + // type, we will let the utxo nursery handle it via Resolve. + // + // TODO(yy): handle the legacy output by offering it to the sweeper. + default: + return nil + } +} diff --git a/contractcourt/htlc_timeout_resolver_test.go b/contractcourt/htlc_timeout_resolver_test.go index 17341f0c20b..89f334aa83b 100644 --- a/contractcourt/htlc_timeout_resolver_test.go +++ b/contractcourt/htlc_timeout_resolver_test.go @@ -40,7 +40,7 @@ type mockWitnessBeacon struct { func newMockWitnessBeacon() *mockWitnessBeacon { return &mockWitnessBeacon{ preImageUpdates: make(chan lntypes.Preimage, 1), - newPreimages: make(chan []lntypes.Preimage), + newPreimages: make(chan []lntypes.Preimage, 1), lookupPreimage: make(map[lntypes.Hash]lntypes.Preimage), } } @@ -280,7 +280,7 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { notifier := &mock.ChainNotifier{ EpochChan: make(chan *chainntnfs.BlockEpoch), - SpendChan: make(chan *chainntnfs.SpendDetail), + SpendChan: make(chan *chainntnfs.SpendDetail, 1), ConfChan: make(chan *chainntnfs.TxConfirmation), } @@ -321,6 +321,7 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { return nil }, + HtlcNotifier: &mockHTLCNotifier{}, }, PutResolverReport: func(_ kvdb.RwTx, _ *channeldb.ResolverReport) error { @@ -356,6 +357,7 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { Amt: testHtlcAmt, }, } + resolver.initLogger("timeoutResolver") var reports []*channeldb.ResolverReport @@ -390,7 +392,12 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { go func() { defer wg.Done() - _, err := resolver.Resolve() + err := resolver.Launch() + if err != nil { + resolveErr <- err + } + + _, err = resolver.Resolve() if err != nil { resolveErr <- err } @@ -406,8 +413,7 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { sweepChan = mockSweeper.sweptInputs } - // The output should be offered to either the sweeper or - // the nursery. + // The output should be offered to either the sweeper or the nursery. select { case <-incubateChan: case <-sweepChan: @@ -431,6 +437,7 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { case notifier.SpendChan <- &chainntnfs.SpendDetail{ SpendingTx: spendingTx, SpenderTxHash: &spendTxHash, + SpentOutPoint: &testChanPoint2, }: case <-time.After(time.Second * 5): t.Fatalf("failed to request spend ntfn") @@ -487,6 +494,7 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { case notifier.SpendChan <- &chainntnfs.SpendDetail{ SpendingTx: spendingTx, SpenderTxHash: &spendTxHash, + SpentOutPoint: &testChanPoint2, }: case <-time.After(time.Second * 5): t.Fatalf("failed to request spend ntfn") @@ -549,6 +557,8 @@ func TestHtlcTimeoutResolver(t *testing.T) { // TestHtlcTimeoutSingleStage tests a remote commitment confirming, and the // local node sweeping the HTLC output directly after timeout. +// +//nolint:ll func TestHtlcTimeoutSingleStage(t *testing.T) { commitOutpoint := wire.OutPoint{Index: 3} @@ -573,6 +583,12 @@ func TestHtlcTimeoutSingleStage(t *testing.T) { SpendTxID: &sweepTxid, } + sweepSpend := &chainntnfs.SpendDetail{ + SpendingTx: sweepTx, + SpentOutPoint: &commitOutpoint, + SpenderTxHash: &sweepTxid, + } + checkpoints := []checkpoint{ { // We send a confirmation the sweep tx from published @@ -582,9 +598,10 @@ func TestHtlcTimeoutSingleStage(t *testing.T) { // The nursery will create and publish a sweep // tx. - ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: sweepTx, - SpenderTxHash: &sweepTxid, + select { + case ctx.notifier.SpendChan <- sweepSpend: + case <-time.After(time.Second * 5): + t.Fatalf("failed to send spend ntfn") } // The resolver should deliver a failure @@ -620,7 +637,9 @@ func TestHtlcTimeoutSingleStage(t *testing.T) { // TestHtlcTimeoutSecondStage tests a local commitment being confirmed, and the // local node claiming the HTLC output using the second-level timeout tx. -func TestHtlcTimeoutSecondStage(t *testing.T) { +// +//nolint:ll +func TestHtlcTimeoutSecondStagex(t *testing.T) { commitOutpoint := wire.OutPoint{Index: 2} htlcOutpoint := wire.OutPoint{Index: 3} @@ -678,23 +697,57 @@ func TestHtlcTimeoutSecondStage(t *testing.T) { SpendTxID: &sweepHash, } + timeoutSpend := &chainntnfs.SpendDetail{ + SpendingTx: timeoutTx, + SpentOutPoint: &commitOutpoint, + SpenderTxHash: &timeoutTxid, + } + + sweepSpend := &chainntnfs.SpendDetail{ + SpendingTx: sweepTx, + SpentOutPoint: &htlcOutpoint, + SpenderTxHash: &sweepHash, + } + checkpoints := []checkpoint{ { + preCheckpoint: func(ctx *htlcResolverTestContext, + _ bool) error { + + // Deliver spend of timeout tx. + ctx.notifier.SpendChan <- timeoutSpend + + return nil + }, + // Output should be handed off to the nursery. incubating: true, + reports: []*channeldb.ResolverReport{ + firstStage, + }, }, { // We send a confirmation for our sweep tx to indicate // that our sweep succeeded. preCheckpoint: func(ctx *htlcResolverTestContext, - _ bool) error { + resumed bool) error { - // The nursery will publish the timeout tx. - ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: timeoutTx, - SpenderTxHash: &timeoutTxid, + // When it's reloaded from disk, we need to + // re-send the notification to mock the first + // `watchHtlcSpend`. + if resumed { + // Deliver spend of timeout tx. + ctx.notifier.SpendChan <- timeoutSpend + + // Deliver spend of timeout tx output. + ctx.notifier.SpendChan <- sweepSpend + + return nil } + // Deliver spend of timeout tx output. + ctx.notifier.SpendChan <- sweepSpend + // The resolver should deliver a failure // resolution message (indicating we // successfully timed out the HTLC). @@ -707,12 +760,6 @@ func TestHtlcTimeoutSecondStage(t *testing.T) { t.Fatalf("resolution not sent") } - // Deliver spend of timeout tx. - ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: sweepTx, - SpenderTxHash: &sweepHash, - } - return nil }, @@ -722,7 +769,7 @@ func TestHtlcTimeoutSecondStage(t *testing.T) { incubating: true, resolved: true, reports: []*channeldb.ResolverReport{ - firstStage, secondState, + secondState, }, }, } @@ -796,10 +843,6 @@ func TestHtlcTimeoutSingleStageRemoteSpend(t *testing.T) { } checkpoints := []checkpoint{ - { - // Output should be handed off to the nursery. - incubating: true, - }, { // We send a spend notification for a remote spend with // the preimage. @@ -812,6 +855,7 @@ func TestHtlcTimeoutSingleStageRemoteSpend(t *testing.T) { // the preimage. ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ SpendingTx: spendTx, + SpentOutPoint: &commitOutpoint, SpenderTxHash: &spendTxHash, } @@ -847,7 +891,7 @@ func TestHtlcTimeoutSingleStageRemoteSpend(t *testing.T) { // After the success tx has confirmed, we expect the // checkpoint to be resolved, and with the above // report. - incubating: true, + incubating: false, resolved: true, reports: []*channeldb.ResolverReport{ claim, @@ -914,6 +958,7 @@ func TestHtlcTimeoutSecondStageRemoteSpend(t *testing.T) { ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ SpendingTx: remoteSuccessTx, + SpentOutPoint: &commitOutpoint, SpenderTxHash: &successTxid, } @@ -967,20 +1012,15 @@ func TestHtlcTimeoutSecondStageRemoteSpend(t *testing.T) { // TestHtlcTimeoutSecondStageSweeper tests that for anchor channels, when a // local commitment confirms, the timeout tx is handed to the sweeper to claim // the HTLC output. +// +//nolint:ll func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { - commitOutpoint := wire.OutPoint{Index: 2} htlcOutpoint := wire.OutPoint{Index: 3} - sweepTx := &wire.MsgTx{ - TxIn: []*wire.TxIn{{}}, - TxOut: []*wire.TxOut{{}}, - } - sweepHash := sweepTx.TxHash() - timeoutTx := &wire.MsgTx{ TxIn: []*wire.TxIn{ { - PreviousOutPoint: commitOutpoint, + PreviousOutPoint: htlcOutpoint, }, }, TxOut: []*wire.TxOut{ @@ -1027,11 +1067,16 @@ func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { }, } reSignedHash := reSignedTimeoutTx.TxHash() - reSignedOutPoint := wire.OutPoint{ + + timeoutTxOutpoint := wire.OutPoint{ Hash: reSignedHash, Index: 1, } + // Make a copy so `isPreimageSpend` can easily pass. + sweepTx := reSignedTimeoutTx.Copy() + sweepHash := sweepTx.TxHash() + // twoStageResolution is a resolution for a htlc on the local // party's commitment, where the timeout tx can be re-signed. twoStageResolution := lnwallet.OutgoingHtlcResolution{ @@ -1045,7 +1090,7 @@ func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { } firstStage := &channeldb.ResolverReport{ - OutPoint: commitOutpoint, + OutPoint: htlcOutpoint, Amount: testHtlcAmt.ToSatoshis(), ResolverType: channeldb.ResolverTypeOutgoingHtlc, ResolverOutcome: channeldb.ResolverOutcomeFirstStage, @@ -1053,12 +1098,45 @@ func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { } secondState := &channeldb.ResolverReport{ - OutPoint: reSignedOutPoint, + OutPoint: timeoutTxOutpoint, Amount: btcutil.Amount(testSignDesc.Output.Value), ResolverType: channeldb.ResolverTypeOutgoingHtlc, ResolverOutcome: channeldb.ResolverOutcomeTimeout, SpendTxID: &sweepHash, } + // mockTimeoutTxSpend is a helper closure to mock `waitForSpend` to + // return the commit spend in `sweepTimeoutTxOutput`. + mockTimeoutTxSpend := func(ctx *htlcResolverTestContext) { + select { + case ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ + SpendingTx: reSignedTimeoutTx, + SpenderInputIndex: 1, + SpenderTxHash: &reSignedHash, + SpendingHeight: 10, + SpentOutPoint: &htlcOutpoint, + }: + + case <-time.After(time.Second * 1): + t.Fatalf("spend not sent") + } + } + + // mockSweepTxSpend is a helper closure to mock `waitForSpend` to + // return the commit spend in `sweepTimeoutTxOutput`. + mockSweepTxSpend := func(ctx *htlcResolverTestContext) { + select { + case ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ + SpendingTx: sweepTx, + SpenderInputIndex: 1, + SpenderTxHash: &sweepHash, + SpendingHeight: 10, + SpentOutPoint: &timeoutTxOutpoint, + }: + + case <-time.After(time.Second * 1): + t.Fatalf("spend not sent") + } + } checkpoints := []checkpoint{ { @@ -1067,28 +1145,40 @@ func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { _ bool) error { resolver := ctx.resolver.(*htlcTimeoutResolver) - inp := <-resolver.Sweeper.(*mockSweeper).sweptInputs + + var ( + inp input.Input + ok bool + ) + + select { + case inp, ok = <-resolver.Sweeper.(*mockSweeper).sweptInputs: + require.True(t, ok) + + case <-time.After(1 * time.Second): + t.Fatal("expected input to be swept") + } + op := inp.OutPoint() - if op != commitOutpoint { + if op != htlcOutpoint { return fmt.Errorf("outpoint %v swept, "+ - "expected %v", op, - commitOutpoint) + "expected %v", op, htlcOutpoint) } - // Emulat the sweeper spending using the - // re-signed timeout tx. - ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: reSignedTimeoutTx, - SpenderInputIndex: 1, - SpenderTxHash: &reSignedHash, - SpendingHeight: 10, - } + // Mock `waitForSpend` twice, called in, + // - `resolveReSignedTimeoutTx` + // - `sweepTimeoutTxOutput`. + mockTimeoutTxSpend(ctx) + mockTimeoutTxSpend(ctx) return nil }, // incubating=true is used to signal that the // second-level transaction was confirmed. incubating: true, + reports: []*channeldb.ResolverReport{ + firstStage, + }, }, { // We send a confirmation for our sweep tx to indicate @@ -1096,18 +1186,18 @@ func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { preCheckpoint: func(ctx *htlcResolverTestContext, resumed bool) error { - // If we are resuming from a checkpoint, we - // expect the resolver to re-subscribe to a - // spend, hence we must resend it. + // Mock `waitForSpend` to return the commit + // spend. if resumed { - ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: reSignedTimeoutTx, - SpenderInputIndex: 1, - SpenderTxHash: &reSignedHash, - SpendingHeight: 10, - } + mockTimeoutTxSpend(ctx) + mockTimeoutTxSpend(ctx) + mockSweepTxSpend(ctx) + + return nil } + mockSweepTxSpend(ctx) + // The resolver should deliver a failure // resolution message (indicating we // successfully timed out the HTLC). @@ -1123,7 +1213,20 @@ func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { // The timeout tx output should now be given to // the sweeper. resolver := ctx.resolver.(*htlcTimeoutResolver) - inp := <-resolver.Sweeper.(*mockSweeper).sweptInputs + + var ( + inp input.Input + ok bool + ) + + select { + case inp, ok = <-resolver.Sweeper.(*mockSweeper).sweptInputs: + require.True(t, ok) + + case <-time.After(1 * time.Second): + t.Fatal("expected input to be swept") + } + op := inp.OutPoint() exp := wire.OutPoint{ Hash: reSignedHash, @@ -1133,14 +1236,6 @@ func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { return fmt.Errorf("wrong outpoint swept") } - // Notify about the spend, which should resolve - // the resolver. - ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: sweepTx, - SpenderTxHash: &sweepHash, - SpendingHeight: 14, - } - return nil }, @@ -1150,7 +1245,6 @@ func TestHtlcTimeoutSecondStageSweeper(t *testing.T) { incubating: true, resolved: true, reports: []*channeldb.ResolverReport{ - firstStage, secondState, }, }, @@ -1231,33 +1325,6 @@ func TestHtlcTimeoutSecondStageSweeperRemoteSpend(t *testing.T) { } checkpoints := []checkpoint{ - { - // The output should be given to the sweeper. - preCheckpoint: func(ctx *htlcResolverTestContext, - _ bool) error { - - resolver := ctx.resolver.(*htlcTimeoutResolver) - inp := <-resolver.Sweeper.(*mockSweeper).sweptInputs - op := inp.OutPoint() - if op != commitOutpoint { - return fmt.Errorf("outpoint %v swept, "+ - "expected %v", op, - commitOutpoint) - } - - // Emulate the remote sweeping the output with the preimage. - // re-signed timeout tx. - ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: spendTx, - SpenderTxHash: &spendTxHash, - } - - return nil - }, - // incubating=true is used to signal that the - // second-level transaction was confirmed. - incubating: true, - }, { // We send a confirmation for our sweep tx to indicate // that our sweep succeeded. @@ -1272,6 +1339,7 @@ func TestHtlcTimeoutSecondStageSweeperRemoteSpend(t *testing.T) { ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ SpendingTx: spendTx, SpenderTxHash: &spendTxHash, + SpentOutPoint: &commitOutpoint, } } @@ -1309,7 +1377,7 @@ func TestHtlcTimeoutSecondStageSweeperRemoteSpend(t *testing.T) { // After the sweep has confirmed, we expect the // checkpoint to be resolved, and with the above // reports. - incubating: true, + incubating: false, resolved: true, reports: []*channeldb.ResolverReport{ claim, @@ -1334,21 +1402,26 @@ func testHtlcTimeout(t *testing.T, resolution lnwallet.OutgoingHtlcResolution, // for the next portion of the test. ctx := newHtlcResolverTestContext(t, func(htlc channeldb.HTLC, cfg ResolverConfig) ContractResolver { - return &htlcTimeoutResolver{ + r := &htlcTimeoutResolver{ contractResolverKit: *newContractResolverKit(cfg), htlc: htlc, htlcResolution: resolution, } + r.initLogger("htlcTimeoutResolver") + + return r }, ) checkpointedState := runFromCheckpoint(t, ctx, checkpoints) + t.Log("Running resolver to completion after restart") + // Now, from every checkpoint created, we re-create the resolver, and // run the test from that checkpoint. for i := range checkpointedState { cp := bytes.NewReader(checkpointedState[i]) - ctx := newHtlcResolverTestContext(t, + ctx := newHtlcResolverTestContextFromReader(t, func(htlc channeldb.HTLC, cfg ResolverConfig) ContractResolver { resolver, err := newTimeoutResolverFromReader(cp, cfg) if err != nil { @@ -1356,7 +1429,8 @@ func testHtlcTimeout(t *testing.T, resolution lnwallet.OutgoingHtlcResolution, } resolver.Supplement(htlc) - resolver.htlcResolution = resolution + resolver.initLogger("htlcTimeoutResolver") + return resolver }, ) From dfaff63e0d688284680aecce0a4dbff058790b7d Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Sun, 17 Nov 2024 10:48:16 +0800 Subject: [PATCH 13/19] invoices: exit early when the subscriber chan is nil When calling `NotifyExitHopHtlc` it is allowed to pass a chan to subscribe to the HTLC's resolution when it's settled. However, this method will also return immediately if there's already a resolution, which means it behaves like a notifier and a getter. If the caller decides to only use the getter to do a non-blocking lookup, it can pass a nil subscriber chan to bypass the notification. --- contractcourt/mock_registry_test.go | 5 +++++ invoices/invoiceregistry.go | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/contractcourt/mock_registry_test.go b/contractcourt/mock_registry_test.go index 5bba11afcb2..0530ab51dd4 100644 --- a/contractcourt/mock_registry_test.go +++ b/contractcourt/mock_registry_test.go @@ -29,6 +29,11 @@ func (r *mockRegistry) NotifyExitHopHtlc(payHash lntypes.Hash, wireCustomRecords lnwire.CustomRecords, payload invoices.Payload) (invoices.HtlcResolution, error) { + // Exit early if the notification channel is nil. + if hodlChan == nil { + return r.notifyResolution, r.notifyErr + } + r.notifyChan <- notifyExitHopData{ hodlChan: hodlChan, payHash: payHash, diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index f5a6c6a95fc..cc76d5aefa5 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -1275,7 +1275,11 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked( invoiceToExpire = makeInvoiceExpiry(ctx.hash, invoice) } - i.hodlSubscribe(hodlChan, ctx.circuitKey) + // Subscribe to the resolution if the caller specified a + // notification channel. + if hodlChan != nil { + i.hodlSubscribe(hodlChan, ctx.circuitKey) + } default: panic("unknown action") From fe4ab4e2e726c3debe7a0d9905e1a722152af4e4 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Sun, 17 Nov 2024 10:47:23 +0800 Subject: [PATCH 14/19] contractcourt: add `Launch` method to incoming contest resolver A minor refactor is done to support implementing `Launch`. --- .../htlc_incoming_contest_resolver.go | 259 +++++++++++++----- .../htlc_incoming_contest_resolver_test.go | 16 +- 2 files changed, 210 insertions(+), 65 deletions(-) diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index ebac4958351..0e0c975248f 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/contractcourt/htlc_incoming_contest_resolver.go @@ -78,6 +78,37 @@ func (h *htlcIncomingContestResolver) processFinalHtlcFail() error { return nil } +// Launch will call the inner resolver's launch method if the preimage can be +// found, otherwise it's a no-op. +func (h *htlcIncomingContestResolver) Launch() error { + // NOTE: we don't mark this resolver as launched as the inner resolver + // will set it when it's launched. + if h.launched { + h.log.Tracef("already launched") + return nil + } + + h.log.Debugf("launching contest resolver...") + + // Query the preimage and apply it if we already know it. + applied, err := h.findAndapplyPreimage() + if err != nil { + return err + } + + // No preimage found, leave it to be handled by the resolver. + if !applied { + return nil + } + + h.log.Debugf("found preimage for htlc=%x, transforming into success "+ + "resolver and launching it", h.htlc.RHash) + + // Once we've applied the preimage, we'll launch the inner resolver to + // attempt to claim the HTLC. + return h.htlcSuccessResolver.Launch() +} + // Resolve attempts to resolve this contract. As we don't yet know of the // preimage for the contract, we'll wait for one of two things to happen: // @@ -94,6 +125,7 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { // If we're already full resolved, then we don't have anything further // to do. if h.resolved { + h.log.Errorf("already resolved") return nil, nil } @@ -101,8 +133,7 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { // now. payload, nextHopOnionBlob, err := h.decodePayload() if err != nil { - log.Debugf("ChannelArbitrator(%v): cannot decode payload of "+ - "htlc %v", h.ChanPoint, h.HtlcPoint()) + h.log.Debugf("cannot decode payload of htlc %v", h.HtlcPoint()) // If we've locked in an htlc with an invalid payload on our // commitment tx, we don't need to resolve it. The other party @@ -177,65 +208,6 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { return nil, h.Checkpoint(h, report) } - // applyPreimage is a helper function that will populate our internal - // resolver with the preimage we learn of. This should be called once - // the preimage is revealed so the inner resolver can properly complete - // its duties. The error return value indicates whether the preimage - // was properly applied. - applyPreimage := func(preimage lntypes.Preimage) error { - // Sanity check to see if this preimage matches our htlc. At - // this point it should never happen that it does not match. - if !preimage.Matches(h.htlc.RHash) { - return errors.New("preimage does not match hash") - } - - // Update htlcResolution with the matching preimage. - h.htlcResolution.Preimage = preimage - - log.Infof("%T(%v): applied preimage=%v", h, - h.htlcResolution.ClaimOutpoint, preimage) - - isSecondLevel := h.htlcResolution.SignedSuccessTx != nil - - // If we didn't have to go to the second level to claim (this - // is the remote commitment transaction), then we don't need to - // modify our canned witness. - if !isSecondLevel { - return nil - } - - isTaproot := txscript.IsPayToTaproot( - h.htlcResolution.SignedSuccessTx.TxOut[0].PkScript, - ) - - // If this is our commitment transaction, then we'll need to - // populate the witness for the second-level HTLC transaction. - switch { - // For taproot channels, the witness for sweeping with success - // looks like: - // - - // - // - // So we'll insert it at the 3rd index of the witness. - case isTaproot: - //nolint:ll - h.htlcResolution.SignedSuccessTx.TxIn[0].Witness[2] = preimage[:] - - // Within the witness for the success transaction, the - // preimage is the 4th element as it looks like: - // - // * <0> - // - // We'll populate it within the witness, as since this - // was a "contest" resolver, we didn't yet know of the - // preimage. - case !isTaproot: - h.htlcResolution.SignedSuccessTx.TxIn[0].Witness[3] = preimage[:] - } - - return nil - } - // Define a closure to process htlc resolutions either directly or // triggered by future notifications. processHtlcResolution := func(e invoices.HtlcResolution) ( @@ -247,7 +219,7 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { // If the htlc resolution was a settle, apply the // preimage and return a success resolver. case *invoices.HtlcSettleResolution: - err := applyPreimage(resolution.Preimage) + err := h.applyPreimage(resolution.Preimage) if err != nil { return nil, err } @@ -312,6 +284,9 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { return nil, err } + h.log.Debugf("received resolution from registry: %v", + resolution) + defer func() { h.Registry.HodlUnsubscribeAll(hodlQueue.ChanIn()) @@ -369,7 +344,9 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { // However, we don't know how to ourselves, so we'll // return our inner resolver which has the knowledge to // do so. - if err := applyPreimage(preimage); err != nil { + h.log.Debugf("Found preimage for htlc=%x", h.htlc.RHash) + + if err := h.applyPreimage(preimage); err != nil { return nil, err } @@ -388,7 +365,10 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { continue } - if err := applyPreimage(preimage); err != nil { + h.log.Debugf("Received preimage for htlc=%x", + h.htlc.RHash) + + if err := h.applyPreimage(preimage); err != nil { return nil, err } @@ -435,6 +415,76 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { } } +// applyPreimage is a helper function that will populate our internal resolver +// with the preimage we learn of. This should be called once the preimage is +// revealed so the inner resolver can properly complete its duties. The error +// return value indicates whether the preimage was properly applied. +func (h *htlcIncomingContestResolver) applyPreimage( + preimage lntypes.Preimage) error { + + // Sanity check to see if this preimage matches our htlc. At this point + // it should never happen that it does not match. + if !preimage.Matches(h.htlc.RHash) { + return errors.New("preimage does not match hash") + } + + // We may already have the preimage since both the `Launch` and + // `Resolve` methods will look for it. + if h.htlcResolution.Preimage != lntypes.ZeroHash { + h.log.Debugf("already applied preimage for htlc=%x", + h.htlc.RHash) + + return nil + } + + // Update htlcResolution with the matching preimage. + h.htlcResolution.Preimage = preimage + + log.Infof("%T(%v): applied preimage=%v", h, + h.htlcResolution.ClaimOutpoint, preimage) + + isSecondLevel := h.htlcResolution.SignedSuccessTx != nil + + // If we didn't have to go to the second level to claim (this + // is the remote commitment transaction), then we don't need to + // modify our canned witness. + if !isSecondLevel { + return nil + } + + isTaproot := txscript.IsPayToTaproot( + h.htlcResolution.SignedSuccessTx.TxOut[0].PkScript, + ) + + // If this is our commitment transaction, then we'll need to + // populate the witness for the second-level HTLC transaction. + switch { + // For taproot channels, the witness for sweeping with success + // looks like: + // - + // + // + // So we'll insert it at the 3rd index of the witness. + case isTaproot: + //nolint:ll + h.htlcResolution.SignedSuccessTx.TxIn[0].Witness[2] = preimage[:] + + // Within the witness for the success transaction, the + // preimage is the 4th element as it looks like: + // + // * <0> + // + // We'll populate it within the witness, as since this + // was a "contest" resolver, we didn't yet know of the + // preimage. + case !isTaproot: + //nolint:ll + h.htlcResolution.SignedSuccessTx.TxIn[0].Witness[3] = preimage[:] + } + + return nil +} + // report returns a report on the resolution state of the contract. func (h *htlcIncomingContestResolver) report() *ContractReport { // No locking needed as these values are read-only. @@ -461,6 +511,8 @@ func (h *htlcIncomingContestResolver) report() *ContractReport { // // NOTE: Part of the ContractResolver interface. func (h *htlcIncomingContestResolver) Stop() { + h.log.Debugf("stopping...") + defer h.log.Debugf("stopped") close(h.quit) } @@ -560,3 +612,82 @@ func (h *htlcIncomingContestResolver) decodePayload() (*hop.Payload, // A compile time assertion to ensure htlcIncomingContestResolver meets the // ContractResolver interface. var _ htlcContractResolver = (*htlcIncomingContestResolver)(nil) + +// findAndapplyPreimage performs a non-blocking read to find the preimage for +// the incoming HTLC. If found, it will be applied to the resolver. This method +// is used for the resolver to decide whether it wants to transform into a +// success resolver during launching. +// +// NOTE: Since we have two places to query the preimage, we need to check both +// the preimage db and the invoice db to look up the preimage. +func (h *htlcIncomingContestResolver) findAndapplyPreimage() (bool, error) { + // Query to see if we already know the preimage. + preimage, ok := h.PreimageDB.LookupPreimage(h.htlc.RHash) + + // If the preimage is known, we'll apply it. + if ok { + if err := h.applyPreimage(preimage); err != nil { + return false, err + } + + // Successfully applied the preimage, we can now return. + return true, nil + } + + // First try to parse the payload. + payload, _, err := h.decodePayload() + if err != nil { + h.log.Errorf("Cannot decode payload of htlc %v", h.HtlcPoint()) + + // If we cannot decode the payload, we will return a nil error + // and let it to be handled in `Resolve`. + return false, nil + } + + // Exit early if this is not the exit hop, which means we are not the + // payment receiver and don't have preimage. + if payload.FwdInfo.NextHop != hop.Exit { + return false, nil + } + + // Notify registry that we are potentially resolving as an exit hop + // on-chain. If this HTLC indeed pays to an existing invoice, the + // invoice registry will tell us what to do with the HTLC. This is + // identical to HTLC resolution in the link. + circuitKey := models.CircuitKey{ + ChanID: h.ShortChanID, + HtlcID: h.htlc.HtlcIndex, + } + + // Try get the resolution - if it doesn't give us a resolution + // immediately, we'll assume we don't know it yet and let the `Resolve` + // handle the waiting. + // + // NOTE: we use a nil subscriber here and a zero current height as we + // are only interested in the settle resolution. + // + // TODO(yy): move this logic to link and let the preimage be accessed + // via the preimage beacon. + resolution, err := h.Registry.NotifyExitHopHtlc( + h.htlc.RHash, h.htlc.Amt, h.htlcExpiry, 0, + circuitKey, nil, nil, payload, + ) + if err != nil { + return false, err + } + + res, ok := resolution.(*invoices.HtlcSettleResolution) + + // Exit early if it's not a settle resolution. + if !ok { + return false, nil + } + + // Otherwise we have a settle resolution, apply the preimage. + err = h.applyPreimage(res.Preimage) + if err != nil { + return false, err + } + + return true, nil +} diff --git a/contractcourt/htlc_incoming_contest_resolver_test.go b/contractcourt/htlc_incoming_contest_resolver_test.go index e8b8eac0c96..f17190e96e8 100644 --- a/contractcourt/htlc_incoming_contest_resolver_test.go +++ b/contractcourt/htlc_incoming_contest_resolver_test.go @@ -5,11 +5,13 @@ import ( "io" "testing" + "github.com/btcsuite/btcd/wire" sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/htlcswitch/hop" + "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/invoices" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnmock" @@ -356,6 +358,7 @@ func newIncomingResolverTestContext(t *testing.T, isExit bool) *incomingResolver return nil }, + Sweeper: newMockSweeper(), }, PutResolverReport: func(_ kvdb.RwTx, _ *channeldb.ResolverReport) error { @@ -374,10 +377,16 @@ func newIncomingResolverTestContext(t *testing.T, isExit bool) *incomingResolver }, } + res := lnwallet.IncomingHtlcResolution{ + SweepSignDesc: input.SignDescriptor{ + Output: &wire.TxOut{}, + }, + } + c.resolver = &htlcIncomingContestResolver{ htlcSuccessResolver: &htlcSuccessResolver{ contractResolverKit: *newContractResolverKit(cfg), - htlcResolution: lnwallet.IncomingHtlcResolution{}, + htlcResolution: res, htlc: channeldb.HTLC{ Amt: lnwire.MilliSatoshi(testHtlcAmount), RHash: testResHash, @@ -386,6 +395,7 @@ func newIncomingResolverTestContext(t *testing.T, isExit bool) *incomingResolver }, htlcExpiry: testHtlcExpiry, } + c.resolver.initLogger("htlcIncomingContestResolver") return c } @@ -395,6 +405,10 @@ func (i *incomingResolverTestContext) resolve() { i.resolveErr = make(chan error, 1) go func() { var err error + + err = i.resolver.Launch() + require.NoError(i.t, err) + i.nextResolver, err = i.resolver.Resolve() i.resolveErr <- err }() From dbfe5df0f392c630b0f9495e649d9e6f1197b232 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 20 Jun 2024 22:11:47 +0800 Subject: [PATCH 15/19] contractcourt: add `Launch` method to outgoing contest resolver --- .../htlc_outgoing_contest_resolver.go | 44 ++++++++++++++++--- .../htlc_outgoing_contest_resolver_test.go | 6 +++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/contractcourt/htlc_outgoing_contest_resolver.go b/contractcourt/htlc_outgoing_contest_resolver.go index 7adce5a6893..b23259b5bb6 100644 --- a/contractcourt/htlc_outgoing_contest_resolver.go +++ b/contractcourt/htlc_outgoing_contest_resolver.go @@ -1,7 +1,6 @@ package contractcourt import ( - "fmt" "io" "github.com/btcsuite/btcd/btcutil" @@ -36,6 +35,37 @@ func newOutgoingContestResolver(res lnwallet.OutgoingHtlcResolution, } } +// Launch will call the inner resolver's launch method if the expiry height has +// been reached, otherwise it's a no-op. +func (h *htlcOutgoingContestResolver) Launch() error { + // NOTE: we don't mark this resolver as launched as the inner resolver + // will set it when it's launched. + if h.launched { + h.log.Tracef("already launched") + return nil + } + + h.log.Debugf("launching contest resolver...") + + _, bestHeight, err := h.ChainIO.GetBestBlock() + if err != nil { + return err + } + + if uint32(bestHeight) < h.htlcResolution.Expiry { + return nil + } + + // If the current height is >= expiry, then a timeout path spend will + // be valid to be included in the next block, and we can immediately + // return the resolver. + h.log.Infof("expired (height=%v, expiry=%v), transforming into "+ + "timeout resolver and launching it", bestHeight, + h.htlcResolution.Expiry) + + return h.htlcTimeoutResolver.Launch() +} + // Resolve commences the resolution of this contract. As this contract hasn't // yet timed out, we'll wait for one of two things to happen // @@ -53,6 +83,7 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { // If we're already full resolved, then we don't have anything further // to do. if h.resolved { + h.log.Errorf("already resolved") return nil, nil } @@ -86,7 +117,6 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { return nil, errResolverShuttingDown } - // TODO(roasbeef): Checkpoint? return nil, h.claimCleanUp(commitSpend) // If it hasn't, then we'll watch for both the expiration, and the @@ -124,12 +154,14 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { // finalized` will be returned and the broadcast will // fail. newHeight := uint32(newBlock.Height) - if newHeight >= h.htlcResolution.Expiry { - log.Infof("%T(%v): HTLC has expired "+ + expiry := h.htlcResolution.Expiry + if newHeight >= expiry { + h.log.Infof("HTLC about to expire "+ "(height=%v, expiry=%v), transforming "+ "into timeout resolver", h, h.htlcResolution.ClaimOutpoint, newHeight, h.htlcResolution.Expiry) + return h.htlcTimeoutResolver, nil } @@ -147,7 +179,7 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { return nil, h.claimCleanUp(commitSpend) case <-h.quit: - return nil, fmt.Errorf("resolver canceled") + return nil, errResolverShuttingDown } } } @@ -178,6 +210,8 @@ func (h *htlcOutgoingContestResolver) report() *ContractReport { // // NOTE: Part of the ContractResolver interface. func (h *htlcOutgoingContestResolver) Stop() { + h.log.Debugf("stopping...") + defer h.log.Debugf("stopped") close(h.quit) } diff --git a/contractcourt/htlc_outgoing_contest_resolver_test.go b/contractcourt/htlc_outgoing_contest_resolver_test.go index 4fa3a6874f9..625df60bf1f 100644 --- a/contractcourt/htlc_outgoing_contest_resolver_test.go +++ b/contractcourt/htlc_outgoing_contest_resolver_test.go @@ -15,6 +15,7 @@ import ( "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwire" + "github.com/stretchr/testify/require" ) const ( @@ -159,6 +160,7 @@ func newOutgoingResolverTestContext(t *testing.T) *outgoingResolverTestContext { return nil }, + ChainIO: &mock.ChainIO{}, }, PutResolverReport: func(_ kvdb.RwTx, _ *channeldb.ResolverReport) error { @@ -195,6 +197,7 @@ func newOutgoingResolverTestContext(t *testing.T) *outgoingResolverTestContext { }, }, } + resolver.initLogger("htlcOutgoingContestResolver") return &outgoingResolverTestContext{ resolver: resolver, @@ -209,6 +212,9 @@ func (i *outgoingResolverTestContext) resolve() { // Start resolver. i.resolverResultChan = make(chan resolveResult, 1) go func() { + err := i.resolver.Launch() + require.NoError(i.t, err) + nextResolver, err := i.resolver.Resolve() i.resolverResultChan <- resolveResult{ nextResolver: nextResolver, From 1c8cbc8e09eb2a4d67dca3edead6168f16978464 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Wed, 10 Jul 2024 18:08:23 +0800 Subject: [PATCH 16/19] contractcourt: fix concurrent access to `resolved` This commit makes `resolved` an atomic bool to avoid data race. This field is now defined in `contractResolverKit` to avoid code duplication. --- contractcourt/anchor_resolver.go | 17 +---- contractcourt/breach_resolver.go | 22 +++--- contractcourt/briefcase_test.go | 67 +++++++++++-------- contractcourt/commit_sweep_resolver.go | 26 +++---- contractcourt/contract_resolver.go | 17 +++++ .../htlc_incoming_contest_resolver.go | 19 ++---- .../htlc_outgoing_contest_resolver.go | 10 +-- contractcourt/htlc_success_resolver.go | 27 +++----- contractcourt/htlc_success_resolver_test.go | 4 +- contractcourt/htlc_timeout_resolver.go | 29 ++++---- contractcourt/htlc_timeout_resolver_test.go | 2 +- 11 files changed, 111 insertions(+), 129 deletions(-) diff --git a/contractcourt/anchor_resolver.go b/contractcourt/anchor_resolver.go index ded3ebd83f2..b70e28323d4 100644 --- a/contractcourt/anchor_resolver.go +++ b/contractcourt/anchor_resolver.go @@ -24,9 +24,6 @@ type anchorResolver struct { // anchor is the outpoint on the commitment transaction. anchor wire.OutPoint - // resolved reflects if the contract has been fully resolved or not. - resolved bool - // broadcastHeight is the height that the original contract was // broadcast to the main-chain at. We'll use this value to bound any // historical queries to the chain for spends/confirmations. @@ -89,7 +86,7 @@ func (c *anchorResolver) ResolverKey() []byte { // NOTE: Part of the ContractResolver interface. func (c *anchorResolver) Resolve() (ContractResolver, error) { // If we're already resolved, then we can exit early. - if c.resolved { + if c.IsResolved() { c.log.Errorf("already resolved") return nil, nil } @@ -139,7 +136,7 @@ func (c *anchorResolver) Resolve() (ContractResolver, error) { ) c.reportLock.Unlock() - c.resolved = true + c.markResolved() return nil, c.PutResolverReport(nil, report) } @@ -154,14 +151,6 @@ func (c *anchorResolver) Stop() { close(c.quit) } -// IsResolved returns true if the stored state in the resolve is fully -// resolved. In this case the target output can be forgotten. -// -// NOTE: Part of the ContractResolver interface. -func (c *anchorResolver) IsResolved() bool { - return c.resolved -} - // SupplementState allows the user of a ContractResolver to supplement it with // state required for the proper resolution of a contract. // @@ -198,7 +187,7 @@ func (c *anchorResolver) Launch() error { c.launched = true // If we're already resolved, then we can exit early. - if c.resolved { + if c.IsResolved() { c.log.Errorf("already resolved") return nil } diff --git a/contractcourt/breach_resolver.go b/contractcourt/breach_resolver.go index 1f74924d397..66a974e67f4 100644 --- a/contractcourt/breach_resolver.go +++ b/contractcourt/breach_resolver.go @@ -12,9 +12,6 @@ import ( // future, this will likely take over the duties the current BreachArbitrator // has. type breachResolver struct { - // resolved reflects if the contract has been fully resolved or not. - resolved bool - // subscribed denotes whether or not the breach resolver has subscribed // to the BreachArbitrator for breach resolution. subscribed bool @@ -62,7 +59,7 @@ func (b *breachResolver) Resolve() (ContractResolver, error) { // If the breach resolution process is already complete, then // we can cleanup and checkpoint the resolved state. if complete { - b.resolved = true + b.markResolved() return nil, b.Checkpoint(b) } @@ -75,8 +72,9 @@ func (b *breachResolver) Resolve() (ContractResolver, error) { // The replyChan has been closed, signalling that the breach // has been fully resolved. Checkpoint the resolved state and // exit. - b.resolved = true + b.markResolved() return nil, b.Checkpoint(b) + case <-b.quit: } @@ -89,19 +87,13 @@ func (b *breachResolver) Stop() { close(b.quit) } -// IsResolved returns true if the breachResolver is fully resolved and cleanup -// can occur. -func (b *breachResolver) IsResolved() bool { - return b.resolved -} - // SupplementState adds additional state to the breachResolver. func (b *breachResolver) SupplementState(_ *channeldb.OpenChannel) { } // Encode encodes the breachResolver to the passed writer. func (b *breachResolver) Encode(w io.Writer) error { - return binary.Write(w, endian, b.resolved) + return binary.Write(w, endian, b.IsResolved()) } // newBreachResolverFromReader attempts to decode an encoded breachResolver @@ -114,9 +106,13 @@ func newBreachResolverFromReader(r io.Reader, resCfg ResolverConfig) ( replyChan: make(chan struct{}), } - if err := binary.Read(r, endian, &b.resolved); err != nil { + var resolved bool + if err := binary.Read(r, endian, &resolved); err != nil { return nil, err } + if resolved { + b.markResolved() + } b.initLogger(fmt.Sprintf("%T(%v)", b, b.ChanPoint)) diff --git a/contractcourt/briefcase_test.go b/contractcourt/briefcase_test.go index 533d0eff782..aa2e711efc2 100644 --- a/contractcourt/briefcase_test.go +++ b/contractcourt/briefcase_test.go @@ -206,8 +206,8 @@ func assertResolversEqual(t *testing.T, originalResolver ContractResolver, ogRes.outputIncubating, diskRes.outputIncubating) } if ogRes.resolved != diskRes.resolved { - t.Fatalf("expected %v, got %v", ogRes.resolved, - diskRes.resolved) + t.Fatalf("expected %v, got %v", ogRes.resolved.Load(), + diskRes.resolved.Load()) } if ogRes.broadcastHeight != diskRes.broadcastHeight { t.Fatalf("expected %v, got %v", @@ -229,8 +229,8 @@ func assertResolversEqual(t *testing.T, originalResolver ContractResolver, ogRes.outputIncubating, diskRes.outputIncubating) } if ogRes.resolved != diskRes.resolved { - t.Fatalf("expected %v, got %v", ogRes.resolved, - diskRes.resolved) + t.Fatalf("expected %v, got %v", ogRes.resolved.Load(), + diskRes.resolved.Load()) } if ogRes.broadcastHeight != diskRes.broadcastHeight { t.Fatalf("expected %v, got %v", @@ -275,8 +275,8 @@ func assertResolversEqual(t *testing.T, originalResolver ContractResolver, ogRes.commitResolution, diskRes.commitResolution) } if ogRes.resolved != diskRes.resolved { - t.Fatalf("expected %v, got %v", ogRes.resolved, - diskRes.resolved) + t.Fatalf("expected %v, got %v", ogRes.resolved.Load(), + diskRes.resolved.Load()) } if ogRes.broadcastHeight != diskRes.broadcastHeight { t.Fatalf("expected %v, got %v", @@ -312,13 +312,14 @@ func TestContractInsertionRetrieval(t *testing.T) { SweepSignDesc: testSignDesc, }, outputIncubating: true, - resolved: true, broadcastHeight: 102, htlc: channeldb.HTLC{ HtlcIndex: 12, }, } - successResolver := htlcSuccessResolver{ + timeoutResolver.resolved.Store(true) + + successResolver := &htlcSuccessResolver{ htlcResolution: lnwallet.IncomingHtlcResolution{ Preimage: testPreimage, SignedSuccessTx: nil, @@ -327,40 +328,49 @@ func TestContractInsertionRetrieval(t *testing.T) { SweepSignDesc: testSignDesc, }, outputIncubating: true, - resolved: true, broadcastHeight: 109, htlc: channeldb.HTLC{ RHash: testPreimage, }, } - resolvers := []ContractResolver{ - &timeoutResolver, - &successResolver, - &commitSweepResolver{ - commitResolution: lnwallet.CommitOutputResolution{ - SelfOutPoint: testChanPoint2, - SelfOutputSignDesc: testSignDesc, - MaturityDelay: 99, - }, - resolved: false, - broadcastHeight: 109, - chanPoint: testChanPoint1, + successResolver.resolved.Store(true) + + commitResolver := &commitSweepResolver{ + commitResolution: lnwallet.CommitOutputResolution{ + SelfOutPoint: testChanPoint2, + SelfOutputSignDesc: testSignDesc, + MaturityDelay: 99, }, + broadcastHeight: 109, + chanPoint: testChanPoint1, + } + commitResolver.resolved.Store(false) + + resolvers := []ContractResolver{ + &timeoutResolver, successResolver, commitResolver, } // All resolvers require a unique ResolverKey() output. To achieve this // for the composite resolvers, we'll mutate the underlying resolver // with a new outpoint. - contestTimeout := timeoutResolver - contestTimeout.htlcResolution.ClaimOutpoint = randOutPoint() + contestTimeout := htlcTimeoutResolver{ + htlcResolution: lnwallet.OutgoingHtlcResolution{ + ClaimOutpoint: randOutPoint(), + SweepSignDesc: testSignDesc, + }, + } resolvers = append(resolvers, &htlcOutgoingContestResolver{ htlcTimeoutResolver: &contestTimeout, }) - contestSuccess := successResolver - contestSuccess.htlcResolution.ClaimOutpoint = randOutPoint() + contestSuccess := &htlcSuccessResolver{ + htlcResolution: lnwallet.IncomingHtlcResolution{ + ClaimOutpoint: randOutPoint(), + SweepSignDesc: testSignDesc, + }, + } resolvers = append(resolvers, &htlcIncomingContestResolver{ htlcExpiry: 100, - htlcSuccessResolver: &contestSuccess, + htlcSuccessResolver: contestSuccess, }) // For quick lookup during the test, we'll create this map which allow @@ -438,12 +448,12 @@ func TestContractResolution(t *testing.T) { SweepSignDesc: testSignDesc, }, outputIncubating: true, - resolved: true, broadcastHeight: 192, htlc: channeldb.HTLC{ HtlcIndex: 9912, }, } + timeoutResolver.resolved.Store(true) // First, we'll insert the resolver into the database and ensure that // we get the same resolver out the other side. We do not need to apply @@ -491,12 +501,13 @@ func TestContractSwapping(t *testing.T) { SweepSignDesc: testSignDesc, }, outputIncubating: true, - resolved: true, broadcastHeight: 102, htlc: channeldb.HTLC{ HtlcIndex: 12, }, } + timeoutResolver.resolved.Store(true) + contestResolver := &htlcOutgoingContestResolver{ htlcTimeoutResolver: timeoutResolver, } diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go index b3323158db0..84efdeb067b 100644 --- a/contractcourt/commit_sweep_resolver.go +++ b/contractcourt/commit_sweep_resolver.go @@ -39,9 +39,6 @@ type commitSweepResolver struct { // this HTLC on-chain. commitResolution lnwallet.CommitOutputResolution - // resolved reflects if the contract has been fully resolved or not. - resolved bool - // broadcastHeight is the height that the original contract was // broadcast to the main-chain at. We'll use this value to bound any // historical queries to the chain for spends/confirmations. @@ -171,7 +168,7 @@ func (c *commitSweepResolver) getCommitTxConfHeight() (uint32, error) { //nolint:funlen func (c *commitSweepResolver) Resolve() (ContractResolver, error) { // If we're already resolved, then we can exit early. - if c.resolved { + if c.IsResolved() { c.log.Errorf("already resolved") return nil, nil } @@ -224,7 +221,7 @@ func (c *commitSweepResolver) Resolve() (ContractResolver, error) { report := c.currentReport.resolverReport( &sweepTxID, channeldb.ResolverTypeCommit, outcome, ) - c.resolved = true + c.markResolved() // Checkpoint the resolver with a closure that will write the outcome // of the resolver and its sweep transaction to disk. @@ -241,14 +238,6 @@ func (c *commitSweepResolver) Stop() { close(c.quit) } -// IsResolved returns true if the stored state in the resolve is fully -// resolved. In this case the target output can be forgotten. -// -// NOTE: Part of the ContractResolver interface. -func (c *commitSweepResolver) IsResolved() bool { - return c.resolved -} - // SupplementState allows the user of a ContractResolver to supplement it with // state required for the proper resolution of a contract. // @@ -277,7 +266,7 @@ func (c *commitSweepResolver) Encode(w io.Writer) error { return err } - if err := binary.Write(w, endian, c.resolved); err != nil { + if err := binary.Write(w, endian, c.IsResolved()); err != nil { return err } if err := binary.Write(w, endian, c.broadcastHeight); err != nil { @@ -312,9 +301,14 @@ func newCommitSweepResolverFromReader(r io.Reader, resCfg ResolverConfig) ( return nil, err } - if err := binary.Read(r, endian, &c.resolved); err != nil { + var resolved bool + if err := binary.Read(r, endian, &resolved); err != nil { return nil, err } + if resolved { + c.markResolved() + } + if err := binary.Read(r, endian, &c.broadcastHeight); err != nil { return nil, err } @@ -383,7 +377,7 @@ func (c *commitSweepResolver) Launch() error { c.launched = true // If we're already resolved, then we can exit early. - if c.resolved { + if c.IsResolved() { c.log.Errorf("already resolved") return nil } diff --git a/contractcourt/contract_resolver.go b/contractcourt/contract_resolver.go index 814c02ff564..5311d466aee 100644 --- a/contractcourt/contract_resolver.go +++ b/contractcourt/contract_resolver.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "sync/atomic" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btclog/v2" @@ -119,6 +120,9 @@ type contractResolverKit struct { // launched specifies whether the resolver has been launched. Calling // `Launch` will be a no-op if this is true. launched bool + + // resolved reflects if the contract has been fully resolved or not. + resolved atomic.Bool } // newContractResolverKit instantiates the mix-in struct. @@ -137,6 +141,19 @@ func (r *contractResolverKit) initLogger(prefix string) { r.log = log.WithPrefix(logPrefix) } +// IsResolved returns true if the stored state in the resolve is fully +// resolved. In this case the target output can be forgotten. +// +// NOTE: Part of the ContractResolver interface. +func (r *contractResolverKit) IsResolved() bool { + return r.resolved.Load() +} + +// markResolved marks the resolver as resolved. +func (r *contractResolverKit) markResolved() { + r.resolved.Store(true) +} + var ( // errResolverShuttingDown is returned when the resolver stops // progressing because it received the quit signal. diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index 0e0c975248f..31790102253 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/contractcourt/htlc_incoming_contest_resolver.go @@ -124,7 +124,7 @@ func (h *htlcIncomingContestResolver) Launch() error { func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { // If we're already full resolved, then we don't have anything further // to do. - if h.resolved { + if h.IsResolved() { h.log.Errorf("already resolved") return nil, nil } @@ -140,7 +140,7 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { // will time it out and get their funds back. This situation // can present itself when we crash before processRemoteAdds in // the link has ran. - h.resolved = true + h.markResolved() if err := h.processFinalHtlcFail(); err != nil { return nil, err @@ -193,7 +193,7 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { log.Infof("%T(%v): HTLC has timed out (expiry=%v, height=%v), "+ "abandoning", h, h.htlcResolution.ClaimOutpoint, h.htlcExpiry, currentHeight) - h.resolved = true + h.markResolved() if err := h.processFinalHtlcFail(); err != nil { return nil, err @@ -234,7 +234,7 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { h.htlcResolution.ClaimOutpoint, h.htlcExpiry, currentHeight) - h.resolved = true + h.markResolved() if err := h.processFinalHtlcFail(); err != nil { return nil, err @@ -395,7 +395,8 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { "(expiry=%v, height=%v), abandoning", h, h.htlcResolution.ClaimOutpoint, h.htlcExpiry, currentHeight) - h.resolved = true + + h.markResolved() if err := h.processFinalHtlcFail(); err != nil { return nil, err @@ -516,14 +517,6 @@ func (h *htlcIncomingContestResolver) Stop() { close(h.quit) } -// IsResolved returns true if the stored state in the resolve is fully -// resolved. In this case the target output can be forgotten. -// -// NOTE: Part of the ContractResolver interface. -func (h *htlcIncomingContestResolver) IsResolved() bool { - return h.resolved -} - // Encode writes an encoded version of the ContractResolver into the passed // Writer. // diff --git a/contractcourt/htlc_outgoing_contest_resolver.go b/contractcourt/htlc_outgoing_contest_resolver.go index b23259b5bb6..8763dfb27eb 100644 --- a/contractcourt/htlc_outgoing_contest_resolver.go +++ b/contractcourt/htlc_outgoing_contest_resolver.go @@ -82,7 +82,7 @@ func (h *htlcOutgoingContestResolver) Launch() error { func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { // If we're already full resolved, then we don't have anything further // to do. - if h.resolved { + if h.IsResolved() { h.log.Errorf("already resolved") return nil, nil } @@ -215,14 +215,6 @@ func (h *htlcOutgoingContestResolver) Stop() { close(h.quit) } -// IsResolved returns true if the stored state in the resolve is fully -// resolved. In this case the target output can be forgotten. -// -// NOTE: Part of the ContractResolver interface. -func (h *htlcOutgoingContestResolver) IsResolved() bool { - return h.resolved -} - // Encode writes an encoded version of the ContractResolver into the passed // Writer. // diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index 20b45166343..b4c6db2b916 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -42,9 +42,6 @@ type htlcSuccessResolver struct { // second-level output (true). outputIncubating bool - // resolved reflects if the contract has been fully resolved or not. - resolved bool - // broadcastHeight is the height that the original contract was // broadcast to the main-chain at. We'll use this value to bound any // historical queries to the chain for spends/confirmations. @@ -122,7 +119,7 @@ func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) { switch { // If we're already resolved, then we can exit early. - case h.resolved: + case h.IsResolved(): h.log.Errorf("already resolved") // If this is an output on the remote party's commitment transaction, @@ -226,7 +223,7 @@ func (h *htlcSuccessResolver) checkpointClaim(spendTx *chainhash.Hash) error { } // Finally, we checkpoint the resolver with our report(s). - h.resolved = true + h.markResolved() return h.Checkpoint(h, reports...) } @@ -241,14 +238,6 @@ func (h *htlcSuccessResolver) Stop() { close(h.quit) } -// IsResolved returns true if the stored state in the resolve is fully -// resolved. In this case the target output can be forgotten. -// -// NOTE: Part of the ContractResolver interface. -func (h *htlcSuccessResolver) IsResolved() bool { - return h.resolved -} - // report returns a report on the resolution state of the contract. func (h *htlcSuccessResolver) report() *ContractReport { // If the sign details are nil, the report will be created by handled @@ -298,7 +287,7 @@ func (h *htlcSuccessResolver) Encode(w io.Writer) error { if err := binary.Write(w, endian, h.outputIncubating); err != nil { return err } - if err := binary.Write(w, endian, h.resolved); err != nil { + if err := binary.Write(w, endian, h.IsResolved()); err != nil { return err } if err := binary.Write(w, endian, h.broadcastHeight); err != nil { @@ -337,9 +326,15 @@ func newSuccessResolverFromReader(r io.Reader, resCfg ResolverConfig) ( if err := binary.Read(r, endian, &h.outputIncubating); err != nil { return nil, err } - if err := binary.Read(r, endian, &h.resolved); err != nil { + + var resolved bool + if err := binary.Read(r, endian, &resolved); err != nil { return nil, err } + if resolved { + h.markResolved() + } + if err := binary.Read(r, endian, &h.broadcastHeight); err != nil { return nil, err } @@ -745,7 +740,7 @@ func (h *htlcSuccessResolver) Launch() error { switch { // If we're already resolved, then we can exit early. - case h.resolved: + case h.IsResolved(): h.log.Errorf("already resolved") return nil diff --git a/contractcourt/htlc_success_resolver_test.go b/contractcourt/htlc_success_resolver_test.go index f395d67215a..417ef907ed0 100644 --- a/contractcourt/htlc_success_resolver_test.go +++ b/contractcourt/htlc_success_resolver_test.go @@ -616,11 +616,11 @@ func runFromCheckpoint(t *testing.T, ctx *htlcResolverTestContext, var resolved, incubating bool if h, ok := resolver.(*htlcSuccessResolver); ok { - resolved = h.resolved + resolved = h.resolved.Load() incubating = h.outputIncubating } if h, ok := resolver.(*htlcTimeoutResolver); ok { - resolved = h.resolved + resolved = h.resolved.Load() incubating = h.outputIncubating } diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index eae52255f2b..125d205477a 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -38,9 +38,6 @@ type htlcTimeoutResolver struct { // incubator (utxo nursery). outputIncubating bool - // resolved reflects if the contract has been fully resolved or not. - resolved bool - // broadcastHeight is the height that the original contract was // broadcast to the main-chain at. We'll use this value to bound any // historical queries to the chain for spends/confirmations. @@ -238,7 +235,7 @@ func (h *htlcTimeoutResolver) claimCleanUp( }); err != nil { return err } - h.resolved = true + h.markResolved() // Checkpoint our resolver with a report which reflects the preimage // claim by the remote party. @@ -424,7 +421,7 @@ func checkSizeAndIndex(witness wire.TxWitness, size, index int) bool { // NOTE: Part of the ContractResolver interface. func (h *htlcTimeoutResolver) Resolve() (ContractResolver, error) { // If we're already resolved, then we can exit early. - if h.resolved { + if h.IsResolved() { h.log.Errorf("already resolved") return nil, nil } @@ -622,14 +619,6 @@ func (h *htlcTimeoutResolver) Stop() { close(h.quit) } -// IsResolved returns true if the stored state in the resolve is fully -// resolved. In this case the target output can be forgotten. -// -// NOTE: Part of the ContractResolver interface. -func (h *htlcTimeoutResolver) IsResolved() bool { - return h.resolved -} - // report returns a report on the resolution state of the contract. func (h *htlcTimeoutResolver) report() *ContractReport { // If we have a SignedTimeoutTx but no SignDetails, this is a local @@ -689,7 +678,7 @@ func (h *htlcTimeoutResolver) Encode(w io.Writer) error { if err := binary.Write(w, endian, h.outputIncubating); err != nil { return err } - if err := binary.Write(w, endian, h.resolved); err != nil { + if err := binary.Write(w, endian, h.IsResolved()); err != nil { return err } if err := binary.Write(w, endian, h.broadcastHeight); err != nil { @@ -730,9 +719,15 @@ func newTimeoutResolverFromReader(r io.Reader, resCfg ResolverConfig) ( if err := binary.Read(r, endian, &h.outputIncubating); err != nil { return nil, err } - if err := binary.Read(r, endian, &h.resolved); err != nil { + + var resolved bool + if err := binary.Read(r, endian, &resolved); err != nil { return nil, err } + if resolved { + h.markResolved() + } + if err := binary.Read(r, endian, &h.broadcastHeight); err != nil { return nil, err } @@ -1149,7 +1144,7 @@ func (h *htlcTimeoutResolver) checkpointClaim( } // Finally, we checkpoint the resolver with our report(s). - h.resolved = true + h.markResolved() return h.Checkpoint(h, report) } @@ -1285,7 +1280,7 @@ func (h *htlcTimeoutResolver) Launch() error { switch { // If we're already resolved, then we can exit early. - case h.resolved: + case h.IsResolved(): h.log.Errorf("already resolved") return nil diff --git a/contractcourt/htlc_timeout_resolver_test.go b/contractcourt/htlc_timeout_resolver_test.go index 89f334aa83b..017d3d38861 100644 --- a/contractcourt/htlc_timeout_resolver_test.go +++ b/contractcourt/htlc_timeout_resolver_test.go @@ -532,7 +532,7 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { wg.Wait() // Finally, the resolver should be marked as resolved. - if !resolver.resolved { + if !resolver.resolved.Load() { t.Fatalf("resolver should be marked as resolved") } } From b6c4dd9ad04e66448bf27b0eddda814f3fd38fc6 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 11 Jul 2024 16:19:01 +0800 Subject: [PATCH 17/19] contractcourt: fix concurrent access to `launched` --- contractcourt/anchor_resolver.go | 4 ++-- contractcourt/breach_resolver.go | 4 ++-- contractcourt/commit_sweep_resolver.go | 4 ++-- contractcourt/contract_resolver.go | 17 +++++++++++++++-- contractcourt/htlc_incoming_contest_resolver.go | 2 +- contractcourt/htlc_outgoing_contest_resolver.go | 2 +- contractcourt/htlc_success_resolver.go | 4 ++-- contractcourt/htlc_timeout_resolver.go | 4 ++-- 8 files changed, 27 insertions(+), 14 deletions(-) diff --git a/contractcourt/anchor_resolver.go b/contractcourt/anchor_resolver.go index b70e28323d4..f0f6e2f5a91 100644 --- a/contractcourt/anchor_resolver.go +++ b/contractcourt/anchor_resolver.go @@ -178,13 +178,13 @@ var _ ContractResolver = (*anchorResolver)(nil) // Launch offers the anchor output to the sweeper. func (c *anchorResolver) Launch() error { - if c.launched { + if c.isLaunched() { c.log.Tracef("already launched") return nil } c.log.Debugf("launching resolver...") - c.launched = true + c.markLaunched() // If we're already resolved, then we can exit early. if c.IsResolved() { diff --git a/contractcourt/breach_resolver.go b/contractcourt/breach_resolver.go index 66a974e67f4..f341128006c 100644 --- a/contractcourt/breach_resolver.go +++ b/contractcourt/breach_resolver.go @@ -130,13 +130,13 @@ var _ ContractResolver = (*breachResolver)(nil) // // TODO(yy): implement it once the outputs are offered to the sweeper. func (b *breachResolver) Launch() error { - if b.launched { + if b.isLaunched() { b.log.Tracef("already launched") return nil } b.log.Debugf("launching resolver...") - b.launched = true + b.markLaunched() return nil } diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go index 84efdeb067b..55ee08e5d83 100644 --- a/contractcourt/commit_sweep_resolver.go +++ b/contractcourt/commit_sweep_resolver.go @@ -368,13 +368,13 @@ var _ reportingContractResolver = (*commitSweepResolver)(nil) // Launch constructs a commit input and offers it to the sweeper. func (c *commitSweepResolver) Launch() error { - if c.launched { + if c.isLaunched() { c.log.Tracef("already launched") return nil } c.log.Debugf("launching resolver...") - c.launched = true + c.markLaunched() // If we're already resolved, then we can exit early. if c.IsResolved() { diff --git a/contractcourt/contract_resolver.go b/contractcourt/contract_resolver.go index 5311d466aee..293c8496623 100644 --- a/contractcourt/contract_resolver.go +++ b/contractcourt/contract_resolver.go @@ -118,8 +118,11 @@ type contractResolverKit struct { sweepResultChan chan sweep.Result // launched specifies whether the resolver has been launched. Calling - // `Launch` will be a no-op if this is true. - launched bool + // `Launch` will be a no-op if this is true. This value is not saved to + // db, as it's fine to relaunch a resolver after a restart. It's only + // used to avoid resending requests to the sweeper when a new blockbeat + // is received. + launched atomic.Bool // resolved reflects if the contract has been fully resolved or not. resolved atomic.Bool @@ -154,6 +157,16 @@ func (r *contractResolverKit) markResolved() { r.resolved.Store(true) } +// isLaunched returns true if the resolver has been launched. +func (r *contractResolverKit) isLaunched() bool { + return r.launched.Load() +} + +// markLaunched marks the resolver as launched. +func (r *contractResolverKit) markLaunched() { + r.launched.Store(true) +} + var ( // errResolverShuttingDown is returned when the resolver stops // progressing because it received the quit signal. diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index 31790102253..bc5948487df 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/contractcourt/htlc_incoming_contest_resolver.go @@ -83,7 +83,7 @@ func (h *htlcIncomingContestResolver) processFinalHtlcFail() error { func (h *htlcIncomingContestResolver) Launch() error { // NOTE: we don't mark this resolver as launched as the inner resolver // will set it when it's launched. - if h.launched { + if h.isLaunched() { h.log.Tracef("already launched") return nil } diff --git a/contractcourt/htlc_outgoing_contest_resolver.go b/contractcourt/htlc_outgoing_contest_resolver.go index 8763dfb27eb..19574dee27e 100644 --- a/contractcourt/htlc_outgoing_contest_resolver.go +++ b/contractcourt/htlc_outgoing_contest_resolver.go @@ -40,7 +40,7 @@ func newOutgoingContestResolver(res lnwallet.OutgoingHtlcResolution, func (h *htlcOutgoingContestResolver) Launch() error { // NOTE: we don't mark this resolver as launched as the inner resolver // will set it when it's launched. - if h.launched { + if h.isLaunched() { h.log.Tracef("already launched") return nil } diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index b4c6db2b916..a4d27ba4e81 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -730,13 +730,13 @@ func (h *htlcSuccessResolver) resolveSuccessTxOutput(op wire.OutPoint) error { // Launch creates an input based on the details of the incoming htlc resolution // and offers it to the sweeper. func (h *htlcSuccessResolver) Launch() error { - if h.launched { + if h.isLaunched() { h.log.Tracef("already launched") return nil } h.log.Debugf("launching resolver...") - h.launched = true + h.markLaunched() switch { // If we're already resolved, then we can exit early. diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index 125d205477a..872a3da92b5 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -1270,13 +1270,13 @@ func (h *htlcTimeoutResolver) resolveTimeoutTxOutput(op wire.OutPoint) error { // Launch creates an input based on the details of the outgoing htlc resolution // and offers it to the sweeper. func (h *htlcTimeoutResolver) Launch() error { - if h.launched { + if h.isLaunched() { h.log.Tracef("already launched") return nil } h.log.Debugf("launching resolver...") - h.launched = true + h.markLaunched() switch { // If we're already resolved, then we can exit early. From b3cec2c42bbe9a20460336a689aa3b8f6b2e9665 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Wed, 26 Jun 2024 07:41:51 +0800 Subject: [PATCH 18/19] contractcourt: break `launchResolvers` into two steps In this commit, we break the old `launchResolvers` into two steps - step one is to launch the resolvers synchronously, and step two is to actually waiting for the resolvers to be resolved. This is critical as in the following commit we will require the resolvers to be launched at the same blockbeat when a force close event is sent by the chain watcher. --- contractcourt/channel_arbitrator.go | 75 +++++++++++++++++++-- contractcourt/channel_arbitrator_test.go | 54 ++++++++------- contractcourt/contract_resolver.go | 11 +++ contractcourt/htlc_success_resolver_test.go | 3 + 4 files changed, 113 insertions(+), 30 deletions(-) diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index ca70f733ff5..9b554cb6487 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -816,7 +816,7 @@ func (c *ChannelArbitrator) relaunchResolvers(commitSet *CommitSet, // TODO(roasbeef): this isn't re-launched? } - c.launchResolvers(unresolvedContracts) + c.resolveContracts(unresolvedContracts) return nil } @@ -1355,7 +1355,7 @@ func (c *ChannelArbitrator) stateStep( // Finally, we'll launch all the required contract resolvers. // Once they're all resolved, we're no longer needed. - c.launchResolvers(resolvers) + c.resolveContracts(resolvers) nextState = StateWaitingFullResolution @@ -1578,18 +1578,75 @@ func (c *ChannelArbitrator) findCommitmentDeadlineAndValue(heightHint uint32, return fn.Some(int32(deadline)), valueLeft, nil } -// launchResolvers updates the activeResolvers list and starts the resolvers. -func (c *ChannelArbitrator) launchResolvers(resolvers []ContractResolver) { +// resolveContracts updates the activeResolvers list and starts to resolve each +// contract concurrently, and launches them. +func (c *ChannelArbitrator) resolveContracts(resolvers []ContractResolver) { c.activeResolversLock.Lock() c.activeResolvers = resolvers c.activeResolversLock.Unlock() + // Launch all resolvers. + c.launchResolvers() + for _, contract := range resolvers { c.wg.Add(1) go c.resolveContract(contract) } } +// launchResolvers launches all the active resolvers concurrently. +func (c *ChannelArbitrator) launchResolvers() { + c.activeResolversLock.Lock() + resolvers := c.activeResolvers + c.activeResolversLock.Unlock() + + // errChans is a map of channels that will be used to receive errors + // returned from launching the resolvers. + errChans := make(map[ContractResolver]chan error, len(resolvers)) + + // Launch each resolver in goroutines. + for _, r := range resolvers { + // If the contract is already resolved, there's no need to + // launch it again. + if r.IsResolved() { + log.Debugf("ChannelArbitrator(%v): skipping resolver "+ + "%T as it's already resolved", c.cfg.ChanPoint, + r) + + continue + } + + // Create a signal chan. + errChan := make(chan error, 1) + errChans[r] = errChan + + go func() { + err := r.Launch() + errChan <- err + }() + } + + // Wait for all resolvers to finish launching. + for r, errChan := range errChans { + select { + case err := <-errChan: + if err == nil { + continue + } + + log.Errorf("ChannelArbitrator(%v): unable to launch "+ + "contract resolver(%T): %v", c.cfg.ChanPoint, r, + err) + + case <-c.quit: + log.Debugf("ChannelArbitrator quit signal received, " + + "exit launchResolvers") + + return + } + } +} + // advanceState is the main driver of our state machine. This method is an // iterative function which repeatedly attempts to advance the internal state // of the channel arbitrator. The state will be advanced until we reach a @@ -2628,6 +2685,13 @@ func (c *ChannelArbitrator) resolveContract(currentContract ContractResolver) { // loop. currentContract = nextContract + // Launch the new contract. + err = currentContract.Launch() + if err != nil { + log.Errorf("Failed to launch %T: %v", + currentContract, err) + } + // If this contract is actually fully resolved, then // we'll mark it as such within the database. case currentContract.IsResolved(): @@ -3144,6 +3208,9 @@ func (c *ChannelArbitrator) handleBlockbeat(beat chainio.Blockbeat) error { } } + // Launch all active resolvers when a new blockbeat is received. + c.launchResolvers() + return nil } diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index d1ef2993d50..827e10b5c7d 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -13,6 +13,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" + "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/chainio" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" @@ -1101,12 +1102,7 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { } // Send a notification that the expiry height has been reached. - // - // TODO(yy): remove the EpochChan and use the blockbeat below once - // resolvers are hooked with the blockbeat. oldNotifier.EpochChan <- &chainntnfs.BlockEpoch{Height: 10} - // beat := chainio.NewBlockbeatFromHeight(10) - // chanArb.BlockbeatChan <- beat // htlcOutgoingContestResolver is now transforming into a // htlcTimeoutResolver and should send the contract off for incubation. @@ -1149,8 +1145,12 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { default: } - // Notify resolver that the second level transaction is spent. - oldNotifier.SpendChan <- &chainntnfs.SpendDetail{SpendingTx: closeTx} + // Notify resolver that the output of the timeout tx has been spent. + oldNotifier.SpendChan <- &chainntnfs.SpendDetail{ + SpendingTx: closeTx, + SpentOutPoint: &wire.OutPoint{}, + SpenderTxHash: &closeTxid, + } // At this point channel should be marked as resolved. chanArbCtxNew.AssertStateTransitions(StateFullyResolved) @@ -2830,27 +2830,28 @@ func TestChannelArbitratorAnchors(t *testing.T) { } chanArb.UpdateContractSignals(signals) - // Set current block height. - beat = newBeatFromHeight(int32(heightHint)) - chanArbCtx.chanArb.BlockbeatChan <- beat - htlcAmt := lnwire.MilliSatoshi(1_000_000) // Create testing HTLCs. - deadlineDelta := uint32(10) - deadlinePreimageDelta := deadlineDelta + 2 + spendingHeight := uint32(beat.Height()) + deadlineDelta := uint32(100) + + deadlinePreimageDelta := deadlineDelta htlcWithPreimage := channeldb.HTLC{ - HtlcIndex: 99, - RefundTimeout: heightHint + deadlinePreimageDelta, + HtlcIndex: 99, + // RefundTimeout is 101. + RefundTimeout: spendingHeight + deadlinePreimageDelta, RHash: rHash, Incoming: true, Amt: htlcAmt, } + expectedDeadline := deadlineDelta/2 + spendingHeight - deadlineHTLCdelta := deadlineDelta + 3 + deadlineHTLCdelta := deadlineDelta + 40 htlc := channeldb.HTLC{ - HtlcIndex: 100, - RefundTimeout: heightHint + deadlineHTLCdelta, + HtlcIndex: 100, + // RefundTimeout is 141. + RefundTimeout: spendingHeight + deadlineHTLCdelta, Amt: htlcAmt, } @@ -2935,7 +2936,9 @@ func TestChannelArbitratorAnchors(t *testing.T) { //nolint:ll chanArb.cfg.ChainEvents.LocalUnilateralClosure <- &LocalUnilateralCloseInfo{ - SpendDetail: &chainntnfs.SpendDetail{}, + SpendDetail: &chainntnfs.SpendDetail{ + SpendingHeight: int32(spendingHeight), + }, LocalForceCloseSummary: &lnwallet.LocalForceCloseSummary{ CloseTx: closeTx, ContractResolutions: fn.Some(lnwallet.ContractResolutions{ @@ -2999,16 +3002,14 @@ func TestChannelArbitratorAnchors(t *testing.T) { // to htlcWithPreimage's CLTV. require.Equal(t, 2, len(chanArbCtx.sweeper.deadlines)) require.EqualValues(t, - heightHint+deadlinePreimageDelta/2, + expectedDeadline, chanArbCtx.sweeper.deadlines[0], "want %d, got %d", - heightHint+deadlinePreimageDelta/2, - chanArbCtx.sweeper.deadlines[0], + expectedDeadline, chanArbCtx.sweeper.deadlines[0], ) require.EqualValues(t, - heightHint+deadlinePreimageDelta/2, + expectedDeadline, chanArbCtx.sweeper.deadlines[1], "want %d, got %d", - heightHint+deadlinePreimageDelta/2, - chanArbCtx.sweeper.deadlines[1], + expectedDeadline, chanArbCtx.sweeper.deadlines[1], ) } @@ -3159,7 +3160,8 @@ func assertResolverReport(t *testing.T, reports chan *channeldb.ResolverReport, select { case report := <-reports: if !reflect.DeepEqual(report, expected) { - t.Fatalf("expected: %v, got: %v", expected, report) + t.Fatalf("expected: %v, got: %v", spew.Sdump(expected), + spew.Sdump(report)) } case <-time.After(defaultTimeout): diff --git a/contractcourt/contract_resolver.go b/contractcourt/contract_resolver.go index 293c8496623..d11bd2f597a 100644 --- a/contractcourt/contract_resolver.go +++ b/contractcourt/contract_resolver.go @@ -37,6 +37,17 @@ type ContractResolver interface { // resides within. ResolverKey() []byte + // Launch starts the resolver by constructing an input and offering it + // to the sweeper. Once offered, it's expected to monitor the sweeping + // result in a goroutine invoked by calling Resolve. + // + // NOTE: We can call `Resolve` inside a goroutine at the end of this + // method to avoid calling it in the ChannelArbitrator. However, there + // are some DB-related operations such as SwapContract/ResolveContract + // which need to be done inside the resolvers instead, which needs a + // deeper refactoring. + Launch() error + // Resolve instructs the contract resolver to resolve the output // on-chain. Once the output has been *fully* resolved, the function // should return immediately with a nil ContractResolver value for the diff --git a/contractcourt/htlc_success_resolver_test.go b/contractcourt/htlc_success_resolver_test.go index 417ef907ed0..fe6ee1ad0ea 100644 --- a/contractcourt/htlc_success_resolver_test.go +++ b/contractcourt/htlc_success_resolver_test.go @@ -146,6 +146,9 @@ func (i *htlcResolverTestContext) resolve() { i.resolverResultChan = make(chan resolveResult, 1) go func() { + err := i.resolver.Launch() + require.NoError(i.t, err) + nextResolver, err := i.resolver.Resolve() i.resolverResultChan <- resolveResult{ nextResolver: nextResolver, From 7751b1bc7cc72d0c6340679aedd662109a46a1e6 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 25 Nov 2024 13:55:11 +0800 Subject: [PATCH 19/19] contractcourt: offer outgoing htlc one block earlier before its expiry We need to offer the outgoing htlc one block earlier to make sure when the expiry height hits, the sweeper will not miss sweeping it in the same block. This also means the outgoing contest resolver now only does one thing - watch for preimage spend till height expiry-1, which can easily be moved into the timeout resolver instead in the future. --- contractcourt/htlc_outgoing_contest_resolver.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/contractcourt/htlc_outgoing_contest_resolver.go b/contractcourt/htlc_outgoing_contest_resolver.go index 19574dee27e..b66a3fdf0b3 100644 --- a/contractcourt/htlc_outgoing_contest_resolver.go +++ b/contractcourt/htlc_outgoing_contest_resolver.go @@ -155,7 +155,14 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { // fail. newHeight := uint32(newBlock.Height) expiry := h.htlcResolution.Expiry - if newHeight >= expiry { + + // Check if the expiry height is about to be reached. + // We offer this HTLC one block earlier to make sure + // when the next block arrives, the sweeper will pick + // up this input and sweep it immediately. The sweeper + // will handle the waiting for the one last block till + // expiry. + if newHeight >= expiry-1 { h.log.Infof("HTLC about to expire "+ "(height=%v, expiry=%v), transforming "+ "into timeout resolver", h,