From 31d45757a456a9ff95e09e19f40a60dd86da99ec Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Thu, 27 Jun 2024 14:20:28 -0700 Subject: [PATCH 1/2] contractcourt: properly detect RBF coop close transactions This commit fixes an issue where we did not properly detect and therefore record the coop close transaction if it used the newer RBF coop close v2 scheme. This only affects coop closes of taproot channels today. --- contractcourt/chain_watcher.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index b17b40aca..f1733ad1f 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -12,6 +12,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/mempool" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/davecgh/go-spew/spew" @@ -689,7 +690,10 @@ func (c *chainWatcher) closeObserver(spendNtfn *chainntnfs.SpendEvent) { // sequence number that's finalized. This won't happen with // regular commitment transactions due to the state hint // encoding scheme. - if commitTxBroadcast.TxIn[0].Sequence == wire.MaxTxInSequenceNum { + switch commitTxBroadcast.TxIn[0].Sequence { + case wire.MaxTxInSequenceNum: + fallthrough + case mempool.MaxRBFSequence: // TODO(roasbeef): rare but possible, need itest case // for err := c.dispatchCooperativeClose(commitSpend) From 09f5e08d32e650dbbf9bc76acc7f25013b465518 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Fri, 28 Jun 2024 12:34:58 -0700 Subject: [PATCH 2/2] contractcourt: Fix heuristic for identifying STC commit broadcaster. This commit fixes the heuristic we use for identifying the party that broadcast a Simple Taproot Channel commitment transaction. Prior to this change we checked if the last script element was an OP_DROP. However, both the local and remote commitment outputs have an OP_DROP at the end. The new approach checks the resolver's SignDescriptor and compares that key to the keys in the channel's local ChannelConfig. If the key is the delay key, we know that it is our commitment transaction. --- contractcourt/commit_sweep_resolver.go | 32 ++++++++++++++++++-------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go index 296ea38e5..f2392192e 100644 --- a/contractcourt/commit_sweep_resolver.go +++ b/contractcourt/commit_sweep_resolver.go @@ -24,6 +24,11 @@ import ( // version of the commitment transaction. We can sweep this output immediately, // as it doesn't have a time-lock delay. type commitSweepResolver struct { + // localChanCfg is used to provide the resolver with the keys required + // to identify whether the commitment transaction was broadcast by the + // local or remote party. + localChanCfg channeldb.ChannelConfig + // commitResolution contains all data required to successfully sweep // this HTLC on-chain. commitResolution lnwallet.CommitOutputResolution @@ -252,18 +257,26 @@ func (c *commitSweepResolver) Resolve(_ bool) (ContractResolver, error) { signDesc = c.commitResolution.SelfOutputSignDesc ) + switch { // For taproot channels, we'll know if this is the local commit based - // on the witness script. For local channels, the witness script has an - // OP_DROP value. - // - // TODO(roasbeef): revisit this after the script changes - // * otherwise need to base off the key in script or the CSV value - // (script num encode) + // on the timelock value. For remote commitment transactions, the + // witness script has a timelock of 1. case c.chanType.IsTaproot(): - scriptLen := len(signDesc.WitnessScript) - isLocalCommitTx = signDesc.WitnessScript[scriptLen-1] == - txscript.OP_DROP + 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 @@ -446,6 +459,7 @@ func (c *commitSweepResolver) SupplementState(state *channeldb.OpenChannel) { if state.ChanType.HasLeaseExpiration() { c.leaseExpiry = state.ThawHeight } + c.localChanCfg = state.LocalChanCfg c.channelInitiator = state.IsInitiator c.chanType = state.ChanType }