[NIT-1297] [Config-Change] Change comma separated string values in config to string slice

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.,

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 {

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}
 

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
 		}

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,

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()