From 34b00b253e67b6d118202fcc71e053a3f368df3f Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 8 Aug 2023 18:23:17 +0800 Subject: [PATCH 1/3] funding: remove unused field `newChanBarriers` This commit removes the occurance of `newChanBarriers` as it's not used anywhere. --- funding/manager.go | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index 2e27051b64..28e4366a35 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -609,12 +609,6 @@ type Manager struct { // requests from a local subsystem within the daemon. fundingRequests chan *InitFundingMsg - // newChanBarriers is a map from a channel ID to a 'barrier' which will - // be signalled once the channel is fully open. This barrier acts as a - // synchronization point for any incoming/outgoing HTLCs before the - // channel has been fully opened. - newChanBarriers *lnutils.SyncMap[lnwire.ChannelID, chan struct{}] - localDiscoverySignals *lnutils.SyncMap[lnwire.ChannelID, chan struct{}] handleChannelReadyBarriers *lnutils.SyncMap[lnwire.ChannelID, struct{}] @@ -671,9 +665,6 @@ func NewFundingManager(cfg Config) (*Manager, error) { signedReservations: make( map[lnwire.ChannelID][32]byte, ), - newChanBarriers: &lnutils.SyncMap[ - lnwire.ChannelID, chan struct{}, - ]{}, fundingMsgs: make( chan *fundingMsg, msgBufferSize, ), @@ -728,7 +719,6 @@ func (f *Manager) start() error { "creating chan barrier", channel.FundingOutpoint) - f.newChanBarriers.Store(chanID, make(chan struct{})) f.localDiscoverySignals.Store( chanID, make(chan struct{}), ) @@ -2192,7 +2182,6 @@ func (f *Manager) continueFundingAccept(resCtx *reservationWithCtx, // fully open. channelID := lnwire.NewChanIDFromOutPoint(outPoint) log.Debugf("Creating chan barrier for ChanID(%v)", channelID) - f.newChanBarriers.Store(channelID, make(chan struct{})) // The next message that advances the funding flow will reference the // channel via its permanent channel ID, so we'll set up this mapping @@ -2362,7 +2351,6 @@ func (f *Manager) handleFundingCreated(peer lnpeer.Peer, // fully open. channelID := lnwire.NewChanIDFromOutPoint(&fundingOut) log.Debugf("Creating chan barrier for ChanID(%v)", channelID) - f.newChanBarriers.Store(channelID, make(chan struct{})) fundingSigned := &lnwire.FundingSigned{} @@ -3870,20 +3858,6 @@ func (f *Manager) handleChannelReady(peer lnpeer.Peer, //nolint:funlen return } - // Launch a defer so we _ensure_ that the channel barrier is properly - // closed even if the target peer is no longer online at this point. - defer func() { - // Close the active channel barrier signaling the readHandler - // that commitment related modifications to this channel can - // now proceed. - chanBarrier, ok := f.newChanBarriers.LoadAndDelete(chanID) - if ok { - log.Tracef("Closing chan barrier for ChanID(%v)", - chanID) - close(chanBarrier) - } - }() - // Before we can add the channel to the peer, we'll need to ensure that // we have an initial forwarding policy set. if err := f.ensureInitialForwardingPolicy(chanID, channel); err != nil { @@ -4889,8 +4863,6 @@ func (f *Manager) cancelReservationCtx(peerKey *btcec.PublicKey, func (f *Manager) deleteReservationCtx(peerKey *btcec.PublicKey, pendingChanID [32]byte) { - // TODO(roasbeef): possibly cancel funding barrier in peer's - // channelManager? peerIDKey := newSerializedKey(peerKey) f.resMtx.Lock() defer f.resMtx.Unlock() From 9c31f93aa3c03c0117c51e86f0f1c4f76321be5f Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 8 Aug 2023 18:35:44 +0800 Subject: [PATCH 2/3] funding: rename method names to clear the funding flow Slightly refactored the names so it's easier to see which side is processing what messages. --- funding/manager.go | 45 ++++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index 28e4366a35..b0246fedf2 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -982,16 +982,16 @@ func (f *Manager) reservationCoordinator() { case fmsg := <-f.fundingMsgs: switch msg := fmsg.msg.(type) { case *lnwire.OpenChannel: - f.handleFundingOpen(fmsg.peer, msg) + f.fundeeProcessOpenChannel(fmsg.peer, msg) case *lnwire.AcceptChannel: - f.handleFundingAccept(fmsg.peer, msg) + f.funderProcessAcceptChannel(fmsg.peer, msg) case *lnwire.FundingCreated: - f.handleFundingCreated(fmsg.peer, msg) + f.fundeeProcessFundingCreated(fmsg.peer, msg) case *lnwire.FundingSigned: - f.handleFundingSigned(fmsg.peer, msg) + f.funderProcessFundingSigned(fmsg.peer, msg) case *lnwire.ChannelReady: f.wg.Add(1) @@ -1301,13 +1301,15 @@ func (f *Manager) ProcessFundingMsg(msg lnwire.Message, peer lnpeer.Peer) { } } -// handleFundingOpen creates an initial 'ChannelReservation' within the wallet, -// then responds to the source peer with an accept channel message progressing -// the funding workflow. +// fundeeProcessOpenChannel creates an initial 'ChannelReservation' within the +// wallet, then responds to the source peer with an accept channel message +// progressing the funding workflow. // // TODO(roasbeef): add error chan to all, let channelManager handle // error+propagate. -func (f *Manager) handleFundingOpen(peer lnpeer.Peer, +// +//nolint:funlen +func (f *Manager) fundeeProcessOpenChannel(peer lnpeer.Peer, msg *lnwire.OpenChannel) { // Check number of pending channels to be smaller than maximum allowed @@ -1830,10 +1832,12 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer, } } -// handleFundingAccept processes a response to the workflow initiation sent by -// the remote peer. This message then queues a message with the funding +// funderProcessAcceptChannel processes a response to the workflow initiation +// sent by the remote peer. This message then queues a message with the funding // outpoint, and a commitment signature to the remote peer. -func (f *Manager) handleFundingAccept(peer lnpeer.Peer, +// +//nolint:funlen +func (f *Manager) funderProcessAcceptChannel(peer lnpeer.Peer, msg *lnwire.AcceptChannel) { pendingChanID := msg.PendingChannelID @@ -2244,11 +2248,14 @@ func (f *Manager) continueFundingAccept(resCtx *reservationWithCtx, } } -// handleFundingCreated progresses the funding workflow when the daemon is on -// the responding side of a single funder workflow. Once this message has been -// processed, a signature is sent to the remote peer allowing it to broadcast -// the funding transaction, progressing the workflow into the final stage. -func (f *Manager) handleFundingCreated(peer lnpeer.Peer, +// fundeeProcessFundingCreated progresses the funding workflow when the daemon +// is on the responding side of a single funder workflow. Once this message has +// been processed, a signature is sent to the remote peer allowing it to +// broadcast the funding transaction, progressing the workflow into the final +// stage. +// +//nolint:funlen +func (f *Manager) fundeeProcessFundingCreated(peer lnpeer.Peer, msg *lnwire.FundingCreated) { peerKey := peer.IdentityKey() @@ -2453,12 +2460,12 @@ func (f *Manager) handleFundingCreated(peer lnpeer.Peer, go f.advanceFundingState(completeChan, pendingChanID, nil) } -// handleFundingSigned processes the final message received in a single funder -// workflow. Once this message is processed, the funding transaction is +// funderProcessFundingSigned processes the final message received in a single +// funder workflow. Once this message is processed, the funding transaction is // broadcast. Once the funding transaction reaches a sufficient number of // confirmations, a message is sent to the responding peer along with a compact // encoding of the location of the channel within the blockchain. -func (f *Manager) handleFundingSigned(peer lnpeer.Peer, +func (f *Manager) funderProcessFundingSigned(peer lnpeer.Peer, msg *lnwire.FundingSigned) { // As the funding signed message will reference the reservation by its From 23c75ae02fba34be2e2c20879cb9e5df00e6889f Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 11 Aug 2023 18:31:41 +0800 Subject: [PATCH 3/3] lnwallet: sanity check empty pending channel ID This commit adds a sanity check to make sure an empty pending channel ID will not be accepted. --- lnwallet/test/test_interface.go | 7 ++++--- lnwallet/wallet.go | 18 +++++++++++++++-- lnwallet/wallet_test.go | 34 +++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 lnwallet/wallet_test.go diff --git a/lnwallet/test/test_interface.go b/lnwallet/test/test_interface.go index 563a975f84..66958278e6 100644 --- a/lnwallet/test/test_interface.go +++ b/lnwallet/test/test_interface.go @@ -773,6 +773,7 @@ func testReservationInitiatorBalanceBelowDustCancel(miner *rpctest.Harness, PushMSat: 0, Flags: lnwire.FFAnnounceChannel, CommitType: lnwallet.CommitmentTypeTweakless, + PendingChanID: [32]byte{1}, } _, err = alice.InitChannelReservation(req) switch { @@ -2744,7 +2745,7 @@ var walletTests = []walletTestCase{ testSingleFunderReservationWorkflow( miner, alice, bob, t, lnwallet.CommitmentTypeLegacy, nil, - nil, [32]byte{}, 0, + nil, [32]byte{1}, 0, ) }, }, @@ -2756,7 +2757,7 @@ var walletTests = []walletTestCase{ testSingleFunderReservationWorkflow( miner, alice, bob, t, lnwallet.CommitmentTypeTweakless, nil, - nil, [32]byte{}, 0, + nil, [32]byte{1}, 0, ) }, }, @@ -2768,7 +2769,7 @@ var walletTests = []walletTestCase{ testSingleFunderReservationWorkflow( miner, alice, bob, t, lnwallet.CommitmentTypeSimpleTaproot, nil, - nil, [32]byte{}, 0, + nil, [32]byte{1}, 0, ) }, }, diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index b0a59d6036..5f0748097e 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -62,6 +62,14 @@ var ( ErrReservedValueInvalidated = errors.New("reserved wallet balance " + "invalidated: transaction would leave insufficient funds for " + "fee bumping anchor channel closings (see debug log for details)") + + // ErrEmptyPendingChanID is returned when an empty value is used for + // the pending channel ID. + ErrEmptyPendingChanID = errors.New("pending channel ID is empty") + + // ErrDuplicatePendingChanID is returned when an existing pending + // channel ID is registered again. + ErrDuplicatePendingChanID = errors.New("duplicate pending channel ID") ) // PsbtFundingRequired is a type that implements the error interface and @@ -670,9 +678,15 @@ func (l *LightningWallet) RegisterFundingIntent(expectedID [32]byte, l.intentMtx.Lock() defer l.intentMtx.Unlock() + // Sanity check the pending channel ID is not empty. + var zeroID [32]byte + if expectedID == zeroID { + return ErrEmptyPendingChanID + } + if _, ok := l.fundingIntents[expectedID]; ok { - return fmt.Errorf("pendingChanID(%x) already has intent "+ - "registered", expectedID[:]) + return fmt.Errorf("%w: already has intent registered: %v", + ErrDuplicatePendingChanID, expectedID[:]) } l.fundingIntents[expectedID] = shimIntent diff --git a/lnwallet/wallet_test.go b/lnwallet/wallet_test.go new file mode 100644 index 0000000000..fc6055ac20 --- /dev/null +++ b/lnwallet/wallet_test.go @@ -0,0 +1,34 @@ +package lnwallet + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestRegisterFundingIntent checks RegisterFundingIntent behaves as expected. +func TestRegisterFundingIntent(t *testing.T) { + t.Parallel() + + require := require.New(t) + + // Create a testing wallet. + lw, err := NewLightningWallet(Config{}) + require.NoError(err) + + // Init an empty testing channel ID. + var testID [32]byte + + // Call the method with empty ID should give us an error. + err = lw.RegisterFundingIntent(testID, nil) + require.ErrorIs(err, ErrEmptyPendingChanID) + + // Modify the ID and call the method again should result in no error. + testID[0] = 1 + err = lw.RegisterFundingIntent(testID, nil) + require.NoError(err) + + // Call the method using the same ID should give us an error. + err = lw.RegisterFundingIntent(testID, nil) + require.ErrorIs(err, ErrDuplicatePendingChanID) +}