From 650bed6de8c98d3020b71a21c7687fc8d5e2142f Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Mon, 11 Dec 2023 14:35:33 -0700 Subject: [PATCH 1/4] Fix data poster time RLP encoding --- arbnode/dataposter/dbstorage/storage.go | 3 +- arbnode/dataposter/slice/slicestorage.go | 3 +- arbnode/dataposter/storage/storage.go | 42 ++++++++++++++++++++++++ arbnode/dataposter/storage/time.go | 34 +++++++++++++++++++ arbnode/dataposter/storage_test.go | 19 +++++++++++ 5 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 arbnode/dataposter/storage/time.go diff --git a/arbnode/dataposter/dbstorage/storage.go b/arbnode/dataposter/dbstorage/storage.go index f2b1854492..473bfa2c3b 100644 --- a/arbnode/dataposter/dbstorage/storage.go +++ b/arbnode/dataposter/dbstorage/storage.go @@ -6,6 +6,7 @@ package dbstorage import ( "bytes" "context" + "encoding/hex" "errors" "fmt" "strconv" @@ -140,7 +141,7 @@ func (s *Storage) Put(ctx context.Context, index uint64, prev, new *storage.Queu return fmt.Errorf("encoding previous item: %w", err) } if !bytes.Equal(stored, prevEnc) { - return fmt.Errorf("replacing different item than expected at index: %v, stored: %v, prevEnc: %v", index, stored, prevEnc) + return fmt.Errorf("replacing different item than expected at index: %v, stored: %v, prevEnc: %v", index, hex.EncodeToString(stored), hex.EncodeToString(prevEnc)) } newEnc, err := s.encDec().Encode(new) if err != nil { diff --git a/arbnode/dataposter/slice/slicestorage.go b/arbnode/dataposter/slice/slicestorage.go index 04286df411..dbd7a3ea5e 100644 --- a/arbnode/dataposter/slice/slicestorage.go +++ b/arbnode/dataposter/slice/slicestorage.go @@ -6,6 +6,7 @@ package slice import ( "bytes" "context" + "encoding/hex" "errors" "fmt" @@ -90,7 +91,7 @@ func (s *Storage) Put(_ context.Context, index uint64, prev, new *storage.Queued return fmt.Errorf("encoding previous item: %w", err) } if !bytes.Equal(prevEnc, s.queue[queueIdx]) { - return fmt.Errorf("replacing different item than expected at index: %v, stored: %v, prevEnc: %v", index, s.queue[queueIdx], prevEnc) + return fmt.Errorf("replacing different item than expected at index: %v, stored: %v, prevEnc: %v", index, hex.EncodeToString(s.queue[queueIdx]), hex.EncodeToString(prevEnc)) } s.queue[queueIdx] = newEnc } else { diff --git a/arbnode/dataposter/storage/storage.go b/arbnode/dataposter/storage/storage.go index 70637c48e0..b55f6363fa 100644 --- a/arbnode/dataposter/storage/storage.go +++ b/arbnode/dataposter/storage/storage.go @@ -1,8 +1,12 @@ +// Copyright 2021-2023, Offchain Labs, Inc. +// For license information, see https://github.com/nitro/blob/master/LICENSE + package storage import ( "errors" "fmt" + "io" "time" "github.com/ethereum/go-ethereum/core/types" @@ -30,6 +34,44 @@ type QueuedTransaction struct { NextReplacement time.Time } +type QueuedTransactionForEncoding struct { + FullTx *types.Transaction + Data types.DynamicFeeTx + Meta []byte + Sent bool + Created *RlpTime `rlp:"optional"` // may be earlier than the tx was given to the tx poster + NextReplacement *RlpTime `rlp:"optional"` +} + +func (qt *QueuedTransaction) EncodeRLP(w io.Writer) error { + return rlp.Encode(w, QueuedTransactionForEncoding{ + FullTx: qt.FullTx, + Data: qt.Data, + Meta: qt.Meta, + Sent: qt.Sent, + Created: (*RlpTime)(&qt.Created), + NextReplacement: (*RlpTime)(&qt.NextReplacement), + }) +} + +func (qt *QueuedTransaction) DecodeRLP(s *rlp.Stream) error { + var qtEnc QueuedTransactionForEncoding + if err := s.Decode(&qtEnc); err != nil { + return err + } + qt.FullTx = qtEnc.FullTx + qt.Data = qtEnc.Data + qt.Meta = qtEnc.Meta + qt.Sent = qtEnc.Sent + if qtEnc.Created != nil { + qt.Created = time.Time(*qtEnc.Created) + } + if qtEnc.NextReplacement != nil { + qt.NextReplacement = time.Time(*qtEnc.NextReplacement) + } + return nil +} + // LegacyQueuedTransaction is used for backwards compatibility. // Before https://github.com/OffchainLabs/nitro/pull/1773: the queuedTransaction // looked like this and was rlp encoded directly. After the pr, we are store diff --git a/arbnode/dataposter/storage/time.go b/arbnode/dataposter/storage/time.go new file mode 100644 index 0000000000..d9911de997 --- /dev/null +++ b/arbnode/dataposter/storage/time.go @@ -0,0 +1,34 @@ +// Copyright 2021-2023, Offchain Labs, Inc. +// For license information, see https://github.com/nitro/blob/master/LICENSE + +package storage + +import ( + "io" + "time" + + "github.com/ethereum/go-ethereum/rlp" +) + +// time.Time doesn't encode as anything in RLP. This fixes that. +// It encodes a timestamp as a uint64 unix timestamp in seconds, +// so any subsecond precision is lost. +type RlpTime time.Time + +func (b *RlpTime) DecodeRLP(s *rlp.Stream) error { + var nanos uint64 + err := s.Decode(&nanos) + if err != nil { + return err + } + *b = RlpTime(time.Unix(int64(nanos), 0)) + return nil +} + +func (b RlpTime) EncodeRLP(w io.Writer) error { + return rlp.Encode(w, uint64(time.Time(b).Unix())) +} + +func (b RlpTime) String() string { + return time.Time(b).String() +} diff --git a/arbnode/dataposter/storage_test.go b/arbnode/dataposter/storage_test.go index adea2073e2..ff5623f778 100644 --- a/arbnode/dataposter/storage_test.go +++ b/arbnode/dataposter/storage_test.go @@ -5,6 +5,7 @@ import ( "math/big" "path" "testing" + "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" @@ -366,3 +367,21 @@ func TestLength(t *testing.T) { } } + +func TestTimeEncoding(t *testing.T) { + // RlpTime cuts off subsecond precision, so for this test, + // we'll use a time that doesn't have any subsecond precision. + now := storage.RlpTime(time.Unix(time.Now().Unix(), 0)) + enc, err := rlp.EncodeToBytes(now) + if err != nil { + t.Fatal("failed to encode time", err) + } + var dec storage.RlpTime + err = rlp.DecodeBytes(enc, &dec) + if err != nil { + t.Fatal("failed to decode time", err) + } + if !time.Time(dec).Equal(time.Time(now)) { + t.Fatalf("time %v encoded then decoded to %v", now, dec) + } +} From 0bc7f5035d16de248f4d8316cc1185916c398445 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Mon, 11 Dec 2023 14:43:33 -0700 Subject: [PATCH 2/4] Always saveTx in sendTx --- arbnode/dataposter/data_poster.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 4138aa8400..f535279118 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -537,10 +537,8 @@ func (p *DataPoster) saveTx(ctx context.Context, prevTx, newTx *storage.QueuedTr } func (p *DataPoster) sendTx(ctx context.Context, prevTx *storage.QueuedTransaction, newTx *storage.QueuedTransaction) error { - if prevTx == nil || (newTx.FullTx.Hash() != prevTx.FullTx.Hash()) { - if err := p.saveTx(ctx, prevTx, newTx); err != nil { - return err - } + if err := p.saveTx(ctx, prevTx, newTx); err != nil { + return err } if err := p.client.SendTransaction(ctx, newTx.FullTx); err != nil { if !strings.Contains(err.Error(), "already known") && !strings.Contains(err.Error(), "nonce too low") { From f7275cfb109041e102858a54f2778c94a0b794b8 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Mon, 11 Dec 2023 17:05:29 -0700 Subject: [PATCH 3/4] Make saveTx skipping check comprehensive --- arbnode/dataposter/data_poster.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index f535279118..23b63cad46 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -5,6 +5,7 @@ package dataposter import ( + "bytes" "context" "crypto/tls" "crypto/x509" @@ -527,8 +528,24 @@ func (p *DataPoster) PostTransaction(ctx context.Context, dataCreatedAt time.Tim // the mutex must be held by the caller func (p *DataPoster) saveTx(ctx context.Context, prevTx, newTx *storage.QueuedTransaction) error { - if prevTx != nil && prevTx.Data.Nonce != newTx.Data.Nonce { - return fmt.Errorf("prevTx nonce %v doesn't match newTx nonce %v", prevTx.Data.Nonce, newTx.Data.Nonce) + if prevTx != nil { + if prevTx.Data.Nonce != newTx.Data.Nonce { + return fmt.Errorf("prevTx nonce %v doesn't match newTx nonce %v", prevTx.Data.Nonce, newTx.Data.Nonce) + } + + // Check if prevTx is the same as newTx and we don't need to do anything + oldEnc, err := rlp.EncodeToBytes(prevTx) + if err != nil { + return fmt.Errorf("failed to encode prevTx: %w", err) + } + newEnc, err := rlp.EncodeToBytes(newTx) + if err != nil { + return fmt.Errorf("failed to encode newTx: %w", err) + } + if bytes.Equal(oldEnc, newEnc) { + // No need to save newTx as it's the same as prevTx + return nil + } } if err := p.queue.Put(ctx, newTx.Data.Nonce, prevTx, newTx); err != nil { return fmt.Errorf("putting new tx in the queue: %w", err) From 8b64fa27317f46785cdae557ea2fb0f68f3ef6dd Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 12 Dec 2023 10:21:58 -0700 Subject: [PATCH 4/4] Add nanoseconds to time storage --- arbnode/dataposter/storage/storage.go | 6 +++--- arbnode/dataposter/storage/time.go | 16 ++++++++++++---- arbnode/dataposter/storage_test.go | 4 +--- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/arbnode/dataposter/storage/storage.go b/arbnode/dataposter/storage/storage.go index b55f6363fa..87bfa8d83d 100644 --- a/arbnode/dataposter/storage/storage.go +++ b/arbnode/dataposter/storage/storage.go @@ -34,7 +34,7 @@ type QueuedTransaction struct { NextReplacement time.Time } -type QueuedTransactionForEncoding struct { +type queuedTransactionForEncoding struct { FullTx *types.Transaction Data types.DynamicFeeTx Meta []byte @@ -44,7 +44,7 @@ type QueuedTransactionForEncoding struct { } func (qt *QueuedTransaction) EncodeRLP(w io.Writer) error { - return rlp.Encode(w, QueuedTransactionForEncoding{ + return rlp.Encode(w, queuedTransactionForEncoding{ FullTx: qt.FullTx, Data: qt.Data, Meta: qt.Meta, @@ -55,7 +55,7 @@ func (qt *QueuedTransaction) EncodeRLP(w io.Writer) error { } func (qt *QueuedTransaction) DecodeRLP(s *rlp.Stream) error { - var qtEnc QueuedTransactionForEncoding + var qtEnc queuedTransactionForEncoding if err := s.Decode(&qtEnc); err != nil { return err } diff --git a/arbnode/dataposter/storage/time.go b/arbnode/dataposter/storage/time.go index d9911de997..df07c5cff9 100644 --- a/arbnode/dataposter/storage/time.go +++ b/arbnode/dataposter/storage/time.go @@ -15,18 +15,26 @@ import ( // so any subsecond precision is lost. type RlpTime time.Time +type rlpTimeEncoding struct { + Seconds uint64 + Nanos uint64 +} + func (b *RlpTime) DecodeRLP(s *rlp.Stream) error { - var nanos uint64 - err := s.Decode(&nanos) + var enc rlpTimeEncoding + err := s.Decode(&enc) if err != nil { return err } - *b = RlpTime(time.Unix(int64(nanos), 0)) + *b = RlpTime(time.Unix(int64(enc.Seconds), int64(enc.Nanos))) return nil } func (b RlpTime) EncodeRLP(w io.Writer) error { - return rlp.Encode(w, uint64(time.Time(b).Unix())) + return rlp.Encode(w, rlpTimeEncoding{ + Seconds: uint64(time.Time(b).Unix()), + Nanos: uint64(time.Time(b).Nanosecond()), + }) } func (b RlpTime) String() string { diff --git a/arbnode/dataposter/storage_test.go b/arbnode/dataposter/storage_test.go index ff5623f778..3344215e90 100644 --- a/arbnode/dataposter/storage_test.go +++ b/arbnode/dataposter/storage_test.go @@ -369,9 +369,7 @@ func TestLength(t *testing.T) { } func TestTimeEncoding(t *testing.T) { - // RlpTime cuts off subsecond precision, so for this test, - // we'll use a time that doesn't have any subsecond precision. - now := storage.RlpTime(time.Unix(time.Now().Unix(), 0)) + now := storage.RlpTime(time.Now()) enc, err := rlp.EncodeToBytes(now) if err != nil { t.Fatal("failed to encode time", err)