From 8077862225501a338e274ecfae4c9d3a10470bdd Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Fri, 9 Aug 2024 12:47:58 -0700 Subject: [PATCH] lnwallet: pack commit chains into Dual This commit packs the LightningChannel's localCommitmentChain and remoteCommitmentChain into a Dual structure for better symmetric access. This will be leveraged by an upcoming commit where we want to more concisely express how we compute the number of pending updates. --- lnwallet/channel.go | 122 ++++++++++++++++++++------------------- lnwallet/channel_test.go | 56 ++++++++++-------- 2 files changed, 93 insertions(+), 85 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index e5cbe6b6a..e55cc8ad2 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -858,15 +858,13 @@ type LightningChannel struct { // accepted. currentHeight uint64 - // remoteCommitChain is the remote node's commitment chain. Any new - // commitments we initiate are added to the tip of this chain. - remoteCommitChain *commitmentChain - - // localCommitChain is our local commitment chain. Any new commitments - // received are added to the tip of this chain. The tail (or lowest - // height) in this chain is our current accepted state, which we are - // able to broadcast safely. - localCommitChain *commitmentChain + // commitChains is a Dual of the local and remote node's commitment + // chains. Any new commitments we initiate are added to Remote chain's + // tip. The Local portion of this field is our local commitment chain. + // Any new commitments received are added to the tip of this chain. + // The tail (or lowest height) in this chain is our current accepted + // state, which we are able to broadcast safely. + commitChains lntypes.Dual[*commitmentChain] channelState *channeldb.OpenChannel @@ -992,14 +990,18 @@ func NewLightningChannel(signer input.Signer, return nil, fmt.Errorf("unable to derive shachain: %w", err) } + commitChains := lntypes.Dual[*commitmentChain]{ + Local: newCommitmentChain(), + Remote: newCommitmentChain(), + } + lc := &LightningChannel{ - Signer: signer, - leafStore: opts.leafStore, - sigPool: sigPool, - currentHeight: localCommit.CommitHeight, - remoteCommitChain: newCommitmentChain(), - localCommitChain: newCommitmentChain(), - channelState: state, + Signer: signer, + leafStore: opts.leafStore, + sigPool: sigPool, + currentHeight: localCommit.CommitHeight, + commitChains: commitChains, + channelState: state, commitBuilder: NewCommitmentBuilder( state, opts.leafStore, ), @@ -1463,10 +1465,10 @@ func (lc *LightningChannel) restoreCommitState( if err != nil { return err } - lc.localCommitChain.addCommitment(localCommit) + lc.commitChains.Local.addCommitment(localCommit) lc.log.Tracef("starting local commitment: %v", - lnutils.SpewLogClosure(lc.localCommitChain.tail())) + lnutils.SpewLogClosure(lc.commitChains.Local.tail())) // We'll also do the same for the remote commitment chain. remoteCommit, err := lc.diskCommitToMemCommit( @@ -1476,10 +1478,10 @@ func (lc *LightningChannel) restoreCommitState( if err != nil { return err } - lc.remoteCommitChain.addCommitment(remoteCommit) + lc.commitChains.Remote.addCommitment(remoteCommit) lc.log.Tracef("starting remote commitment: %v", - lnutils.SpewLogClosure(lc.remoteCommitChain.tail())) + lnutils.SpewLogClosure(lc.commitChains.Remote.tail())) var ( pendingRemoteCommit *commitment @@ -1508,10 +1510,10 @@ func (lc *LightningChannel) restoreCommitState( if err != nil { return err } - lc.remoteCommitChain.addCommitment(pendingRemoteCommit) + lc.commitChains.Remote.addCommitment(pendingRemoteCommit) lc.log.Debugf("pending remote commitment: %v", - lnutils.SpewLogClosure(lc.remoteCommitChain.tip())) + lnutils.SpewLogClosure(lc.commitChains.Remote.tip())) // We'll also re-create the set of commitment keys needed to // fully re-derive the state. @@ -2655,10 +2657,10 @@ func (lc *LightningChannel) fetchCommitmentView( ourLogIndex, ourHtlcIndex, theirLogIndex, theirHtlcIndex uint64, keyRing *CommitmentKeyRing) (*commitment, error) { - commitChain := lc.localCommitChain + commitChain := lc.commitChains.Local dustLimit := lc.channelState.LocalChanCfg.DustLimit if whoseCommitChain.IsRemote() { - commitChain = lc.remoteCommitChain + commitChain = lc.commitChains.Remote dustLimit = lc.channelState.RemoteChanCfg.DustLimit } @@ -3480,10 +3482,10 @@ func (lc *LightningChannel) getUnsignedAckedUpdates() []channeldb.LogUpdate { chanID := lnwire.NewChanIDFromOutPoint(lc.channelState.FundingOutpoint) // Fetch the last remote update that we have signed for. - lastRemoteCommitted := lc.remoteCommitChain.tail().theirMessageIndex + lastRemoteCommitted := lc.commitChains.Remote.tail().theirMessageIndex // Fetch the last remote update that we have acked. - lastLocalCommitted := lc.localCommitChain.tail().theirMessageIndex + lastLocalCommitted := lc.commitChains.Local.tail().theirMessageIndex // We'll now run through the remote update log to locate the items that // we haven't signed for yet. This will be the set of items we need to @@ -3709,9 +3711,9 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, ) error { // First fetch the initial balance before applying any updates. - commitChain := lc.localCommitChain + commitChain := lc.commitChains.Local if whoseCommitChain.IsRemote() { - commitChain = lc.remoteCommitChain + commitChain = lc.commitChains.Remote } ourInitialBalance := commitChain.tip().ourBalance theirInitialBalance := commitChain.tip().theirBalance @@ -3961,7 +3963,7 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { // party, then we're unable to create new states. Each time we create a // new state, we consume a prior revocation point. commitPoint := lc.channelState.RemoteNextRevocation - unacked := lc.remoteCommitChain.hasUnackedCommitment() + unacked := lc.commitChains.Remote.hasUnackedCommitment() if unacked || commitPoint == nil { lc.log.Tracef("waiting for remote ack=%v, nil "+ "RemoteNextRevocation: %v", unacked, commitPoint == nil) @@ -3969,8 +3971,8 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { } // Determine the last update on the remote log that has been locked in. - remoteACKedIndex := lc.localCommitChain.tail().theirMessageIndex - remoteHtlcIndex := lc.localCommitChain.tail().theirHtlcIndex + remoteACKedIndex := lc.commitChains.Local.tail().theirMessageIndex + remoteHtlcIndex := lc.commitChains.Local.tail().theirHtlcIndex // Before we extend this new commitment to the remote commitment chain, // ensure that we aren't violating any of the constraints the remote @@ -4117,7 +4119,7 @@ func (lc *LightningChannel) SignNextCommitment() (*NewCommitState, error) { // Extend the remote commitment chain by one with the addition of our // latest commitment update. - lc.remoteCommitChain.addCommitment(newCommitView) + lc.commitChains.Remote.addCommitment(newCommitView) return &NewCommitState{ CommitSigs: &CommitSigs{ @@ -4233,9 +4235,9 @@ func (lc *LightningChannel) ProcessChanSyncMsg( // Take note of our current commit chain heights before we begin adding // more to them. var ( - localTailHeight = lc.localCommitChain.tail().height - remoteTailHeight = lc.remoteCommitChain.tail().height - remoteTipHeight = lc.remoteCommitChain.tip().height + localTailHeight = lc.commitChains.Local.tail().height + remoteTailHeight = lc.commitChains.Remote.tail().height + remoteTipHeight = lc.commitChains.Remote.tip().height ) // We'll now check that their view of our local chain is up-to-date. @@ -4503,10 +4505,10 @@ func (lc *LightningChannel) computeView(view *HtlcView, dryRunFee fn.Option[chainfee.SatPerKWeight]) (lnwire.MilliSatoshi, lnwire.MilliSatoshi, lntypes.WeightUnit, *HtlcView, error) { - commitChain := lc.localCommitChain + commitChain := lc.commitChains.Local dustLimit := lc.channelState.LocalChanCfg.DustLimit if whoseCommitChain.IsRemote() { - commitChain = lc.remoteCommitChain + commitChain = lc.commitChains.Remote dustLimit = lc.channelState.RemoteChanCfg.DustLimit } @@ -4969,8 +4971,8 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSigs *CommitSigs) error { } // Determine the last update on the local log that has been locked in. - localACKedIndex := lc.remoteCommitChain.tail().ourMessageIndex - localHtlcIndex := lc.remoteCommitChain.tail().ourHtlcIndex + localACKedIndex := lc.commitChains.Remote.tail().ourMessageIndex + localHtlcIndex := lc.commitChains.Remote.tail().ourHtlcIndex // Ensure that this new local update from the remote node respects all // the constraints we specified during initial channel setup. If not, @@ -5207,7 +5209,7 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSigs *CommitSigs) error { localCommitmentView.sig = commitSigs.CommitSig.ToSignatureBytes() //nolint:lll } - lc.localCommitChain.addCommitment(localCommitmentView) + lc.commitChains.Local.addCommitment(localCommitmentView) return nil } @@ -5225,13 +5227,13 @@ func (lc *LightningChannel) IsChannelClean() bool { defer lc.RUnlock() // Check whether we have a pending commitment for our local state. - if lc.localCommitChain.hasUnackedCommitment() { + if lc.commitChains.Local.hasUnackedCommitment() { return false } // Check whether our counterparty has a pending commitment for their // state. - if lc.remoteCommitChain.hasUnackedCommitment() { + if lc.commitChains.Remote.hasUnackedCommitment() { return false } @@ -5284,8 +5286,8 @@ func (lc *LightningChannel) oweCommitment(issuer lntypes.ChannelParty) bool { var ( remoteUpdatesPending, localUpdatesPending bool - lastLocalCommit = lc.localCommitChain.tip() - lastRemoteCommit = lc.remoteCommitChain.tip() + lastLocalCommit = lc.commitChains.Local.tip() + lastRemoteCommit = lc.commitChains.Remote.tip() perspective string ) @@ -5337,7 +5339,7 @@ func (lc *LightningChannel) PendingLocalUpdateCount() uint64 { lc.RLock() defer lc.RUnlock() - lastRemoteCommit := lc.remoteCommitChain.tip() + lastRemoteCommit := lc.commitChains.Remote.tip() return lc.localUpdateLog.logIndex - lastRemoteCommit.ourMessageIndex } @@ -5362,16 +5364,16 @@ func (lc *LightningChannel) RevokeCurrentCommitment() (*lnwire.RevokeAndAck, } lc.log.Tracef("revoking height=%v, now at height=%v", - lc.localCommitChain.tail().height, + lc.commitChains.Local.tail().height, lc.currentHeight+1) // Advance our tail, as we've revoked our previous state. - lc.localCommitChain.advanceTail() + lc.commitChains.Local.advanceTail() lc.currentHeight++ // Additionally, generate a channel delta for this state transition for // persistent storage. - chainTail := lc.localCommitChain.tail() + chainTail := lc.commitChains.Local.tail() newCommitment := chainTail.toDiskCommit(lntypes.Local) // Get the unsigned acked remotes updates that are currently in memory. @@ -5451,13 +5453,13 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) ( lc.log.Tracef("remote party accepted state transition, revoked height "+ "%v, now at %v", - lc.remoteCommitChain.tail().height, - lc.remoteCommitChain.tail().height+1) + lc.commitChains.Remote.tail().height, + lc.commitChains.Remote.tail().height+1) // Add one to the remote tail since this will be height *after* we write // the revocation to disk, the local height will remain unchanged. - remoteChainTail := lc.remoteCommitChain.tail().height + 1 - localChainTail := lc.localCommitChain.tail().height + remoteChainTail := lc.commitChains.Remote.tail().height + 1 + localChainTail := lc.commitChains.Local.tail().height source := lc.ShortChanID() chanID := lnwire.NewChanIDFromOutPoint(lc.channelState.FundingOutpoint) @@ -5598,8 +5600,8 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) ( // We use the remote commitment chain's tip as it will soon become the tail // once advanceTail is called. - remoteMessageIndex := lc.remoteCommitChain.tip().ourMessageIndex - localMessageIndex := lc.localCommitChain.tail().ourMessageIndex + remoteMessageIndex := lc.commitChains.Remote.tip().ourMessageIndex + localMessageIndex := lc.commitChains.Local.tail().ourMessageIndex localPeerUpdates := lc.unsignedLocalUpdates( remoteMessageIndex, localMessageIndex, chanID, @@ -5660,7 +5662,7 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) ( // Since they revoked the current lowest height in their commitment // chain, we can advance their chain by a single commitment. - lc.remoteCommitChain.advanceTail() + lc.commitChains.Remote.advanceTail() // As we've just completed a new state transition, attempt to see if we // can remove any entries from the update log which have been removed @@ -5921,7 +5923,7 @@ func (lc *LightningChannel) validateAddHtlc(pd *PaymentDescriptor, buffer BufferType) error { // Make sure adding this HTLC won't violate any of the constraints we // must keep on the commitment transactions. - remoteACKedIndex := lc.localCommitChain.tail().theirMessageIndex + remoteACKedIndex := lc.commitChains.Local.tail().theirMessageIndex // First we'll check whether this HTLC can be added to the remote // commitment transaction without violation any of the constraints. @@ -5977,7 +5979,7 @@ func (lc *LightningChannel) ReceiveHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, // PR). } - localACKedIndex := lc.remoteCommitChain.tail().ourMessageIndex + localACKedIndex := lc.commitChains.Remote.tail().ourMessageIndex // Clamp down on the number of HTLC's we can receive by checking the // commitment sanity. @@ -8123,7 +8125,7 @@ func (lc *LightningChannel) availableBalance( // We'll grab the current set of log updates that the remote has // ACKed. - remoteACKedIndex := lc.localCommitChain.tip().theirMessageIndex + remoteACKedIndex := lc.commitChains.Local.tip().theirMessageIndex htlcView := lc.fetchHTLCView(remoteACKedIndex, lc.localUpdateLog.logIndex) @@ -8308,7 +8310,7 @@ func (lc *LightningChannel) validateFeeRate(feePerKw chainfee.SatPerKWeight) err availableBalance, txWeight := lc.availableBalance(AdditionalHtlc) oldFee := lnwire.NewMSatFromSatoshis( - lc.localCommitChain.tip().feePerKw.FeeForWeight(txWeight), + lc.commitChains.Local.tip().feePerKw.FeeForWeight(txWeight), ) // Our base balance is the total amount of satoshis we can commit @@ -8641,7 +8643,7 @@ func (lc *LightningChannel) MaxFeeRate( // exactly why it was introduced to react for sharp fee changes. availableBalance, weight := lc.availableBalance(AdditionalHtlc) - currentFee := lc.localCommitChain.tip().feePerKw.FeeForWeight(weight) + currentFee := lc.commitChains.Local.tip().feePerKw.FeeForWeight(weight) // baseBalance is the maximum amount available for us to spend on fees. baseBalance := availableBalance.ToSatoshis() + currentFee diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index de6bb219c..3b6ee77de 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -1435,12 +1435,12 @@ func TestHTLCDustLimit(t *testing.T) { // while Bob's should not, because the value falls beneath his dust // limit. The amount of the HTLC should be applied to fees in Bob's // commitment transaction. - aliceCommitment := aliceChannel.localCommitChain.tip() + aliceCommitment := aliceChannel.commitChains.Local.tip() if len(aliceCommitment.txn.TxOut) != 3 { t.Fatalf("incorrect # of outputs: expected %v, got %v", 3, len(aliceCommitment.txn.TxOut)) } - bobCommitment := bobChannel.localCommitChain.tip() + bobCommitment := bobChannel.commitChains.Local.tip() if len(bobCommitment.txn.TxOut) != 2 { t.Fatalf("incorrect # of outputs: expected %v, got %v", 2, len(bobCommitment.txn.TxOut)) @@ -1465,7 +1465,7 @@ func TestHTLCDustLimit(t *testing.T) { // At this point, for Alice's commitment chains, the value of the HTLC // should have been added to Alice's balance and TotalSatoshisSent. - commitment := aliceChannel.localCommitChain.tip() + commitment := aliceChannel.commitChains.Local.tip() if len(commitment.txn.TxOut) != 2 { t.Fatalf("incorrect # of outputs: expected %v, got %v", 2, len(commitment.txn.TxOut)) @@ -1698,7 +1698,7 @@ func TestChannelBalanceDustLimit(t *testing.T) { // output for Alice's balance should have been removed as dust, leaving // only a single output that will send the remaining funds in the // channel to Bob. - commitment := bobChannel.localCommitChain.tip() + commitment := bobChannel.commitChains.Local.tip() if len(commitment.txn.TxOut) != 1 { t.Fatalf("incorrect # of outputs: expected %v, got %v", 1, len(commitment.txn.TxOut)) @@ -1816,25 +1816,25 @@ func TestStateUpdatePersistence(t *testing.T) { // After the state transition the fee update is fully locked in, and // should've been removed from both channels' update logs. - if aliceChannel.localCommitChain.tail().feePerKw != fee { + if aliceChannel.commitChains.Local.tail().feePerKw != fee { t.Fatalf("fee not locked in") } - if bobChannel.localCommitChain.tail().feePerKw != fee { + if bobChannel.commitChains.Local.tail().feePerKw != fee { t.Fatalf("fee not locked in") } assertNumLogUpdates(3, 1) // The latest commitment from both sides should have all the HTLCs. - numAliceOutgoing := aliceChannel.localCommitChain.tail().outgoingHTLCs - numAliceIncoming := aliceChannel.localCommitChain.tail().incomingHTLCs + numAliceOutgoing := aliceChannel.commitChains.Local.tail().outgoingHTLCs + numAliceIncoming := aliceChannel.commitChains.Local.tail().incomingHTLCs if len(numAliceOutgoing) != 3 { t.Fatalf("expected %v htlcs, instead got %v", 3, numAliceOutgoing) } if len(numAliceIncoming) != 1 { t.Fatalf("expected %v htlcs, instead got %v", 1, numAliceIncoming) } - numBobOutgoing := bobChannel.localCommitChain.tail().outgoingHTLCs - numBobIncoming := bobChannel.localCommitChain.tail().incomingHTLCs + numBobOutgoing := bobChannel.commitChains.Local.tail().outgoingHTLCs + numBobIncoming := bobChannel.commitChains.Local.tail().incomingHTLCs if len(numBobOutgoing) != 1 { t.Fatalf("expected %v htlcs, instead got %v", 1, numBobOutgoing) } @@ -2090,20 +2090,24 @@ func TestCancelHTLC(t *testing.T) { // Now HTLCs should be present on the commitment transaction for either // side. - if len(aliceChannel.localCommitChain.tip().outgoingHTLCs) != 0 || - len(aliceChannel.remoteCommitChain.tip().outgoingHTLCs) != 0 { + if len(aliceChannel.commitChains.Local.tip().outgoingHTLCs) != 0 || + len(aliceChannel.commitChains.Remote.tip().outgoingHTLCs) != 0 { + t.Fatalf("htlc's still active from alice's POV") } - if len(aliceChannel.localCommitChain.tip().incomingHTLCs) != 0 || - len(aliceChannel.remoteCommitChain.tip().incomingHTLCs) != 0 { + if len(aliceChannel.commitChains.Local.tip().incomingHTLCs) != 0 || + len(aliceChannel.commitChains.Remote.tip().incomingHTLCs) != 0 { + t.Fatalf("htlc's still active from alice's POV") } - if len(bobChannel.localCommitChain.tip().outgoingHTLCs) != 0 || - len(bobChannel.remoteCommitChain.tip().outgoingHTLCs) != 0 { + if len(bobChannel.commitChains.Local.tip().outgoingHTLCs) != 0 || + len(bobChannel.commitChains.Remote.tip().outgoingHTLCs) != 0 { + t.Fatalf("htlc's still active from bob's POV") } - if len(bobChannel.localCommitChain.tip().incomingHTLCs) != 0 || - len(bobChannel.remoteCommitChain.tip().incomingHTLCs) != 0 { + if len(bobChannel.commitChains.Local.tip().incomingHTLCs) != 0 || + len(bobChannel.commitChains.Remote.tip().incomingHTLCs) != 0 { + t.Fatalf("htlc's still active from bob's POV") } @@ -5207,7 +5211,9 @@ func TestChanCommitWeightDustHtlcs(t *testing.T) { // When sending htlcs we enforce the feebuffer on the commitment // transaction. remoteCommitWeight := func(lc *LightningChannel) lntypes.WeightUnit { - remoteACKedIndex := lc.localCommitChain.tip().theirMessageIndex + remoteACKedIndex := + lc.commitChains.Local.tip().theirMessageIndex + htlcView := lc.fetchHTLCView(remoteACKedIndex, lc.localUpdateLog.logIndex) @@ -5830,7 +5836,7 @@ func TestChannelUnilateralClosePendingCommit(t *testing.T) { // At this point, Alice's commitment chain should have a new pending // commit for Bob. We'll extract it so we can simulate Bob broadcasting // the commitment due to an issue. - bobCommit := aliceChannel.remoteCommitChain.tip().txn + bobCommit := aliceChannel.commitChains.Remote.tip().txn bobTxHash := bobCommit.TxHash() spendDetail := &chainntnfs.SpendDetail{ SpenderTxHash: &bobTxHash, @@ -7523,7 +7529,7 @@ func TestForceCloseBorkedState(t *testing.T) { // We manually advance the commitment tail here since the above // ReceiveRevocation call will fail before it's actually advanced. - aliceChannel.remoteCommitChain.advanceTail() + aliceChannel.commitChains.Remote.advanceTail() _, err = aliceChannel.SignNextCommitment() if err != channeldb.ErrChanBorked { t.Fatalf("sign commitment should have failed: %v", err) @@ -7739,7 +7745,7 @@ func TestIdealCommitFeeRate(t *testing.T) { maxFeeAlloc float64) chainfee.SatPerKWeight { balance, weight := c.availableBalance(AdditionalHtlc) - feeRate := c.localCommitChain.tip().feePerKw + feeRate := c.commitChains.Local.tip().feePerKw currentFee := feeRate.FeeForWeight(weight) maxBalance := balance.ToSatoshis() + currentFee @@ -7756,7 +7762,7 @@ func TestIdealCommitFeeRate(t *testing.T) { // currentFeeRate calculates the current fee rate of the channel. The // ideal fee rate is floored at the current fee rate of the channel. currentFeeRate := func(c *LightningChannel) chainfee.SatPerKWeight { - return c.localCommitChain.tip().feePerKw + return c.commitChains.Local.tip().feePerKw } // testCase definies the test cases when calculating the ideal fee rate @@ -11099,7 +11105,7 @@ func TestBlindingPointPersistence(t *testing.T) { require.NoError(t, err, "unable to restart alice") // Assert that the blinding point is restored from disk. - remoteCommit := aliceChannel.remoteCommitChain.tip() + remoteCommit := aliceChannel.commitChains.Remote.tip() require.Len(t, remoteCommit.outgoingHTLCs, 1) require.Equal(t, blinding, remoteCommit.outgoingHTLCs[0].BlindingPoint.UnwrapOrFailV(t)) @@ -11116,7 +11122,7 @@ func TestBlindingPointPersistence(t *testing.T) { require.NoError(t, err, "unable to restart bob's channel") // Assert that Bob is able to recover the blinding point from disk. - bobCommit := bobChannel.localCommitChain.tip() + bobCommit := bobChannel.commitChains.Local.tip() require.Len(t, bobCommit.incomingHTLCs, 1) require.Equal(t, blinding, bobCommit.incomingHTLCs[0].BlindingPoint.UnwrapOrFailV(t))