diff --git a/docs/release-notes/release-notes-0.17.1.md b/docs/release-notes/release-notes-0.17.1.md index 2caf937c6..54238d898 100644 --- a/docs/release-notes/release-notes-0.17.1.md +++ b/docs/release-notes/release-notes-0.17.1.md @@ -23,6 +23,9 @@ bit when sending `update_fail_malformed_htlc`. This avoids a force close with other implementations. +* A bug that would cause taproot channels to sometimes not display as active + [has been fixed](https://github.com/lightningnetwork/lnd/pull/8104). + # New Features ## Functional Enhancements @@ -67,4 +70,5 @@ # Contributors (Alphabetical Order) * Eugene Siegel +* Olaoluwa Osuntokun * Yong Yu diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 0674d646b..9b1499bd8 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -462,6 +462,10 @@ var allTestCases = []*lntest.TestCase{ Name: "taproot", TestFunc: testTaproot, }, + { + Name: "simple taproot channel activation", + TestFunc: testSimpleTaprootChannelActivation, + }, { Name: "wallet import account", TestFunc: testWalletImportAccount, diff --git a/itest/lnd_open_channel_test.go b/itest/lnd_open_channel_test.go index 674d9a62b..d8cf66485 100644 --- a/itest/lnd_open_channel_test.go +++ b/itest/lnd_open_channel_test.go @@ -768,3 +768,57 @@ func testFundingExpiryBlocksOnPending(ht *lntest.HarnessTest) { chanPoint := lntest.ChanPointFromPendingUpdate(update) ht.CloseChannel(alice, chanPoint) } + +// testSimpleTaprootChannelActivation ensures that a simple taproot channel is +// active if the initiator disconnects and reconnects in between channel opening +// and channel confirmation. +func testSimpleTaprootChannelActivation(ht *lntest.HarnessTest) { + simpleTaprootChanArgs := lntest.NodeArgsForCommitType( + lnrpc.CommitmentType_SIMPLE_TAPROOT, + ) + + // Make the new set of participants. + alice := ht.NewNode("alice", simpleTaprootChanArgs) + defer ht.Shutdown(alice) + bob := ht.NewNode("bob", simpleTaprootChanArgs) + defer ht.Shutdown(bob) + + ht.FundCoins(btcutil.SatoshiPerBitcoin, alice) + + // Make sure Alice and Bob are connected. + ht.EnsureConnected(alice, bob) + + // Create simple taproot channel opening parameters. + params := lntest.OpenChannelParams{ + FundMax: true, + CommitmentType: lnrpc.CommitmentType_SIMPLE_TAPROOT, + Private: true, + } + + // Alice opens the channel to Bob. + pendingChan := ht.OpenChannelAssertPending(alice, bob, params) + + // We'll create the channel point to be able to close the channel once + // our test is done. + chanPoint := &lnrpc.ChannelPoint{ + FundingTxid: &lnrpc.ChannelPoint_FundingTxidBytes{ + FundingTxidBytes: pendingChan.Txid, + }, + OutputIndex: pendingChan.OutputIndex, + } + + // We disconnect and reconnect Alice and Bob before the channel is + // confirmed. Our expectation is that the channel is active once the + // channel is confirmed. + ht.DisconnectNodes(alice, bob) + ht.EnsureConnected(alice, bob) + + // Mine six blocks to confirm the channel funding transaction. + ht.MineBlocksAndAssertNumTxes(6, 1) + + // Verify that Alice sees an active channel to Bob. + ht.AssertChannelActive(alice, chanPoint) + + // Our test is done and Alice closes her channel to Bob. + ht.CloseChannel(alice, chanPoint) +} diff --git a/lntest/harness_assertion.go b/lntest/harness_assertion.go index 76ff292ef..6ead7d277 100644 --- a/lntest/harness_assertion.go +++ b/lntest/harness_assertion.go @@ -352,6 +352,31 @@ func (h *HarnessTest) AssertTopologyChannelOpen(hn *node.HarnessNode, func (h *HarnessTest) AssertChannelExists(hn *node.HarnessNode, cp *lnrpc.ChannelPoint) *lnrpc.Channel { + return h.assertChannelStatus(hn, cp, true) +} + +// AssertChannelActive checks if a channel identified by the specified channel +// point is active. +func (h *HarnessTest) AssertChannelActive(hn *node.HarnessNode, + cp *lnrpc.ChannelPoint) *lnrpc.Channel { + + return h.assertChannelStatus(hn, cp, true) +} + +// AssertChannelInactive checks if a channel identified by the specified channel +// point is inactive. +func (h *HarnessTest) AssertChannelInactive(hn *node.HarnessNode, + cp *lnrpc.ChannelPoint) *lnrpc.Channel { + + return h.assertChannelStatus(hn, cp, false) +} + +// assertChannelStatus asserts that a channel identified by the specified +// channel point exists from the point-of-view of the node and that it is either +// active or inactive depending on the value of the active parameter. +func (h *HarnessTest) assertChannelStatus(hn *node.HarnessNode, + cp *lnrpc.ChannelPoint, active bool) *lnrpc.Channel { + var ( channel *lnrpc.Channel err error @@ -364,11 +389,12 @@ func (h *HarnessTest) AssertChannelExists(hn *node.HarnessNode, } // Check whether the channel is active, exit early if it is. - if channel.Active { + if channel.Active == active { return nil } - return fmt.Errorf("channel point not active") + return fmt.Errorf("expected channel_active=%v, got %v", + active, channel.Active) }, DefaultTimeout) require.NoErrorf(h, err, "%s: timeout checking for channel point: %v", diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 5ca771cb1..faef703a1 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1344,6 +1344,9 @@ type LightningChannel struct { // fundingOutput is the funding output (script+value). fundingOutput wire.TxOut + // opts is the set of options that channel was initialized with. + opts *channelOpts + sync.RWMutex } @@ -1351,6 +1354,14 @@ type LightningChannel struct { // is created. type ChannelOpt func(*channelOpts) +// channelOpts is the set of options used to create a new channel. +type channelOpts struct { + localNonce *musig2.Nonces + remoteNonce *musig2.Nonces + + skipNonceInit bool +} + // WithLocalMusigNonces is used to bind an existing verification/local nonce to // a new channel. func WithLocalMusigNonces(nonce *musig2.Nonces) ChannelOpt { @@ -1367,10 +1378,15 @@ func WithRemoteMusigNonces(nonces *musig2.Nonces) ChannelOpt { } } -// channelOpts is the set of options used to create a new channel. -type channelOpts struct { - localNonce *musig2.Nonces - remoteNonce *musig2.Nonces +// WithSkipNonceInit is used to modify the way nonces are handled during +// channel initialization for taproot channels. If this option is specified, +// then when we receive the chan reest message from the remote party, we won't +// modify our nonce state. This is needed if we create a channel, get a channel +// ready message, then also get the chan reest message after that. +func WithSkipNonceInit() ChannelOpt { + return func(o *channelOpts) { + o.skipNonceInit = true + } } // defaultChannelOpts returns the set of default options for a new channel. @@ -1429,6 +1445,7 @@ func NewLightningChannel(signer input.Signer, RemoteFundingKey: state.RemoteChanCfg.MultiSigKey.PubKey, taprootNonceProducer: taprootNonceProducer, log: build.NewPrefixLog(logPrefix, walletLog), + opts: opts, } switch { @@ -4263,6 +4280,12 @@ func (lc *LightningChannel) ProcessChanSyncMsg( "not sent") case lc.channelState.ChanType.IsTaproot() && msg.LocalNonce != nil: + if lc.opts.skipNonceInit { + // Don't call InitRemoteMusigNonces if we have already + // done so. + break + } + err := lc.InitRemoteMusigNonces(&musig2.Nonces{ PubNonce: *msg.LocalNonce, }) @@ -8763,6 +8786,9 @@ func (lc *LightningChannel) InitRemoteMusigNonces(remoteNonce *musig2.Nonces, lc.pendingVerificationNonce = nil + lc.opts.localNonce = nil + lc.opts.remoteNonce = nil + return nil } diff --git a/peer/brontide.go b/peer/brontide.go index 86aa99aae..9453be39f 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -3824,17 +3824,6 @@ func (p *Brontide) addActiveChannel(c *lnpeer.NewChannel) error { chanPoint := &c.FundingOutpoint chanID := lnwire.NewChanIDFromOutPoint(chanPoint) - // If not already active, we'll add this channel to the set of active - // channels, so we can look it up later easily according to its channel - // ID. - lnChan, err := lnwallet.NewLightningChannel( - p.cfg.Signer, c.OpenChannel, - p.cfg.SigPool, c.ChanOpts..., - ) - if err != nil { - return fmt.Errorf("unable to create LightningChannel: %w", err) - } - // If we've reached this point, there are two possible scenarios. If // the channel was in the active channels map as nil, then it was // loaded from disk and we need to send reestablish. Else, it was not @@ -3842,6 +3831,24 @@ func (p *Brontide) addActiveChannel(c *lnpeer.NewChannel) error { // fresh channel. shouldReestablish := p.isLoadedFromDisk(chanID) + chanOpts := c.ChanOpts + if shouldReestablish { + // If we have to do the reestablish dance for this channel, + // ensure that we don't try to call InitRemoteMusigNonces twice + // by calling SkipNonceInit. + chanOpts = append(chanOpts, lnwallet.WithSkipNonceInit()) + } + + // If not already active, we'll add this channel to the set of active + // channels, so we can look it up later easily according to its channel + // ID. + lnChan, err := lnwallet.NewLightningChannel( + p.cfg.Signer, c.OpenChannel, p.cfg.SigPool, chanOpts..., + ) + if err != nil { + return fmt.Errorf("unable to create LightningChannel: %w", err) + } + // Store the channel in the activeChannels map. p.activeChannels.Store(chanID, lnChan)