From 24cbf1a748545c33e963b639d50064c6e5fb5edb Mon Sep 17 00:00:00 2001 From: Thiago Coimbra Lemos Date: Thu, 3 Aug 2023 05:02:56 -0300 Subject: [PATCH] add GasPriceMarginFactor and MaxGasPrice to eth-tx-manager (#2360) * add GasPriceMarginFactor and MaxGasPrice to eth-tx-manager * add logs, fix config * update config file documentation --------- Co-authored-by: joanestebanr <129153821+joanestebanr@users.noreply.github.com> --- config/config_test.go | 8 ++ config/default.go | 2 + docs/config-file/node-config-doc.html | 2 +- docs/config-file/node-config-doc.md | 69 +++++++++++++++-- docs/config-file/node-config-schema.json | 10 +++ ethtxmanager/config.go | 33 +++++++++ ethtxmanager/ethtxmanager.go | 35 ++++++++- ethtxmanager/ethtxmanager_test.go | 94 ++++++++++++++++++++++++ 8 files changed, 242 insertions(+), 11 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 269a2e19ba..d94ed5d603 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -264,6 +264,14 @@ func Test_Defaults(t *testing.T) { path: "EthTxManager.ForcedGas", expectedValue: uint64(0), }, + { + path: "EthTxManager.GasPriceMarginFactor", + expectedValue: float64(1), + }, + { + path: "EthTxManager.MaxGasPriceLimit", + expectedValue: uint64(0), + }, { path: "L2GasPriceSuggester.DefaultGasPriceWei", expectedValue: uint64(2000000000), diff --git a/config/default.go b/config/default.go index 8d44065ab8..132b6f3a90 100644 --- a/config/default.go +++ b/config/default.go @@ -49,6 +49,8 @@ MultiGasProvider = false FrequencyToMonitorTxs = "1s" WaitTxToBeMined = "2m" ForcedGas = 0 +GasPriceMarginFactor = 1 +MaxGasPriceLimit = 0 [RPC] Host = "0.0.0.0" diff --git a/docs/config-file/node-config-doc.html b/docs/config-file/node-config-doc.html index 04f2a688b7..55fb39748c 100644 --- a/docs/config-file/node-config-doc.html +++ b/docs/config-file/node-config-doc.html @@ -2,7 +2,7 @@
"300ms"
 

Default: "2m0s"Type: string

WaitTxToBeMined time to wait after transaction was sent to the ethereum


Examples:

"1m"
 
"300ms"
-

Type: array of object

PrivateKeys defines all the key store files that are going
to be read in order to provide the private keys to sign the L1 txs

Each item of this array must be:

Type: string

Path is the file path for the key store file


Type: string

Password is the password to decrypt the key store file



Default: 0Type: integer

ForcedGas is the amount of gas to be forced in case of gas estimation error


Pool service configuration
Default: "5m0s"Type: string

IntervalToRefreshBlockedAddresses is the time it takes to sync the
blocked address list from db to memory


Examples:

"1m"
+

Type: array of object

PrivateKeys defines all the key store files that are going
to be read in order to provide the private keys to sign the L1 txs

Each item of this array must be:

Type: string

Path is the file path for the key store file


Type: string

Password is the password to decrypt the key store file



Default: 0Type: integer

ForcedGas is the amount of gas to be forced in case of gas estimation error


Default: 1Type: number

GasPriceMarginFactor is used to multiply the suggested gas price provided by the network
in order to allow a different gas price to be set for all the transactions and making it
easier to have the txs prioritized in the pool, default value is 1.

ex:
suggested gas price: 100
GasPriceMarginFactor: 1
gas price = 100

suggested gas price: 100
GasPriceMarginFactor: 1.1
gas price = 110


Default: 0Type: integer

MaxGasPriceLimit helps avoiding transactions to be sent over an specified
gas price amount, default value is 0, which means no limit.
If the gas price provided by the network and adjusted by the GasPriceMarginFactor
is greater than this configuration, transaction will have its gas price set to
the value configured in this config as the limit.

ex:

suggested gas price: 100
gas price margin factor: 20%
max gas price limit: 150
tx gas price = 120

suggested gas price: 100
gas price margin factor: 20%
max gas price limit: 110
tx gas price = 110


Pool service configuration
Default: "5m0s"Type: string

IntervalToRefreshBlockedAddresses is the time it takes to sync the
blocked address list from db to memory


Examples:

"1m"
 
"300ms"
 

Default: "5s"Type: string

IntervalToRefreshGasPrices is the time to wait to refresh the gas prices


Examples:

"1m"
 
"300ms"
diff --git a/docs/config-file/node-config-doc.md b/docs/config-file/node-config-doc.md
index 69c8f8bb43..211cd0e1c7 100644
--- a/docs/config-file/node-config-doc.md
+++ b/docs/config-file/node-config-doc.md
@@ -224,12 +224,14 @@ Url=""
 **Type:** : `object`
 **Description:** Configuration for ethereum transaction manager
 
-| Property                                                        | Pattern | Type            | Deprecated | Definition | Title/Description                                                                                                                  |
-| --------------------------------------------------------------- | ------- | --------------- | ---------- | ---------- | ---------------------------------------------------------------------------------------------------------------------------------- |
-| - [FrequencyToMonitorTxs](#EthTxManager_FrequencyToMonitorTxs ) | No      | string          | No         | -          | Duration                                                                                                                           |
-| - [WaitTxToBeMined](#EthTxManager_WaitTxToBeMined )             | No      | string          | No         | -          | Duration                                                                                                                           |
-| - [PrivateKeys](#EthTxManager_PrivateKeys )                     | No      | array of object | No         | -          | PrivateKeys defines all the key store files that are going
to be read in order to provide the private keys to sign the L1 txs | -| - [ForcedGas](#EthTxManager_ForcedGas ) | No | integer | No | - | ForcedGas is the amount of gas to be forced in case of gas estimation error | +| Property | Pattern | Type | Deprecated | Definition | Title/Description | +| --------------------------------------------------------------- | ------- | --------------- | ---------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| - [FrequencyToMonitorTxs](#EthTxManager_FrequencyToMonitorTxs ) | No | string | No | - | Duration | +| - [WaitTxToBeMined](#EthTxManager_WaitTxToBeMined ) | No | string | No | - | Duration | +| - [PrivateKeys](#EthTxManager_PrivateKeys ) | No | array of object | No | - | PrivateKeys defines all the key store files that are going
to be read in order to provide the private keys to sign the L1 txs | +| - [ForcedGas](#EthTxManager_ForcedGas ) | No | integer | No | - | ForcedGas is the amount of gas to be forced in case of gas estimation error | +| - [GasPriceMarginFactor](#EthTxManager_GasPriceMarginFactor ) | No | number | No | - | GasPriceMarginFactor is used to multiply the suggested gas price provided by the network
in order to allow a different gas price to be set for all the transactions and making it
easier to have the txs prioritized in the pool, default value is 1.

ex:
suggested gas price: 100
GasPriceMarginFactor: 1
gas price = 100

suggested gas price: 100
GasPriceMarginFactor: 1.1
gas price = 110 | +| - [MaxGasPriceLimit](#EthTxManager_MaxGasPriceLimit ) | No | integer | No | - | MaxGasPriceLimit helps avoiding transactions to be sent over an specified
gas price amount, default value is 0, which means no limit.
If the gas price provided by the network and adjusted by the GasPriceMarginFactor
is greater than this configuration, transaction will have its gas price set to
the value configured in this config as the limit.

ex:

suggested gas price: 100
gas price margin factor: 20%
max gas price limit: 150
tx gas price = 120

suggested gas price: 100
gas price margin factor: 20%
max gas price limit: 110
tx gas price = 110 | ### 6.1. `EthTxManager.FrequencyToMonitorTxs` @@ -335,6 +337,61 @@ to be read in order to provide the private keys to sign the L1 txs ForcedGas=0 ``` +### 6.5. `EthTxManager.GasPriceMarginFactor` + +**Type:** : `number` + +**Default:** `1` + +**Description:** GasPriceMarginFactor is used to multiply the suggested gas price provided by the network +in order to allow a different gas price to be set for all the transactions and making it +easier to have the txs prioritized in the pool, default value is 1. + +ex: +suggested gas price: 100 +GasPriceMarginFactor: 1 +gas price = 100 + +suggested gas price: 100 +GasPriceMarginFactor: 1.1 +gas price = 110 + +**Example setting the default value** (1): +``` +[EthTxManager] +GasPriceMarginFactor=1 +``` + +### 6.6. `EthTxManager.MaxGasPriceLimit` + +**Type:** : `integer` + +**Default:** `0` + +**Description:** MaxGasPriceLimit helps avoiding transactions to be sent over an specified +gas price amount, default value is 0, which means no limit. +If the gas price provided by the network and adjusted by the GasPriceMarginFactor +is greater than this configuration, transaction will have its gas price set to +the value configured in this config as the limit. + +ex: + +suggested gas price: 100 +gas price margin factor: 20% +max gas price limit: 150 +tx gas price = 120 + +suggested gas price: 100 +gas price margin factor: 20% +max gas price limit: 110 +tx gas price = 110 + +**Example setting the default value** (0): +``` +[EthTxManager] +MaxGasPriceLimit=0 +``` + ## 7. `[Pool]` **Type:** : `object` diff --git a/docs/config-file/node-config-schema.json b/docs/config-file/node-config-schema.json index bed2336c99..64c4967768 100644 --- a/docs/config-file/node-config-schema.json +++ b/docs/config-file/node-config-schema.json @@ -136,6 +136,16 @@ "type": "integer", "description": "ForcedGas is the amount of gas to be forced in case of gas estimation error", "default": 0 + }, + "GasPriceMarginFactor": { + "type": "number", + "description": "GasPriceMarginFactor is used to multiply the suggested gas price provided by the network\nin order to allow a different gas price to be set for all the transactions and making it\neasier to have the txs prioritized in the pool, default value is 1.\n\nex:\nsuggested gas price: 100\nGasPriceMarginFactor: 1\ngas price = 100\n\nsuggested gas price: 100\nGasPriceMarginFactor: 1.1\ngas price = 110", + "default": 1 + }, + "MaxGasPriceLimit": { + "type": "integer", + "description": "MaxGasPriceLimit helps avoiding transactions to be sent over an specified\ngas price amount, default value is 0, which means no limit.\nIf the gas price provided by the network and adjusted by the GasPriceMarginFactor\nis greater than this configuration, transaction will have its gas price set to\nthe value configured in this config as the limit.\n\nex:\n\nsuggested gas price: 100\ngas price margin factor: 20%\nmax gas price limit: 150\ntx gas price = 120\n\nsuggested gas price: 100\ngas price margin factor: 20%\nmax gas price limit: 110\ntx gas price = 110", + "default": 0 } }, "additionalProperties": false, diff --git a/ethtxmanager/config.go b/ethtxmanager/config.go index 3bf486bc04..060cb2ff51 100644 --- a/ethtxmanager/config.go +++ b/ethtxmanager/config.go @@ -15,4 +15,37 @@ type Config struct { // ForcedGas is the amount of gas to be forced in case of gas estimation error ForcedGas uint64 `mapstructure:"ForcedGas"` + + // GasPriceMarginFactor is used to multiply the suggested gas price provided by the network + // in order to allow a different gas price to be set for all the transactions and making it + // easier to have the txs prioritized in the pool, default value is 1. + // + // ex: + // suggested gas price: 100 + // GasPriceMarginFactor: 1 + // gas price = 100 + // + // suggested gas price: 100 + // GasPriceMarginFactor: 1.1 + // gas price = 110 + GasPriceMarginFactor float64 `mapstructure:"GasPriceMarginFactor"` + + // MaxGasPriceLimit helps avoiding transactions to be sent over an specified + // gas price amount, default value is 0, which means no limit. + // If the gas price provided by the network and adjusted by the GasPriceMarginFactor + // is greater than this configuration, transaction will have its gas price set to + // the value configured in this config as the limit. + // + // ex: + // + // suggested gas price: 100 + // gas price margin factor: 20% + // max gas price limit: 150 + // tx gas price = 120 + // + // suggested gas price: 100 + // gas price margin factor: 20% + // max gas price limit: 110 + // tx gas price = 110 + MaxGasPriceLimit uint64 `mapstructure:"MaxGasPriceLimit"` } diff --git a/ethtxmanager/ethtxmanager.go b/ethtxmanager/ethtxmanager.go index e7099afddc..03df29c859 100644 --- a/ethtxmanager/ethtxmanager.go +++ b/ethtxmanager/ethtxmanager.go @@ -90,7 +90,7 @@ func (c *Client) Add(ctx context.Context, owner, id string, from common.Address, } // get gas price - gasPrice, err := c.etherman.SuggestedGasPrice(ctx) + gasPrice, err := c.suggestedGasPrice(ctx) if err != nil { err := fmt.Errorf("failed to get suggested gas price: %w", err) log.Errorf(err.Error()) @@ -113,6 +113,9 @@ func (c *Client) Add(ctx context.Context, owner, id string, from common.Address, return err } + mTxLog := log.WithFields("monitoredTx", mTx.id, "createdAt", mTx.createdAt) + mTxLog.Infof("created") + return nil } @@ -261,7 +264,7 @@ func (c *Client) monitorTxs(ctx context.Context) error { for _, mTx := range mTxs { mTx := mTx // force variable shadowing to avoid pointer conflicts - mTxLog := log.WithFields("monitoredTx", mTx.id) + mTxLog := log.WithFields("monitoredTx", mTx.id, "createdAt", mTx.createdAt) mTxLog.Info("processing") // check if any of the txs in the history was mined @@ -355,7 +358,7 @@ func (c *Client) monitorTxs(ctx context.Context) error { // rebuild transaction tx := mTx.Tx() - mTxLog.Debugf("unsigned tx %v created", tx.Hash().String(), mTx.id) + mTxLog.Debugf("unsigned tx %v created", tx.Hash().String()) // sign tx signedTx, err = c.etherman.SignTx(ctx, mTx.from, tx) @@ -519,7 +522,7 @@ func (c *Client) ReviewMonitoredTx(ctx context.Context, mTx *monitoredTx) error } // get gas price - gasPrice, err := c.etherman.SuggestedGasPrice(ctx) + gasPrice, err := c.suggestedGasPrice(ctx) if err != nil { err := fmt.Errorf("failed to get suggested gas price: %w", err) mTxLog.Errorf(err.Error()) @@ -558,6 +561,30 @@ func (c *Client) ReviewMonitoredTxNonce(ctx context.Context, mTx *monitoredTx) e return nil } +func (c *Client) suggestedGasPrice(ctx context.Context) (*big.Int, error) { + // get gas price + gasPrice, err := c.etherman.SuggestedGasPrice(ctx) + if err != nil { + return nil, err + } + + // adjust the gas price by the margin factor + marginFactor := big.NewFloat(0).SetFloat64(c.cfg.GasPriceMarginFactor) + fGasPrice := big.NewFloat(0).SetInt(gasPrice) + adjustedGasPrice, _ := big.NewFloat(0).Mul(fGasPrice, marginFactor).Int(big.NewInt(0)) + + // if there is a max gas price limit configured and the current + // adjusted gas price is over this limit, set the gas price as the limit + if c.cfg.MaxGasPriceLimit > 0 { + maxGasPrice := big.NewInt(0).SetUint64(c.cfg.MaxGasPriceLimit) + if adjustedGasPrice.Cmp(maxGasPrice) == 1 { + adjustedGasPrice.Set(maxGasPrice) + } + } + + return adjustedGasPrice, nil +} + // logErrorAndWait used when an error is detected before trying again func (c *Client) logErrorAndWait(msg string, err error) { log.Errorf(msg, err) diff --git a/ethtxmanager/ethtxmanager_test.go b/ethtxmanager/ethtxmanager_test.go index 9d5b4e2b4b..0e18763707 100644 --- a/ethtxmanager/ethtxmanager_test.go +++ b/ethtxmanager/ethtxmanager_test.go @@ -3,6 +3,7 @@ package ethtxmanager import ( "context" "errors" + "fmt" "math/big" "testing" "time" @@ -20,6 +21,8 @@ import ( var defaultEthTxmanagerConfigForTests = Config{ FrequencyToMonitorTxs: types.NewDuration(time.Millisecond), WaitTxToBeMined: types.NewDuration(time.Second), + GasPriceMarginFactor: 1, + MaxGasPriceLimit: 0, } func TestTxGetMined(t *testing.T) { @@ -676,3 +679,94 @@ func TestExecutionReverted(t *testing.T) { require.NoError(t, err) require.Equal(t, MonitoredTxStatusConfirmed, result.Status) } + +func TestGasPriceMarginAndLimit(t *testing.T) { + type testCase struct { + name string + gasPriceMarginFactor float64 + maxGasPriceLimit uint64 + suggestedGasPrice int64 + expectedGasPrice int64 + } + + testCases := []testCase{ + { + name: "no margin and no limit", + gasPriceMarginFactor: 1, + maxGasPriceLimit: 0, + suggestedGasPrice: 100, + expectedGasPrice: 100, + }, + { + name: "20% margin", + gasPriceMarginFactor: 1.2, + maxGasPriceLimit: 0, + suggestedGasPrice: 100, + expectedGasPrice: 120, + }, + { + name: "20% margin but limited", + gasPriceMarginFactor: 1.2, + maxGasPriceLimit: 110, + suggestedGasPrice: 100, + expectedGasPrice: 110, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dbCfg := dbutils.NewStateConfigFromEnv() + require.NoError(t, dbutils.InitOrResetState(dbCfg)) + + etherman := newEthermanMock(t) + st := newStateMock(t) + storage, err := NewPostgresStorage(dbCfg) + require.NoError(t, err) + + var cfg = Config{ + FrequencyToMonitorTxs: defaultEthTxmanagerConfigForTests.FrequencyToMonitorTxs, + WaitTxToBeMined: defaultEthTxmanagerConfigForTests.WaitTxToBeMined, + GasPriceMarginFactor: tc.gasPriceMarginFactor, + MaxGasPriceLimit: tc.maxGasPriceLimit, + } + + ethTxManagerClient := New(cfg, etherman, storage, st) + + owner := "owner" + id := "unique_id" + from := common.HexToAddress("") + var to *common.Address + var value *big.Int + var data []byte = nil + + ctx := context.Background() + + currentNonce := uint64(1) + etherman. + On("CurrentNonce", ctx, from). + Return(currentNonce, nil). + Once() + + estimatedGas := uint64(1) + etherman. + On("EstimateGas", ctx, from, to, value, data). + Return(estimatedGas, nil). + Once() + + suggestedGasPrice := big.NewInt(int64(tc.suggestedGasPrice)) + etherman. + On("SuggestedGasPrice", ctx). + Return(suggestedGasPrice, nil). + Once() + + expectedSuggestedGasPrice := big.NewInt(tc.expectedGasPrice) + + err = ethTxManagerClient.Add(ctx, owner, id, from, to, value, data, nil) + require.NoError(t, err) + + monitoredTx, err := storage.Get(ctx, owner, id, nil) + require.NoError(t, err) + require.Equal(t, monitoredTx.gasPrice.Cmp(expectedSuggestedGasPrice), 0, fmt.Sprintf("expected gas price %v, found %v", expectedSuggestedGasPrice.String(), monitoredTx.gasPrice.String())) + }) + } +}