From 01dd072c106609ec3c3cdb5c12415141a5585f97 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Fri, 1 Dec 2023 22:11:48 +0530 Subject: [PATCH 1/3] Make the formula to calculate max fee cap configurable and add time based factor to default formula --- arbnode/dataposter/data_poster.go | 54 ++++++++++++++++++++------- arbnode/dataposter/dataposter_test.go | 25 +++++++++++++ go.mod | 1 + go.sum | 1 + 4 files changed, 67 insertions(+), 14 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 266131a6b9..999e2de593 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -17,6 +17,7 @@ import ( "sync" "time" + "github.com/Knetic/govaluate" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -71,6 +72,8 @@ type DataPoster struct { nonce uint64 queue QueueStorage errorCount map[uint64]int // number of consecutive intermittent errors rbf-ing or sending, per nonce + + maxFeeCapExpression *govaluate.EvaluableExpression } // signerFn is a signer function callback when a contract requires a method to @@ -152,6 +155,10 @@ func NewDataPoster(ctx context.Context, opts *DataPosterOpts) (*DataPoster, erro default: queue = slice.NewStorage(func() storage.EncoderDecoderInterface { return &storage.EncoderDecoder{} }) } + expression, err := govaluate.NewEvaluableExpression(cfg.MaxFeeCapFormula) + if err != nil { + return nil, fmt.Errorf("error creating govaluate evaluable expression for calculating maxFeeCap: %w", err) + } dp := &DataPoster{ headerReader: opts.HeaderReader, client: opts.HeaderReader.Client(), @@ -159,13 +166,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, - metadataRetriever: opts.MetadataRetriever, - queue: queue, - redisLock: opts.RedisLock, - errorCount: make(map[uint64]int), + config: opts.Config, + usingNoOpStorage: useNoOpStorage, + replacementTimes: replacementTimes, + metadataRetriever: opts.MetadataRetriever, + queue: queue, + redisLock: opts.RedisLock, + errorCount: make(map[uint64]int), + maxFeeCapExpression: expression, } if cfg.ExternalSigner.URL != "" { signer, sender, err := externalSigner(ctx, &cfg.ExternalSigner) @@ -389,13 +397,19 @@ func (p *DataPoster) feeAndTipCaps(ctx context.Context, nonce uint64, gasLimit u } elapsed := time.Since(dataCreatedAt) - // MaxFeeCap = (BacklogOfBatches^2 * UrgencyGWei^2 + TargetPriceGWei) * GWei - maxFeeCap := - arbmath.FloatToBig( - (float64(arbmath.SquareUint(backlogOfBatches))* - arbmath.SquareFloat(config.UrgencyGwei) + - config.TargetPriceGwei) * - params.GWei) + parameters := make(map[string]interface{}) + parameters["BacklogOfBatches"] = float64(backlogOfBatches) + parameters["UrgencyGWei"] = config.UrgencyGwei + parameters["ElapsedTime"] = float64(elapsed) + parameters["ElapsedTimeBase"] = float64(config.ElapsedTimeBase) + parameters["ElapsedTimeImportance"] = config.ElapsedTimeImportance + parameters["TargetPriceGWei"] = config.TargetPriceGwei + parameters["GWei"] = params.GWei + result, err := p.maxFeeCapExpression.Evaluate(parameters) + if err != nil { + return nil, nil, fmt.Errorf("error evaluating maxFeeCapExpression: %w", err) + } + maxFeeCap := arbmath.FloatToBig(result.(float64)) if arbmath.BigGreaterThan(newFeeCap, maxFeeCap) { log.Warn( "reducing proposed fee cap to current maximum", @@ -756,6 +770,9 @@ type DataPosterConfig struct { LegacyStorageEncoding bool `koanf:"legacy-storage-encoding" reload:"hot"` Dangerous DangerousConfig `koanf:"dangerous"` ExternalSigner ExternalSignerCfg `koanf:"external-signer"` + MaxFeeCapFormula string `koanf:"max-fee-cap-formula" reload:"hot"` + ElapsedTimeBase time.Duration `koanf:"elapsed-time-base" reload:"hot"` + ElapsedTimeImportance float64 `koanf:"elapsed-time-importance" reload:"hot"` } type ExternalSignerCfg struct { @@ -802,6 +819,9 @@ func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet, defaultDataPost f.Bool(prefix+".use-db-storage", defaultDataPosterConfig.UseDBStorage, "uses database storage when enabled") f.Bool(prefix+".use-noop-storage", defaultDataPosterConfig.UseNoOpStorage, "uses noop storage, it doesn't store anything") f.Bool(prefix+".legacy-storage-encoding", defaultDataPosterConfig.LegacyStorageEncoding, "encodes items in a legacy way (as it was before dropping generics)") + f.String(prefix+".max-fee-cap-formula", defaultDataPosterConfig.MaxFeeCapFormula, "mathematical formula to calculate maximum fee cap") + f.Duration(prefix+".elapsed-time-base", defaultDataPosterConfig.ElapsedTimeBase, "unit to measure the time elapsed since creation of transaction used for maximum fee cap calculation") + f.Float64(prefix+".elapsed-time-importance", defaultDataPosterConfig.ElapsedTimeImportance, "weight given to the units of time elapsed used for maximum fee cap calculation") signature.SimpleHmacConfigAddOptions(prefix+".redis-signer", f) addDangerousOptions(prefix+".dangerous", f) @@ -836,6 +856,9 @@ var DefaultDataPosterConfig = DataPosterConfig{ LegacyStorageEncoding: false, Dangerous: DangerousConfig{ClearDBStorage: false}, ExternalSigner: ExternalSignerCfg{Method: "eth_signTransaction"}, + MaxFeeCapFormula: "(BacklogOfBatches * BacklogOfBatches * UrgencyGWei * UrgencyGWei + (ElapsedTime/ElapsedTimeBase) * (ElapsedTime/ElapsedTimeBase) * ElapsedTimeImportance + TargetPriceGWei) * GWei", + ElapsedTimeBase: 10 * time.Minute, + ElapsedTimeImportance: 10, } var DefaultDataPosterConfigForValidator = func() DataPosterConfig { @@ -859,6 +882,9 @@ var TestDataPosterConfig = DataPosterConfig{ UseNoOpStorage: false, LegacyStorageEncoding: false, ExternalSigner: ExternalSignerCfg{Method: "eth_signTransaction"}, + MaxFeeCapFormula: "(BacklogOfBatches * BacklogOfBatches * UrgencyGWei * UrgencyGWei + (ElapsedTime/ElapsedTimeBase) * (ElapsedTime/ElapsedTimeBase) * ElapsedTimeImportance + TargetPriceGWei) * GWei", + ElapsedTimeBase: 10 * time.Minute, + ElapsedTimeImportance: 10, } var TestDataPosterConfigForValidator = func() DataPosterConfig { diff --git a/arbnode/dataposter/dataposter_test.go b/arbnode/dataposter/dataposter_test.go index d4d72bbbf4..0038ed2212 100644 --- a/arbnode/dataposter/dataposter_test.go +++ b/arbnode/dataposter/dataposter_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/Knetic/govaluate" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" @@ -242,3 +243,27 @@ func (s *server) mux(w http.ResponseWriter, r *http.Request) { fmt.Printf("error writing response: %v\n", err) } } + +func TestMaxFeeCapFormulaCalculation(t *testing.T) { + // This test alerts, by failing, if the max fee cap formula were to be changed in the DefaultDataPosterConfig to + // use new variables other than the ones that are keys of 'parameters' map below + expression, err := govaluate.NewEvaluableExpression(DefaultDataPosterConfig.MaxFeeCapFormula) + if err != nil { + t.Fatalf("error creating govaluate evaluable expression for default maxFeeCap formula: %t", err) + } + parameters := make(map[string]interface{}) + parameters["BacklogOfBatches"] = 0 + parameters["UrgencyGWei"] = DefaultDataPosterConfig.UrgencyGwei + parameters["ElapsedTime"] = 0 + parameters["ElapsedTimeBase"] = float64(DefaultDataPosterConfig.ElapsedTimeBase) + parameters["ElapsedTimeImportance"] = DefaultDataPosterConfig.ElapsedTimeImportance + parameters["TargetPriceGWei"] = DefaultDataPosterConfig.TargetPriceGwei + parameters["GWei"] = 0 + result, err := expression.Evaluate(parameters) + if err != nil { + t.Fatalf("error evaluating expression:: %t", err) + } + if result.(float64) != 0 { + t.Fatalf("unexpected result. Got error: %f, want: %d", result.(float64), 0) + } +} diff --git a/go.mod b/go.mod index 598c48e920..bdda6a61a1 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ replace github.com/VictoriaMetrics/fastcache => ./fastcache replace github.com/ethereum/go-ethereum => ./go-ethereum require ( + github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible github.com/Shopify/toxiproxy v2.1.4+incompatible github.com/alicebob/miniredis/v2 v2.21.0 github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 diff --git a/go.sum b/go.sum index 1fb8a405f0..bf8b4b826d 100644 --- a/go.sum +++ b/go.sum @@ -52,6 +52,7 @@ github.com/CloudyKit/jet/v3 v3.0.0/go.mod h1:HKQPgSJmdK8hdoAbKUUWajkHyHo4RaU5rMd github.com/DataDog/zstd v1.5.2 h1:vUG4lAyuPCXO0TLbXvPv7EB7cNK1QV/luu55UHLrrn8= github.com/DataDog/zstd v1.5.2/go.mod h1:g4AWEaM3yOg3HYfnJ3YIawPnVdXJh9QME85blwSAmyw= github.com/Joker/hpp v1.0.0/go.mod h1:8x5n+M1Hp5hC0g8okX3sR3vFQwynaX/UgSOM9MeBKzY= +github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible h1:1G1pk05UrOh0NlF1oeaaix1x8XzrfjIDK47TY0Zehcw= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/Kubuxu/go-os-helper v0.0.1/go.mod h1:N8B+I7vPCT80IcP58r50u4+gEEcsZETFUpAzWW2ep1Y= github.com/OneOfOne/xxhash v1.2.2 h1:KMrpdQIwFcEqXDklaen+P1axHaj9BSKzvpUUfnHldSE= From ab98a73e70882f4e2778f50780ca3a88bf70a06d Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Tue, 5 Dec 2023 20:36:40 +0530 Subject: [PATCH 2/3] address PR comments --- arbnode/dataposter/data_poster.go | 38 +++++++++++++++++---------- arbnode/dataposter/dataposter_test.go | 24 ++++++++--------- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 999e2de593..a03ac78d5a 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -358,6 +358,25 @@ func (p *DataPoster) GetNextNonceAndMeta(ctx context.Context) (uint64, []byte, e const minRbfIncrease = arbmath.OneInBips * 11 / 10 +// evalMaxFeeCapExpr uses MaxFeeCapFormula from config to calculate the expression's result by plugging in appropriate parameter values +func (p *DataPoster) evalMaxFeeCapExpr(backlogOfBatches uint64, elapsed time.Duration) (*big.Int, error) { + config := p.config() + parameters := map[string]any{ + "BacklogOfBatches": float64(backlogOfBatches), + "UrgencyGWei": config.UrgencyGwei, + "ElapsedTime": float64(elapsed), + "ElapsedTimeBase": float64(config.ElapsedTimeBase), + "ElapsedTimeImportance": config.ElapsedTimeImportance, + "TargetPriceGWei": config.TargetPriceGwei, + "GWei": params.GWei, + } + result, err := p.maxFeeCapExpression.Evaluate(parameters) + if err != nil { + return nil, fmt.Errorf("error evaluating maxFeeCapExpression: %w", err) + } + return arbmath.FloatToBig(result.(float64)), nil +} + 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) @@ -397,19 +416,10 @@ func (p *DataPoster) feeAndTipCaps(ctx context.Context, nonce uint64, gasLimit u } elapsed := time.Since(dataCreatedAt) - parameters := make(map[string]interface{}) - parameters["BacklogOfBatches"] = float64(backlogOfBatches) - parameters["UrgencyGWei"] = config.UrgencyGwei - parameters["ElapsedTime"] = float64(elapsed) - parameters["ElapsedTimeBase"] = float64(config.ElapsedTimeBase) - parameters["ElapsedTimeImportance"] = config.ElapsedTimeImportance - parameters["TargetPriceGWei"] = config.TargetPriceGwei - parameters["GWei"] = params.GWei - result, err := p.maxFeeCapExpression.Evaluate(parameters) + maxFeeCap, err := p.evalMaxFeeCapExpr(backlogOfBatches, elapsed) if err != nil { - return nil, nil, fmt.Errorf("error evaluating maxFeeCapExpression: %w", err) + return nil, nil, err } - maxFeeCap := arbmath.FloatToBig(result.(float64)) if arbmath.BigGreaterThan(newFeeCap, maxFeeCap) { log.Warn( "reducing proposed fee cap to current maximum", @@ -819,7 +829,7 @@ func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet, defaultDataPost f.Bool(prefix+".use-db-storage", defaultDataPosterConfig.UseDBStorage, "uses database storage when enabled") f.Bool(prefix+".use-noop-storage", defaultDataPosterConfig.UseNoOpStorage, "uses noop storage, it doesn't store anything") f.Bool(prefix+".legacy-storage-encoding", defaultDataPosterConfig.LegacyStorageEncoding, "encodes items in a legacy way (as it was before dropping generics)") - f.String(prefix+".max-fee-cap-formula", defaultDataPosterConfig.MaxFeeCapFormula, "mathematical formula to calculate maximum fee cap") + f.String(prefix+".max-fee-cap-formula", defaultDataPosterConfig.MaxFeeCapFormula, "mathematical formula to calculate maximum fee cap. Refer https://github.com/Knetic/govaluate/blob/master/MANUAL.md to find all available mathematical operators") f.Duration(prefix+".elapsed-time-base", defaultDataPosterConfig.ElapsedTimeBase, "unit to measure the time elapsed since creation of transaction used for maximum fee cap calculation") f.Float64(prefix+".elapsed-time-importance", defaultDataPosterConfig.ElapsedTimeImportance, "weight given to the units of time elapsed used for maximum fee cap calculation") @@ -856,7 +866,7 @@ var DefaultDataPosterConfig = DataPosterConfig{ LegacyStorageEncoding: false, Dangerous: DangerousConfig{ClearDBStorage: false}, ExternalSigner: ExternalSignerCfg{Method: "eth_signTransaction"}, - MaxFeeCapFormula: "(BacklogOfBatches * BacklogOfBatches * UrgencyGWei * UrgencyGWei + (ElapsedTime/ElapsedTimeBase) * (ElapsedTime/ElapsedTimeBase) * ElapsedTimeImportance + TargetPriceGWei) * GWei", + MaxFeeCapFormula: "(((BacklogOfBatches * UrgencyGWei) ** 2) + ((ElapsedTime/ElapsedTimeBase) ** 2) * ElapsedTimeImportance + TargetPriceGWei) * GWei", ElapsedTimeBase: 10 * time.Minute, ElapsedTimeImportance: 10, } @@ -882,7 +892,7 @@ var TestDataPosterConfig = DataPosterConfig{ UseNoOpStorage: false, LegacyStorageEncoding: false, ExternalSigner: ExternalSignerCfg{Method: "eth_signTransaction"}, - MaxFeeCapFormula: "(BacklogOfBatches * BacklogOfBatches * UrgencyGWei * UrgencyGWei + (ElapsedTime/ElapsedTimeBase) * (ElapsedTime/ElapsedTimeBase) * ElapsedTimeImportance + TargetPriceGWei) * GWei", + MaxFeeCapFormula: "(((BacklogOfBatches * UrgencyGWei) ** 2) + ((ElapsedTime/ElapsedTimeBase) ** 2) * ElapsedTimeImportance + TargetPriceGWei) * GWei", ElapsedTimeBase: 10 * time.Minute, ElapsedTimeImportance: 10, } diff --git a/arbnode/dataposter/dataposter_test.go b/arbnode/dataposter/dataposter_test.go index 0038ed2212..8c18288e5f 100644 --- a/arbnode/dataposter/dataposter_test.go +++ b/arbnode/dataposter/dataposter_test.go @@ -249,21 +249,19 @@ func TestMaxFeeCapFormulaCalculation(t *testing.T) { // use new variables other than the ones that are keys of 'parameters' map below expression, err := govaluate.NewEvaluableExpression(DefaultDataPosterConfig.MaxFeeCapFormula) if err != nil { - t.Fatalf("error creating govaluate evaluable expression for default maxFeeCap formula: %t", err) + t.Fatalf("Error creating govaluate evaluable expression for calculating default maxFeeCap formula: %v", err) } - parameters := make(map[string]interface{}) - parameters["BacklogOfBatches"] = 0 - parameters["UrgencyGWei"] = DefaultDataPosterConfig.UrgencyGwei - parameters["ElapsedTime"] = 0 - parameters["ElapsedTimeBase"] = float64(DefaultDataPosterConfig.ElapsedTimeBase) - parameters["ElapsedTimeImportance"] = DefaultDataPosterConfig.ElapsedTimeImportance - parameters["TargetPriceGWei"] = DefaultDataPosterConfig.TargetPriceGwei - parameters["GWei"] = 0 - result, err := expression.Evaluate(parameters) + config := DefaultDataPosterConfig + config.TargetPriceGwei = 0 + p := &DataPoster{ + config: func() *DataPosterConfig { return &config }, + maxFeeCapExpression: expression, + } + result, err := p.evalMaxFeeCapExpr(0, 0) if err != nil { - t.Fatalf("error evaluating expression:: %t", err) + t.Fatalf("Error evaluating MaxFeeCap expression: %v", err) } - if result.(float64) != 0 { - t.Fatalf("unexpected result. Got error: %f, want: %d", result.(float64), 0) + if result.Cmp(common.Big0) != 0 { + t.Fatalf("Unexpected result. Got: %d, want: 0", result.Uint64()) } } From 02747d558091c7609ffc5eadc10080af0e243d93 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Wed, 6 Dec 2023 20:38:23 +0530 Subject: [PATCH 3/3] update flag usage --- arbnode/dataposter/data_poster.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index a03ac78d5a..824499433a 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -829,7 +829,9 @@ func DataPosterConfigAddOptions(prefix string, f *pflag.FlagSet, defaultDataPost f.Bool(prefix+".use-db-storage", defaultDataPosterConfig.UseDBStorage, "uses database storage when enabled") f.Bool(prefix+".use-noop-storage", defaultDataPosterConfig.UseNoOpStorage, "uses noop storage, it doesn't store anything") f.Bool(prefix+".legacy-storage-encoding", defaultDataPosterConfig.LegacyStorageEncoding, "encodes items in a legacy way (as it was before dropping generics)") - f.String(prefix+".max-fee-cap-formula", defaultDataPosterConfig.MaxFeeCapFormula, "mathematical formula to calculate maximum fee cap. Refer https://github.com/Knetic/govaluate/blob/master/MANUAL.md to find all available mathematical operators") + f.String(prefix+".max-fee-cap-formula", defaultDataPosterConfig.MaxFeeCapFormula, "mathematical formula to calculate maximum fee cap the result of which would be float64.\n"+ + "This expression is expected to be evaluated please refer https://github.com/Knetic/govaluate/blob/master/MANUAL.md to find all available mathematical operators.\n"+ + "Currently available variables to construct the formula are BacklogOfBatches, UrgencyGWei, ElapsedTime, ElapsedTimeBase, ElapsedTimeImportance, TargetPriceGWei and GWei") f.Duration(prefix+".elapsed-time-base", defaultDataPosterConfig.ElapsedTimeBase, "unit to measure the time elapsed since creation of transaction used for maximum fee cap calculation") f.Float64(prefix+".elapsed-time-importance", defaultDataPosterConfig.ElapsedTimeImportance, "weight given to the units of time elapsed used for maximum fee cap calculation")