From ccfb0065a03ee026743f7cfa81cc72e4f26725ab Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Wed, 23 Aug 2023 17:13:28 -0500 Subject: [PATCH 01/22] Initialize dev wallet as a chain owner in nitro-testnode --- nitro-testnode | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nitro-testnode b/nitro-testnode index 14f24a1bad..7ad12c0f1b 160000 --- a/nitro-testnode +++ b/nitro-testnode @@ -1 +1 @@ -Subproject commit 14f24a1bad2625412602d06156156c380bd589d2 +Subproject commit 7ad12c0f1be75a72c7360d5258e0090f8225594e From 2754cf3322f7a359ff9c20186be706e7c8033040 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 29 Aug 2023 11:50:24 -0600 Subject: [PATCH 02/22] Fix batch bounds calculation on L3s --- arbnode/batch_poster.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index e9a1663741..429701be7e 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -787,7 +787,8 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) if err != nil { return false, err } - blockNumberWithPadding := arbmath.SaturatingUAdd(arbmath.BigToUintSaturating(latestHeader.Number), uint64(config.L1BlockBoundBypass/ethPosBlockTime)) + latestBlockNumber := arbutil.ParentHeaderToL1BlockNumber(latestHeader) + blockNumberWithPadding := arbmath.SaturatingUAdd(latestBlockNumber, uint64(config.L1BlockBoundBypass/ethPosBlockTime)) timestampWithPadding := arbmath.SaturatingUAdd(latestHeader.Time, uint64(config.L1BlockBoundBypass/time.Second)) l1BoundMinBlockNumber = arbmath.SaturatingUSub(blockNumberWithPadding, arbmath.BigToUintSaturating(maxTimeVariation.DelayBlocks)) From de3b6a9305b4cacca9a915e48ea0b8c4b874ef93 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 29 Aug 2023 15:06:24 -0600 Subject: [PATCH 03/22] Fix clippy warning about an Arc not being Send+Sync --- arbitrator/prover/src/utils.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arbitrator/prover/src/utils.rs b/arbitrator/prover/src/utils.rs index 6c11e9af05..efd94dcd7c 100644 --- a/arbitrator/prover/src/utils.rs +++ b/arbitrator/prover/src/utils.rs @@ -158,6 +158,10 @@ impl From<&[u8]> for CBytes { } } +// There's no thread safety concerns for CBytes +unsafe impl Send for CBytes {} +unsafe impl Sync for CBytes {} + #[derive(Serialize, Deserialize)] #[serde(remote = "Type")] enum RemoteType { From 9227df788929352184e0bd792898f15beabac0b9 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 29 Aug 2023 15:32:04 -0600 Subject: [PATCH 04/22] Fix formatting --- arbitrator/jit/src/syscall.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arbitrator/jit/src/syscall.rs b/arbitrator/jit/src/syscall.rs index 4cd0363b49..c81641a7f8 100644 --- a/arbitrator/jit/src/syscall.rs +++ b/arbitrator/jit/src/syscall.rs @@ -306,10 +306,10 @@ pub fn js_value_index(mut env: WasmEnvMut, sp: u32) { pub fn js_value_call(mut env: WasmEnvMut, sp: u32) -> MaybeEscape { let Some(resume) = env.data().exports.resume.clone() else { - return Escape::failure(format!("wasmer failed to bind {}", "resume".red())) + return Escape::failure(format!("wasmer failed to bind {}", "resume".red())); }; let Some(get_stack_pointer) = env.data().exports.get_stack_pointer.clone() else { - return Escape::failure(format!("wasmer failed to bind {}", "getsp".red())) + return Escape::failure(format!("wasmer failed to bind {}", "getsp".red())); }; let sp = GoStack::simple(sp, &env); let data = env.data_mut(); From ad64d967f68b84b1e8ed29a3d6907bd639b8cb16 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 30 Aug 2023 09:37:29 -0600 Subject: [PATCH 05/22] Make PreimageResolver Send+Sync --- arbitrator/prover/src/machine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arbitrator/prover/src/machine.rs b/arbitrator/prover/src/machine.rs index fff9c0f3d8..d5a9c52d92 100644 --- a/arbitrator/prover/src/machine.rs +++ b/arbitrator/prover/src/machine.rs @@ -651,7 +651,7 @@ pub struct MachineState<'a> { initial_hash: Bytes32, } -pub type PreimageResolver = Arc Option>; +pub type PreimageResolver = Arc Option + Send + Sync>; /// Wraps a preimage resolver to provide an easier API /// and cache the last preimage retrieved. From 283b5d71a1d8eff096e878d8f028cf33356585e9 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 30 Aug 2023 09:53:11 -0600 Subject: [PATCH 06/22] Extend comment explaining why CBytes is Send+Sync --- arbitrator/prover/src/utils.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arbitrator/prover/src/utils.rs b/arbitrator/prover/src/utils.rs index efd94dcd7c..e86ea96768 100644 --- a/arbitrator/prover/src/utils.rs +++ b/arbitrator/prover/src/utils.rs @@ -158,7 +158,10 @@ impl From<&[u8]> for CBytes { } } -// There's no thread safety concerns for CBytes +// There's no thread safety concerns for CBytes. +// This type is basically a Box<[u8]> (which is Send + Sync) with libc as an allocator. +// Any data races between threads are prevented by Rust borrowing rules, +// and the data isn't thread-local so there's no concern moving it between threads. unsafe impl Send for CBytes {} unsafe impl Sync for CBytes {} From f6f659e7b2513ab48f0a276340676636350d98f0 Mon Sep 17 00:00:00 2001 From: Nodar Date: Wed, 30 Aug 2023 18:04:20 +0200 Subject: [PATCH 07/22] Implement legacy storage encoding with hot-reload flag to change encoding strategy on the fly --- arbnode/dataposter/data_poster.go | 24 +++-- arbnode/dataposter/leveldb/leveldb.go | 26 ++--- arbnode/dataposter/redis/redisstorage.go | 28 +++--- arbnode/dataposter/slice/slicestorage.go | 22 ++--- arbnode/dataposter/storage/storage.go | 117 +++++++++++++++++++++++ arbnode/dataposter/storage_test.go | 64 ++++++++----- 6 files changed, 202 insertions(+), 79 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index b1db655d71..adfde88f74 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -101,17 +101,23 @@ func NewDataPoster(db ethdb.Database, headerReader *headerreader.HeaderReader, a initConfig.UseNoOpStorage = true log.Info("Disabling data poster storage, as parent chain appears to be an Arbitrum chain without a mempool") } + encF := func() storage.EncoderDecoderInterface { + if config().LegacyStorageEncoding { + return &storage.LegacyEncoderDecoder{} + } + return &storage.EncoderDecoder{} + } var queue QueueStorage switch { case initConfig.UseNoOpStorage: queue = &noop.Storage{} case initConfig.UseLevelDB: - queue = leveldb.New(db) + queue = leveldb.New(db, encF) case redisClient == nil: - queue = slice.NewStorage() + queue = slice.NewStorage(encF) default: var err error - queue, err = redisstorage.NewStorage(redisClient, "data-poster.queue", &initConfig.RedisSigner) + queue, err = redisstorage.NewStorage(redisClient, "data-poster.queue", &initConfig.RedisSigner, encF) if err != nil { return nil, err } @@ -174,7 +180,7 @@ func (p *DataPoster) getNextNonceAndMaybeMeta(ctx context.Context) (uint64, []by } lastQueueItem, err := p.queue.FetchLast(ctx) if err != nil { - return 0, nil, false, err + return 0, nil, false, fmt.Errorf("fetching last element from queue: %w", err) } if lastQueueItem != nil { nextNonce := lastQueueItem.Data.Nonce + 1 @@ -364,7 +370,10 @@ func (p *DataPoster) saveTx(ctx context.Context, prevTx, newTx *storage.QueuedTr 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) } - return p.queue.Put(ctx, newTx.Data.Nonce, prevTx, newTx) + if err := p.queue.Put(ctx, newTx.Data.Nonce, prevTx, newTx); err != nil { + return fmt.Errorf("putting new tx in the queue: %w", err) + } + return nil } func (p *DataPoster) sendTx(ctx context.Context, prevTx *storage.QueuedTransaction, newTx *storage.QueuedTransaction) error { @@ -546,7 +555,7 @@ func (p *DataPoster) Start(ctxIn context.Context) { // replacing them by fee. queueContents, err := p.queue.FetchContents(ctx, unconfirmedNonce, maxTxsToRbf) if err != nil { - log.Error("Failed to get tx queue contents", "err", err) + log.Error("Failed to fetch tx queue contents", "err", err) return minWait } for index, tx := range queueContents { @@ -616,6 +625,7 @@ type DataPosterConfig struct { AllocateMempoolBalance bool `koanf:"allocate-mempool-balance" reload:"hot"` UseLevelDB bool `koanf:"use-leveldb"` UseNoOpStorage bool `koanf:"use-noop-storage"` + LegacyStorageEncoding bool `koanf:"legacy-storage-encoding" reload:"hot"` } // ConfigFetcher function type is used instead of directly passing config so @@ -636,6 +646,7 @@ func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Bool(prefix+".allocate-mempool-balance", DefaultDataPosterConfig.AllocateMempoolBalance, "if true, don't put transactions in the mempool that spend a total greater than the batch poster's balance") f.Bool(prefix+".use-leveldb", DefaultDataPosterConfig.UseLevelDB, "uses leveldb when enabled") f.Bool(prefix+".use-noop-storage", DefaultDataPosterConfig.UseNoOpStorage, "uses noop storage, it doesn't store anything") + f.Bool(prefix+".legacy-storage-encoding", DefaultDataPosterConfig.LegacyStorageEncoding, "encodes items in a legacy way (as it was before dropping generics)") signature.SimpleHmacConfigAddOptions(prefix+".redis-signer", f) } @@ -651,6 +662,7 @@ var DefaultDataPosterConfig = DataPosterConfig{ AllocateMempoolBalance: true, UseLevelDB: false, UseNoOpStorage: false, + LegacyStorageEncoding: true, } var DefaultDataPosterConfigForValidator = func() DataPosterConfig { diff --git a/arbnode/dataposter/leveldb/leveldb.go b/arbnode/dataposter/leveldb/leveldb.go index e41a8665a6..cfb34b04f7 100644 --- a/arbnode/dataposter/leveldb/leveldb.go +++ b/arbnode/dataposter/leveldb/leveldb.go @@ -12,14 +12,14 @@ import ( "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/ethdb/memorydb" - "github.com/ethereum/go-ethereum/rlp" "github.com/offchainlabs/nitro/arbnode/dataposter/storage" "github.com/syndtr/goleveldb/leveldb" ) // Storage implements leveldb based storage for batch poster. type Storage struct { - db ethdb.Database + db ethdb.Database + encDec storage.EncoderDecoderF } var ( @@ -31,16 +31,8 @@ var ( countKey = []byte(".count_key") ) -func New(db ethdb.Database) *Storage { - return &Storage{db: db} -} - -func (s *Storage) decodeItem(data []byte) (*storage.QueuedTransaction, error) { - var item storage.QueuedTransaction - if err := rlp.DecodeBytes(data, &item); err != nil { - return nil, fmt.Errorf("decoding item: %w", err) - } - return &item, nil +func New(db ethdb.Database, enc storage.EncoderDecoderF) *Storage { + return &Storage{db: db, encDec: enc} } func idxToKey(idx uint64) []byte { @@ -55,7 +47,7 @@ func (s *Storage) FetchContents(_ context.Context, startingIndex uint64, maxResu if !it.Next() { break } - item, err := s.decodeItem(it.Value()) + item, err := s.encDec().Decode(it.Value()) if err != nil { return nil, err } @@ -84,7 +76,7 @@ func (s *Storage) FetchLast(ctx context.Context) (*storage.QueuedTransaction, er if err != nil { return nil, err } - return s.decodeItem(val) + return s.encDec().Decode(val) } func (s *Storage) Prune(ctx context.Context, until uint64) error { @@ -117,7 +109,7 @@ func (s *Storage) valueAt(_ context.Context, key []byte) ([]byte, error) { val, err := s.db.Get(key) if err != nil { if isErrNotFound(err) { - return rlp.EncodeToBytes((*storage.QueuedTransaction)(nil)) + return s.encDec().Encode((*storage.QueuedTransaction)(nil)) } return nil, err } @@ -130,14 +122,14 @@ func (s *Storage) Put(ctx context.Context, index uint64, prev, new *storage.Queu if err != nil { return err } - prevEnc, err := rlp.EncodeToBytes(prev) + prevEnc, err := s.encDec().Encode(prev) if err != nil { 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) } - newEnc, err := rlp.EncodeToBytes(new) + newEnc, err := s.encDec().Encode(new) if err != nil { return fmt.Errorf("encoding new item: %w", err) } diff --git a/arbnode/dataposter/redis/redisstorage.go b/arbnode/dataposter/redis/redisstorage.go index e6fe666c69..5123cea154 100644 --- a/arbnode/dataposter/redis/redisstorage.go +++ b/arbnode/dataposter/redis/redisstorage.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" - "github.com/ethereum/go-ethereum/rlp" "github.com/go-redis/redis/v8" "github.com/offchainlabs/nitro/arbnode/dataposter/storage" "github.com/offchainlabs/nitro/util/signature" @@ -23,14 +22,15 @@ type Storage struct { client redis.UniversalClient signer *signature.SimpleHmac key string + encDec storage.EncoderDecoderF } -func NewStorage(client redis.UniversalClient, key string, signerConf *signature.SimpleHmacConfig) (*Storage, error) { +func NewStorage(client redis.UniversalClient, key string, signerConf *signature.SimpleHmacConfig, enc storage.EncoderDecoderF) (*Storage, error) { signer, err := signature.NewSimpleHmac(signerConf) if err != nil { return nil, err } - return &Storage{client, signer, key}, nil + return &Storage{client, signer, key, enc}, nil } func joinHmacMsg(msg []byte, sig []byte) ([]byte, error) { @@ -65,16 +65,15 @@ func (s *Storage) FetchContents(ctx context.Context, startingIndex uint64, maxRe } var items []*storage.QueuedTransaction for _, itemString := range itemStrings { - var item storage.QueuedTransaction data, err := s.peelVerifySignature([]byte(itemString)) if err != nil { return nil, err } - err = rlp.DecodeBytes(data, &item) + item, err := s.encDec().Decode(data) if err != nil { return nil, err } - items = append(items, &item) + items = append(items, item) } return items, nil } @@ -95,16 +94,15 @@ func (s *Storage) FetchLast(ctx context.Context) (*storage.QueuedTransaction, er } var ret *storage.QueuedTransaction if len(itemStrings) > 0 { - var item storage.QueuedTransaction data, err := s.peelVerifySignature([]byte(itemStrings[0])) if err != nil { return nil, err } - err = rlp.DecodeBytes(data, &item) + item, err := s.encDec().Decode(data) if err != nil { return nil, err } - ret = &item + ret = item } return ret, nil } @@ -144,21 +142,20 @@ func (s *Storage) Put(ctx context.Context, index uint64, prev, new *storage.Queu if err != nil { return fmt.Errorf("failed to validate item already in redis at index%v: %w", index, err) } - prevItemEncoded, err := rlp.EncodeToBytes(prev) + prevItemEncoded, err := s.encDec().Encode(prev) if err != nil { return err } if !bytes.Equal(verifiedItem, prevItemEncoded) { return fmt.Errorf("%w: replacing different item than expected at index %v", storage.ErrStorageRace, index) } - err = pipe.ZRem(ctx, s.key, haveItems[0]).Err() - if err != nil { + if err := pipe.ZRem(ctx, s.key, haveItems[0]).Err(); err != nil { return err } } else { return fmt.Errorf("expected only one return value for Put but got %v", len(haveItems)) } - newItemEncoded, err := rlp.EncodeToBytes(*new) + newItemEncoded, err := s.encDec().Encode(new) if err != nil { return err } @@ -170,11 +167,10 @@ func (s *Storage) Put(ctx context.Context, index uint64, prev, new *storage.Queu if err != nil { return err } - err = pipe.ZAdd(ctx, s.key, &redis.Z{ + if err := pipe.ZAdd(ctx, s.key, &redis.Z{ Score: float64(index), Member: string(signedItem), - }).Err() - if err != nil { + }).Err(); err != nil { return err } _, err = pipe.Exec(ctx) diff --git a/arbnode/dataposter/slice/slicestorage.go b/arbnode/dataposter/slice/slicestorage.go index 6eda5ca9a3..04286df411 100644 --- a/arbnode/dataposter/slice/slicestorage.go +++ b/arbnode/dataposter/slice/slicestorage.go @@ -9,25 +9,17 @@ import ( "errors" "fmt" - "github.com/ethereum/go-ethereum/rlp" "github.com/offchainlabs/nitro/arbnode/dataposter/storage" ) type Storage struct { firstNonce uint64 queue [][]byte + encDec func() storage.EncoderDecoderInterface } -func NewStorage() *Storage { - return &Storage{} -} - -func (s *Storage) decodeItem(data []byte) (*storage.QueuedTransaction, error) { - var item storage.QueuedTransaction - if err := rlp.DecodeBytes(data, &item); err != nil { - return nil, fmt.Errorf("decoding item: %w", err) - } - return &item, nil +func NewStorage(encDec func() storage.EncoderDecoderInterface) *Storage { + return &Storage{encDec: encDec} } func (s *Storage) FetchContents(_ context.Context, startingIndex uint64, maxResults uint64) ([]*storage.QueuedTransaction, error) { @@ -43,7 +35,7 @@ func (s *Storage) FetchContents(_ context.Context, startingIndex uint64, maxResu } var res []*storage.QueuedTransaction for _, r := range txs { - item, err := s.decodeItem(r) + item, err := s.encDec().Decode(r) if err != nil { return nil, err } @@ -56,7 +48,7 @@ func (s *Storage) FetchLast(context.Context) (*storage.QueuedTransaction, error) if len(s.queue) == 0 { return nil, nil } - return s.decodeItem(s.queue[len(s.queue)-1]) + return s.encDec().Decode(s.queue[len(s.queue)-1]) } func (s *Storage) Prune(_ context.Context, until uint64) error { @@ -73,7 +65,7 @@ func (s *Storage) Put(_ context.Context, index uint64, prev, new *storage.Queued if new == nil { return fmt.Errorf("tried to insert nil item at index %v", index) } - newEnc, err := rlp.EncodeToBytes(new) + newEnc, err := s.encDec().Encode(new) if err != nil { return fmt.Errorf("encoding new item: %w", err) } @@ -93,7 +85,7 @@ func (s *Storage) Put(_ context.Context, index uint64, prev, new *storage.Queued if queueIdx > len(s.queue) { return fmt.Errorf("attempted to set out-of-bounds index %v in queue starting at %v of length %v", index, s.firstNonce, len(s.queue)) } - prevEnc, err := rlp.EncodeToBytes(prev) + prevEnc, err := s.encDec().Encode(prev) if err != nil { return fmt.Errorf("encoding previous item: %w", err) } diff --git a/arbnode/dataposter/storage/storage.go b/arbnode/dataposter/storage/storage.go index 174ab131ac..57085c4fb1 100644 --- a/arbnode/dataposter/storage/storage.go +++ b/arbnode/dataposter/storage/storage.go @@ -2,9 +2,12 @@ package storage import ( "errors" + "fmt" "time" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/rlp" + "github.com/offchainlabs/nitro/arbutil" ) var ( @@ -24,3 +27,117 @@ type QueuedTransaction struct { Created time.Time // may be earlier than the tx was given to the tx poster NextReplacement time.Time } + +// 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 +// rlp encoding of Meta into queuedTransaction and rlp encoding it once more +// to store it. +type LegacyQueuedTransaction struct { + FullTx *types.Transaction `rlp:"nil"` + Data types.DynamicFeeTx + Meta BatchPosterPosition + Sent bool + Created time.Time // may be earlier than the tx was given to the tx poster + NextReplacement time.Time +} + +// This is also for legacy reason. Since Batchposter is in arbnode package, +// we can't refer to BatchPosterPosition type there even if we export it (that +// would create cyclic dependency). +// Ideally we'll factor out Batch Poster from arbnode into separate package +// and BatchPosterPosition into another separate package as well. +// For the sake of minimal refactoring, that struct is duplicated here. +type BatchPosterPosition struct { + MessageCount arbutil.MessageIndex + DelayedMessageCount uint64 + NextSeqNum uint64 +} + +func DecodeLegacyQueuedTransaction(data []byte) (*LegacyQueuedTransaction, error) { + var val LegacyQueuedTransaction + if err := rlp.DecodeBytes(data, &val); err != nil { + return nil, fmt.Errorf("decoding legacy queued transaction: %w", err) + } + return &val, nil +} + +func LegacyToQueuedTransaction(legacyQT *LegacyQueuedTransaction) (*QueuedTransaction, error) { + meta, err := rlp.EncodeToBytes(legacyQT.Meta) + if err != nil { + return nil, fmt.Errorf("converting legacy to queued transaction: %w", err) + } + return &QueuedTransaction{ + FullTx: legacyQT.FullTx, + Data: legacyQT.Data, + Meta: meta, + Sent: legacyQT.Sent, + Created: legacyQT.Created, + NextReplacement: legacyQT.NextReplacement, + }, nil +} + +func QueuedTransactionToLegacy(qt *QueuedTransaction) (*LegacyQueuedTransaction, error) { + if qt == nil { + return nil, nil + } + var meta BatchPosterPosition + if qt.Meta != nil { + if err := rlp.DecodeBytes(qt.Meta, &meta); err != nil { + return nil, fmt.Errorf("converting queued transaction to legacy: %w", err) + } + } + return &LegacyQueuedTransaction{ + FullTx: qt.FullTx, + Data: qt.Data, + Meta: meta, + Sent: qt.Sent, + Created: qt.Created, + NextReplacement: qt.NextReplacement, + }, nil +} + +type EncoderDecoder struct{} + +func (e *EncoderDecoder) Encode(qt *QueuedTransaction) ([]byte, error) { + return rlp.EncodeToBytes(qt) +} + +func (e *EncoderDecoder) Decode(data []byte) (*QueuedTransaction, error) { + var item QueuedTransaction + if err := rlp.DecodeBytes(data, &item); err != nil { + return nil, fmt.Errorf("decoding item: %w", err) + } + return &item, nil +} + +type LegacyEncoderDecoder struct{} + +func (e *LegacyEncoderDecoder) Encode(qt *QueuedTransaction) ([]byte, error) { + legacyQt, err := QueuedTransactionToLegacy(qt) + if err != nil { + return nil, fmt.Errorf("encoding legacy item: %w", err) + } + return rlp.EncodeToBytes(legacyQt) +} + +func (le *LegacyEncoderDecoder) Decode(data []byte) (*QueuedTransaction, error) { + val, err := DecodeLegacyQueuedTransaction(data) + if err != nil { + return nil, fmt.Errorf("decoding legacy item: %w", err) + } + return LegacyToQueuedTransaction(val) +} + +// Typically interfaces belong to where they are being used, not at implementing +// site, but this is used in all storages (besides no-op) and all of them +// require all the functions for this interface. +type EncoderDecoderInterface interface { + Encode(*QueuedTransaction) ([]byte, error) + Decode([]byte) (*QueuedTransaction, error) +} + +// EncoderDecoderF is a function type that returns encoder/decoder interface. +// This is needed to implement hot-reloading flag to switch encoding/decoding +// strategy on the fly. +type EncoderDecoderF func() EncoderDecoderInterface diff --git a/arbnode/dataposter/storage_test.go b/arbnode/dataposter/storage_test.go index 2424ac0845..eac05502be 100644 --- a/arbnode/dataposter/storage_test.go +++ b/arbnode/dataposter/storage_test.go @@ -8,6 +8,7 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/rlp" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/offchainlabs/nitro/arbnode/dataposter/leveldb" @@ -27,36 +28,41 @@ var ignoreData = cmp.Options{ cmpopts.IgnoreFields(types.Transaction{}, "hash", "size", "from"), } -func newLevelDBStorage(t *testing.T) *leveldb.Storage { +func newLevelDBStorage(t *testing.T, encF storage.EncoderDecoderF) *leveldb.Storage { t.Helper() db, err := rawdb.NewLevelDBDatabase(path.Join(t.TempDir(), "level.db"), 0, 0, "default", false) if err != nil { t.Fatalf("NewLevelDBDatabase() unexpected error: %v", err) } - return leveldb.New(db) + return leveldb.New(db, encF) } -func newSliceStorage() *slice.Storage { - return slice.NewStorage() +func newSliceStorage(encF storage.EncoderDecoderF) *slice.Storage { + return slice.NewStorage(encF) } -func newRedisStorage(ctx context.Context, t *testing.T) *redis.Storage { +func newRedisStorage(ctx context.Context, t *testing.T, encF storage.EncoderDecoderF) *redis.Storage { t.Helper() redisUrl := redisutil.CreateTestRedis(ctx, t) client, err := redisutil.RedisClientFromURL(redisUrl) if err != nil { t.Fatalf("RedisClientFromURL(%q) unexpected error: %v", redisUrl, err) } - s, err := redis.NewStorage(client, "", &signature.TestSimpleHmacConfig) + s, err := redis.NewStorage(client, "", &signature.TestSimpleHmacConfig, encF) if err != nil { t.Fatalf("redis.NewStorage() unexpected error: %v", err) } return s } -func valueOf(i int) *storage.QueuedTransaction { +func valueOf(t *testing.T, i int) *storage.QueuedTransaction { + t.Helper() + meta, err := rlp.EncodeToBytes(storage.BatchPosterPosition{DelayedMessageCount: uint64(i)}) + if err != nil { + t.Fatalf("Encoding batch poster position, error: %v", err) + } return &storage.QueuedTransaction{ - Meta: []byte{byte(i)}, + Meta: meta, Data: types.DynamicFeeTx{ ChainID: big.NewInt(int64(i)), Nonce: uint64(i), @@ -73,10 +79,10 @@ func valueOf(i int) *storage.QueuedTransaction { } } -func values(from, to int) []*storage.QueuedTransaction { +func values(t *testing.T, from, to int) []*storage.QueuedTransaction { var res []*storage.QueuedTransaction for i := from; i <= to; i++ { - res = append(res, valueOf(i)) + res = append(res, valueOf(t, i)) } return res } @@ -85,7 +91,7 @@ func values(from, to int) []*storage.QueuedTransaction { func initStorage(ctx context.Context, t *testing.T, s QueueStorage) QueueStorage { t.Helper() for i := 0; i < 20; i++ { - if err := s.Put(ctx, uint64(i), nil, valueOf(i)); err != nil { + if err := s.Put(ctx, uint64(i), nil, valueOf(t, i)); err != nil { t.Fatalf("Error putting a key/value: %v", err) } } @@ -95,10 +101,18 @@ func initStorage(ctx context.Context, t *testing.T, s QueueStorage) QueueStorage // Returns a map of all empty storages. func storages(t *testing.T) map[string]QueueStorage { t.Helper() + f := func(enc storage.EncoderDecoderInterface) storage.EncoderDecoderF { + return func() storage.EncoderDecoderInterface { + return enc + } + } return map[string]QueueStorage{ - "levelDB": newLevelDBStorage(t), - "slice": newSliceStorage(), - "redis": newRedisStorage(context.Background(), t), + "levelDBLegacy": newLevelDBStorage(t, f(&storage.LegacyEncoderDecoder{})), + "sliceLegacy": newSliceStorage(f(&storage.LegacyEncoderDecoder{})), + "redisLegacy": newRedisStorage(context.Background(), t, f(&storage.LegacyEncoderDecoder{})), + "levelDB": newLevelDBStorage(t, f(&storage.EncoderDecoder{})), + "slice": newSliceStorage(f(&storage.EncoderDecoder{})), + "redis": newRedisStorage(context.Background(), t, f(&storage.EncoderDecoder{})), } } @@ -125,13 +139,13 @@ func TestFetchContents(t *testing.T) { desc: "sequence with single digits", startIdx: 5, maxResults: 3, - want: values(5, 7), + want: values(t, 5, 7), }, { desc: "corner case of single element", startIdx: 0, maxResults: 1, - want: values(0, 0), + want: values(t, 0, 0), }, { desc: "no elements", @@ -143,13 +157,13 @@ func TestFetchContents(t *testing.T) { desc: "sequence with variable number of digits", startIdx: 9, maxResults: 3, - want: values(9, 11), + want: values(t, 9, 11), }, { desc: "max results goes over the last element", startIdx: 13, maxResults: 10, - want: values(13, 19), + want: values(t, 13, 19), }, } { t.Run(name+"_"+tc.desc, func(t *testing.T) { @@ -171,7 +185,7 @@ func TestLast(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := context.Background() for i := 0; i < cnt; i++ { - val := valueOf(i) + val := valueOf(t, i) if err := s.Put(ctx, uint64(i), nil, val); err != nil { t.Fatalf("Error putting a key/value: %v", err) } @@ -185,12 +199,12 @@ func TestLast(t *testing.T) { } }) - last := valueOf(cnt - 1) + last := valueOf(t, cnt-1) t.Run(name+"_update_entries", func(t *testing.T) { ctx := context.Background() for i := 0; i < cnt-1; i++ { - prev := valueOf(i) - newVal := valueOf(cnt + i) + prev := valueOf(t, i) + newVal := valueOf(t, cnt+i) if err := s.Put(ctx, uint64(i), prev, newVal); err != nil { t.Fatalf("Error putting a key/value: %v, prev: %v, new: %v", err, prev, newVal) } @@ -227,17 +241,17 @@ func TestPrune(t *testing.T) { { desc: "prune all but one", pruneFrom: 19, - want: values(19, 19), + want: values(t, 19, 19), }, { desc: "pruning first element", pruneFrom: 1, - want: values(1, 19), + want: values(t, 1, 19), }, { desc: "pruning first 11 elements", pruneFrom: 11, - want: values(11, 19), + want: values(t, 11, 19), }, { desc: "pruning from higher than biggest index", From bb08e9d3fb5a5424f7e5885e411733978a2df736 Mon Sep 17 00:00:00 2001 From: Nodar Date: Wed, 30 Aug 2023 18:51:23 +0200 Subject: [PATCH 08/22] Use new encoding for slicestorage, attempt both decodings no matter what encoding is enabled --- arbnode/dataposter/data_poster.go | 2 +- arbnode/dataposter/storage/storage.go | 29 ++++++++++++++++++--------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index adfde88f74..a289a17626 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -114,7 +114,7 @@ func NewDataPoster(db ethdb.Database, headerReader *headerreader.HeaderReader, a case initConfig.UseLevelDB: queue = leveldb.New(db, encF) case redisClient == nil: - queue = slice.NewStorage(encF) + queue = slice.NewStorage(func() storage.EncoderDecoderInterface { return &storage.EncoderDecoder{} }) default: var err error queue, err = redisstorage.NewStorage(redisClient, "data-poster.queue", &initConfig.RedisSigner, encF) diff --git a/arbnode/dataposter/storage/storage.go b/arbnode/dataposter/storage/storage.go index 57085c4fb1..a132f375e5 100644 --- a/arbnode/dataposter/storage/storage.go +++ b/arbnode/dataposter/storage/storage.go @@ -6,6 +6,7 @@ import ( "time" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" "github.com/offchainlabs/nitro/arbutil" ) @@ -97,6 +98,22 @@ func QueuedTransactionToLegacy(qt *QueuedTransaction) (*LegacyQueuedTransaction, }, nil } +// Decode tries to decode QueuedTransaction, if that fails it tries to decode +// into legacy queued transaction and converts to queued +func decode(data []byte) (*QueuedTransaction, error) { + var item QueuedTransaction + if err := rlp.DecodeBytes(data, &item); err != nil { + log.Warn("Failed to decode QueuedTransaction, attempting to decide legacy queued transaction", "error", err) + val, err := DecodeLegacyQueuedTransaction(data) + if err != nil { + return nil, fmt.Errorf("decoding legacy item: %w", err) + } + return LegacyToQueuedTransaction(val) + } + return &item, nil + +} + type EncoderDecoder struct{} func (e *EncoderDecoder) Encode(qt *QueuedTransaction) ([]byte, error) { @@ -104,11 +121,7 @@ func (e *EncoderDecoder) Encode(qt *QueuedTransaction) ([]byte, error) { } func (e *EncoderDecoder) Decode(data []byte) (*QueuedTransaction, error) { - var item QueuedTransaction - if err := rlp.DecodeBytes(data, &item); err != nil { - return nil, fmt.Errorf("decoding item: %w", err) - } - return &item, nil + return decode(data) } type LegacyEncoderDecoder struct{} @@ -122,11 +135,7 @@ func (e *LegacyEncoderDecoder) Encode(qt *QueuedTransaction) ([]byte, error) { } func (le *LegacyEncoderDecoder) Decode(data []byte) (*QueuedTransaction, error) { - val, err := DecodeLegacyQueuedTransaction(data) - if err != nil { - return nil, fmt.Errorf("decoding legacy item: %w", err) - } - return LegacyToQueuedTransaction(val) + return decode(data) } // Typically interfaces belong to where they are being used, not at implementing From 2e6d59ac0c3bfa6679011c974d37fdd16fad2992 Mon Sep 17 00:00:00 2001 From: Nodar Date: Wed, 30 Aug 2023 18:52:20 +0200 Subject: [PATCH 09/22] drop empty line --- arbnode/dataposter/storage/storage.go | 1 - 1 file changed, 1 deletion(-) diff --git a/arbnode/dataposter/storage/storage.go b/arbnode/dataposter/storage/storage.go index a132f375e5..ed848b9b7d 100644 --- a/arbnode/dataposter/storage/storage.go +++ b/arbnode/dataposter/storage/storage.go @@ -111,7 +111,6 @@ func decode(data []byte) (*QueuedTransaction, error) { return LegacyToQueuedTransaction(val) } return &item, nil - } type EncoderDecoder struct{} From 06930b67bf24630f002abbe523c2ae9e3fd71362 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 30 Aug 2023 12:56:15 -0600 Subject: [PATCH 10/22] Fix pollForReverts channel reading --- arbnode/batch_poster.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 429701be7e..7efa2b3f73 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -309,8 +309,8 @@ func (b *BatchPoster) pollForReverts(ctx context.Context) { // - polling is through context, or // - we see a transaction in the block from dataposter that was reverted. select { - case h, closed := <-headerCh: - if closed { + case h, ok := <-headerCh: + if !ok { log.Info("L1 headers channel has been closed") return } From 5ce0f8164c3b05c2056cc22083cc1e2ebdc6d33d Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 30 Aug 2023 14:26:51 -0600 Subject: [PATCH 11/22] Disable wait-for-l1-finality on L3 data posters --- arbnode/dataposter/data_poster.go | 39 +++++++++++++++++-------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index b1db655d71..f20d7dd597 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -162,11 +162,14 @@ func (p *DataPoster) canPostWithNonce(ctx context.Context, nextNonce uint64) err return nil } +func (p *DataPoster) waitForL1Finality() bool { + return p.config().WaitForL1Finality && !p.headerReader.IsParentChainArbitrum() +} + // Requires the caller hold the mutex. // Returns the next nonce, its metadata if stored, a bool indicating if the metadata is present, and an error. // Unlike GetNextNonceAndMeta, this does not call the metadataRetriever if the metadata is not stored in the queue. func (p *DataPoster) getNextNonceAndMaybeMeta(ctx context.Context) (uint64, []byte, bool, error) { - config := p.config() // Ensure latest finalized block state is available. blockNum, err := p.client.BlockNumber(ctx) if err != nil { @@ -185,7 +188,7 @@ func (p *DataPoster) getNextNonceAndMaybeMeta(ctx context.Context) (uint64, []by } if err := p.updateNonce(ctx); err != nil { - if !p.queue.IsPersistent() && config.WaitForL1Finality { + if !p.queue.IsPersistent() && p.waitForL1Finality() { return 0, nil, false, fmt.Errorf("error getting latest finalized nonce (and queue is not persistent): %w", err) } // Fall back to using a recent block to get the nonce. This is safe because there's nothing in the queue. @@ -433,7 +436,7 @@ func (p *DataPoster) replaceTx(ctx context.Context, prevTx *storage.QueuedTransa // The mutex must be held by the caller. func (p *DataPoster) updateNonce(ctx context.Context) error { var blockNumQuery *big.Int - if p.config().WaitForL1Finality { + if p.waitForL1Finality() { blockNumQuery = big.NewInt(int64(rpc.FinalizedBlockNumber)) } header, err := p.client.HeaderByNumber(ctx, blockNumQuery) @@ -602,20 +605,22 @@ type QueueStorage interface { } type DataPosterConfig struct { - RedisSigner signature.SimpleHmacConfig `koanf:"redis-signer"` - ReplacementTimes string `koanf:"replacement-times"` - WaitForL1Finality bool `koanf:"wait-for-l1-finality" reload:"hot"` - MaxMempoolTransactions uint64 `koanf:"max-mempool-transactions" reload:"hot"` - MaxQueuedTransactions int `koanf:"max-queued-transactions" reload:"hot"` - TargetPriceGwei float64 `koanf:"target-price-gwei" reload:"hot"` - UrgencyGwei float64 `koanf:"urgency-gwei" reload:"hot"` - MinFeeCapGwei float64 `koanf:"min-fee-cap-gwei" reload:"hot"` - MinTipCapGwei float64 `koanf:"min-tip-cap-gwei" reload:"hot"` - MaxTipCapGwei float64 `koanf:"max-tip-cap-gwei" reload:"hot"` - NonceRbfSoftConfs uint64 `koanf:"nonce-rbf-soft-confs" reload:"hot"` - AllocateMempoolBalance bool `koanf:"allocate-mempool-balance" reload:"hot"` - UseLevelDB bool `koanf:"use-leveldb"` - UseNoOpStorage bool `koanf:"use-noop-storage"` + RedisSigner signature.SimpleHmacConfig `koanf:"redis-signer"` + ReplacementTimes string `koanf:"replacement-times"` + // This is forcibly disabled if the parent chain is an Arbitrum chain, + // so you should probably use DataPoster's waitForL1Finality method instead of reading this field directly. + WaitForL1Finality bool `koanf:"wait-for-l1-finality" reload:"hot"` + MaxMempoolTransactions uint64 `koanf:"max-mempool-transactions" reload:"hot"` + MaxQueuedTransactions int `koanf:"max-queued-transactions" reload:"hot"` + TargetPriceGwei float64 `koanf:"target-price-gwei" reload:"hot"` + UrgencyGwei float64 `koanf:"urgency-gwei" reload:"hot"` + MinFeeCapGwei float64 `koanf:"min-fee-cap-gwei" reload:"hot"` + MinTipCapGwei float64 `koanf:"min-tip-cap-gwei" reload:"hot"` + MaxTipCapGwei float64 `koanf:"max-tip-cap-gwei" reload:"hot"` + NonceRbfSoftConfs uint64 `koanf:"nonce-rbf-soft-confs" reload:"hot"` + AllocateMempoolBalance bool `koanf:"allocate-mempool-balance" reload:"hot"` + UseLevelDB bool `koanf:"use-leveldb"` + UseNoOpStorage bool `koanf:"use-noop-storage"` } // ConfigFetcher function type is used instead of directly passing config so From 370ac231089a56649372e7bbe21ce3c26cefda80 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 30 Aug 2023 20:43:12 -0600 Subject: [PATCH 12/22] Fix off by 1 in validator logging --- staker/block_validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staker/block_validator.go b/staker/block_validator.go index f04b852041..94bc2a0806 100644 --- a/staker/block_validator.go +++ b/staker/block_validator.go @@ -597,7 +597,7 @@ func (v *BlockValidator) iterativeValidationPrint(ctx context.Context) time.Dura var batchMsgs arbutil.MessageIndex var printedCount int64 if validated.GlobalState.Batch > 0 { - batchMsgs, err = v.inboxTracker.GetBatchMessageCount(validated.GlobalState.Batch) + batchMsgs, err = v.inboxTracker.GetBatchMessageCount(validated.GlobalState.Batch - 1) } if err != nil { printedCount = -1 From a7e26b2c038471bcd11831fbcc517238114efbb9 Mon Sep 17 00:00:00 2001 From: Nodar Date: Thu, 31 Aug 2023 15:30:04 +0200 Subject: [PATCH 13/22] Drop normalize method in koanf.go, ignore case when comparing tag and a field --- linter/koanf/koanf.go | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/linter/koanf/koanf.go b/linter/koanf/koanf.go index bc94a9c20e..2127fb23b0 100644 --- a/linter/koanf/koanf.go +++ b/linter/koanf/koanf.go @@ -6,7 +6,6 @@ import ( "go/token" "reflect" "strings" - "unicode" "github.com/fatih/structtag" "golang.org/x/tools/go/analysis" @@ -84,7 +83,7 @@ func checkStruct(pass *analysis.Pass, s *ast.StructType) Result { if err != nil { continue } - tagName := normalize(tag.Name) + tagName := strings.ReplaceAll(tag.Name, "-", "") fieldName := f.Names[0].Name if !strings.EqualFold(tagName, fieldName) { res.Errors = append(res.Errors, koanfError{ @@ -96,25 +95,6 @@ func checkStruct(pass *analysis.Pass, s *ast.StructType) Result { return res } -func normalize(s string) string { - ans := s[:1] - for i := 1; i < len(s); i++ { - c := rune(s[i]) - if !isAlphanumeric(c) { - continue - } - if !isAlphanumeric(rune(s[i-1])) && unicode.IsLower(c) { - c = unicode.ToUpper(c) - } - ans += string(c) - } - return ans -} - -func isAlphanumeric(c rune) bool { - return unicode.IsLetter(c) || unicode.IsDigit(c) -} - func main() { singlechecker.Main(Analyzer) } From 4d2118660a75fb1dcf04491e07049a2041d3fd83 Mon Sep 17 00:00:00 2001 From: Nodar Date: Thu, 31 Aug 2023 15:38:54 +0200 Subject: [PATCH 14/22] drop tag normalization in koanf.go --- linter/koanf/handlers.go | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/linter/koanf/handlers.go b/linter/koanf/handlers.go index 2381a230ee..5826004014 100644 --- a/linter/koanf/handlers.go +++ b/linter/koanf/handlers.go @@ -5,7 +5,6 @@ import ( "go/ast" "go/token" "strings" - "unicode" "github.com/fatih/structtag" "golang.org/x/tools/go/analysis" @@ -195,7 +194,7 @@ func tagFromField(f *ast.Field) string { if err != nil { return "" } - return strings.ReplaceAll(tag.Name, "-", "") + return normalizeTag(tag.Name) } // checkStruct returns violations where koanf tag name doesn't match field names. @@ -219,22 +218,7 @@ func checkStruct(pass *analysis.Pass, s *ast.StructType) Result { } func normalizeTag(s string) string { - ans := s[:1] - for i := 1; i < len(s); i++ { - c := rune(s[i]) - if !isAlphanumeric(c) { - continue - } - if !isAlphanumeric(rune(s[i-1])) && unicode.IsLower(c) { - c = unicode.ToUpper(c) - } - ans += string(c) - } - return ans -} - -func isAlphanumeric(c rune) bool { - return unicode.IsLetter(c) || unicode.IsDigit(c) + return strings.ReplaceAll(s, "-", "") } func normalizeID(pass *analysis.Pass, id string) string { From 8cb2606040a993d7fd1f6cf08e5af842ff1b2d1d Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Thu, 31 Aug 2023 10:49:42 -0500 Subject: [PATCH 15/22] add new type Uint64OrHex --- go-ethereum | 2 +- system_tests/conditionaltx_test.go | 29 ++++++++++++++--------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/go-ethereum b/go-ethereum index c905292f8a..f8363dc42d 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit c905292f8af601f7fca261e65a7d4bc144261e29 +Subproject commit f8363dc42d6cf8dbd76aed2550207a90919e88ee diff --git a/system_tests/conditionaltx_test.go b/system_tests/conditionaltx_test.go index c65103694a..b758ba1807 100644 --- a/system_tests/conditionaltx_test.go +++ b/system_tests/conditionaltx_test.go @@ -16,7 +16,6 @@ import ( "github.com/ethereum/go-ethereum/arbitrum" "github.com/ethereum/go-ethereum/arbitrum_types" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/rpc" @@ -103,23 +102,23 @@ func getOptions(address common.Address, rootHash common.Hash, slotValueMap map[c } func getFulfillableBlockTimeLimits(t *testing.T, blockNumber uint64, timestamp uint64) []*arbitrum_types.ConditionalOptions { - future := hexutil.Uint64(timestamp + 30) - past := hexutil.Uint64(timestamp - 1) - futureBlockNumber := hexutil.Uint64(blockNumber + 1000) - currentBlockNumber := hexutil.Uint64(blockNumber) + future := common.Uint64OrHex(timestamp + 30) + past := common.Uint64OrHex(timestamp - 1) + futureBlockNumber := common.Uint64OrHex(blockNumber + 1000) + currentBlockNumber := common.Uint64OrHex(blockNumber) return getBlockTimeLimits(t, currentBlockNumber, futureBlockNumber, past, future) } func getUnfulfillableBlockTimeLimits(t *testing.T, blockNumber uint64, timestamp uint64) []*arbitrum_types.ConditionalOptions { - future := hexutil.Uint64(timestamp + 30) - past := hexutil.Uint64(timestamp - 1) - futureBlockNumber := hexutil.Uint64(blockNumber + 1000) - previousBlockNumber := hexutil.Uint64(blockNumber - 1) + future := common.Uint64OrHex(timestamp + 30) + past := common.Uint64OrHex(timestamp - 1) + futureBlockNumber := common.Uint64OrHex(blockNumber + 1000) + previousBlockNumber := common.Uint64OrHex(blockNumber - 1) // skip first empty options return getBlockTimeLimits(t, futureBlockNumber, previousBlockNumber, future, past)[1:] } -func getBlockTimeLimits(t *testing.T, blockMin, blockMax hexutil.Uint64, timeMin, timeMax hexutil.Uint64) []*arbitrum_types.ConditionalOptions { +func getBlockTimeLimits(t *testing.T, blockMin, blockMax common.Uint64OrHex, timeMin, timeMax common.Uint64OrHex) []*arbitrum_types.ConditionalOptions { basic := []*arbitrum_types.ConditionalOptions{ {}, {TimestampMin: &timeMin}, @@ -157,9 +156,9 @@ func optionsProduct(optionsA, optionsB []*arbitrum_types.ConditionalOptions) []* c.KnownAccounts[k] = v } limitTriples := []struct { - a *hexutil.Uint64 - b *hexutil.Uint64 - c **hexutil.Uint64 + a *common.Uint64OrHex + b *common.Uint64OrHex + c **common.Uint64OrHex }{ {a.BlockNumberMin, b.BlockNumberMin, &c.BlockNumberMin}, {a.BlockNumberMax, b.BlockNumberMax, &c.BlockNumberMax}, @@ -168,10 +167,10 @@ func optionsProduct(optionsA, optionsB []*arbitrum_types.ConditionalOptions) []* } for _, tripple := range limitTriples { if tripple.b != nil { - value := hexutil.Uint64(*tripple.b) + value := common.Uint64OrHex(*tripple.b) *tripple.c = &value } else if tripple.a != nil { - value := hexutil.Uint64(*tripple.a) + value := common.Uint64OrHex(*tripple.a) *tripple.c = &value } else { *tripple.c = nil From 1f6ddc4f16497f80a203fdfb8f7c4cca04bfba93 Mon Sep 17 00:00:00 2001 From: Nodar Date: Thu, 31 Aug 2023 17:55:40 +0200 Subject: [PATCH 16/22] Change warning to debug when decoding queued transaction fails, add another debug line when it succeeds --- arbnode/dataposter/storage/storage.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arbnode/dataposter/storage/storage.go b/arbnode/dataposter/storage/storage.go index ed848b9b7d..734e2770ee 100644 --- a/arbnode/dataposter/storage/storage.go +++ b/arbnode/dataposter/storage/storage.go @@ -103,11 +103,12 @@ func QueuedTransactionToLegacy(qt *QueuedTransaction) (*LegacyQueuedTransaction, func decode(data []byte) (*QueuedTransaction, error) { var item QueuedTransaction if err := rlp.DecodeBytes(data, &item); err != nil { - log.Warn("Failed to decode QueuedTransaction, attempting to decide legacy queued transaction", "error", err) + log.Debug("Failed to decode QueuedTransaction, attempting to decide legacy queued transaction", "error", err) val, err := DecodeLegacyQueuedTransaction(data) if err != nil { return nil, fmt.Errorf("decoding legacy item: %w", err) } + log.Debug("Succeeded decoding QueuedTransaction with legacy encoder") return LegacyToQueuedTransaction(val) } return &item, nil From 8f3f8162b88f22e3ef56e3ebbc6ba627b10b8ecb Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Thu, 31 Aug 2023 12:59:35 -0500 Subject: [PATCH 17/22] reuse type math.HexOrDecimal64 --- go-ethereum | 2 +- system_tests/conditionaltx_test.go | 29 +++++++++++++++-------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/go-ethereum b/go-ethereum index f8363dc42d..b4bd0da114 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit f8363dc42d6cf8dbd76aed2550207a90919e88ee +Subproject commit b4bd0da1142fe6bb81cac7e0794ebb4746b9885a diff --git a/system_tests/conditionaltx_test.go b/system_tests/conditionaltx_test.go index b758ba1807..14aa000313 100644 --- a/system_tests/conditionaltx_test.go +++ b/system_tests/conditionaltx_test.go @@ -16,6 +16,7 @@ import ( "github.com/ethereum/go-ethereum/arbitrum" "github.com/ethereum/go-ethereum/arbitrum_types" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/rpc" @@ -102,23 +103,23 @@ func getOptions(address common.Address, rootHash common.Hash, slotValueMap map[c } func getFulfillableBlockTimeLimits(t *testing.T, blockNumber uint64, timestamp uint64) []*arbitrum_types.ConditionalOptions { - future := common.Uint64OrHex(timestamp + 30) - past := common.Uint64OrHex(timestamp - 1) - futureBlockNumber := common.Uint64OrHex(blockNumber + 1000) - currentBlockNumber := common.Uint64OrHex(blockNumber) + future := math.HexOrDecimal64(timestamp + 30) + past := math.HexOrDecimal64(timestamp - 1) + futureBlockNumber := math.HexOrDecimal64(blockNumber + 1000) + currentBlockNumber := math.HexOrDecimal64(blockNumber) return getBlockTimeLimits(t, currentBlockNumber, futureBlockNumber, past, future) } func getUnfulfillableBlockTimeLimits(t *testing.T, blockNumber uint64, timestamp uint64) []*arbitrum_types.ConditionalOptions { - future := common.Uint64OrHex(timestamp + 30) - past := common.Uint64OrHex(timestamp - 1) - futureBlockNumber := common.Uint64OrHex(blockNumber + 1000) - previousBlockNumber := common.Uint64OrHex(blockNumber - 1) + future := math.HexOrDecimal64(timestamp + 30) + past := math.HexOrDecimal64(timestamp - 1) + futureBlockNumber := math.HexOrDecimal64(blockNumber + 1000) + previousBlockNumber := math.HexOrDecimal64(blockNumber - 1) // skip first empty options return getBlockTimeLimits(t, futureBlockNumber, previousBlockNumber, future, past)[1:] } -func getBlockTimeLimits(t *testing.T, blockMin, blockMax common.Uint64OrHex, timeMin, timeMax common.Uint64OrHex) []*arbitrum_types.ConditionalOptions { +func getBlockTimeLimits(t *testing.T, blockMin, blockMax math.HexOrDecimal64, timeMin, timeMax math.HexOrDecimal64) []*arbitrum_types.ConditionalOptions { basic := []*arbitrum_types.ConditionalOptions{ {}, {TimestampMin: &timeMin}, @@ -156,9 +157,9 @@ func optionsProduct(optionsA, optionsB []*arbitrum_types.ConditionalOptions) []* c.KnownAccounts[k] = v } limitTriples := []struct { - a *common.Uint64OrHex - b *common.Uint64OrHex - c **common.Uint64OrHex + a *math.HexOrDecimal64 + b *math.HexOrDecimal64 + c **math.HexOrDecimal64 }{ {a.BlockNumberMin, b.BlockNumberMin, &c.BlockNumberMin}, {a.BlockNumberMax, b.BlockNumberMax, &c.BlockNumberMax}, @@ -167,10 +168,10 @@ func optionsProduct(optionsA, optionsB []*arbitrum_types.ConditionalOptions) []* } for _, tripple := range limitTriples { if tripple.b != nil { - value := common.Uint64OrHex(*tripple.b) + value := math.HexOrDecimal64(*tripple.b) *tripple.c = &value } else if tripple.a != nil { - value := common.Uint64OrHex(*tripple.a) + value := math.HexOrDecimal64(*tripple.a) *tripple.c = &value } else { *tripple.c = nil From d23602ffc8215367d7705c0e4867cb357949ba84 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 31 Aug 2023 19:42:15 -0600 Subject: [PATCH 18/22] Fix batch poster fixAccErr not resetting when the error stops --- arbnode/batch_poster.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 429701be7e..53cd5d5594 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -970,10 +970,14 @@ func (b *BatchPoster) Start(ctxIn context.Context) { return b.config().PollInterval } posted, err := b.maybePostSequencerBatch(ctx) + ephemeralError := errors.Is(err, AccumulatorNotFoundErr) || errors.Is(err, storage.ErrStorageRace) + if !ephemeralError { + b.firstAccErr = time.Time{} + } if err != nil { b.building = nil logLevel := log.Error - if errors.Is(err, AccumulatorNotFoundErr) || errors.Is(err, storage.ErrStorageRace) { + if ephemeralError { // Likely the inbox tracker just isn't caught up. // Let's see if this error disappears naturally. if b.firstAccErr == (time.Time{}) { @@ -982,8 +986,6 @@ func (b *BatchPoster) Start(ctxIn context.Context) { } else if time.Since(b.firstAccErr) < time.Minute { logLevel = log.Debug } - } else { - b.firstAccErr = time.Time{} } logLevel("error posting batch", "err", err) return b.config().ErrorDelay From 053e13a4d81da7e417be72c70f023bf86aa11189 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 31 Aug 2023 19:47:50 -0600 Subject: [PATCH 19/22] Rename firstAccErr to firstEphemeralError --- arbnode/batch_poster.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 53cd5d5594..43cf97f2ae 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -56,20 +56,20 @@ type batchPosterPosition struct { type BatchPoster struct { stopwaiter.StopWaiter - l1Reader *headerreader.HeaderReader - inbox *InboxTracker - streamer *TransactionStreamer - config BatchPosterConfigFetcher - seqInbox *bridgegen.SequencerInbox - bridge *bridgegen.Bridge - syncMonitor *SyncMonitor - seqInboxABI *abi.ABI - seqInboxAddr common.Address - building *buildingBatch - daWriter das.DataAvailabilityServiceWriter - dataPoster *dataposter.DataPoster - redisLock *redislock.Simple - firstAccErr time.Time // first time a continuous missing accumulator occurred + l1Reader *headerreader.HeaderReader + inbox *InboxTracker + streamer *TransactionStreamer + config BatchPosterConfigFetcher + seqInbox *bridgegen.SequencerInbox + bridge *bridgegen.Bridge + syncMonitor *SyncMonitor + seqInboxABI *abi.ABI + seqInboxAddr common.Address + building *buildingBatch + daWriter das.DataAvailabilityServiceWriter + dataPoster *dataposter.DataPoster + redisLock *redislock.Simple + firstEphemeralError time.Time // first time a continuous error suspected to be ephemeral occurred // An estimate of the number of batches we want to post but haven't yet. // This doesn't include batches which we don't want to post yet due to the L1 bounds. backlog uint64 @@ -972,7 +972,7 @@ func (b *BatchPoster) Start(ctxIn context.Context) { posted, err := b.maybePostSequencerBatch(ctx) ephemeralError := errors.Is(err, AccumulatorNotFoundErr) || errors.Is(err, storage.ErrStorageRace) if !ephemeralError { - b.firstAccErr = time.Time{} + b.firstEphemeralError = time.Time{} } if err != nil { b.building = nil @@ -980,10 +980,10 @@ func (b *BatchPoster) Start(ctxIn context.Context) { if ephemeralError { // Likely the inbox tracker just isn't caught up. // Let's see if this error disappears naturally. - if b.firstAccErr == (time.Time{}) { - b.firstAccErr = time.Now() + if b.firstEphemeralError == (time.Time{}) { + b.firstEphemeralError = time.Now() logLevel = log.Debug - } else if time.Since(b.firstAccErr) < time.Minute { + } else if time.Since(b.firstEphemeralError) < time.Minute { logLevel = log.Debug } } From 2f9db18cb552cec9647c9c6dfd9cbe4d7ef079b9 Mon Sep 17 00:00:00 2001 From: Nodar Date: Fri, 1 Sep 2023 16:34:51 +0200 Subject: [PATCH 20/22] Compare correct encodings in redis storage, drop nil rlp tag from queuedTransaction struct --- arbnode/dataposter/data_poster.go | 2 +- arbnode/dataposter/redis/redisstorage.go | 15 +++++++++++++++ arbnode/dataposter/storage/storage.go | 8 +++----- arbnode/dataposter/storage_test.go | 9 +++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 5231a1c332..dff2602cac 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -112,7 +112,7 @@ func NewDataPoster(db ethdb.Database, headerReader *headerreader.HeaderReader, a case initConfig.UseNoOpStorage: queue = &noop.Storage{} case initConfig.UseLevelDB: - queue = leveldb.New(db, encF) + queue = leveldb.New(db, func() storage.EncoderDecoderInterface { return &storage.EncoderDecoder{} }) case redisClient == nil: queue = slice.NewStorage(func() storage.EncoderDecoderInterface { return &storage.EncoderDecoder{} }) default: diff --git a/arbnode/dataposter/redis/redisstorage.go b/arbnode/dataposter/redis/redisstorage.go index 5123cea154..f2393611b2 100644 --- a/arbnode/dataposter/redis/redisstorage.go +++ b/arbnode/dataposter/redis/redisstorage.go @@ -114,6 +114,17 @@ func (s *Storage) Prune(ctx context.Context, until uint64) error { return nil } +// normalizeDecoding decodes data (regardless of what encoding it used), and +// encodes it according to current encoding for storage. +// As a result, encoded data is transformed to currently used encoding. +func (s *Storage) normalizeDecoding(data []byte) ([]byte, error) { + item, err := s.encDec().Decode(data) + if err != nil { + return nil, err + } + return s.encDec().Encode(item) +} + func (s *Storage) Put(ctx context.Context, index uint64, prev, new *storage.QueuedTransaction) error { if new == nil { return fmt.Errorf("tried to insert nil item at index %v", index) @@ -142,6 +153,10 @@ func (s *Storage) Put(ctx context.Context, index uint64, prev, new *storage.Queu if err != nil { return fmt.Errorf("failed to validate item already in redis at index%v: %w", index, err) } + verifiedItem, err = s.normalizeDecoding(verifiedItem) + if err != nil { + return fmt.Errorf("error normalizing encoding for verified item: %w", err) + } prevItemEncoded, err := s.encDec().Encode(prev) if err != nil { return err diff --git a/arbnode/dataposter/storage/storage.go b/arbnode/dataposter/storage/storage.go index 734e2770ee..b59bf7bf62 100644 --- a/arbnode/dataposter/storage/storage.go +++ b/arbnode/dataposter/storage/storage.go @@ -21,7 +21,7 @@ var ( ) type QueuedTransaction struct { - FullTx *types.Transaction `rlp:"nil"` + FullTx *types.Transaction Data types.DynamicFeeTx Meta []byte Sent bool @@ -35,7 +35,7 @@ type QueuedTransaction struct { // rlp encoding of Meta into queuedTransaction and rlp encoding it once more // to store it. type LegacyQueuedTransaction struct { - FullTx *types.Transaction `rlp:"nil"` + FullTx *types.Transaction Data types.DynamicFeeTx Meta BatchPosterPosition Sent bool @@ -46,9 +46,7 @@ type LegacyQueuedTransaction struct { // This is also for legacy reason. Since Batchposter is in arbnode package, // we can't refer to BatchPosterPosition type there even if we export it (that // would create cyclic dependency). -// Ideally we'll factor out Batch Poster from arbnode into separate package -// and BatchPosterPosition into another separate package as well. -// For the sake of minimal refactoring, that struct is duplicated here. +// We'll drop this struct in a few releases when we drop legacy encoding. type BatchPosterPosition struct { MessageCount arbutil.MessageIndex DelayedMessageCount uint64 diff --git a/arbnode/dataposter/storage_test.go b/arbnode/dataposter/storage_test.go index eac05502be..d536e5da05 100644 --- a/arbnode/dataposter/storage_test.go +++ b/arbnode/dataposter/storage_test.go @@ -6,6 +6,7 @@ import ( "path" "testing" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/rlp" @@ -22,6 +23,7 @@ import ( var ignoreData = cmp.Options{ cmpopts.IgnoreUnexported( + types.Transaction{}, types.DynamicFeeTx{}, big.Int{}, ), @@ -62,6 +64,13 @@ func valueOf(t *testing.T, i int) *storage.QueuedTransaction { t.Fatalf("Encoding batch poster position, error: %v", err) } return &storage.QueuedTransaction{ + FullTx: types.NewTransaction( + uint64(i), + common.Address{}, + big.NewInt(int64(i)), + uint64(i), + big.NewInt(int64(i)), + []byte{byte(i)}), Meta: meta, Data: types.DynamicFeeTx{ ChainID: big.NewInt(int64(i)), From 0bf724b80497f821ff31fc36519ffe53b37d9676 Mon Sep 17 00:00:00 2001 From: Nodar Date: Fri, 1 Sep 2023 17:09:28 +0200 Subject: [PATCH 21/22] Fix incorrect merge with base pr --- linter/koanf/koanf.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linter/koanf/koanf.go b/linter/koanf/koanf.go index d483cf7751..c7c38e2571 100644 --- a/linter/koanf/koanf.go +++ b/linter/koanf/koanf.go @@ -116,7 +116,7 @@ func checkFlagDefs(pass *analysis.Pass, f *ast.FuncDecl) Result { if !ok { continue } - if normSL := normalize(sl); !strings.EqualFold(normSL, s) { + if normSL := strings.ReplaceAll(sl, "-", ""); !strings.EqualFold(normSL, s) { res.Errors = append(res.Errors, koanfError{ Pos: pass.Fset.Position(f.Pos()), Message: fmt.Sprintf("koanf tag name: %q doesn't match the field: %q", sl, s), From 1182da1bc3fef5f9117edb64323f0480b6e1f63b Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Fri, 1 Sep 2023 09:54:04 -0600 Subject: [PATCH 22/22] Fix changing the basefee in non-mutating calls --- arbos/tx_processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arbos/tx_processor.go b/arbos/tx_processor.go index 09a4692eae..0d44ac548e 100644 --- a/arbos/tx_processor.go +++ b/arbos/tx_processor.go @@ -677,7 +677,7 @@ func (p *TxProcessor) GetPaidGasPrice() *big.Int { if version != 9 { gasPrice = p.evm.Context.BaseFee if p.msg.TxRunMode != core.MessageCommitMode && p.msg.GasFeeCap.Sign() == 0 { - gasPrice.SetInt64(0) // gasprice zero behavior + gasPrice = common.Big0 } } return gasPrice