diff --git a/channeldb/channel.go b/channeldb/channel.go index e6dc31b45..4ece47a57 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -2306,7 +2306,7 @@ func (c *OpenChannel) InsertNextRevocation(revKey *btcec.PublicKey) error { // set of local updates that the peer still needs to send us a signature for. // We store this set of updates in case we go down. func (c *OpenChannel) AdvanceCommitChainTail(fwdPkg *FwdPkg, - updates []LogUpdate) error { + updates []LogUpdate, ourOutputIndex, theirOutputIndex uint32) error { c.Lock() defer c.Unlock() @@ -2352,7 +2352,7 @@ func (c *OpenChannel) AdvanceCommitChainTail(fwdPkg *FwdPkg, // TODO(roasbeef): could make the deltas relative, would save // space, but then tradeoff for more disk-seeks to recover the // full state. - logKey := revocationLogBucketDeprecated + logKey := revocationLogBucket logBucket, err := chanBucket.CreateBucketIfNotExists(logKey) if err != nil { return err @@ -2379,9 +2379,10 @@ func (c *OpenChannel) AdvanceCommitChainTail(fwdPkg *FwdPkg, // With the commitment pointer swapped, we can now add the // revoked (prior) state to the revocation log. - // - // TODO(roasbeef): store less - err = appendChannelLogEntry(logBucket, &c.RemoteCommitment) + err = putRevocationLog( + logBucket, &c.RemoteCommitment, + ourOutputIndex, theirOutputIndex, + ) if err != nil { return err } @@ -2591,9 +2592,9 @@ func (c *OpenChannel) revocationLogTailCommitHeight() (uint64, error) { return err } - logBucket := chanBucket.NestedReadBucket(revocationLogBucketDeprecated) - if logBucket == nil { - return ErrNoPastDeltas + logBucket, err := fetchLogBucket(chanBucket) + if err != nil { + return err } // Once we have the bucket that stores the revocation log from @@ -2654,11 +2655,15 @@ func (c *OpenChannel) CommitmentHeight() (uint64, error) { // intended to be used for obtaining the relevant data needed to claim all // funds rightfully spendable in the case of an on-chain broadcast of the // commitment transaction. -func (c *OpenChannel) FindPreviousState(updateNum uint64) (*ChannelCommitment, error) { +func (c *OpenChannel) FindPreviousState( + updateNum uint64) (*RevocationLog, *ChannelCommitment, error) { + c.RLock() defer c.RUnlock() - var commit ChannelCommitment + commit := &ChannelCommitment{} + rl := &RevocationLog{} + err := kvdb.View(c.Db.backend, func(tx kvdb.RTx) error { chanBucket, err := fetchChanBucket( tx, c.IdentityPub, &c.FundingOutpoint, c.ChainHash, @@ -2667,24 +2672,24 @@ func (c *OpenChannel) FindPreviousState(updateNum uint64) (*ChannelCommitment, e return err } - logBucket := chanBucket.NestedReadBucket(revocationLogBucketDeprecated) - if logBucket == nil { - return ErrNoPastDeltas - } - - c, err := fetchOldRevocationLog(logBucket, updateNum) + // Find the revocation log from both the new and the old + // bucket. + r, c, err := fetchRevocationLogCompatible(chanBucket, updateNum) if err != nil { return err } + rl = r commit = c return nil }, func() {}) if err != nil { - return nil, err + return nil, nil, err } - return &commit, nil + // Either the `rl` or the `commit` is nil here. We return them as-is + // and leave it to the caller to decide its following action. + return rl, commit, nil } // ClosureType is an enum like structure that details exactly _how_ a channel @@ -2881,12 +2886,8 @@ func (c *OpenChannel) CloseChannel(summary *ChannelCloseSummary, // With the base channel data deleted, attempt to delete the // information stored within the revocation log. - logBucket := chanBucket.NestedReadWriteBucket(revocationLogBucketDeprecated) - if logBucket != nil { - err = chanBucket.DeleteNestedBucket(revocationLogBucketDeprecated) - if err != nil { - return err - } + if err := deleteLogBucket(chanBucket); err != nil { + return err } err = chainBucket.DeleteNestedBucket(chanPointBuf.Bytes()) @@ -3643,19 +3644,6 @@ func makeLogKey(updateNum uint64) [8]byte { return key } -// TODO: delete -func appendChannelLogEntry(log kvdb.RwBucket, - commit *ChannelCommitment) error { - - var b bytes.Buffer - if err := serializeChanCommit(&b, commit); err != nil { - return err - } - - logEntrykey := makeLogKey(commit.CommitHeight) - return log.Put(logEntrykey[:], b.Bytes()) -} - func fetchThawHeight(chanBucket kvdb.RBucket) (uint32, error) { var height uint32 diff --git a/channeldb/channel_test.go b/channeldb/channel_test.go index c6033d9ed..35607fd55 100644 --- a/channeldb/channel_test.go +++ b/channeldb/channel_test.go @@ -52,8 +52,17 @@ var ( Port: 18555, } - // keyLocIndex is the KeyLocator Index we use for TestKeyLocatorEncoding. + // keyLocIndex is the KeyLocator Index we use for + // TestKeyLocatorEncoding. keyLocIndex = uint32(2049) + + // dummyLocalOutputIndex specifics a default value for our output index + // in this test. + dummyLocalOutputIndex = uint32(0) + + // dummyRemoteOutIndex specifics a default value for their output index + // in this test. + dummyRemoteOutIndex = uint32(1) ) // testChannelParams is a struct which details the specifics of how a channel @@ -548,6 +557,32 @@ func assertCommitmentEqual(t *testing.T, a, b *ChannelCommitment) { } } +// assertRevocationLogEntryEqual asserts that, for all the fields of a given +// revocation log entry, their values match those on a given ChannelCommitment. +func assertRevocationLogEntryEqual(t *testing.T, c *ChannelCommitment, + r *RevocationLog) { + + // Check the common fields. + require.EqualValues( + t, r.CommitTxHash, c.CommitTx.TxHash(), "CommitTx mismatch", + ) + + // Now check the common fields from the HTLCs. + require.Equal(t, len(r.HTLCEntries), len(c.Htlcs), "HTLCs len mismatch") + for i, rHtlc := range r.HTLCEntries { + cHtlc := c.Htlcs[i] + require.Equal(t, rHtlc.RHash, cHtlc.RHash, "RHash mismatch") + require.Equal(t, rHtlc.Amt, cHtlc.Amt.ToSatoshis(), + "Amt mismatch") + require.Equal(t, rHtlc.RefundTimeout, cHtlc.RefundTimeout, + "RefundTimeout mismatch") + require.EqualValues(t, rHtlc.OutputIndex, cHtlc.OutputIndex, + "OutputIndex mismatch") + require.Equal(t, rHtlc.Incoming, cHtlc.Incoming, + "Incoming mismatch") + } +} + func TestChannelStateTransition(t *testing.T) { t.Parallel() @@ -748,7 +783,9 @@ func TestChannelStateTransition(t *testing.T) { fwdPkg := NewFwdPkg(channel.ShortChanID(), oldRemoteCommit.CommitHeight, diskCommitDiff.LogUpdates, nil) - err = channel.AdvanceCommitChainTail(fwdPkg, nil) + err = channel.AdvanceCommitChainTail( + fwdPkg, nil, dummyLocalOutputIndex, dummyRemoteOutIndex, + ) if err != nil { t.Fatalf("unable to append to revocation log: %v", err) } @@ -761,16 +798,24 @@ func TestChannelStateTransition(t *testing.T) { // We should be able to fetch the channel delta created above by its // update number with all the state properly reconstructed. - diskPrevCommit, err := channel.FindPreviousState( + diskPrevCommit, _, err := channel.FindPreviousState( oldRemoteCommit.CommitHeight, ) if err != nil { t.Fatalf("unable to fetch past delta: %v", err) } + // Check the output indexes are saved as expected. + require.EqualValues( + t, dummyLocalOutputIndex, diskPrevCommit.OurOutputIndex, + ) + require.EqualValues( + t, dummyRemoteOutIndex, diskPrevCommit.TheirOutputIndex, + ) + // The two deltas (the original vs the on-disk version) should // identical, and all HTLC data should properly be retained. - assertCommitmentEqual(t, &oldRemoteCommit, diskPrevCommit) + assertRevocationLogEntryEqual(t, &oldRemoteCommit, diskPrevCommit) // The state number recovered from the tail of the revocation log // should be identical to this current state. @@ -796,17 +841,30 @@ func TestChannelStateTransition(t *testing.T) { fwdPkg = NewFwdPkg(channel.ShortChanID(), oldRemoteCommit.CommitHeight, nil, nil) - err = channel.AdvanceCommitChainTail(fwdPkg, nil) + err = channel.AdvanceCommitChainTail( + fwdPkg, nil, dummyLocalOutputIndex, dummyRemoteOutIndex, + ) if err != nil { t.Fatalf("unable to append to revocation log: %v", err) } // Once again, fetch the state and ensure it has been properly updated. - prevCommit, err := channel.FindPreviousState(oldRemoteCommit.CommitHeight) + prevCommit, _, err := channel.FindPreviousState( + oldRemoteCommit.CommitHeight, + ) if err != nil { t.Fatalf("unable to fetch past delta: %v", err) } - assertCommitmentEqual(t, &oldRemoteCommit, prevCommit) + + // Check the output indexes are saved as expected. + require.EqualValues( + t, dummyLocalOutputIndex, diskPrevCommit.OurOutputIndex, + ) + require.EqualValues( + t, dummyRemoteOutIndex, diskPrevCommit.TheirOutputIndex, + ) + + assertRevocationLogEntryEqual(t, &oldRemoteCommit, prevCommit) // Once again, state number recovered from the tail of the revocation // log should be identical to this current state. @@ -860,7 +918,9 @@ func TestChannelStateTransition(t *testing.T) { // Attempting to find previous states on the channel should fail as the // revocation log has been deleted. - _, err = updatedChannel[0].FindPreviousState(oldRemoteCommit.CommitHeight) + _, _, err = updatedChannel[0].FindPreviousState( + oldRemoteCommit.CommitHeight, + ) if err == nil { t.Fatal("revocation log search should have failed") } diff --git a/channeldb/db_test.go b/channeldb/db_test.go index 6cc5c6d44..b741a2ad7 100644 --- a/channeldb/db_test.go +++ b/channeldb/db_test.go @@ -409,7 +409,9 @@ func TestRestoreChannelShells(t *testing.T) { if err != ErrNoRestoredChannelMutation { t.Fatalf("able to mutate restored channel") } - err = channel.AdvanceCommitChainTail(nil, nil) + err = channel.AdvanceCommitChainTail( + nil, nil, dummyLocalOutputIndex, dummyRemoteOutIndex, + ) if err != ErrNoRestoredChannelMutation { t.Fatalf("able to mutate restored channel") } diff --git a/lnwallet/channel.go b/lnwallet/channel.go index f27c55608..b9cde8dc8 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2282,7 +2282,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // Query the on-disk revocation log for the snapshot which was recorded // at this particular state num. - revokedSnapshot, err := chanState.FindPreviousState(stateNum) + _, revokedSnapshot, err := chanState.FindPreviousState(stateNum) if err != nil { return nil, err } @@ -4872,12 +4872,28 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) ( source, remoteChainTail, addUpdates, settleFailUpdates, ) + // We will soon be saving the current remote commitment to revocation + // log bucket, which is `lc.channelState.RemoteCommitment`. After that, + // the `RemoteCommitment` will be replaced with a newer version found + // in `CommitDiff`. Thus we need to compute the output indexes here + // before the change since the indexes are meant for the current, + // revoked remote commitment. + ourOutputIndex, theirOutputIndex, err := findOutputIndexesFromRemote( + revocation, lc.channelState, + ) + if err != nil { + return nil, nil, nil, nil, err + } + // At this point, the revocation has been accepted, and we've rotated // the current revocation key+hash for the remote party. Therefore we // sync now to ensure the revocation producer state is consistent with // the current commitment height and also to advance the on-disk // commitment chain. - err = lc.channelState.AdvanceCommitChainTail(fwdPkg, localPeerUpdates) + err = lc.channelState.AdvanceCommitChainTail( + fwdPkg, localPeerUpdates, + ourOutputIndex, theirOutputIndex, + ) if err != nil { return nil, nil, nil, nil, err }