From 0ef1c8399021dcf5ac6f44274f32a0ffd1f86123 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Fri, 9 Jun 2023 19:07:26 -0700 Subject: [PATCH 01/34] Use go-ethereum v1.20.0 Changes: - MakeSigner now takes block time. - IsShanghai now takes block number. - VerifyHeader removed seal param, we don't use it anyway. - GetEVM needs block context now, so we pass it into the hook. - StartMining no longer takes threads param. --- arbnode/execution/sequencer.go | 2 +- arbnode/execution/tx_pre_checker.go | 4 ++-- arbos/block_processor.go | 2 +- arbos/engine.go | 6 +++--- go-ethereum | 2 +- go.mod | 3 +-- go.sum | 5 ++--- nodeInterface/virtual-contracts.go | 6 ++---- system_tests/common_test.go | 2 +- 9 files changed, 14 insertions(+), 18 deletions(-) diff --git a/arbnode/execution/sequencer.go b/arbnode/execution/sequencer.go index 3bc3c90cb1..1fe6ce83a1 100644 --- a/arbnode/execution/sequencer.go +++ b/arbnode/execution/sequencer.go @@ -639,7 +639,7 @@ func (s *Sequencer) precheckNonces(queueItems []txQueueItem) []txQueueItem { return queueItems } nextHeaderNumber := arbmath.BigAdd(latestHeader.Number, common.Big1) - signer := types.MakeSigner(bc.Config(), nextHeaderNumber) + signer := types.MakeSigner(bc.Config(), nextHeaderNumber, latestHeader.Time) outputQueueItems := make([]txQueueItem, 0, len(queueItems)) var nextQueueItem *txQueueItem var queueItemsIdx int diff --git a/arbnode/execution/tx_pre_checker.go b/arbnode/execution/tx_pre_checker.go index 01cef6d7a4..c5f44ccd3c 100644 --- a/arbnode/execution/tx_pre_checker.go +++ b/arbnode/execution/tx_pre_checker.go @@ -115,7 +115,7 @@ func PreCheckTx(bc *core.BlockChain, chainConfig *params.ChainConfig, header *ty if tx.Gas() < params.TxGas { return core.ErrIntrinsicGas } - sender, err := types.Sender(types.MakeSigner(chainConfig, header.Number), tx) + sender, err := types.Sender(types.MakeSigner(chainConfig, header.Number, header.Time), tx) if err != nil { return err } @@ -134,7 +134,7 @@ func PreCheckTx(bc *core.BlockChain, chainConfig *params.ChainConfig, header *ty return MakeNonceError(sender, tx.Nonce(), stateNonce) } extraInfo := types.DeserializeHeaderExtraInformation(header) - intrinsic, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, chainConfig.IsHomestead(header.Number), chainConfig.IsIstanbul(header.Number), chainConfig.IsShanghai(header.Time, extraInfo.ArbOSFormatVersion)) + intrinsic, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, chainConfig.IsHomestead(header.Number), chainConfig.IsIstanbul(header.Number), chainConfig.IsShanghai(header.Number, header.Time, extraInfo.ArbOSFormatVersion)) if err != nil { return err } diff --git a/arbos/block_processor.go b/arbos/block_processor.go index c5270e49ef..bed85b930a 100644 --- a/arbos/block_processor.go +++ b/arbos/block_processor.go @@ -189,7 +189,7 @@ func ProduceBlockAdvanced( } header := createNewHeader(lastBlockHeader, l1Info, state, chainConfig) - signer := types.MakeSigner(chainConfig, header.Number) + signer := types.MakeSigner(chainConfig, header.Number, header.Time) // Note: blockGasLeft will diverge from the actual gas left during execution in the event of invalid txs, // but it's only used as block-local representation limiting the amount of work done in a block. blockGasLeft, _ := state.L2PricingState().PerBlockGasLimit() diff --git a/arbos/engine.go b/arbos/engine.go index ebc27c0886..0014e8ab96 100644 --- a/arbos/engine.go +++ b/arbos/engine.go @@ -23,15 +23,15 @@ func (e Engine) Author(header *types.Header) (common.Address, error) { return header.Coinbase, nil } -func (e Engine) VerifyHeader(chain consensus.ChainHeaderReader, header *types.Header, seal bool) error { +func (e Engine) VerifyHeader(chain consensus.ChainHeaderReader, header *types.Header) error { // TODO what verification should be done here? return nil } -func (e Engine) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error) { +func (e Engine) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header) (chan<- struct{}, <-chan error) { errors := make(chan error, len(headers)) for i := range headers { - errors <- e.VerifyHeader(chain, headers[i], seals[i]) + errors <- e.VerifyHeader(chain, headers[i]) } return make(chan struct{}), errors } diff --git a/go-ethereum b/go-ethereum index 63816ba74a..c8150bfc1b 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit 63816ba74a6945f9c4b4ebb7be8461367019ca80 +Subproject commit c8150bfc1b7dfa9c2d65914dceaef2517db4c8a8 diff --git a/go.mod b/go.mod index ff7332c3d6..9341eb2e2c 100644 --- a/go.mod +++ b/go.mod @@ -280,7 +280,6 @@ require ( github.com/VictoriaMetrics/fastcache v1.6.0 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect - github.com/edsrzf/mmap-go v1.0.0 // indirect github.com/fjl/memsize v0.0.0-20190710130421-bcb5799ab5e5 // indirect github.com/gballet/go-libpcsclite v0.0.0-20190607065134-2772fd86a8ff // indirect github.com/go-ole/go-ole v1.2.1 // indirect @@ -311,7 +310,7 @@ require ( golang.org/x/crypto v0.6.0 golang.org/x/net v0.8.0 // indirect golang.org/x/sync v0.1.0 // indirect - golang.org/x/sys v0.6.0 + golang.org/x/sys v0.7.0 golang.org/x/text v0.8.0 // indirect golang.org/x/time v0.0.0-20220922220347-f3bd1da661af // indirect gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce // indirect diff --git a/go.sum b/go.sum index ea4ee68a5d..2926b21998 100644 --- a/go.sum +++ b/go.sum @@ -302,7 +302,6 @@ github.com/eapache/go-resiliency v1.1.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5m github.com/eapache/go-resiliency v1.2.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs= github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU= github.com/eapache/queue v1.1.0/go.mod h1:6eCeP0CKFpHLu8blIFXhExK/dRa7WDZfr6jVFPTqq+I= -github.com/edsrzf/mmap-go v1.0.0 h1:CEBF7HpRnUCSJgGUb5h1Gm7e3VkmVDrR8lvWVLtrOFw= github.com/edsrzf/mmap-go v1.0.0/go.mod h1:YO35OhQPt3KJa3ryjFM5Bs14WD66h8eGKpfaBNrHW5M= github.com/eknkc/amber v0.0.0-20171010120322-cdade1c07385/go.mod h1:0vRUJqYpeSZifjYj7uP3BG/gKcuzL9xWVV/Y+cK33KM= github.com/elastic/gosigar v0.12.0/go.mod h1:iXRIGg2tLnu7LBdpqzyQfGDEidKCfWcCMS0WKyPWoMs= @@ -1950,8 +1949,8 @@ golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= +golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.6.0 h1:clScbb1cHjoCkyRbWwBEUZ5H/tIFu5TAXIqaZD0Gcjw= diff --git a/nodeInterface/virtual-contracts.go b/nodeInterface/virtual-contracts.go index 29ca3f2b82..ee81c1c3e6 100644 --- a/nodeInterface/virtual-contracts.go +++ b/nodeInterface/virtual-contracts.go @@ -53,6 +53,7 @@ func init() { statedb *state.StateDB, header *types.Header, backend core.NodeInterfaceBackendAPI, + blockCtx *vm.BlockContext, ) (*core.Message, *ExecutionResult, error) { to := msg.To arbosVersion := arbosState.ArbOSVersion(statedb) // check ArbOS has been installed @@ -87,10 +88,7 @@ func init() { return msg, nil, nil } - evm, vmError, err := backend.GetEVM(ctx, msg, statedb, header, &vm.Config{NoBaseFee: true}) - if err != nil { - return msg, nil, err - } + evm, vmError := backend.GetEVM(ctx, msg, statedb, header, &vm.Config{NoBaseFee: true}, blockCtx) go func() { <-ctx.Done() evm.Cancel() diff --git a/system_tests/common_test.go b/system_tests/common_test.go index e471899ff9..d6b4d9c5b6 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -421,7 +421,7 @@ func createTestL1BlockChainWithConfig(t *testing.T, l1info info, stackConfig *no }}) Require(t, stack.Start()) - Require(t, l1backend.StartMining(1)) + Require(t, l1backend.StartMining()) rpcClient, err := stack.Attach() Require(t, err) From ddd2882169709bd6e168c6acc6f0dab5e95133fb Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Tue, 13 Jun 2023 11:04:21 -0700 Subject: [PATCH 02/34] Force leveldb instead of pebbledb --- cmd/nitro/nitro.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index 0035171078..bb20fb2691 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -132,6 +132,7 @@ func mainImpl() int { } stackConf := node.DefaultConfig stackConf.DataDir = nodeConfig.Persistent.Chain + stackConf.DBEngine = "leveldb" nodeConfig.HTTP.Apply(&stackConf) nodeConfig.WS.Apply(&stackConf) nodeConfig.AuthRPC.Apply(&stackConf) From 70d09f489635cfc15cceb993c54294a77b765f8f Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Mon, 31 Jul 2023 15:48:32 -0700 Subject: [PATCH 03/34] Use go-ethereum version with fixed build --- go-ethereum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-ethereum b/go-ethereum index 949af3144e..31addf8423 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit 949af3144ecbfcd09b457b440ff42eb672e671c8 +Subproject commit 31addf8423c0e028aa4b9308dca9f570aa0602ff From 5f182227e3ab21170517088df02448bda16ccd24 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 10 Aug 2023 12:24:19 -0600 Subject: [PATCH 04/34] Don't treat hitting batch L1 bounds as a backlog --- arbnode/batch_poster.go | 55 ++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 601302e536..1443382ca8 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -55,21 +55,22 @@ 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 *SimpleRedisLock - firstAccErr time.Time // first time a continuous missing accumulator occurred - backlog uint64 // An estimate of the number of unposted batches + 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 *SimpleRedisLock + firstAccErr time.Time // first time a continuous missing accumulator occurred + backlog uint64 // An estimate of the number of unposted batches + lastHitL1Bounds time.Time // The last time we wanted to post a message but hit the L1 bounds batchReverted atomic.Bool // indicates whether data poster batch was reverted } @@ -92,7 +93,7 @@ type BatchPosterConfig struct { MaxBatchSize int `koanf:"max-size" reload:"hot"` MaxBatchPostDelay time.Duration `koanf:"max-delay" reload:"hot"` WaitForMaxBatchPostDelay bool `koanf:"wait-for-max-delay" reload:"hot"` - BatchPollDelay time.Duration `koanf:"poll-delay" reload:"hot"` + PollInterval time.Duration `koanf:"poll-interval" reload:"hot"` PostingErrorDelay time.Duration `koanf:"error-delay" reload:"hot"` CompressionLevel int `koanf:"compression-level" reload:"hot"` DASRetentionPeriod time.Duration `koanf:"das-retention-period" reload:"hot"` @@ -141,7 +142,7 @@ func BatchPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Int(prefix+".max-size", DefaultBatchPosterConfig.MaxBatchSize, "maximum batch size") f.Duration(prefix+".max-delay", DefaultBatchPosterConfig.MaxBatchPostDelay, "maximum batch posting delay") f.Bool(prefix+".wait-for-max-delay", DefaultBatchPosterConfig.WaitForMaxBatchPostDelay, "wait for the max batch delay, even if the batch is full") - f.Duration(prefix+".poll-delay", DefaultBatchPosterConfig.BatchPollDelay, "how long to delay after successfully posting batch") + f.Duration(prefix+".poll-interval", DefaultBatchPosterConfig.PollInterval, "how long to wait after no batches are ready to be posted before checking again") f.Duration(prefix+".error-delay", DefaultBatchPosterConfig.PostingErrorDelay, "how long to delay after error posting batch") f.Int(prefix+".compression-level", DefaultBatchPosterConfig.CompressionLevel, "batch compression level") f.Duration(prefix+".das-retention-period", DefaultBatchPosterConfig.DASRetentionPeriod, "In AnyTrust mode, the period which DASes are requested to retain the stored batches.") @@ -159,7 +160,7 @@ var DefaultBatchPosterConfig = BatchPosterConfig{ Enable: false, DisableDasFallbackStoreDataOnChain: false, MaxBatchSize: 100000, - BatchPollDelay: time.Second * 10, + PollInterval: time.Second * 10, PostingErrorDelay: time.Second * 10, MaxBatchPostDelay: time.Hour, WaitForMaxBatchPostDelay: false, @@ -184,7 +185,7 @@ var DefaultBatchPosterL1WalletConfig = genericconf.WalletConfig{ var TestBatchPosterConfig = BatchPosterConfig{ Enable: true, MaxBatchSize: 100000, - BatchPollDelay: time.Millisecond * 10, + PollInterval: time.Millisecond * 10, PostingErrorDelay: time.Millisecond * 10, MaxBatchPostDelay: 0, WaitForMaxBatchPostDelay: false, @@ -797,6 +798,7 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) l1BoundMaxTimestamp = math.MaxUint64 } if msg.Message.Header.BlockNumber > l1BoundMaxBlockNumber || msg.Message.Header.Timestamp > l1BoundMaxTimestamp { + b.lastHitL1Bounds = time.Now() log.Info( "not posting more messages because block number or timestamp exceed L1 bounds", "blockNumber", msg.Message.Header.BlockNumber, @@ -884,16 +886,20 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) "current delayed", b.building.segments.delayedMsg, "total segments", len(b.building.segments.rawSegments), ) + recentlyHitL1Bounds := time.Since(b.lastHitL1Bounds) < config.PollInterval*3 postedMessages := b.building.msgCount - batchPosition.MessageCount unpostedMessages := msgCount - b.building.msgCount b.backlog = uint64(unpostedMessages) / uint64(postedMessages) if b.backlog > 10 { logLevel := log.Warn - if b.backlog > 30 { + if recentlyHitL1Bounds { + logLevel = log.Info + } else if b.backlog > 30 { logLevel = log.Error } logLevel( "a large batch posting backlog exists", + "recentlyHitL1Bounds", recentlyHitL1Bounds, "currentPosition", b.building.msgCount, "messageCount", msgCount, "lastPostedMessages", postedMessages, @@ -901,6 +907,11 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) "batchBacklogEstimate", b.backlog, ) } + if recentlyHitL1Bounds { + // This backlog isn't "real" in that we don't want to post any more messages. + // Setting the backlog to 0 here ensures that we don't lower compression as a result. + b.backlog = 0 + } b.building = nil return true, nil } @@ -930,7 +941,7 @@ func (b *BatchPoster) Start(ctxIn context.Context) { } if !b.redisLock.AttemptLock(ctx) { b.building = nil - return b.config().BatchPollDelay + return b.config().PollInterval } posted, err := b.maybePostSequencerBatch(ctx) if err != nil { @@ -953,7 +964,7 @@ func (b *BatchPoster) Start(ctxIn context.Context) { } else if posted { return 0 } else { - return b.config().BatchPollDelay + return b.config().PollInterval } }) } From f1be9c0d0dd5b81a34fb5ba23ee22b3591ad0667 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 15 Aug 2023 12:51:21 -0600 Subject: [PATCH 05/34] Update backlog comment to mention L1 bounds --- arbnode/batch_poster.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 5c34fc07c2..62681a3f4f 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -56,21 +56,23 @@ 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 - backlog uint64 // An estimate of the number of unposted batches + 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 + // 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 lastHitL1Bounds time.Time // The last time we wanted to post a message but hit the L1 bounds batchReverted atomic.Bool // indicates whether data poster batch was reverted From 2d89a05745c0990daa42b5352b606a3995ecfe03 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 17 Aug 2023 19:16:44 -0600 Subject: [PATCH 06/34] In the data poster, properly allocate fee spending across transactions --- arbnode/dataposter/data_poster.go | 53 +++++++++++++++++++++---------- util/arbmath/math.go | 5 +++ 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 1e7b74834e..e4fe798a9c 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -201,7 +201,7 @@ func (p *DataPoster) GetNextNonceAndMeta(ctx context.Context) (uint64, []byte, e const minRbfIncrease = arbmath.OneInBips * 11 / 10 -func (p *DataPoster) feeAndTipCaps(ctx context.Context, gasLimit uint64, lastFeeCap *big.Int, lastTipCap *big.Int, dataCreatedAt time.Time, backlogOfBatches uint64) (*big.Int, *big.Int, error) { +func (p *DataPoster) feeAndTipCaps(ctx context.Context, nonce uint64, gasLimit uint64, lastFeeCap *big.Int, lastTipCap *big.Int, dataCreatedAt time.Time, backlogOfBatches uint64) (*big.Int, *big.Int, error) { config := p.config() latestHeader, err := p.headerReader.LastHeader(ctx) if err != nil { @@ -210,6 +210,11 @@ func (p *DataPoster) feeAndTipCaps(ctx context.Context, gasLimit uint64, lastFee if latestHeader.BaseFee == nil { return nil, nil, fmt.Errorf("latest parent chain block %v missing BaseFee (either the parent chain does not have EIP-1559 or the parent chain node is not synced)", latestHeader.Number) } + softConfBlock := arbmath.BigSubByUint(latestHeader.Number, config.NonceRbfSoftConfs) + softConfNonce, err := p.client.NonceAt(ctx, p.sender, softConfBlock) + if err != nil { + return nil, nil, fmt.Errorf("failed to get latest nonce %v blocks ago (block %v): %w", config.NonceRbfSoftConfs, softConfBlock, err) + } newFeeCap := new(big.Int).Mul(latestHeader.BaseFee, big.NewInt(2)) newFeeCap = arbmath.BigMax(newFeeCap, arbmath.FloatToBig(config.MinFeeCapGwei*params.GWei)) @@ -252,14 +257,29 @@ func (p *DataPoster) feeAndTipCaps(ctx context.Context, gasLimit uint64, lastFee newFeeCap = maxFeeCap } - balanceFeeCap := new(big.Int).Div(p.balance, new(big.Int).SetUint64(gasLimit)) + latestBalance := p.balance + balanceForTx := new(big.Int).Set(latestBalance) + if !config.UseNoOpStorage { + // We reserve half the balance for the first transaction, and then split the remaining balance for all after that. + // With noop storage, we don't try to replace-by-fee, so we don't need to worry about this. + balanceForTx.Div(balanceForTx, common.Big2) + if nonce != softConfNonce && config.MaxMempoolTransactions > 1 { + // balanceForTx /= config.MaxMempoolTransactions-1 + balanceForTx.Div(balanceForTx, arbmath.UintToBig(config.MaxMempoolTransactions-1)) + } + } + balanceFeeCap := arbmath.BigDivByUint(balanceForTx, gasLimit) if arbmath.BigGreaterThan(newFeeCap, balanceFeeCap) { log.Error( "lack of L1 balance prevents posting transaction with desired fee cap", - "balance", p.balance, + "balance", latestBalance, + "maxTransactions", config.MaxMempoolTransactions, + "balanceForTransaction", balanceForTx, "gasLimit", gasLimit, "desiredFeeCap", newFeeCap, "balanceFeeCap", balanceFeeCap, + "nonce", nonce, + "softConfNonce", softConfNonce, ) newFeeCap = balanceFeeCap } @@ -284,7 +304,7 @@ func (p *DataPoster) PostTransaction(ctx context.Context, dataCreatedAt time.Tim return nil, fmt.Errorf("failed to update data poster balance: %w", err) } - feeCap, tipCap, err := p.feeAndTipCaps(ctx, gasLimit, nil, nil, dataCreatedAt, 0) + feeCap, tipCap, err := p.feeAndTipCaps(ctx, nonce, gasLimit, nil, nil, dataCreatedAt, 0) if err != nil { return nil, err } @@ -342,7 +362,7 @@ func (p *DataPoster) sendTx(ctx context.Context, prevTx *storage.QueuedTransacti // The mutex must be held by the caller. func (p *DataPoster) replaceTx(ctx context.Context, prevTx *storage.QueuedTransaction, backlogOfBatches uint64) error { - newFeeCap, newTipCap, err := p.feeAndTipCaps(ctx, prevTx.Data.Gas, prevTx.Data.GasFeeCap, prevTx.Data.GasTipCap, prevTx.Created, backlogOfBatches) + newFeeCap, newTipCap, err := p.feeAndTipCaps(ctx, prevTx.Data.Nonce, prevTx.Data.Gas, prevTx.Data.GasFeeCap, prevTx.Data.GasTipCap, prevTx.Created, backlogOfBatches) if err != nil { return err } @@ -459,7 +479,7 @@ func (p *DataPoster) maybeLogError(err error, tx *storage.QueuedTransaction, msg } else { delete(p.errorCount, nonce) } - logLevel(msg, "err", err, "nonce", nonce, "feeCap", tx.Data.GasFeeCap, "tipCap", tx.Data.GasTipCap) + logLevel(msg, "err", err, "nonce", nonce, "feeCap", tx.Data.GasFeeCap, "tipCap", tx.Data.GasTipCap, "gas", tx.Data.Gas) } const minWait = time.Second * 10 @@ -499,7 +519,7 @@ func (p *DataPoster) Start(ctxIn context.Context) { // replacing them by fee. queueContents, err := p.queue.FetchContents(ctx, unconfirmedNonce, maxTxsToRbf) if err != nil { - log.Warn("Failed to get tx queue contents", "err", err) + log.Error("Failed to get tx queue contents", "err", err) return minWait } for index, tx := range queueContents { @@ -564,10 +584,10 @@ type DataPosterConfig struct { 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"` UseLevelDB bool `koanf:"use-leveldb" reload:"hot"` UseNoOpStorage bool `koanf:"use-noop-storage" reload:"hot"` - MaxTipCapGwei float64 `koanf:"max-tip-cap-gwei" reload:"hot"` - EnableLevelDB bool `koanf:"enable-leveldb" reload:"hot"` } // ConfigFetcher function type is used instead of directly passing config so @@ -582,7 +602,8 @@ func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Float64(prefix+".target-price-gwei", DefaultDataPosterConfig.TargetPriceGwei, "the target price to use for maximum fee cap calculation") f.Float64(prefix+".urgency-gwei", DefaultDataPosterConfig.UrgencyGwei, "the urgency to use for maximum fee cap calculation") f.Float64(prefix+".min-fee-cap-gwei", DefaultDataPosterConfig.MinFeeCapGwei, "the minimum fee cap to post transactions at") - f.Float64(prefix+".min-tip-cap-gwei", DefaultDataPosterConfig.MinTipCapGwei, "the minimum tip cap to post transactions at") + f.Float64(prefix+".max-tip-cap-gwei", DefaultDataPosterConfig.MaxTipCapGwei, "the maximum tip cap to post transactions at") + f.Uint64(prefix+".nonce-rbf-soft-confs", DefaultDataPosterConfig.NonceRbfSoftConfs, "the maximum probable reorg depth, used to determine when a transaction will no longer likely need replaced-by-fee") f.Bool(prefix+".use-leveldb", DefaultDataPosterConfig.UseLevelDB, "uses leveldb when enabled") f.Bool(prefix+".use-noop-storage", DefaultDataPosterConfig.UseLevelDB, "uses noop storage, it doesn't store anything") signature.SimpleHmacConfigAddOptions(prefix+".redis-signer", f) @@ -593,12 +614,12 @@ var DefaultDataPosterConfig = DataPosterConfig{ WaitForL1Finality: true, TargetPriceGwei: 60., UrgencyGwei: 2., - MaxMempoolTransactions: 64, + MaxMempoolTransactions: 10, MinTipCapGwei: 0.05, + MaxTipCapGwei: 5, + NonceRbfSoftConfs: 2, UseLevelDB: false, UseNoOpStorage: false, - MaxTipCapGwei: 5, - EnableLevelDB: false, } var TestDataPosterConfig = DataPosterConfig{ @@ -607,10 +628,10 @@ var TestDataPosterConfig = DataPosterConfig{ WaitForL1Finality: false, TargetPriceGwei: 60., UrgencyGwei: 2., - MaxMempoolTransactions: 64, + MaxMempoolTransactions: 10, MinTipCapGwei: 0.05, + MaxTipCapGwei: 5, + NonceRbfSoftConfs: 1, UseLevelDB: false, UseNoOpStorage: false, - MaxTipCapGwei: 5, - EnableLevelDB: false, } diff --git a/util/arbmath/math.go b/util/arbmath/math.go index a9758db1c0..467ee58a14 100644 --- a/util/arbmath/math.go +++ b/util/arbmath/math.go @@ -175,6 +175,11 @@ func BigAddByUint(augend *big.Int, addend uint64) *big.Int { return new(big.Int).Add(augend, UintToBig(addend)) } +// BigSub subtracts a uint from a huge +func BigSubByUint(minuend *big.Int, subtrahend uint64) *big.Int { + return new(big.Int).Sub(minuend, UintToBig(subtrahend)) +} + // BigMulByFrac multiply a huge by a rational func BigMulByFrac(value *big.Int, numerator, denominator int64) *big.Int { value = new(big.Int).Set(value) From 33790dee755740c01c34f02a7f8cad126b2f973d Mon Sep 17 00:00:00 2001 From: Joshua Colvin Date: Thu, 17 Aug 2023 19:03:04 -0700 Subject: [PATCH 07/34] Improve help messages for relay options --- relay/relay.go | 2 +- wsbroadcastserver/wsbroadcastserver.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/relay/relay.go b/relay/relay.go index 81d931c0ce..bb07251190 100644 --- a/relay/relay.go +++ b/relay/relay.go @@ -175,7 +175,7 @@ func ConfigAddOptions(f *flag.FlagSet) { f.Bool("pprof", ConfigDefault.PProf, "enable pprof") genericconf.PProfAddOptions("pprof-cfg", f) NodeConfigAddOptions("node", f) - f.Int("queue", ConfigDefault.Queue, "size of relay queue") + f.Int("queue", ConfigDefault.Queue, "queue for incoming messages from sequencer") } type NodeConfig struct { diff --git a/wsbroadcastserver/wsbroadcastserver.go b/wsbroadcastserver/wsbroadcastserver.go index 913eae81f3..014995cee0 100644 --- a/wsbroadcastserver/wsbroadcastserver.go +++ b/wsbroadcastserver/wsbroadcastserver.go @@ -83,7 +83,7 @@ func BroadcasterConfigAddOptions(prefix string, f *flag.FlagSet) { f.String(prefix+".port", DefaultBroadcasterConfig.Port, "port to bind the relay feed output to") f.Duration(prefix+".ping", DefaultBroadcasterConfig.Ping, "duration for ping interval") f.Duration(prefix+".client-timeout", DefaultBroadcasterConfig.ClientTimeout, "duration to wait before timing out connections to client") - f.Int(prefix+".queue", DefaultBroadcasterConfig.Queue, "queue size") + f.Int(prefix+".queue", DefaultBroadcasterConfig.Queue, "queue size for HTTP to WS upgrade") f.Int(prefix+".workers", DefaultBroadcasterConfig.Workers, "number of threads to reserve for HTTP to WS upgrade") f.Int(prefix+".max-send-queue", DefaultBroadcasterConfig.MaxSendQueue, "maximum number of messages allowed to accumulate before client is disconnected") f.Bool(prefix+".require-version", DefaultBroadcasterConfig.RequireVersion, "don't connect if client version not present") From 8b7ff56f92a2b0332f2bc50ab8132c830f5cb57e Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 17 Aug 2023 20:47:03 -0600 Subject: [PATCH 08/34] Add config option for mempool balance allocation --- arbnode/dataposter/data_poster.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index e4fe798a9c..73a0e92ce8 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -259,7 +259,7 @@ func (p *DataPoster) feeAndTipCaps(ctx context.Context, nonce uint64, gasLimit u latestBalance := p.balance balanceForTx := new(big.Int).Set(latestBalance) - if !config.UseNoOpStorage { + if config.AllocateMempoolBalance && !config.UseNoOpStorage { // We reserve half the balance for the first transaction, and then split the remaining balance for all after that. // With noop storage, we don't try to replace-by-fee, so we don't need to worry about this. balanceForTx.Div(balanceForTx, common.Big2) @@ -586,6 +586,7 @@ type DataPosterConfig struct { 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" reload:"hot"` UseNoOpStorage bool `koanf:"use-noop-storage" reload:"hot"` } @@ -604,6 +605,7 @@ func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Float64(prefix+".min-fee-cap-gwei", DefaultDataPosterConfig.MinFeeCapGwei, "the minimum fee cap to post transactions at") f.Float64(prefix+".max-tip-cap-gwei", DefaultDataPosterConfig.MaxTipCapGwei, "the maximum tip cap to post transactions at") f.Uint64(prefix+".nonce-rbf-soft-confs", DefaultDataPosterConfig.NonceRbfSoftConfs, "the maximum probable reorg depth, used to determine when a transaction will no longer likely need replaced-by-fee") + 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.UseLevelDB, "uses noop storage, it doesn't store anything") signature.SimpleHmacConfigAddOptions(prefix+".redis-signer", f) @@ -617,7 +619,8 @@ var DefaultDataPosterConfig = DataPosterConfig{ MaxMempoolTransactions: 10, MinTipCapGwei: 0.05, MaxTipCapGwei: 5, - NonceRbfSoftConfs: 2, + NonceRbfSoftConfs: 1, + AllocateMempoolBalance: true, UseLevelDB: false, UseNoOpStorage: false, } @@ -632,6 +635,7 @@ var TestDataPosterConfig = DataPosterConfig{ MinTipCapGwei: 0.05, MaxTipCapGwei: 5, NonceRbfSoftConfs: 1, + AllocateMempoolBalance: true, UseLevelDB: false, UseNoOpStorage: false, } From 89aa1131d3b04d85714d345aa5a326d47e021667 Mon Sep 17 00:00:00 2001 From: Nodar Date: Fri, 18 Aug 2023 17:01:00 +0200 Subject: [PATCH 09/34] Introduce linter for detecting pointer comparisons --- .gitignore | 1 - Makefile | 1 + go.mod | 3 +- go.sum | 2 + linter/pointercheck/pointer.go | 99 +++++++++++++++++++ linter/pointercheck/pointer_test.go | 31 ++++++ .../testdata/src/pointercheck/pointercheck.go | 34 +++++++ 7 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 linter/pointercheck/pointer.go create mode 100644 linter/pointercheck/pointer_test.go create mode 100644 linter/testdata/src/pointercheck/pointercheck.go diff --git a/.gitignore b/.gitignore index f0eb5c2ec3..60df842f0e 100644 --- a/.gitignore +++ b/.gitignore @@ -19,5 +19,4 @@ solgen/go/ target/ yarn-error.log local/ -testdata system_tests/test-data/* diff --git a/Makefile b/Makefile index 205025dfe9..d180a8cfff 100644 --- a/Makefile +++ b/Makefile @@ -304,6 +304,7 @@ contracts/test/prover/proofs/%.json: $(arbitrator_cases)/%.wasm $(arbitrator_pro # strategic rules to minimize dependency building .make/lint: $(DEP_PREDICATE) build-node-deps $(ORDER_ONLY_PREDICATE) .make + go run linter/pointercheck/pointer.go ./... golangci-lint run --fix yarn --cwd contracts solhint @touch $@ diff --git a/go.mod b/go.mod index 5adfd19388..b0e0b09d08 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/codeclysm/extract/v3 v3.0.2 github.com/dgraph-io/badger/v3 v3.2103.2 github.com/ethereum/go-ethereum v1.10.26 + github.com/fatih/structtag v1.2.0 github.com/google/go-cmp v0.5.9 github.com/hashicorp/golang-lru/v2 v2.0.1 github.com/ipfs/go-cid v0.3.2 @@ -32,6 +33,7 @@ require ( github.com/spf13/pflag v1.0.5 github.com/wealdtech/go-merkletree v1.0.0 golang.org/x/term v0.6.0 + golang.org/x/tools v0.7.0 gopkg.in/natefinch/lumberjack.v2 v2.0.0 ) @@ -257,7 +259,6 @@ require ( go4.org v0.0.0-20200411211856-f5505b9728dd // indirect golang.org/x/exp v0.0.0-20230206171751-46f607a40771 // indirect golang.org/x/mod v0.9.0 // indirect - golang.org/x/tools v0.7.0 // indirect golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect google.golang.org/genproto v0.0.0-20211118181313-81c1377c94b1 // indirect google.golang.org/grpc v1.46.0 // indirect diff --git a/go.sum b/go.sum index 58155db124..e24fe60926 100644 --- a/go.sum +++ b/go.sum @@ -326,6 +326,8 @@ github.com/facebookgo/atomicfile v0.0.0-20151019160806-2de1f203e7d5/go.mod h1:Jp github.com/fasthttp-contrib/websocket v0.0.0-20160511215533-1f3b11f56072/go.mod h1:duJ4Jxv5lDcvg4QuQr0oowTf7dz4/CR8NtyCooz9HL8= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= +github.com/fatih/structtag v1.2.0 h1:/OdNE99OxoI/PqaW/SuSK9uxxT3f/tcSZgon/ssNSx4= +github.com/fatih/structtag v1.2.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4/aAZl94= github.com/fjl/memsize v0.0.0-20190710130421-bcb5799ab5e5 h1:FtmdgXiUlNeRsoNMFlKLDt+S+6hbjVMEW6RGQ7aUf7c= github.com/fjl/memsize v0.0.0-20190710130421-bcb5799ab5e5/go.mod h1:VvhXpOYNQvB+uIk2RvXzuaQtkQJzzIx6lSBe1xv7hi0= github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc= diff --git a/linter/pointercheck/pointer.go b/linter/pointercheck/pointer.go new file mode 100644 index 0000000000..2054921029 --- /dev/null +++ b/linter/pointercheck/pointer.go @@ -0,0 +1,99 @@ +package main + +import ( + "fmt" + "go/ast" + "go/token" + "go/types" + "reflect" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func New(conf any) ([]*analysis.Analyzer, error) { + return []*analysis.Analyzer{Analyzer}, nil +} + +var Analyzer = &analysis.Analyzer{ + Name: "pointercheck", + Doc: "check for pointer comparison", + Run: func(p *analysis.Pass) (interface{}, error) { return run(false, p) }, + ResultType: reflect.TypeOf(Result{}), +} + +var analyzerForTests = &analysis.Analyzer{ + Name: "testpointercheck", + Doc: "check for pointer comparison (for tests)", + Run: func(p *analysis.Pass) (interface{}, error) { return run(true, p) }, + ResultType: reflect.TypeOf(Result{}), +} + +// pointerCmpError indicates the position of pointer comparison. +type pointerCmpError struct { + Pos token.Position + Message string +} + +// Result is returned from the checkStruct function, and holds all the +// configuration errors. +type Result struct { + Errors []pointerCmpError +} + +func run(dryRun bool, pass *analysis.Pass) (interface{}, error) { + var ret Result + for _, f := range pass.Files { + ast.Inspect(f, func(node ast.Node) bool { + var res *Result + switch e := node.(type) { + case *ast.BinaryExpr: + res = checkExpr(pass, e) + default: + } + if res == nil { + return true + } + for _, err := range res.Errors { + ret.Errors = append(ret.Errors, err) + if !dryRun { + pass.Report(analysis.Diagnostic{ + Pos: pass.Fset.File(f.Pos()).Pos(err.Pos.Offset), + Message: err.Message, + Category: "pointercheck", + }) + } + } + return true + }, + ) + } + return ret, nil +} + +func checkExpr(pass *analysis.Pass, e *ast.BinaryExpr) *Result { + if e.Op != token.EQL && e.Op != token.NEQ { + return nil + } + ret := &Result{} + if ptrIdent(pass, e.X) && ptrIdent(pass, e.Y) { + ret.Errors = append(ret.Errors, pointerCmpError{ + Pos: pass.Fset.Position(e.OpPos), + Message: fmt.Sprintf("comparison of two pointers in expression %q", e), + }) + } + return ret +} + +func ptrIdent(pass *analysis.Pass, e ast.Expr) bool { + if _, ok := e.(*ast.Ident); ok { + et := pass.TypesInfo.Types[e].Type + _, isPtr := (et).(*types.Pointer) + return isPtr + } + return false +} + +func main() { + singlechecker.Main(Analyzer) +} diff --git a/linter/pointercheck/pointer_test.go b/linter/pointercheck/pointer_test.go new file mode 100644 index 0000000000..6ed74a9685 --- /dev/null +++ b/linter/pointercheck/pointer_test.go @@ -0,0 +1,31 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAll(t *testing.T) { + wd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get wd: %s", err) + } + testdata := filepath.Join(filepath.Dir(wd), "testdata") + res := analysistest.Run(t, testdata, analyzerForTests, "pointercheck") + if cnt := countErrors(res); cnt != 5 { + t.Errorf("analysistest.Run() got %v errors, expected 5", cnt) + } +} + +func countErrors(errs []*analysistest.Result) int { + cnt := 0 + for _, e := range errs { + if r, ok := e.Result.(Result); ok { + cnt += len(r.Errors) + } + } + return cnt +} diff --git a/linter/testdata/src/pointercheck/pointercheck.go b/linter/testdata/src/pointercheck/pointercheck.go new file mode 100644 index 0000000000..55ebd18293 --- /dev/null +++ b/linter/testdata/src/pointercheck/pointercheck.go @@ -0,0 +1,34 @@ +package pointercheck + +import "fmt" + +type A struct { + x, y int +} + +// pointerCmp compares pointers, sometimes inside +func pointerCmp() { + a, b := &A{}, &A{} + // Simple comparions. + if a != b { + fmt.Println("Not Equal") + } + if a == b { + fmt.Println("Equals") + } + // Nested binary expressions. + if (2 > 1) && (a != b) { + fmt.Println("Still not equal") + } + if (174%15 > 3) && (2 > 1 && (1+2 > 2 || a != b)) { + fmt.Println("Who knows at this point") + } + // Nested and inside unary operator. + if 10 > 5 && !(2 > 1 || a == b) { + fmt.Println("Not equal") + } + c, d := 1, 2 + if &c != &d { + fmt.Println("Not equal") + } +} From 00dfaed58a2037cdc5c15275eaebe8d73305c71a Mon Sep 17 00:00:00 2001 From: Nodar Date: Fri, 18 Aug 2023 17:40:02 +0200 Subject: [PATCH 10/34] Drop pointer comparisons --- arbnode/dataposter/data_poster.go | 2 +- arbnode/execution/executionengine.go | 2 +- arbnode/execution/tx_pre_checker.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 1e7b74834e..713eca6dd5 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -321,7 +321,7 @@ 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 != newTx { + if prevTx == nil || (newTx != nil && newTx.FullTx.Hash() != prevTx.FullTx.Hash()) { if err := p.saveTx(ctx, prevTx, newTx); err != nil { return err } diff --git a/arbnode/execution/executionengine.go b/arbnode/execution/executionengine.go index d8029650d7..71610b308c 100644 --- a/arbnode/execution/executionengine.go +++ b/arbnode/execution/executionengine.go @@ -599,7 +599,7 @@ func (s *ExecutionEngine) Start(ctx_in context.Context) { s.latestBlockMutex.Lock() block := s.latestBlock s.latestBlockMutex.Unlock() - if block != lastBlock && block != nil { + if block != nil && (lastBlock == nil || block.TxHash() != lastBlock.TxHash()) { log.Info( "created block", "l2Block", block.Number(), diff --git a/arbnode/execution/tx_pre_checker.go b/arbnode/execution/tx_pre_checker.go index 01cef6d7a4..4a0645e97b 100644 --- a/arbnode/execution/tx_pre_checker.go +++ b/arbnode/execution/tx_pre_checker.go @@ -170,7 +170,7 @@ func PreCheckTx(bc *core.BlockChain, chainConfig *params.ChainConfig, header *ty oldHeader = previousHeader blocksTraversed++ } - if oldHeader != header { + if oldHeader == nil || (header != nil && oldHeader.Hash() != header.Hash()) { secondOldStatedb, err := bc.StateAt(oldHeader.Root) if err != nil { return fmt.Errorf("failed to get old state: %w", err) From 9979eb1083cde85bbcc5ff24bee50c2bf6273f94 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Fri, 18 Aug 2023 11:35:42 -0600 Subject: [PATCH 11/34] Fix removed config option --- arbnode/dataposter/data_poster.go | 1 + 1 file changed, 1 insertion(+) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 73a0e92ce8..979cbe3b7e 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -603,6 +603,7 @@ func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Float64(prefix+".target-price-gwei", DefaultDataPosterConfig.TargetPriceGwei, "the target price to use for maximum fee cap calculation") f.Float64(prefix+".urgency-gwei", DefaultDataPosterConfig.UrgencyGwei, "the urgency to use for maximum fee cap calculation") f.Float64(prefix+".min-fee-cap-gwei", DefaultDataPosterConfig.MinFeeCapGwei, "the minimum fee cap to post transactions at") + f.Float64(prefix+".min-tip-cap-gwei", DefaultDataPosterConfig.MinTipCapGwei, "the minimum tip cap to post transactions at") f.Float64(prefix+".max-tip-cap-gwei", DefaultDataPosterConfig.MaxTipCapGwei, "the maximum tip cap to post transactions at") f.Uint64(prefix+".nonce-rbf-soft-confs", DefaultDataPosterConfig.NonceRbfSoftConfs, "the maximum probable reorg depth, used to determine when a transaction will no longer likely need replaced-by-fee") 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") From 916b5cec36ba77c1cf67d61926c2d96d0374cd69 Mon Sep 17 00:00:00 2001 From: Nodar Date: Mon, 21 Aug 2023 17:54:48 +0200 Subject: [PATCH 12/34] Fix flag initializations in sample_hmac.go, init.go and data_poster.go --- arbnode/dataposter/data_poster.go | 2 +- cmd/nitro/init.go | 2 +- util/signature/simple_hmac.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 979cbe3b7e..dfcb1f4ba1 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -608,7 +608,7 @@ func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Uint64(prefix+".nonce-rbf-soft-confs", DefaultDataPosterConfig.NonceRbfSoftConfs, "the maximum probable reorg depth, used to determine when a transaction will no longer likely need replaced-by-fee") 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.UseLevelDB, "uses noop storage, it doesn't store anything") + f.Bool(prefix+".use-noop-storage", DefaultDataPosterConfig.UseNoOpStorage, "uses noop storage, it doesn't store anything") signature.SimpleHmacConfigAddOptions(prefix+".redis-signer", f) } diff --git a/cmd/nitro/init.go b/cmd/nitro/init.go index 6480526897..bdba7c1210 100644 --- a/cmd/nitro/init.go +++ b/cmd/nitro/init.go @@ -86,7 +86,7 @@ func InitConfigAddOptions(prefix string, f *pflag.FlagSet) { f.Bool(prefix+".dev-init", InitConfigDefault.DevInit, "init with dev data (1 account with balance) instead of file import") f.String(prefix+".dev-init-address", InitConfigDefault.DevInitAddress, "Address of dev-account. Leave empty to use the dev-wallet.") f.Uint64(prefix+".dev-init-blocknum", InitConfigDefault.DevInitBlockNum, "Number of preinit blocks. Must exist in ancient database.") - f.Bool(prefix+".empty", InitConfigDefault.DevInit, "init with empty state") + f.Bool(prefix+".empty", InitConfigDefault.Empty, "init with empty state") f.Bool(prefix+".then-quit", InitConfigDefault.ThenQuit, "quit after init is done") f.String(prefix+".import-file", InitConfigDefault.ImportFile, "path for json data to import") f.Uint(prefix+".accounts-per-sync", InitConfigDefault.AccountsPerSync, "during init - sync database every X accounts. Lower value for low-memory systems. 0 disables.") diff --git a/util/signature/simple_hmac.go b/util/signature/simple_hmac.go index b1c683742b..4899b5c22c 100644 --- a/util/signature/simple_hmac.go +++ b/util/signature/simple_hmac.go @@ -58,7 +58,7 @@ func SimpleHmacDangerousConfigAddOptions(prefix string, f *flag.FlagSet) { func SimpleHmacConfigAddOptions(prefix string, f *flag.FlagSet) { f.String(prefix+".signing-key", EmptySimpleHmacConfig.SigningKey, "a 32-byte (64-character) hex string used to sign messages, or a path to a file containing it") - f.String(prefix+".fallback-verification-key", EmptySimpleHmacConfig.SigningKey, "a fallback key used for message verification") + f.String(prefix+".fallback-verification-key", EmptySimpleHmacConfig.FallbackVerificationKey, "a fallback key used for message verification") SimpleHmacDangerousConfigAddOptions(prefix+".dangerous", f) } From eba3d33cb3f79d424e3ed564c86523ee7bb53438 Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 22 Aug 2023 16:39:00 +0200 Subject: [PATCH 13/34] Fix linter error where we implicitly alias memory in for loop --- arbos/arbosState/initialize.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arbos/arbosState/initialize.go b/arbos/arbosState/initialize.go index e98ab08485..9f24d96765 100644 --- a/arbos/arbosState/initialize.go +++ b/arbos/arbosState/initialize.go @@ -189,7 +189,8 @@ func initializeRetryables(statedb *state.StateDB, rs *retryables.RetryableState, for _, r := range retryablesList { var to *common.Address if r.To != (common.Address{}) { - to = &r.To + addr := r.To + to = &addr } statedb.AddBalance(retryables.RetryableEscrowAddress(r.Id), r.Callvalue) _, err := rs.CreateRetryable(r.Id, r.Timeout, r.From, to, r.Callvalue, r.Beneficiary, r.Calldata) From d0e6c7c8dbb0e73322ce1a9e0251cf9eb884dacb Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 22 Aug 2023 17:38:54 +0200 Subject: [PATCH 14/34] Detect pointers in selectors --- linter/pointercheck/pointer.go | 9 +++++---- linter/pointercheck/pointer_test.go | 4 ++-- linter/testdata/src/pointercheck/pointercheck.go | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/linter/pointercheck/pointer.go b/linter/pointercheck/pointer.go index 2054921029..6500b01222 100644 --- a/linter/pointercheck/pointer.go +++ b/linter/pointercheck/pointer.go @@ -78,16 +78,17 @@ func checkExpr(pass *analysis.Pass, e *ast.BinaryExpr) *Result { ret := &Result{} if ptrIdent(pass, e.X) && ptrIdent(pass, e.Y) { ret.Errors = append(ret.Errors, pointerCmpError{ - Pos: pass.Fset.Position(e.OpPos), - Message: fmt.Sprintf("comparison of two pointers in expression %q", e), + Pos: pass.Fset.Position(e.Pos()), + Message: fmt.Sprintf("comparison of two pointers in expression %v", e), }) } return ret } func ptrIdent(pass *analysis.Pass, e ast.Expr) bool { - if _, ok := e.(*ast.Ident); ok { - et := pass.TypesInfo.Types[e].Type + switch tp := e.(type) { + case *ast.Ident, *ast.SelectorExpr: + et := pass.TypesInfo.Types[tp].Type _, isPtr := (et).(*types.Pointer) return isPtr } diff --git a/linter/pointercheck/pointer_test.go b/linter/pointercheck/pointer_test.go index 6ed74a9685..290e3826de 100644 --- a/linter/pointercheck/pointer_test.go +++ b/linter/pointercheck/pointer_test.go @@ -15,8 +15,8 @@ func TestAll(t *testing.T) { } testdata := filepath.Join(filepath.Dir(wd), "testdata") res := analysistest.Run(t, testdata, analyzerForTests, "pointercheck") - if cnt := countErrors(res); cnt != 5 { - t.Errorf("analysistest.Run() got %v errors, expected 5", cnt) + if cnt := countErrors(res); cnt != 6 { + t.Errorf("analysistest.Run() got %v errors, expected 6", cnt) } } diff --git a/linter/testdata/src/pointercheck/pointercheck.go b/linter/testdata/src/pointercheck/pointercheck.go index 55ebd18293..f63fdd1743 100644 --- a/linter/testdata/src/pointercheck/pointercheck.go +++ b/linter/testdata/src/pointercheck/pointercheck.go @@ -32,3 +32,19 @@ func pointerCmp() { fmt.Println("Not equal") } } + +func legitCmps() { + a, b := &A{}, &A{} + if a.x == b.x { + fmt.Println("Allowed") + } +} + +type cache struct { + dirty *A +} + +// matches does pointer comparison. +func (c *cache) matches(a *A) bool { + return c.dirty == a +} From 90c1d460a6031220d8a65a91d284f7157f847c4b Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 22 Aug 2023 17:40:25 +0200 Subject: [PATCH 15/34] compare block hash instead of txHash when comparing block pointers --- arbnode/execution/executionengine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arbnode/execution/executionengine.go b/arbnode/execution/executionengine.go index 71610b308c..da01e27983 100644 --- a/arbnode/execution/executionengine.go +++ b/arbnode/execution/executionengine.go @@ -599,7 +599,7 @@ func (s *ExecutionEngine) Start(ctx_in context.Context) { s.latestBlockMutex.Lock() block := s.latestBlock s.latestBlockMutex.Unlock() - if block != nil && (lastBlock == nil || block.TxHash() != lastBlock.TxHash()) { + if block != nil && (lastBlock == nil || block.Hash() != lastBlock.Hash()) { log.Info( "created block", "l2Block", block.Number(), From 182794c72cddf50b8d221c3b7192aa884ed72ee7 Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 22 Aug 2023 18:03:20 +0200 Subject: [PATCH 16/34] Drop pointer comparison for selectors --- arbnode/execution/sequencer.go | 2 +- arbos/arbostypes/incomingmessage.go | 11 +++++++++-- util/headerreader/header_reader.go | 9 ++++++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/arbnode/execution/sequencer.go b/arbnode/execution/sequencer.go index ea818beb6b..7065bd87d9 100644 --- a/arbnode/execution/sequencer.go +++ b/arbnode/execution/sequencer.go @@ -184,7 +184,7 @@ func (c *nonceCache) matches(header *types.Header) bool { if c.dirty != nil { // The header is updated as the block is built, // so instead of checking its hash, we do a pointer comparison. - return c.dirty == header + return headerreader.HeadersEqual(c.dirty, header) } return c.block == header.ParentHash } diff --git a/arbos/arbostypes/incomingmessage.go b/arbos/arbostypes/incomingmessage.go index e9a5466d46..60f53cdaab 100644 --- a/arbos/arbostypes/incomingmessage.go +++ b/arbos/arbostypes/incomingmessage.go @@ -127,14 +127,21 @@ func (msg *L1IncomingMessage) Equals(other *L1IncomingMessage) bool { return msg.Header.Equals(other.Header) && bytes.Equal(msg.L2msg, other.L2msg) } +func hashesEqual(ha, hb *common.Hash) bool { + if (ha == nil) != (hb == nil) { + return false + } + return (ha == nil) && (hb == nil) || *ha == *hb +} + func (h *L1IncomingMessageHeader) Equals(other *L1IncomingMessageHeader) bool { // These are all non-pointer types so it's safe to use the == operator return h.Kind == other.Kind && h.Poster == other.Poster && h.BlockNumber == other.BlockNumber && h.Timestamp == other.Timestamp && - h.RequestId == other.RequestId && - h.L1BaseFee == other.L1BaseFee + hashesEqual(h.RequestId, other.RequestId) && + arbmath.BigEquals(h.L1BaseFee, other.L1BaseFee) } func ComputeBatchGasCost(data []byte) uint64 { diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index 28fef8ee07..87cc6d69b0 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -393,6 +393,13 @@ func headerIndicatesFinalitySupport(header *types.Header) bool { return false } +func HeadersEqual(ha, hb *types.Header) bool { + if (ha == nil) != (hb == nil) { + return false + } + return (ha == nil) && (hb == nil) || ha.Hash() == hb.Hash() +} + func (s *HeaderReader) getCached(ctx context.Context, c *cachedHeader) (*types.Header, error) { c.mutex.Lock() defer c.mutex.Unlock() @@ -400,7 +407,7 @@ func (s *HeaderReader) getCached(ctx context.Context, c *cachedHeader) (*types.H if err != nil { return nil, err } - if currentHead == c.headWhenCached { + if HeadersEqual(currentHead, c.headWhenCached) { return c.header, nil } if !s.config().UseFinalityData || !headerIndicatesFinalitySupport(currentHead) { From 93cb20b2f51698164e4b5a9f217a1b0868b2a423 Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 22 Aug 2023 18:07:57 +0200 Subject: [PATCH 17/34] Use headerreader.HeadersEqual in execution to compare headers --- arbnode/execution/tx_pre_checker.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arbnode/execution/tx_pre_checker.go b/arbnode/execution/tx_pre_checker.go index 4a0645e97b..5dae32a192 100644 --- a/arbnode/execution/tx_pre_checker.go +++ b/arbnode/execution/tx_pre_checker.go @@ -18,6 +18,7 @@ import ( "github.com/offchainlabs/nitro/arbos/arbosState" "github.com/offchainlabs/nitro/arbos/l1pricing" "github.com/offchainlabs/nitro/util/arbmath" + "github.com/offchainlabs/nitro/util/headerreader" flag "github.com/spf13/pflag" ) @@ -170,7 +171,7 @@ func PreCheckTx(bc *core.BlockChain, chainConfig *params.ChainConfig, header *ty oldHeader = previousHeader blocksTraversed++ } - if oldHeader == nil || (header != nil && oldHeader.Hash() != header.Hash()) { + if headerreader.HeadersEqual(oldHeader, header) { secondOldStatedb, err := bc.StateAt(oldHeader.Root) if err != nil { return fmt.Errorf("failed to get old state: %w", err) From 12811e290e002cea2a1aafa30a14d96c7c4ae8a1 Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 22 Aug 2023 18:55:31 +0200 Subject: [PATCH 18/34] Add system_tests\/testdata and arbos\/testdata in .gitignore so that fuzzing output will be ignored --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 60df842f0e..f9b920e4fe 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,5 @@ target/ yarn-error.log local/ system_tests/test-data/* +system_tests/testdata/* +arbos/testdata/* \ No newline at end of file From 9099b51689b53eb5249bef134e77470e74bb79ea Mon Sep 17 00:00:00 2001 From: Nodar Date: Tue, 22 Aug 2023 18:56:35 +0200 Subject: [PATCH 19/34] Add blank line to .gitignore, otherwise github complains --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index f9b920e4fe..8a628e29c4 100644 --- a/.gitignore +++ b/.gitignore @@ -21,4 +21,4 @@ yarn-error.log local/ system_tests/test-data/* system_tests/testdata/* -arbos/testdata/* \ No newline at end of file +arbos/testdata/* From 6555a39b15c1b296a9455abf2b7ab9e4b4569a54 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 22 Aug 2023 13:34:36 -0600 Subject: [PATCH 20/34] Bump go-ethereum pin --- go-ethereum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-ethereum b/go-ethereum index d312afd03b..88b1bd3b54 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit d312afd03bba77aa2b4ea36e80b7308cd6528e80 +Subproject commit 88b1bd3b5465725c6ecaec8b4d124c175d75268d From e1b438cf1cf0c42f0fe78f93bea9cf9cbb8a6bd1 Mon Sep 17 00:00:00 2001 From: Tristan Wilson Date: Tue, 22 Aug 2023 13:18:58 -0700 Subject: [PATCH 21/34] Update go-ethereum submod to latest merge-v1.12.0 --- go-ethereum | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-ethereum b/go-ethereum index 0685207850..c905292f8a 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit 068520785011e63b5760c2195676674647269337 +Subproject commit c905292f8af601f7fca261e65a7d4bc144261e29 From cf355d96b2dd5a60bc6305d7d13e027855396910 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 22 Aug 2023 15:31:33 -0600 Subject: [PATCH 22/34] Fix running a watchtower staker without a wallet --- arbnode/node.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index a4025429c6..42435838ca 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -543,8 +543,13 @@ func checkArbDbSchemaVersion(arbDb ethdb.Database) error { return nil } -func ValidatorDataposter(db ethdb.Database, l1Reader *headerreader.HeaderReader, - transactOpts *bind.TransactOpts, cfgFetcher ConfigFetcher, syncMonitor *SyncMonitor) (*dataposter.DataPoster, error) { +func ValidatorDataposter( + db ethdb.Database, l1Reader *headerreader.HeaderReader, + transactOpts *bind.TransactOpts, cfgFetcher ConfigFetcher, syncMonitor *SyncMonitor, +) (*dataposter.DataPoster, error) { + if transactOpts == nil { + return nil, nil + } cfg := cfgFetcher.Get() mdRetriever := func(ctx context.Context, blockNum *big.Int) ([]byte, error) { return nil, nil From 0b72ffe08125012051720dbdf61ef6f3274810a5 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 22 Aug 2023 16:52:32 -0600 Subject: [PATCH 23/34] Add sepolia-rollup testnet chain information --- cmd/chaininfo/arbitrum_chain_info.json | 51 +++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/cmd/chaininfo/arbitrum_chain_info.json b/cmd/chaininfo/arbitrum_chain_info.json index 5352f9760f..224cd469c0 100644 --- a/cmd/chaininfo/arbitrum_chain_info.json +++ b/cmd/chaininfo/arbitrum_chain_info.json @@ -214,5 +214,54 @@ "GenesisBlockNum": 0 } } + }, + { + "chain-id": 421614, + "parent-chain-id": 11155111, + "chain-name": "sepolia-rollup", + "sequencer-url": "https://sepolia-rollup-sequencer.arbitrum.io/rpc", + "feed-url": "https://sepolia-rollup.arbitrum.io/feed", + "chain-config": + { + "chainId": 421614, + "homesteadBlock": 0, + "daoForkBlock": null, + "daoForkSupport": true, + "eip150Block": 0, + "eip150Hash": "0x0000000000000000000000000000000000000000000000000000000000000000", + "eip155Block": 0, + "eip158Block": 0, + "byzantiumBlock": 0, + "constantinopleBlock": 0, + "petersburgBlock": 0, + "istanbulBlock": 0, + "muirGlacierBlock": 0, + "berlinBlock": 0, + "londonBlock": 0, + "clique": + { + "period": 0, + "epoch": 0 + }, + "arbitrum": + { + "EnableArbOS": true, + "AllowDebugPrecompiles": false, + "DataAvailabilityCommittee": false, + "InitialArbOSVersion": 10, + "InitialChainOwner": "0x71B61c2E250AFa05dFc36304D6c91501bE0965D8", + "GenesisBlockNum": 0 + } + }, + "rollup": + { + "bridge": "0x38f918D0E9F1b721EDaA41302E399fa1B79333a9", + "inbox": "0xaAe29B0366299461418F5324a79Afc425BE5ae21", + "sequencer-inbox": "0x6c97864CE4bEf387dE0b3310A44230f7E3F1be0D", + "rollup": "0xd80810638dbDF9081b72C1B33c65375e807281C8", + "validator-utils": "0x1f6860C3cac255fFFa72B7410b1183c3a0D261e0", + "validator-wallet-creator": "0x894fC71fA0A666352824EC954B401573C861D664", + "deployed-at": 4139226 + } } -] \ No newline at end of file +] From b9ad5a1baa5c9118cfeb099a3c64d085e4445fa6 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 22 Aug 2023 21:26:05 -0600 Subject: [PATCH 24/34] Fix sepolia-rollup feed connection scheme --- cmd/chaininfo/arbitrum_chain_info.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/chaininfo/arbitrum_chain_info.json b/cmd/chaininfo/arbitrum_chain_info.json index 224cd469c0..f5fa56102c 100644 --- a/cmd/chaininfo/arbitrum_chain_info.json +++ b/cmd/chaininfo/arbitrum_chain_info.json @@ -220,7 +220,7 @@ "parent-chain-id": 11155111, "chain-name": "sepolia-rollup", "sequencer-url": "https://sepolia-rollup-sequencer.arbitrum.io/rpc", - "feed-url": "https://sepolia-rollup.arbitrum.io/feed", + "feed-url": "wss://sepolia-rollup.arbitrum.io/feed", "chain-config": { "chainId": 421614, From d464cc7215349573b3a0ab0ec75cb81064e970ee Mon Sep 17 00:00:00 2001 From: Nodar Date: Wed, 23 Aug 2023 14:25:52 +0200 Subject: [PATCH 25/34] Add parenthesis in condition to make it more explicit, do inequality of headers instead of equality in execution package --- arbnode/execution/tx_pre_checker.go | 2 +- arbos/arbostypes/incomingmessage.go | 2 +- util/headerreader/header_reader.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arbnode/execution/tx_pre_checker.go b/arbnode/execution/tx_pre_checker.go index 5dae32a192..ee494fba3d 100644 --- a/arbnode/execution/tx_pre_checker.go +++ b/arbnode/execution/tx_pre_checker.go @@ -171,7 +171,7 @@ func PreCheckTx(bc *core.BlockChain, chainConfig *params.ChainConfig, header *ty oldHeader = previousHeader blocksTraversed++ } - if headerreader.HeadersEqual(oldHeader, header) { + if !headerreader.HeadersEqual(oldHeader, header) { secondOldStatedb, err := bc.StateAt(oldHeader.Root) if err != nil { return fmt.Errorf("failed to get old state: %w", err) diff --git a/arbos/arbostypes/incomingmessage.go b/arbos/arbostypes/incomingmessage.go index 60f53cdaab..04ce8ebe2e 100644 --- a/arbos/arbostypes/incomingmessage.go +++ b/arbos/arbostypes/incomingmessage.go @@ -131,7 +131,7 @@ func hashesEqual(ha, hb *common.Hash) bool { if (ha == nil) != (hb == nil) { return false } - return (ha == nil) && (hb == nil) || *ha == *hb + return (ha == nil && hb == nil) || *ha == *hb } func (h *L1IncomingMessageHeader) Equals(other *L1IncomingMessageHeader) bool { diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index 87cc6d69b0..739a96b438 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -397,7 +397,7 @@ func HeadersEqual(ha, hb *types.Header) bool { if (ha == nil) != (hb == nil) { return false } - return (ha == nil) && (hb == nil) || ha.Hash() == hb.Hash() + return (ha == nil && hb == nil) || ha.Hash() == hb.Hash() } func (s *HeaderReader) getCached(ctx context.Context, c *cachedHeader) (*types.Header, error) { From c217d9f705181fbfb35a3fe3ed9ee3e5e8d782f4 Mon Sep 17 00:00:00 2001 From: Nodar Date: Wed, 23 Aug 2023 14:31:00 +0200 Subject: [PATCH 26/34] update method comment in sequencer.go --- arbnode/execution/sequencer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arbnode/execution/sequencer.go b/arbnode/execution/sequencer.go index 5bb097ace6..402958399d 100644 --- a/arbnode/execution/sequencer.go +++ b/arbnode/execution/sequencer.go @@ -182,8 +182,8 @@ func newNonceCache(size int) *nonceCache { func (c *nonceCache) matches(header *types.Header) bool { if c.dirty != nil { - // The header is updated as the block is built, - // so instead of checking its hash, we do a pointer comparison. + // Note, even though the of the header changes, c.dirty points to the + // same header, hence hashes will be the same and this check will pass. return headerreader.HeadersEqual(c.dirty, header) } return c.block == header.ParentHash From 85faf666a4572d7a698056504efc31eda5aa0b72 Mon Sep 17 00:00:00 2001 From: Nodar Date: Wed, 23 Aug 2023 14:33:15 +0200 Subject: [PATCH 27/34] Drop nil check of newTx in dataposter --- arbnode/dataposter/data_poster.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index dd8366208f..3d936485ec 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -341,7 +341,7 @@ 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 != nil && newTx.FullTx.Hash() != prevTx.FullTx.Hash()) { + if prevTx == nil || (newTx.FullTx.Hash() != prevTx.FullTx.Hash()) { if err := p.saveTx(ctx, prevTx, newTx); err != nil { return err } From 7b45b0352a48c14d3ebefa84a32f5086a395a2ee Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 23 Aug 2023 10:52:14 -0600 Subject: [PATCH 28/34] Move ExtraGas to L1ValidatorConfig and use it for eoa wallets --- arbnode/node.go | 7 ++++--- staker/block_validator.go | 4 ---- staker/eoa_validator_wallet.go | 7 +++++-- staker/staker.go | 3 +++ staker/validator_wallet.go | 8 ++++---- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index 42435838ca..abc4367a39 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -299,7 +299,7 @@ type Config struct { BlockValidator staker.BlockValidatorConfig `koanf:"block-validator" reload:"hot"` RecordingDatabase arbitrum.RecordingDatabaseConfig `koanf:"recording-database"` Feed broadcastclient.FeedConfig `koanf:"feed" reload:"hot"` - Staker staker.L1ValidatorConfig `koanf:"staker"` + Staker staker.L1ValidatorConfig `koanf:"staker" reload:"hot"` SeqCoordinator SeqCoordinatorConfig `koanf:"seq-coordinator"` DataAvailability das.DataAvailabilityConfig `koanf:"data-availability"` SyncMonitor SyncMonitorConfig `koanf:"sync-monitor"` @@ -817,6 +817,7 @@ func createNodeImpl( if err != nil { return nil, err } + getExtraGas := func() uint64 { return configFetcher.Get().Staker.ExtraGas } var wallet staker.ValidatorWalletInterface if config.Staker.UseSmartContractWallet || txOptsValidator == nil { var existingWalletAddress *common.Address @@ -828,7 +829,7 @@ func createNodeImpl( tmpAddress := common.HexToAddress(config.Staker.ContractWalletAddress) existingWalletAddress = &tmpAddress } - wallet, err = staker.NewContractValidatorWallet(dp, existingWalletAddress, deployInfo.ValidatorWalletCreator, deployInfo.Rollup, l1Reader, txOptsValidator, int64(deployInfo.DeployedAt), func(common.Address) {}, config.BlockValidator.ExtraGas) + wallet, err = staker.NewContractValidatorWallet(dp, existingWalletAddress, deployInfo.ValidatorWalletCreator, deployInfo.Rollup, l1Reader, txOptsValidator, int64(deployInfo.DeployedAt), func(common.Address) {}, getExtraGas) if err != nil { return nil, err } @@ -836,7 +837,7 @@ func createNodeImpl( if len(config.Staker.ContractWalletAddress) > 0 { return nil, errors.New("validator contract wallet specified but flag to use a smart contract wallet was not specified") } - wallet, err = staker.NewEoaValidatorWallet(dp, deployInfo.Rollup, l1client, txOptsValidator) + wallet, err = staker.NewEoaValidatorWallet(dp, deployInfo.Rollup, l1client, txOptsValidator, getExtraGas) if err != nil { return nil, err } diff --git a/staker/block_validator.go b/staker/block_validator.go index 333a096813..5da4365143 100644 --- a/staker/block_validator.go +++ b/staker/block_validator.go @@ -91,7 +91,6 @@ type BlockValidatorConfig struct { DataPoster dataposter.DataPosterConfig `koanf:"data-poster" reload:"hot"` RedisUrl string `koanf:"redis-url"` RedisLock redislock.SimpleCfg `koanf:"redis-lock" reload:"hot"` - ExtraGas uint64 `koanf:"extra-gas" reload:"hot"` } func (c *BlockValidatorConfig) Validate() error { @@ -113,7 +112,6 @@ func BlockValidatorConfigAddOptions(prefix string, f *flag.FlagSet) { f.String(prefix+".current-module-root", DefaultBlockValidatorConfig.CurrentModuleRoot, "current wasm module root ('current' read from chain, 'latest' from machines/latest dir, or provide hash)") f.String(prefix+".pending-upgrade-module-root", DefaultBlockValidatorConfig.PendingUpgradeModuleRoot, "pending upgrade wasm module root to additionally validate (hash, 'latest' or empty)") f.Bool(prefix+".failure-is-fatal", DefaultBlockValidatorConfig.FailureIsFatal, "failing a validation is treated as a fatal error") - f.Uint64(prefix+".extra-gas", DefaultBlockValidatorConfig.ExtraGas, "use this much more gas than estimation says is necessary to post transactions") BlockValidatorDangerousConfigAddOptions(prefix+".dangerous", f) dataposter.DataPosterConfigAddOptions(prefix+".data-poster", f) f.String(prefix+".redis-url", DefaultBlockValidatorConfig.RedisUrl, "redis url for block validator") @@ -137,7 +135,6 @@ var DefaultBlockValidatorConfig = BlockValidatorConfig{ DataPoster: dataposter.DefaultDataPosterConfig, RedisUrl: "", RedisLock: redislock.DefaultCfg, - ExtraGas: 50000, } var TestBlockValidatorConfig = BlockValidatorConfig{ @@ -153,7 +150,6 @@ var TestBlockValidatorConfig = BlockValidatorConfig{ DataPoster: dataposter.TestDataPosterConfig, RedisUrl: "", RedisLock: redislock.DefaultCfg, - ExtraGas: 50000, } var DefaultBlockValidatorDangerousConfig = BlockValidatorDangerousConfig{ diff --git a/staker/eoa_validator_wallet.go b/staker/eoa_validator_wallet.go index f514969434..e3d149c99f 100644 --- a/staker/eoa_validator_wallet.go +++ b/staker/eoa_validator_wallet.go @@ -29,17 +29,19 @@ type EoaValidatorWallet struct { challengeManagerAddress common.Address dataPoster *dataposter.DataPoster txCount atomic.Uint64 + getExtraGas func() uint64 } var _ ValidatorWalletInterface = (*EoaValidatorWallet)(nil) -func NewEoaValidatorWallet(dataPoster *dataposter.DataPoster, rollupAddress common.Address, l1Client arbutil.L1Interface, auth *bind.TransactOpts) (*EoaValidatorWallet, error) { +func NewEoaValidatorWallet(dataPoster *dataposter.DataPoster, rollupAddress common.Address, l1Client arbutil.L1Interface, auth *bind.TransactOpts, getExtraGas func() uint64) (*EoaValidatorWallet, error) { return &EoaValidatorWallet{ auth: auth, client: l1Client, rollupAddress: rollupAddress, dataPoster: dataPoster, txCount: atomic.Uint64{}, + getExtraGas: getExtraGas, }, nil } @@ -126,7 +128,8 @@ func (w *EoaValidatorWallet) ExecuteTransactions(ctx context.Context, builder *V log.Warn("Precondition failure, dataposter nonce is higher than validator transactio count", "dataposter nonce", nonce, "validator tx count", w.txCount.Load()) } tx := builder.transactions[0] // we ignore future txs and only execute the first - trans, err := w.dataPoster.PostTransaction(ctx, time.Now(), nonce, nil, *tx.To(), tx.Data(), tx.Gas(), tx.Value()) + gas := tx.Gas() + w.getExtraGas() + trans, err := w.dataPoster.PostTransaction(ctx, time.Now(), nonce, nil, *tx.To(), tx.Data(), gas, tx.Value()) if err != nil { return nil, fmt.Errorf("post transaction: %w", err) } diff --git a/staker/staker.go b/staker/staker.go index a35f5088c1..eb39bcb134 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -81,6 +81,7 @@ type L1ValidatorConfig struct { StartValidationFromStaked bool `koanf:"start-validation-from-staked"` ContractWalletAddress string `koanf:"contract-wallet-address"` GasRefunderAddress string `koanf:"gas-refunder-address"` + ExtraGas uint64 `koanf:"extra-gas" reload:"hot"` Dangerous DangerousConfig `koanf:"dangerous"` ParentChainWallet genericconf.WalletConfig `koanf:"parent-chain-wallet"` @@ -144,6 +145,7 @@ var DefaultL1ValidatorConfig = L1ValidatorConfig{ StartValidationFromStaked: true, ContractWalletAddress: "", GasRefunderAddress: "", + ExtraGas: 50000, Dangerous: DefaultDangerousConfig, ParentChainWallet: DefaultValidatorL1WalletConfig, } @@ -169,6 +171,7 @@ func L1ValidatorConfigAddOptions(prefix string, f *flag.FlagSet) { f.Bool(prefix+".start-validation-from-staked", DefaultL1ValidatorConfig.StartValidationFromStaked, "assume staked nodes are valid") f.String(prefix+".contract-wallet-address", DefaultL1ValidatorConfig.ContractWalletAddress, "validator smart contract wallet public address") f.String(prefix+".gas-refunder-address", DefaultL1ValidatorConfig.GasRefunderAddress, "The gas refunder contract address (optional)") + f.Uint64(prefix+".extra-gas", DefaultL1ValidatorConfig.ExtraGas, "use this much more gas than estimation says is necessary to post transactions") DangerousConfigAddOptions(prefix+".dangerous", f) genericconf.WalletConfigAddOptions(prefix+".parent-chain-wallet", f, DefaultL1ValidatorConfig.ParentChainWallet.Pathname) } diff --git a/staker/validator_wallet.go b/staker/validator_wallet.go index 6c940d8550..3e9c3b5425 100644 --- a/staker/validator_wallet.go +++ b/staker/validator_wallet.go @@ -76,13 +76,13 @@ type ContractValidatorWallet struct { rollupAddress common.Address challengeManagerAddress common.Address dataPoster *dataposter.DataPoster - extraGas uint64 + getExtraGas func() uint64 } var _ ValidatorWalletInterface = (*ContractValidatorWallet)(nil) func NewContractValidatorWallet(dp *dataposter.DataPoster, address *common.Address, walletFactoryAddr, rollupAddress common.Address, l1Reader *headerreader.HeaderReader, auth *bind.TransactOpts, rollupFromBlock int64, onWalletCreated func(common.Address), - extraGas uint64) (*ContractValidatorWallet, error) { + getExtraGas func() uint64) (*ContractValidatorWallet, error) { var con *rollupgen.ValidatorWallet if address != nil { var err error @@ -105,7 +105,7 @@ func NewContractValidatorWallet(dp *dataposter.DataPoster, address *common.Addre rollup: rollup, rollupFromBlock: rollupFromBlock, dataPoster: dp, - extraGas: extraGas, + getExtraGas: getExtraGas, } // Go complains if we make an address variable before wallet and copy it in wallet.address.Store(address) @@ -344,7 +344,7 @@ func (v *ContractValidatorWallet) estimateGas(ctx context.Context, value *big.In if err != nil { return 0, fmt.Errorf("estimating gas: %w", err) } - return g + v.extraGas, nil + return g + v.getExtraGas(), nil } func (v *ContractValidatorWallet) TimeoutChallenges(ctx context.Context, challenges []uint64) (*types.Transaction, error) { From 4c81dcc7d41f4b8527f0d97f0233ae1417f6046a Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 23 Aug 2023 12:28:00 -0600 Subject: [PATCH 29/34] Fix wallet initialization in system tests --- system_tests/staker_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/system_tests/staker_test.go b/system_tests/staker_test.go index 468463d58f..b47e3bb5d8 100644 --- a/system_tests/staker_test.go +++ b/system_tests/staker_test.go @@ -134,7 +134,7 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) if err != nil { t.Fatalf("Error creating validator dataposter: %v", err) } - valWalletA, err := staker.NewContractValidatorWallet(dpA, nil, l2nodeA.DeployInfo.ValidatorWalletCreator, l2nodeA.DeployInfo.Rollup, l2nodeA.L1Reader, &l1authA, 0, func(common.Address) {}, 10000) + valWalletA, err := staker.NewContractValidatorWallet(dpA, nil, l2nodeA.DeployInfo.ValidatorWalletCreator, l2nodeA.DeployInfo.Rollup, l2nodeA.L1Reader, &l1authA, 0, func(common.Address) {}, func() uint64 { return 10000 }) Require(t, err) if honestStakerInactive { valConfig.Strategy = "Defensive" @@ -182,7 +182,7 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) if err != nil { t.Fatalf("Error creating validator dataposter: %v", err) } - valWalletB, err := staker.NewEoaValidatorWallet(dpB, l2nodeB.DeployInfo.Rollup, l2nodeB.L1Reader.Client(), &l1authB) + valWalletB, err := staker.NewEoaValidatorWallet(dpB, l2nodeB.DeployInfo.Rollup, l2nodeB.L1Reader.Client(), &l1authB, func() uint64 { return 0 }) Require(t, err) valConfig.Strategy = "MakeNodes" statelessB, err := staker.NewStatelessBlockValidator( @@ -221,7 +221,7 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) if err != nil { t.Fatalf("Error creating validator dataposter: %v", err) } - valWalletC, err := staker.NewContractValidatorWallet(dpC, nil, l2nodeA.DeployInfo.ValidatorWalletCreator, l2nodeA.DeployInfo.Rollup, l2nodeA.L1Reader, nil, 0, func(common.Address) {}, 10000) + valWalletC, err := staker.NewContractValidatorWallet(dpC, nil, l2nodeA.DeployInfo.ValidatorWalletCreator, l2nodeA.DeployInfo.Rollup, l2nodeA.L1Reader, nil, 0, func(common.Address) {}, func() uint64 { return 10000 }) Require(t, err) valConfig.Strategy = "Watchtower" stakerC, err := staker.NewStaker( From ffe45b8690bc6be3b17b631b898139de5680f7da Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 23 Aug 2023 16:50:14 -0600 Subject: [PATCH 30/34] Check that the nonce is correct in the data poster --- arbnode/dataposter/data_poster.go | 52 ++++++++++++++++++++++--------- staker/eoa_validator_wallet.go | 40 ++++-------------------- staker/staker.go | 26 ++++++++++++++++ staker/validator_wallet.go | 6 ++++ 4 files changed, 75 insertions(+), 49 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 9451fbbafe..89b36e3e51 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -157,46 +157,59 @@ func (p *DataPoster) canPostWithNonce(ctx context.Context, nextNonce uint64) err return nil } -// GetNextNonceAndMeta retrieves generates next nonce, validates that a -// transaction can be posted with that nonce, and fetches "Meta" either last -// queued iterm (if queue isn't empty) or retrieves with last block. -func (p *DataPoster) GetNextNonceAndMeta(ctx context.Context) (uint64, []byte, error) { +// 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() - p.mutex.Lock() - defer p.mutex.Unlock() // Ensure latest finalized block state is available. blockNum, err := p.client.BlockNumber(ctx) if err != nil { - return 0, nil, err + return 0, nil, false, err } lastQueueItem, err := p.queue.FetchLast(ctx) if err != nil { - return 0, nil, err + return 0, nil, false, err } if lastQueueItem != nil { nextNonce := lastQueueItem.Data.Nonce + 1 if err := p.canPostWithNonce(ctx, nextNonce); err != nil { - return 0, nil, err + return 0, nil, false, err } - return nextNonce, lastQueueItem.Meta, nil + return nextNonce, lastQueueItem.Meta, true, nil } if err := p.updateNonce(ctx); err != nil { if !p.queue.IsPersistent() && config.WaitForL1Finality { - return 0, nil, fmt.Errorf("error getting latest finalized nonce (and queue is not persistent): %w", err) + 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. nonceQueryBlock := arbmath.UintToBig(arbmath.SaturatingUSub(blockNum, 1)) log.Warn("failed to update nonce with queue empty; falling back to using a recent block", "recentBlock", nonceQueryBlock, "err", err) nonce, err := p.client.NonceAt(ctx, p.sender, nonceQueryBlock) if err != nil { - return 0, nil, fmt.Errorf("failed to get nonce at block %v: %w", nonceQueryBlock, err) + return 0, nil, false, fmt.Errorf("failed to get nonce at block %v: %w", nonceQueryBlock, err) } p.lastBlock = nonceQueryBlock p.nonce = nonce } - meta, err := p.metadataRetriever(ctx, p.lastBlock) - return p.nonce, meta, err + return p.nonce, nil, false, nil +} + +// GetNextNonceAndMeta retrieves generates next nonce, validates that a +// transaction can be posted with that nonce, and fetches "Meta" either last +// queued iterm (if queue isn't empty) or retrieves with last block. +func (p *DataPoster) GetNextNonceAndMeta(ctx context.Context) (uint64, []byte, error) { + p.mutex.Lock() + defer p.mutex.Unlock() + nonce, meta, hasMeta, err := p.getNextNonceAndMaybeMeta(ctx) + if err != nil { + return 0, nil, err + } + if !hasMeta { + meta, err = p.metadataRetriever(ctx, p.lastBlock) + } + return nonce, meta, err } const minRbfIncrease = arbmath.OneInBips * 11 / 10 @@ -299,7 +312,16 @@ func (p *DataPoster) feeAndTipCaps(ctx context.Context, nonce uint64, gasLimit u func (p *DataPoster) PostTransaction(ctx context.Context, dataCreatedAt time.Time, nonce uint64, meta []byte, to common.Address, calldata []byte, gasLimit uint64, value *big.Int) (*types.Transaction, error) { p.mutex.Lock() defer p.mutex.Unlock() - err := p.updateBalance(ctx) + + expectedNonce, _, _, err := p.getNextNonceAndMaybeMeta(ctx) + if err != nil { + return nil, err + } + if nonce != expectedNonce { + return nil, fmt.Errorf("data poster expected next transaction to have nonce %v but was requested to post transaction with nonce %v", expectedNonce, nonce) + } + + err = p.updateBalance(ctx) if err != nil { return nil, fmt.Errorf("failed to update data poster balance: %w", err) } diff --git a/staker/eoa_validator_wallet.go b/staker/eoa_validator_wallet.go index e3d149c99f..a91510cbbe 100644 --- a/staker/eoa_validator_wallet.go +++ b/staker/eoa_validator_wallet.go @@ -12,7 +12,6 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/log" "github.com/offchainlabs/nitro/arbnode/dataposter" "github.com/offchainlabs/nitro/arbutil" "github.com/offchainlabs/nitro/solgen/go/challengegen" @@ -88,44 +87,13 @@ func (w *EoaValidatorWallet) TestTransactions(context.Context, []*types.Transact return nil } -// Polls until the nonce from dataposter catches up with transactions posted -// by validator wallet. -func (w *EoaValidatorWallet) pollForNonce(ctx context.Context) (uint64, error) { - var nonce uint64 - flag := true - for flag { - var err error - select { - // TODO: consider adding config for eoa validator wallet and pull this - // polling time from there. - case <-time.After(100 * time.Millisecond): - nonce, _, err = w.dataPoster.GetNextNonceAndMeta(ctx) - if err != nil { - return 0, fmt.Errorf("get next nonce and meta: %w", err) - } - if nonce >= w.txCount.Load() { - flag = false - break - } - log.Warn("Dataposter nonce too low", "nonce", nonce, "validator tx count", w.txCount.Load()) - case <-ctx.Done(): - return 0, ctx.Err() - } - } - return nonce, nil -} - func (w *EoaValidatorWallet) ExecuteTransactions(ctx context.Context, builder *ValidatorTxBuilder, _ common.Address) (*types.Transaction, error) { if len(builder.transactions) == 0 { return nil, nil } - nonce, err := w.pollForNonce(ctx) + nonce, err := w.L1Client().NonceAt(ctx, w.auth.From, nil) if err != nil { - return nil, fmt.Errorf("polling for dataposter nonce to catch up: %w", err) - } - if nonce > w.txCount.Load() { - // If this happens, it probably means the dataposter is used by another client, besides validator. - log.Warn("Precondition failure, dataposter nonce is higher than validator transactio count", "dataposter nonce", nonce, "validator tx count", w.txCount.Load()) + return nil, err } tx := builder.transactions[0] // we ignore future txs and only execute the first gas := tx.Gas() + w.getExtraGas() @@ -163,3 +131,7 @@ func (b *EoaValidatorWallet) StopAndWait() { b.StopWaiter.StopAndWait() b.dataPoster.StopAndWait() } + +func (b *EoaValidatorWallet) DataPoster() *dataposter.DataPoster { + return b.dataPoster +} diff --git a/staker/staker.go b/staker/staker.go index eb39bcb134..6ee561b867 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -493,7 +493,33 @@ func (s *Staker) shouldAct(ctx context.Context) bool { return true } +func (s *Staker) confirmDataPosterIsReady(ctx context.Context) error { + dp := s.wallet.DataPoster() + if dp == nil { + return nil + } + dataPosterNonce, _, err := dp.GetNextNonceAndMeta(ctx) + if err != nil { + return err + } + latestNonce, err := s.l1Reader.Client().NonceAt(ctx, dp.Sender(), nil) + if err != nil { + return err + } + if dataPosterNonce > latestNonce { + return fmt.Errorf("data poster nonce %v is ahead of on-chain nonce %v -- probably waiting for a pending transaction to be included in a block", dataPosterNonce, latestNonce) + } + if dataPosterNonce < latestNonce { + return fmt.Errorf("data poster nonce %v is behind on-chain nonce %v -- is something else making transactions on this address?", dataPosterNonce, latestNonce) + } + return nil +} + func (s *Staker) Act(ctx context.Context) (*types.Transaction, error) { + err := s.confirmDataPosterIsReady(ctx) + if err != nil { + return nil, err + } if s.config.strategy != WatchtowerStrategy { whitelisted, err := s.IsWhitelisted(ctx) if err != nil { diff --git a/staker/validator_wallet.go b/staker/validator_wallet.go index 3e9c3b5425..133a808eac 100644 --- a/staker/validator_wallet.go +++ b/staker/validator_wallet.go @@ -61,6 +61,8 @@ type ValidatorWalletInterface interface { AuthIfEoa() *bind.TransactOpts Start(context.Context) StopAndWait() + // May be nil + DataPoster() *dataposter.DataPoster } type ContractValidatorWallet struct { @@ -418,6 +420,10 @@ func (b *ContractValidatorWallet) StopAndWait() { b.StopWaiter.StopAndWait() } +func (b *ContractValidatorWallet) DataPoster() *dataposter.DataPoster { + return b.dataPoster +} + func GetValidatorWalletContract( ctx context.Context, validatorWalletFactoryAddr common.Address, From 4a314b74563a9014494f6f46b021992d42371166 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 23 Aug 2023 16:50:19 -0600 Subject: [PATCH 31/34] Fix flaky tests --- system_tests/forwarder_test.go | 3 ++- system_tests/retryable_test.go | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/system_tests/forwarder_test.go b/system_tests/forwarder_test.go index 93c928d145..0a954719d8 100644 --- a/system_tests/forwarder_test.go +++ b/system_tests/forwarder_test.go @@ -127,6 +127,7 @@ func createForwardingNode( nodeConfig := arbnode.ConfigDefaultL1Test() nodeConfig.Sequencer.Enable = false nodeConfig.DelayedSequencer.Enable = false + nodeConfig.BatchPoster.Enable = false nodeConfig.Forwarder.RedisUrl = redisUrl nodeConfig.ForwardingTarget = fallbackPath // nodeConfig.Feed.Output.Enable = false @@ -148,7 +149,7 @@ func createSequencer( ipcConfig.Path = ipcPath ipcConfig.Apply(stackConfig) nodeConfig := arbnode.ConfigDefaultL1Test() - nodeConfig.BatchPoster.Enable = true + nodeConfig.BatchPoster.Enable = false nodeConfig.SeqCoordinator.Enable = true nodeConfig.SeqCoordinator.RedisUrl = redisUrl nodeConfig.SeqCoordinator.MyUrl = ipcPath diff --git a/system_tests/retryable_test.go b/system_tests/retryable_test.go index 29bfdd6e6f..b1dd32d1dc 100644 --- a/system_tests/retryable_test.go +++ b/system_tests/retryable_test.go @@ -319,7 +319,7 @@ func TestSubmissionGasCosts(t *testing.T) { usefulGas := params.TxGas excessGasLimit := uint64(808) - maxSubmissionFee := big.NewInt(1e13) + maxSubmissionFee := big.NewInt(1e14) retryableGas := arbmath.UintToBig(usefulGas + excessGasLimit) // will only burn the intrinsic cost retryableL2CallValue := big.NewInt(1e4) retryableCallData := []byte{} @@ -358,8 +358,10 @@ func TestSubmissionGasCosts(t *testing.T) { if redeemReceipt.Status != types.ReceiptStatusSuccessful { Fatal(t, "first retry tx failed") } + redeemBlock, err := l2client.HeaderByNumber(ctx, redeemReceipt.BlockNumber) + Require(t, err) - l2BaseFee := GetBaseFee(t, l2client, ctx) + l2BaseFee := redeemBlock.BaseFee excessGasPrice := arbmath.BigSub(gasFeeCap, l2BaseFee) excessWei := arbmath.BigMulByUint(l2BaseFee, excessGasLimit) excessWei.Add(excessWei, arbmath.BigMul(excessGasPrice, retryableGas)) From e557bd8bc29c37196b271afb8d0826ab72b86dba Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 23 Aug 2023 17:01:06 -0600 Subject: [PATCH 32/34] Move data poster config from BlockValidatorConfig to L1ValidatorConfig --- arbnode/dataposter/data_poster.go | 12 ++++++ arbnode/node.go | 8 ++-- staker/block_validator.go | 14 ------- staker/staker.go | 62 +++++++++++++++++++++++-------- system_tests/staker_test.go | 4 +- 5 files changed, 66 insertions(+), 34 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 89b36e3e51..1504fcf58b 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -648,6 +648,12 @@ var DefaultDataPosterConfig = DataPosterConfig{ UseNoOpStorage: false, } +var DefaultDataPosterConfigForValidator = func() DataPosterConfig { + config := DefaultDataPosterConfig + config.MaxMempoolTransactions = 1 // the validator cannot queue transactions + return config +}() + var TestDataPosterConfig = DataPosterConfig{ ReplacementTimes: "1s,2s,5s,10s,20s,30s,1m,5m", RedisSigner: signature.TestSimpleHmacConfig, @@ -662,3 +668,9 @@ var TestDataPosterConfig = DataPosterConfig{ UseLevelDB: false, UseNoOpStorage: false, } + +var TestDataPosterConfigForValidator = func() DataPosterConfig { + config := TestDataPosterConfig + config.MaxMempoolTransactions = 1 // the validator cannot queue transactions + return config +}() diff --git a/arbnode/node.go b/arbnode/node.go index abc4367a39..e6960a3f22 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -434,6 +434,7 @@ func ConfigDefaultL1NonSequencerTest() *Config { config.BatchPoster.Enable = false config.SeqCoordinator.Enable = false config.BlockValidator = staker.TestBlockValidatorConfig + config.Staker = staker.TestL1ValidatorConfig config.Staker.Enable = false config.BlockValidator.ValidationServer.URL = "" config.Forwarder = execution.DefaultTestForwarderConfig @@ -451,6 +452,7 @@ func ConfigDefaultL2Test() *Config { config.Feed.Output.Signed = false config.SeqCoordinator.Signer.ECDSA.AcceptSequencer = false config.SeqCoordinator.Signer.ECDSA.Dangerous.AcceptMissing = true + config.Staker = staker.TestL1ValidatorConfig config.Staker.Enable = false config.BlockValidator.ValidationServer.URL = "" config.TransactionStreamer = DefaultTransactionStreamerConfig @@ -554,19 +556,19 @@ func ValidatorDataposter( mdRetriever := func(ctx context.Context, blockNum *big.Int) ([]byte, error) { return nil, nil } - redisC, err := redisutil.RedisClientFromURL(cfg.BlockValidator.RedisUrl) + redisC, err := redisutil.RedisClientFromURL(cfg.Staker.RedisUrl) if err != nil { return nil, fmt.Errorf("creating redis client from url: %w", err) } lockCfgFetcher := func() *redislock.SimpleCfg { - return &cfg.BlockValidator.RedisLock + return &cfg.Staker.RedisLock } redisLock, err := redislock.NewSimple(redisC, lockCfgFetcher, func() bool { return syncMonitor.Synced() }) if err != nil { return nil, err } dpCfg := func() *dataposter.DataPosterConfig { - return &cfg.BlockValidator.DataPoster + return &cfg.Staker.DataPoster } return dataposter.NewDataPoster(db, l1Reader, transactOpts, redisC, redisLock, dpCfg, mdRetriever) } diff --git a/staker/block_validator.go b/staker/block_validator.go index 5da4365143..f04b852041 100644 --- a/staker/block_validator.go +++ b/staker/block_validator.go @@ -19,8 +19,6 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/rlp" - "github.com/offchainlabs/nitro/arbnode/dataposter" - "github.com/offchainlabs/nitro/arbnode/redislock" "github.com/offchainlabs/nitro/arbutil" "github.com/offchainlabs/nitro/util/containers" "github.com/offchainlabs/nitro/util/rpcclient" @@ -88,9 +86,6 @@ type BlockValidatorConfig struct { PendingUpgradeModuleRoot string `koanf:"pending-upgrade-module-root"` // TODO(magic) requires StatelessBlockValidator recreation on hot reload FailureIsFatal bool `koanf:"failure-is-fatal" reload:"hot"` Dangerous BlockValidatorDangerousConfig `koanf:"dangerous"` - DataPoster dataposter.DataPosterConfig `koanf:"data-poster" reload:"hot"` - RedisUrl string `koanf:"redis-url"` - RedisLock redislock.SimpleCfg `koanf:"redis-lock" reload:"hot"` } func (c *BlockValidatorConfig) Validate() error { @@ -113,9 +108,6 @@ func BlockValidatorConfigAddOptions(prefix string, f *flag.FlagSet) { f.String(prefix+".pending-upgrade-module-root", DefaultBlockValidatorConfig.PendingUpgradeModuleRoot, "pending upgrade wasm module root to additionally validate (hash, 'latest' or empty)") f.Bool(prefix+".failure-is-fatal", DefaultBlockValidatorConfig.FailureIsFatal, "failing a validation is treated as a fatal error") BlockValidatorDangerousConfigAddOptions(prefix+".dangerous", f) - dataposter.DataPosterConfigAddOptions(prefix+".data-poster", f) - f.String(prefix+".redis-url", DefaultBlockValidatorConfig.RedisUrl, "redis url for block validator") - redislock.AddConfigOptions(prefix+".redis-lock", f) } func BlockValidatorDangerousConfigAddOptions(prefix string, f *flag.FlagSet) { @@ -132,9 +124,6 @@ var DefaultBlockValidatorConfig = BlockValidatorConfig{ PendingUpgradeModuleRoot: "latest", FailureIsFatal: true, Dangerous: DefaultBlockValidatorDangerousConfig, - DataPoster: dataposter.DefaultDataPosterConfig, - RedisUrl: "", - RedisLock: redislock.DefaultCfg, } var TestBlockValidatorConfig = BlockValidatorConfig{ @@ -147,9 +136,6 @@ var TestBlockValidatorConfig = BlockValidatorConfig{ PendingUpgradeModuleRoot: "latest", FailureIsFatal: true, Dangerous: DefaultBlockValidatorDangerousConfig, - DataPoster: dataposter.TestDataPosterConfig, - RedisUrl: "", - RedisLock: redislock.DefaultCfg, } var DefaultBlockValidatorDangerousConfig = BlockValidatorDangerousConfig{ diff --git a/staker/staker.go b/staker/staker.go index 6ee561b867..9b7e6c238e 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -20,6 +20,8 @@ import ( "github.com/ethereum/go-ethereum/rpc" flag "github.com/spf13/pflag" + "github.com/offchainlabs/nitro/arbnode/dataposter" + "github.com/offchainlabs/nitro/arbnode/redislock" "github.com/offchainlabs/nitro/arbutil" "github.com/offchainlabs/nitro/cmd/genericconf" "github.com/offchainlabs/nitro/util/arbmath" @@ -69,21 +71,24 @@ func L1PostingStrategyAddOptions(prefix string, f *flag.FlagSet) { } type L1ValidatorConfig struct { - Enable bool `koanf:"enable"` - Strategy string `koanf:"strategy"` - StakerInterval time.Duration `koanf:"staker-interval"` - MakeAssertionInterval time.Duration `koanf:"make-assertion-interval"` - PostingStrategy L1PostingStrategy `koanf:"posting-strategy"` - DisableChallenge bool `koanf:"disable-challenge"` - ConfirmationBlocks int64 `koanf:"confirmation-blocks"` - UseSmartContractWallet bool `koanf:"use-smart-contract-wallet"` - OnlyCreateWalletContract bool `koanf:"only-create-wallet-contract"` - StartValidationFromStaked bool `koanf:"start-validation-from-staked"` - ContractWalletAddress string `koanf:"contract-wallet-address"` - GasRefunderAddress string `koanf:"gas-refunder-address"` - ExtraGas uint64 `koanf:"extra-gas" reload:"hot"` - Dangerous DangerousConfig `koanf:"dangerous"` - ParentChainWallet genericconf.WalletConfig `koanf:"parent-chain-wallet"` + Enable bool `koanf:"enable"` + Strategy string `koanf:"strategy"` + StakerInterval time.Duration `koanf:"staker-interval"` + MakeAssertionInterval time.Duration `koanf:"make-assertion-interval"` + PostingStrategy L1PostingStrategy `koanf:"posting-strategy"` + DisableChallenge bool `koanf:"disable-challenge"` + ConfirmationBlocks int64 `koanf:"confirmation-blocks"` + UseSmartContractWallet bool `koanf:"use-smart-contract-wallet"` + OnlyCreateWalletContract bool `koanf:"only-create-wallet-contract"` + StartValidationFromStaked bool `koanf:"start-validation-from-staked"` + ContractWalletAddress string `koanf:"contract-wallet-address"` + GasRefunderAddress string `koanf:"gas-refunder-address"` + DataPoster dataposter.DataPosterConfig `koanf:"data-poster" reload:"hot"` + RedisUrl string `koanf:"redis-url"` + RedisLock redislock.SimpleCfg `koanf:"redis-lock" reload:"hot"` + ExtraGas uint64 `koanf:"extra-gas" reload:"hot"` + Dangerous DangerousConfig `koanf:"dangerous"` + ParentChainWallet genericconf.WalletConfig `koanf:"parent-chain-wallet"` strategy StakerStrategy gasRefunder common.Address @@ -145,6 +150,30 @@ var DefaultL1ValidatorConfig = L1ValidatorConfig{ StartValidationFromStaked: true, ContractWalletAddress: "", GasRefunderAddress: "", + DataPoster: dataposter.DefaultDataPosterConfigForValidator, + RedisUrl: "", + RedisLock: redislock.DefaultCfg, + ExtraGas: 50000, + Dangerous: DefaultDangerousConfig, + ParentChainWallet: DefaultValidatorL1WalletConfig, +} + +var TestL1ValidatorConfig = L1ValidatorConfig{ + Enable: true, + Strategy: "Watchtower", + StakerInterval: time.Millisecond * 10, + MakeAssertionInterval: 0, + PostingStrategy: L1PostingStrategy{}, + DisableChallenge: false, + ConfirmationBlocks: 0, + UseSmartContractWallet: false, + OnlyCreateWalletContract: false, + StartValidationFromStaked: true, + ContractWalletAddress: "", + GasRefunderAddress: "", + DataPoster: dataposter.TestDataPosterConfigForValidator, + RedisUrl: "", + RedisLock: redislock.DefaultCfg, ExtraGas: 50000, Dangerous: DefaultDangerousConfig, ParentChainWallet: DefaultValidatorL1WalletConfig, @@ -171,7 +200,10 @@ func L1ValidatorConfigAddOptions(prefix string, f *flag.FlagSet) { f.Bool(prefix+".start-validation-from-staked", DefaultL1ValidatorConfig.StartValidationFromStaked, "assume staked nodes are valid") f.String(prefix+".contract-wallet-address", DefaultL1ValidatorConfig.ContractWalletAddress, "validator smart contract wallet public address") f.String(prefix+".gas-refunder-address", DefaultL1ValidatorConfig.GasRefunderAddress, "The gas refunder contract address (optional)") + f.String(prefix+".redis-url", DefaultL1ValidatorConfig.RedisUrl, "redis url for L1 validator") f.Uint64(prefix+".extra-gas", DefaultL1ValidatorConfig.ExtraGas, "use this much more gas than estimation says is necessary to post transactions") + dataposter.DataPosterConfigAddOptions(prefix+".data-poster", f) + redislock.AddConfigOptions(prefix+".redis-lock", f) DangerousConfigAddOptions(prefix+".dangerous", f) genericconf.WalletConfigAddOptions(prefix+".parent-chain-wallet", f, DefaultL1ValidatorConfig.ParentChainWallet.Pathname) } diff --git a/system_tests/staker_test.go b/system_tests/staker_test.go index b47e3bb5d8..82eede9f60 100644 --- a/system_tests/staker_test.go +++ b/system_tests/staker_test.go @@ -128,13 +128,13 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) validatorUtils, err := rollupgen.NewValidatorUtils(l2nodeA.DeployInfo.ValidatorUtils, l1client) Require(t, err) - valConfig := staker.L1ValidatorConfig{} + valConfig := staker.TestL1ValidatorConfig dpA, err := arbnode.ValidatorDataposter(rawdb.NewTable(l2nodeB.ArbDB, storage.BlockValidatorPrefix), l2nodeA.L1Reader, &l1authA, NewFetcherFromConfig(arbnode.ConfigDefaultL1NonSequencerTest()), nil) if err != nil { t.Fatalf("Error creating validator dataposter: %v", err) } - valWalletA, err := staker.NewContractValidatorWallet(dpA, nil, l2nodeA.DeployInfo.ValidatorWalletCreator, l2nodeA.DeployInfo.Rollup, l2nodeA.L1Reader, &l1authA, 0, func(common.Address) {}, func() uint64 { return 10000 }) + valWalletA, err := staker.NewContractValidatorWallet(dpA, nil, l2nodeA.DeployInfo.ValidatorWalletCreator, l2nodeA.DeployInfo.Rollup, l2nodeA.L1Reader, &l1authA, 0, func(common.Address) {}, func() uint64 { return valConfig.ExtraGas }) Require(t, err) if honestStakerInactive { valConfig.Strategy = "Defensive" From 69a5c3e76115a9bb022f2ef41c8487b8646d697b Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 23 Aug 2023 18:36:44 -0600 Subject: [PATCH 33/34] Improve batch poster reliability for L3s --- arbnode/batch_poster.go | 24 +++++++++++++++++++++--- arbnode/dataposter/data_poster.go | 19 ++++++++++++------- arbutil/wait_for_l1.go | 4 ++-- util/headerreader/header_reader.go | 4 ++++ 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 768f73276b..e9a1663741 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -259,6 +259,8 @@ func NewBatchPoster(dataPosterDB ethdb.Database, l1Reader *headerreader.HeaderRe // checkRevert checks blocks with number in range [from, to] whether they // contain reverted batch_poster transaction. +// It returns true if it finds batch posting needs to halt, which is true if a batch reverts +// unless the data poster is configured with noop storage which can tolerate reverts. func (b *BatchPoster) checkReverts(ctx context.Context, from, to int64) (bool, error) { if from > to { return false, fmt.Errorf("wrong range, from: %d is more to: %d", from, to) @@ -280,8 +282,13 @@ func (b *BatchPoster) checkReverts(ctx context.Context, from, to int64) (bool, e return false, fmt.Errorf("getting a receipt for transaction: %v, %w", tx.Hash(), err) } if r.Status == types.ReceiptStatusFailed { - log.Error("Transaction from batch poster reverted", "nonce", tx.Nonce(), "txHash", tx.Hash(), "blockNumber", r.BlockNumber, "blockHash", r.BlockHash) - return true, nil + shouldHalt := !b.config().DataPoster.UseNoOpStorage + logLevel := log.Warn + if shouldHalt { + logLevel = log.Error + } + logLevel("Transaction from batch poster reverted", "nonce", tx.Nonce(), "txHash", tx.Hash(), "blockNumber", r.BlockNumber, "blockHash", r.BlockHash) + return shouldHalt, nil } } } @@ -881,7 +888,8 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) if err != nil { return false, err } - if _, err := b.dataPoster.PostTransaction(ctx, firstMsgTime, nonce, newMeta, b.seqInboxAddr, data, gasLimit, new(big.Int)); err != nil { + tx, err := b.dataPoster.PostTransaction(ctx, firstMsgTime, nonce, newMeta, b.seqInboxAddr, data, gasLimit, new(big.Int)) + if err != nil { return false, err } log.Info( @@ -920,6 +928,16 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) b.backlog = 0 } b.building = nil + + // If we aren't queueing up transactions, wait for the receipt before moving on to the next batch. + if config.DataPoster.UseNoOpStorage { + receipt, err := b.l1Reader.WaitForTxApproval(ctx, tx) + if err != nil { + return false, fmt.Errorf("error waiting for tx receipt: %w", err) + } + log.Info("Got successful receipt from batch poster transaction", "txHash", tx.Hash(), "blockNumber", receipt.BlockNumber, "blockHash", receipt.BlockHash) + } + return true, nil } diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 9451fbbafe..3c8da8ef5f 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -92,21 +92,26 @@ func parseReplacementTimes(val string) ([]time.Duration, error) { } func NewDataPoster(db ethdb.Database, headerReader *headerreader.HeaderReader, auth *bind.TransactOpts, redisClient redis.UniversalClient, redisLock AttemptLocker, config ConfigFetcher, metadataRetriever func(ctx context.Context, blockNum *big.Int) ([]byte, error)) (*DataPoster, error) { - replacementTimes, err := parseReplacementTimes(config().ReplacementTimes) + initConfig := config() + replacementTimes, err := parseReplacementTimes(initConfig.ReplacementTimes) if err != nil { return nil, err } + if headerReader.IsParentChainArbitrum() && !initConfig.UseNoOpStorage { + initConfig.UseNoOpStorage = true + log.Info("Disabling data poster storage, as parent chain appears to be an Arbitrum chain without a mempool") + } var queue QueueStorage switch { - case config().UseLevelDB: - queue = leveldb.New(db) - case config().UseNoOpStorage: + case initConfig.UseNoOpStorage: queue = &noop.Storage{} + case initConfig.UseLevelDB: + queue = leveldb.New(db) case redisClient == nil: queue = slice.NewStorage() default: var err error - queue, err = redisstorage.NewStorage(redisClient, "data-poster.queue", &config().RedisSigner) + queue, err = redisstorage.NewStorage(redisClient, "data-poster.queue", &initConfig.RedisSigner) if err != nil { return nil, err } @@ -587,8 +592,8 @@ type DataPosterConfig struct { 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" reload:"hot"` - UseNoOpStorage bool `koanf:"use-noop-storage" reload:"hot"` + UseLevelDB bool `koanf:"use-leveldb"` + UseNoOpStorage bool `koanf:"use-noop-storage"` } // ConfigFetcher function type is used instead of directly passing config so diff --git a/arbutil/wait_for_l1.go b/arbutil/wait_for_l1.go index ec6bb5a380..12d494a230 100644 --- a/arbutil/wait_for_l1.go +++ b/arbutil/wait_for_l1.go @@ -12,8 +12,8 @@ import ( "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/vm" ) type L1Interface interface { @@ -88,7 +88,7 @@ func DetailTxError(ctx context.Context, client L1Interface, tx *types.Transactio } _, err = SendTxAsCall(ctx, client, tx, from, txRes.BlockNumber, true) if err == nil { - return fmt.Errorf("%w for tx hash %v", core.ErrGasLimitReached, tx.Hash()) + return fmt.Errorf("%w for tx hash %v", vm.ErrOutOfGas, tx.Hash()) } return fmt.Errorf("SendTxAsCall got: %w for tx hash %v", err, tx.Hash()) } diff --git a/util/headerreader/header_reader.go b/util/headerreader/header_reader.go index 739a96b438..e5807224c0 100644 --- a/util/headerreader/header_reader.go +++ b/util/headerreader/header_reader.go @@ -462,6 +462,10 @@ func (s *HeaderReader) UseFinalityData() bool { return s.config().UseFinalityData } +func (s *HeaderReader) IsParentChainArbitrum() bool { + return s.isParentChainArbitrum +} + func (s *HeaderReader) Start(ctxIn context.Context) { s.StopWaiter.Start(ctxIn, s) s.LaunchThread(s.broadcastLoop) From 3c0bb4190c064bf11cf46ae41eddd4c8451bb9e1 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 23 Aug 2023 18:59:21 -0600 Subject: [PATCH 34/34] Use data poster for eoa to timeout challenges --- staker/eoa_validator_wallet.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/staker/eoa_validator_wallet.go b/staker/eoa_validator_wallet.go index a91510cbbe..09175332bf 100644 --- a/staker/eoa_validator_wallet.go +++ b/staker/eoa_validator_wallet.go @@ -6,7 +6,6 @@ package staker import ( "context" "fmt" - "sync/atomic" "time" "github.com/ethereum/go-ethereum/accounts/abi/bind" @@ -27,7 +26,6 @@ type EoaValidatorWallet struct { challengeManager *challengegen.ChallengeManager challengeManagerAddress common.Address dataPoster *dataposter.DataPoster - txCount atomic.Uint64 getExtraGas func() uint64 } @@ -39,7 +37,6 @@ func NewEoaValidatorWallet(dataPoster *dataposter.DataPoster, rollupAddress comm client: l1Client, rollupAddress: rollupAddress, dataPoster: dataPoster, - txCount: atomic.Uint64{}, getExtraGas: getExtraGas, }, nil } @@ -91,18 +88,21 @@ func (w *EoaValidatorWallet) ExecuteTransactions(ctx context.Context, builder *V if len(builder.transactions) == 0 { return nil, nil } + tx := builder.transactions[0] // we ignore future txs and only execute the first + return w.postTransaction(ctx, tx) +} + +func (w *EoaValidatorWallet) postTransaction(ctx context.Context, baseTx *types.Transaction) (*types.Transaction, error) { nonce, err := w.L1Client().NonceAt(ctx, w.auth.From, nil) if err != nil { return nil, err } - tx := builder.transactions[0] // we ignore future txs and only execute the first - gas := tx.Gas() + w.getExtraGas() - trans, err := w.dataPoster.PostTransaction(ctx, time.Now(), nonce, nil, *tx.To(), tx.Data(), gas, tx.Value()) + gas := baseTx.Gas() + w.getExtraGas() + newTx, err := w.dataPoster.PostTransaction(ctx, time.Now(), nonce, nil, *baseTx.To(), baseTx.Data(), gas, baseTx.Value()) if err != nil { return nil, fmt.Errorf("post transaction: %w", err) } - w.txCount.Store(nonce) - return trans, nil + return newTx, nil } func (w *EoaValidatorWallet) TimeoutChallenges(ctx context.Context, timeouts []uint64) (*types.Transaction, error) { @@ -111,7 +111,12 @@ func (w *EoaValidatorWallet) TimeoutChallenges(ctx context.Context, timeouts []u } auth := *w.auth auth.Context = ctx - return w.challengeManager.Timeout(&auth, timeouts[0]) + auth.NoSend = true + tx, err := w.challengeManager.Timeout(&auth, timeouts[0]) + if err != nil { + return nil, err + } + return w.postTransaction(ctx, tx) } func (w *EoaValidatorWallet) CanBatchTxs() bool {