diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 001fdab76e..15446fe855 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -68,18 +68,16 @@ var ( // DataPoster must be RLP serializable and deserializable type DataPoster struct { stopwaiter.StopWaiter - headerReader *headerreader.HeaderReader - client arbutil.L1Interface - auth *bind.TransactOpts - signer signerFn - config ConfigFetcher - usingNoOpStorage bool - replacementTimes []time.Duration - blobTxReplacementTimes []time.Duration - metadataRetriever func(ctx context.Context, blockNum *big.Int) ([]byte, error) - extraBacklog func() uint64 - parentChainID *big.Int - parentChainID256 *uint256.Int + headerReader *headerreader.HeaderReader + client arbutil.L1Interface + auth *bind.TransactOpts + signer signerFn + config ConfigFetcher + usingNoOpStorage bool + metadataRetriever func(ctx context.Context, blockNum *big.Int) ([]byte, error) + extraBacklog func() uint64 + parentChainID *big.Int + parentChainID256 *uint256.Int // These fields are protected by the mutex. // TODO: factor out these fields into separate structure, since now one @@ -101,27 +99,6 @@ type DataPoster struct { // This can be local or external, hence the context parameter. type signerFn func(context.Context, common.Address, *types.Transaction) (*types.Transaction, error) -func parseReplacementTimes(val string) ([]time.Duration, error) { - var res []time.Duration - var lastReplacementTime time.Duration - for _, s := range strings.Split(val, ",") { - t, err := time.ParseDuration(s) - if err != nil { - return nil, fmt.Errorf("parsing durations: %w", err) - } - if t <= lastReplacementTime { - return nil, errors.New("replacement times must be increasing") - } - res = append(res, t) - lastReplacementTime = t - } - if len(res) == 0 { - log.Warn("Disabling replace-by-fee for data poster") - } - // To avoid special casing "don't replace again", replace in 10 years. - return append(res, time.Hour*24*365*10), nil -} - type DataPosterOpts struct { Database ethdb.Database HeaderReader *headerreader.HeaderReader @@ -136,14 +113,6 @@ type DataPosterOpts struct { func NewDataPoster(ctx context.Context, opts *DataPosterOpts) (*DataPoster, error) { cfg := opts.Config() - replacementTimes, err := parseReplacementTimes(cfg.ReplacementTimes) - if err != nil { - return nil, err - } - blobTxReplacementTimes, err := parseReplacementTimes(cfg.BlobTxReplacementTimes) - if err != nil { - return nil, err - } useNoOpStorage := cfg.UseNoOpStorage if opts.HeaderReader.IsParentChainArbitrum() && !cfg.UseNoOpStorage { useNoOpStorage = true @@ -187,16 +156,14 @@ func NewDataPoster(ctx context.Context, opts *DataPosterOpts) (*DataPoster, erro signer: func(_ context.Context, addr common.Address, tx *types.Transaction) (*types.Transaction, error) { return opts.Auth.Signer(addr, tx) }, - config: opts.Config, - usingNoOpStorage: useNoOpStorage, - replacementTimes: replacementTimes, - blobTxReplacementTimes: blobTxReplacementTimes, - metadataRetriever: opts.MetadataRetriever, - queue: queue, - errorCount: make(map[uint64]int), - maxFeeCapExpression: expression, - extraBacklog: opts.ExtraBacklog, - parentChainID: opts.ParentChainID, + config: opts.Config, + usingNoOpStorage: useNoOpStorage, + metadataRetriever: opts.MetadataRetriever, + queue: queue, + errorCount: make(map[uint64]int), + maxFeeCapExpression: expression, + extraBacklog: opts.ExtraBacklog, + parentChainID: opts.ParentChainID, } var overflow bool dp.parentChainID256, overflow = uint256.FromBig(opts.ParentChainID) @@ -781,9 +748,9 @@ func (p *DataPoster) PostTransaction(ctx context.Context, dataCreatedAt time.Tim var deprecatedData types.DynamicFeeTx var inner types.TxData - replacementTimes := p.replacementTimes + replacementTimes := p.config().ReplacementTimes if len(kzgBlobs) > 0 { - replacementTimes = p.blobTxReplacementTimes + replacementTimes = p.config().BlobTxReplacementTimes value256, overflow := uint256.FromBig(value) if overflow { return nil, fmt.Errorf("blob transaction callvalue %v overflows uint256", value) @@ -1026,9 +993,9 @@ func (p *DataPoster) replaceTx(ctx context.Context, prevTx *storage.QueuedTransa return p.sendTx(ctx, prevTx, &newTx) } - replacementTimes := p.replacementTimes + replacementTimes := p.config().ReplacementTimes if len(prevTx.FullTx.BlobHashes()) > 0 { - replacementTimes = p.blobTxReplacementTimes + replacementTimes = p.config().BlobTxReplacementTimes } elapsed := time.Since(prevTx.Created) @@ -1155,7 +1122,7 @@ func (p *DataPoster) Start(ctxIn context.Context) { log.Warn("failed to update tx poster nonce", "err", err) } now := time.Now() - nextCheck := now.Add(arbmath.MinInt(p.replacementTimes[0], p.blobTxReplacementTimes[0])) + nextCheck := now.Add(arbmath.MinInt(p.config().ReplacementTimes[0], p.config().BlobTxReplacementTimes[0])) maxTxsToRbf := p.config().MaxMempoolTransactions if maxTxsToRbf == 0 { maxTxsToRbf = 512 @@ -1260,8 +1227,8 @@ type QueueStorage interface { type DataPosterConfig struct { RedisSigner signature.SimpleHmacConfig `koanf:"redis-signer"` - ReplacementTimes string `koanf:"replacement-times"` - BlobTxReplacementTimes string `koanf:"blob-tx-replacement-times"` + ReplacementTimes []time.Duration `koanf:"replacement-times"` + BlobTxReplacementTimes []time.Duration `koanf:"blob-tx-replacement-times"` // This is forcibly disabled if the parent chain is an Arbitrum chain, // so you should probably use DataPoster's waitForL1Finality method instead of reading this field directly. WaitForL1Finality bool `koanf:"wait-for-l1-finality" reload:"hot"` @@ -1322,8 +1289,8 @@ type DangerousConfig struct { type ConfigFetcher func() *DataPosterConfig func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet, defaultDataPosterConfig DataPosterConfig) { - f.String(prefix+".replacement-times", defaultDataPosterConfig.ReplacementTimes, "comma-separated list of durations since first posting to attempt a replace-by-fee") - f.String(prefix+".blob-tx-replacement-times", defaultDataPosterConfig.BlobTxReplacementTimes, "comma-separated list of durations since first posting a blob transaction to attempt a replace-by-fee") + f.DurationSlice(prefix+".replacement-times", defaultDataPosterConfig.ReplacementTimes, "comma-separated list of durations since first posting to attempt a replace-by-fee") + f.DurationSlice(prefix+".blob-tx-replacement-times", defaultDataPosterConfig.BlobTxReplacementTimes, "comma-separated list of durations since first posting a blob transaction to attempt a replace-by-fee") f.Bool(prefix+".wait-for-l1-finality", defaultDataPosterConfig.WaitForL1Finality, "only treat a transaction as confirmed after L1 finality has been achieved (recommended)") f.Uint64(prefix+".max-mempool-transactions", defaultDataPosterConfig.MaxMempoolTransactions, "the maximum number of transactions to have queued in the mempool at once (0 = unlimited)") f.Uint64(prefix+".max-mempool-weight", defaultDataPosterConfig.MaxMempoolWeight, "the maximum number of weight (weight = min(1, tx.blobs)) to have queued in the mempool at once (0 = unlimited)") @@ -1367,8 +1334,8 @@ func addExternalSignerOptions(prefix string, f *pflag.FlagSet) { } var DefaultDataPosterConfig = DataPosterConfig{ - ReplacementTimes: "5m,10m,20m,30m,1h,2h,4h,6h,8h,12h,16h,18h,20h,22h", - BlobTxReplacementTimes: "5m,10m,30m,1h,4h,8h,16h,22h", + ReplacementTimes: []time.Duration{5 * time.Minute, 10 * time.Minute, 20 * time.Minute, 30 * time.Minute, time.Hour, 2 * time.Hour, 4 * time.Hour, 6 * time.Hour, 8 * time.Hour, 12 * time.Hour, 16 * time.Hour, 18 * time.Hour, 20 * time.Hour, 22 * time.Hour}, + BlobTxReplacementTimes: []time.Duration{5 * time.Minute, 10 * time.Minute, 30 * time.Minute, time.Hour, 4 * time.Hour, 8 * time.Hour, 16 * time.Hour, 22 * time.Hour}, WaitForL1Finality: true, TargetPriceGwei: 60., UrgencyGwei: 2., @@ -1401,8 +1368,8 @@ var DefaultDataPosterConfigForValidator = func() DataPosterConfig { }() var TestDataPosterConfig = DataPosterConfig{ - ReplacementTimes: "1s,2s,5s,10s,20s,30s,1m,5m", - BlobTxReplacementTimes: "1s,10s,30s,5m", + ReplacementTimes: []time.Duration{1 * time.Second, 2 * time.Second, 5 * time.Second, 10 * time.Second, 20 * time.Second, 30 * time.Second, time.Minute, 5 * time.Minute}, + BlobTxReplacementTimes: []time.Duration{1 * time.Second, 10 * time.Second, 30 * time.Second, 5 * time.Minute}, RedisSigner: signature.TestSimpleHmacConfig, WaitForL1Finality: false, TargetPriceGwei: 60., diff --git a/arbnode/dataposter/dataposter_test.go b/arbnode/dataposter/dataposter_test.go index 172b486df0..7f2f61c07e 100644 --- a/arbnode/dataposter/dataposter_test.go +++ b/arbnode/dataposter/dataposter_test.go @@ -22,41 +22,6 @@ import ( "github.com/offchainlabs/nitro/util/arbmath" ) -func TestParseReplacementTimes(t *testing.T) { - for _, tc := range []struct { - desc, replacementTimes string - want []time.Duration - wantErr bool - }{ - { - desc: "valid case", - replacementTimes: "1s,2s,1m,5m", - want: []time.Duration{ - time.Duration(time.Second), - time.Duration(2 * time.Second), - time.Duration(time.Minute), - time.Duration(5 * time.Minute), - time.Duration(time.Hour * 24 * 365 * 10), - }, - }, - { - desc: "non-increasing replacement times", - replacementTimes: "1s,2s,1m,5m,1s", - wantErr: true, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - got, err := parseReplacementTimes(tc.replacementTimes) - if gotErr := (err != nil); gotErr != tc.wantErr { - t.Fatalf("Got error: %t, want: %t", gotErr, tc.wantErr) - } - if diff := cmp.Diff(tc.want, got); diff != "" { - t.Errorf("parseReplacementTimes(%s) unexpected diff:\n%s", tc.replacementTimes, diff) - } - }) - } -} - func signerTestCfg(addr common.Address, url string) (*ExternalSignerCfg, error) { cp, err := externalsignertest.CertPaths() if err != nil { diff --git a/cmd/util/confighelpers/configuration.go b/cmd/util/confighelpers/configuration.go index ff33da6732..55c9ec330f 100644 --- a/cmd/util/confighelpers/configuration.go +++ b/cmd/util/confighelpers/configuration.go @@ -7,7 +7,9 @@ import ( "errors" "fmt" "os" + "reflect" "strings" + "time" "github.com/knadh/koanf" "github.com/knadh/koanf/parsers/json" @@ -98,19 +100,24 @@ var envvarsToSplitOnComma map[string]any = map[string]any{ "chain.info-files": struct{}{}, "conf.file": struct{}{}, "execution.secondary-forwarding-target": struct{}{}, + "execution.sequencer.sender-whitelist": struct{}{}, "graphql.corsdomain": struct{}{}, "graphql.vhosts": struct{}{}, "http.api": struct{}{}, "http.corsdomain": struct{}{}, "http.vhosts": struct{}{}, - "node.data-availability.rest-aggregator.urls": struct{}{}, - "node.feed.input.secondary-url": struct{}{}, - "node.feed.input.url": struct{}{}, - "node.feed.input.verify.allowed-addresses": struct{}{}, - "node.seq-coordinator.signer.ecdsa.allowed-addresses": struct{}{}, - "p2p.bootnodes": struct{}{}, - "p2p.bootnodes-v5": struct{}{}, - "validation.api-auth": struct{}{}, + "node.batch-poster.data-poster.blob-tx-replacement-times": time.Duration(0), + "node.batch-poster.data-poster.replacement-times": time.Duration(0), + "node.data-availability.rest-aggregator.urls": struct{}{}, + "node.feed.input.secondary-url": struct{}{}, + "node.feed.input.url": struct{}{}, + "node.feed.input.verify.allowed-addresses": struct{}{}, + "node.seq-coordinator.signer.ecdsa.allowed-addresses": struct{}{}, + "node.staker.batch-poster.data-poster.blob-tx-replacement-times": time.Duration(0), + "node.staker.batch-poster.data-poster.replacement-times": time.Duration(0), + "p2p.bootnodes": struct{}{}, + "p2p.bootnodes-v5": struct{}{}, + "validation.api-auth": struct{}{}, "validation.arbitrator.redis-validation-server-config.module-roots": struct{}{}, "validation.wasm.allowed-wasm-module-roots": struct{}{}, "ws.api": struct{}{}, @@ -126,8 +133,22 @@ func loadEnvironmentVariables(k *koanf.Koanf) error { strings.TrimPrefix(key, envPrefix+"_")), "__", "-") key = strings.ReplaceAll(key, "_", ".") - if _, found := envvarsToSplitOnComma[key]; found { + if value, found := envvarsToSplitOnComma[key]; found { // If there are commas in the value, split the value into a slice. + if _, ok := value.(time.Duration); ok { + // Special case for time.Duration + // v[1:len(v)-1] removes the '[' , ']' around the string + durationStrings := strings.Split(v[1:len(v)-1], ",") + var durations []time.Duration + for _, durationString := range durationStrings { + duration, err := time.ParseDuration(durationString) + if err != nil { + return key, nil + } + durations = append(durations, duration) + } + return key, durations + } if strings.Contains(v, ",") { return key, strings.Split(v, ",") @@ -227,6 +248,7 @@ func EndCommonParse(k *koanf.Koanf, config interface{}) error { // Default values DecodeHook: mapstructure.ComposeDecodeHookFunc( + stringToSliceDurationHookFunc(","), mapstructure.StringToTimeDurationHookFunc()), Metadata: nil, Result: config, @@ -240,6 +262,36 @@ func EndCommonParse(k *koanf.Koanf, config interface{}) error { return nil } +func stringToSliceDurationHookFunc(sep string) mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if f.Kind() != reflect.String { + return data, nil + } + if t != reflect.TypeOf([]time.Duration{}) { + return data, nil + } + + raw, _ := data.(string) + if raw == "" { + return []time.Duration{}, nil + } + // raw[1:len(raw)-1] removes the '[' , ']' around the string + durationStrings := strings.Split(raw[1:len(raw)-1], sep) + var durations []time.Duration + for _, durationString := range durationStrings { + duration, err := time.ParseDuration(durationString) + if err != nil { + return nil, err + } + durations = append(durations, duration) + } + return durations, nil + } +} + func DumpConfig(k *koanf.Koanf, extraOverrideFields map[string]interface{}) error { overrideFields := map[string]interface{}{"conf.dump": false} diff --git a/execution/gethexec/sequencer.go b/execution/gethexec/sequencer.go index e620e6ccd1..90e3082062 100644 --- a/execution/gethexec/sequencer.go +++ b/execution/gethexec/sequencer.go @@ -11,7 +11,6 @@ import ( "math/big" "runtime/debug" "strconv" - "strings" "sync" "sync/atomic" "time" @@ -66,7 +65,7 @@ type SequencerConfig struct { MaxBlockSpeed time.Duration `koanf:"max-block-speed" reload:"hot"` MaxRevertGasReject uint64 `koanf:"max-revert-gas-reject" reload:"hot"` MaxAcceptableTimestampDelta time.Duration `koanf:"max-acceptable-timestamp-delta" reload:"hot"` - SenderWhitelist string `koanf:"sender-whitelist"` + SenderWhitelist []string `koanf:"sender-whitelist"` Forwarder ForwarderConfig `koanf:"forwarder"` QueueSize int `koanf:"queue-size"` QueueTimeout time.Duration `koanf:"queue-timeout" reload:"hot"` @@ -82,8 +81,7 @@ type SequencerConfig struct { } func (c *SequencerConfig) Validate() error { - entries := strings.Split(c.SenderWhitelist, ",") - for _, address := range entries { + for _, address := range c.SenderWhitelist { if len(address) == 0 { continue } @@ -118,6 +116,7 @@ var DefaultSequencerConfig = SequencerConfig{ MaxBlockSpeed: time.Millisecond * 250, MaxRevertGasReject: 0, MaxAcceptableTimestampDelta: time.Hour, + SenderWhitelist: []string{}, Forwarder: DefaultSequencerForwarderConfig, QueueSize: 1024, QueueTimeout: time.Second * 12, @@ -137,7 +136,7 @@ func SequencerConfigAddOptions(prefix string, f *flag.FlagSet) { f.Duration(prefix+".max-block-speed", DefaultSequencerConfig.MaxBlockSpeed, "minimum delay between blocks (sets a maximum speed of block production)") f.Uint64(prefix+".max-revert-gas-reject", DefaultSequencerConfig.MaxRevertGasReject, "maximum gas executed in a revert for the sequencer to reject the transaction instead of posting it (anti-DOS)") f.Duration(prefix+".max-acceptable-timestamp-delta", DefaultSequencerConfig.MaxAcceptableTimestampDelta, "maximum acceptable time difference between the local time and the latest L1 block's timestamp") - f.String(prefix+".sender-whitelist", DefaultSequencerConfig.SenderWhitelist, "comma separated whitelist of authorized senders (if empty, everyone is allowed)") + f.StringSlice(prefix+".sender-whitelist", DefaultSequencerConfig.SenderWhitelist, "comma separated whitelist of authorized senders (if empty, everyone is allowed)") AddOptionsForSequencerForwarderConfig(prefix+".forwarder", f) f.Int(prefix+".queue-size", DefaultSequencerConfig.QueueSize, "size of the pending tx queue") f.Duration(prefix+".queue-timeout", DefaultSequencerConfig.QueueTimeout, "maximum amount of time transaction can wait in queue") @@ -324,8 +323,7 @@ func NewSequencer(execEngine *ExecutionEngine, l1Reader *headerreader.HeaderRead return nil, err } senderWhitelist := make(map[common.Address]struct{}) - entries := strings.Split(config.SenderWhitelist, ",") - for _, address := range entries { + for _, address := range config.SenderWhitelist { if len(address) == 0 { continue } diff --git a/system_tests/common_test.go b/system_tests/common_test.go index d170173cc0..c6964b37c0 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -183,7 +183,7 @@ var TestSequencerConfig = gethexec.SequencerConfig{ MaxBlockSpeed: time.Millisecond * 10, MaxRevertGasReject: params.TxGas + 10000, MaxAcceptableTimestampDelta: time.Hour, - SenderWhitelist: "", + SenderWhitelist: []string{}, Forwarder: DefaultTestForwarderConfig, QueueSize: 128, QueueTimeout: time.Second * 5, diff --git a/system_tests/seq_whitelist_test.go b/system_tests/seq_whitelist_test.go index efa30171ac..db73cb23c0 100644 --- a/system_tests/seq_whitelist_test.go +++ b/system_tests/seq_whitelist_test.go @@ -16,7 +16,7 @@ func TestSequencerWhitelist(t *testing.T) { defer cancel() builder := NewNodeBuilder(ctx).DefaultConfig(t, false) - builder.execConfig.Sequencer.SenderWhitelist = GetTestAddressForAccountName(t, "Owner").String() + "," + GetTestAddressForAccountName(t, "User").String() + builder.execConfig.Sequencer.SenderWhitelist = []string{GetTestAddressForAccountName(t, "Owner").String(), GetTestAddressForAccountName(t, "User").String()} cleanup := builder.Build(t) defer cleanup()