From bcde70e74a69ab8972a80ef2849b84e607ca889f Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 20 Feb 2017 21:58:34 -0800 Subject: [PATCH] channeldb: fix bug when writing revocation log states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes a bug that was introduced when we moved to using 64-bit integers for storing the revocation log state. When we made this change, we forgot to increase the size of the buffer which stores the key for the particular channel state from 40 to 44 bytes to account for the 4 additional bytes in the new 64-bit integer. This bug has been fixed by properly sizing the key buffer. We’ve also added an additional test to ensure that we retrieve the proper state after multiple state updates. --- channeldb/channel.go | 10 ++++++---- channeldb/channel_test.go | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/channeldb/channel.go b/channeldb/channel.go index 890a8788..d179878f 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -1817,18 +1817,20 @@ func deserializeChannelDelta(r io.Reader) (*ChannelDelta, error) { return delta, nil } -func makeLogKey(o *wire.OutPoint, updateNum uint64) [40]byte { +func makeLogKey(o *wire.OutPoint, updateNum uint64) [44]byte { var ( scratch [8]byte n int - k [40]byte + + // txid (32) || index (4) || update_num (8) + // 32 + 4 + 8 = 44 + k [44]byte ) n += copy(k[:], o.Hash[:]) byteOrder.PutUint32(scratch[:4], o.Index) - copy(k[n:], scratch[:4]) - n += 4 + n += copy(k[n:], scratch[:4]) byteOrder.PutUint64(scratch[:], updateNum) copy(k[n:], scratch[:]) diff --git a/channeldb/channel_test.go b/channeldb/channel_test.go index df36f7cb..c56f6197 100644 --- a/channeldb/channel_test.go +++ b/channeldb/channel_test.go @@ -389,7 +389,10 @@ func TestChannelStateTransition(t *testing.T) { // Add some HTLCs which were added during this new state transition. // Half of the HTLCs are incoming, while the other half are outgoing. - var htlcs []*HTLC + var ( + htlcs []*HTLC + htlcAmt btcutil.Amount + ) for i := uint32(0); i < 10; i++ { var incoming bool if i > 5 { @@ -397,13 +400,14 @@ func TestChannelStateTransition(t *testing.T) { } htlc := &HTLC{ Incoming: incoming, - Amt: 50000, + Amt: 10, RHash: key, RefundTimeout: i, RevocationDelay: i + 2, OutputIndex: uint16(i * 3), } htlcs = append(htlcs, htlc) + htlcAmt += htlc.Amt } // Create a new channel delta which includes the above HTLCs, some @@ -508,6 +512,33 @@ func TestChannelStateTransition(t *testing.T) { } } + // Next modify the delta slightly, then create a new entry within the + // revocation log. + delta.UpdateNum = 2 + delta.LocalBalance -= htlcAmt + delta.RemoteBalance += htlcAmt + delta.Htlcs = nil + if err := channel.AppendToRevocationLog(delta); err != nil { + t.Fatalf("unable to append to revocation log: %v", err) + } + + // Once again, fetch the state and ensure it has been properly updated. + diskDelta, err = channel.FindPreviousState(uint64(delta.UpdateNum)) + if err != nil { + t.Fatalf("unable to fetch past delta: %v", err) + } + if len(diskDelta.Htlcs) != 0 { + t.Fatalf("expected %v htlcs, got %v", 0, len(diskDelta.Htlcs)) + } + if delta.LocalBalance != 1e8-htlcAmt { + t.Fatalf("mismatched balances, expected %v got %v", 1e8-htlcAmt, + delta.LocalBalance) + } + if delta.RemoteBalance != 1e8+htlcAmt { + t.Fatalf("mismatched balances, expected %v got %v", 1e8+htlcAmt, + delta.RemoteBalance) + } + // The revocation state stored on-disk should now also be identical. updatedChannel, err = cdb.FetchOpenChannels(channel.IdentityPub) if err != nil {