ethtxmanager improvements (0xPolygonHermez#2664)

* fix null effective_percentage * fix forkID calculation * fix script * generate json-schema + docs for node config file and network_custom * fix unittest * Hotfixv0.1.4 to v0.2.0 (0xPolygonHermez#2255) * Hotfix v0.1.4 to main (0xPolygonHermez#2250) * fix concurrent web socket writes * fix eth_syncing * fix custom trace internal tx call error handling and update prover * add test to custom tracer depth issue; fix internal call error and gas used * fix custom tracer for internal tx with error and no more steps after it * remove debug code * Make max grpc message size configurable (0xPolygonHermez#2179) * make max grpc message size configurable * fix state tests * fix tests * fix tests * get SequencerNodeURI from SC if empty and not IsTrustedSequencer * Optimize trace (0xPolygonHermez#2183) * optimize trace * fix memory reading * update docker image * update prover image * fix converter * fix memory * fix step memory * fix structlogs * fix structlogs * fix structlogs * fix structlogs * fix structlogs * fix structlogs * fix structlogs * fix structlogs * update prover image * fix struclogs * fix memory size * fix memory size * fix memory size * refactor memory resize * refactor memory resize * move log for the best fitting tx (0xPolygonHermez#2192) * fix load zkCounters from pool * remove unnecessary log.info * add custom tracer support to CREATES opcode without depth increase (0xPolygonHermez#2213) * logs * fix getting stateroot from previous batch (GetWIPBatch) * logs * Fix GetWipBatch when previous last batch is a forced batch * fix forcedBatch trusted state * Revert "fix getting stateroot from previous batch (GetWIPBatch)" This reverts commit 860f0e7. * force GHA * add pool limits (0xPolygonHermez#2189) * Hotfix/batch l2 data (0xPolygonHermez#2223) * Fix BatchL2Data * Force GHA * remove failed txs from the pool limit check (0xPolygonHermez#2233) * debug trace by batch number via external rpc requests (0xPolygonHermez#2235) * fix trace batch remote requests in parallel limitation (0xPolygonHermez#2244) * Added RPC.TraceBatchUseHTTPS config parameter * fix executor version --------- Co-authored-by: tclemos <[email protected]> Co-authored-by: tclemos <[email protected]> Co-authored-by: Toni Ramírez <[email protected]> Co-authored-by: agnusmor <[email protected]> Co-authored-by: agnusmor <[email protected]> Co-authored-by: Thiago Coimbra Lemos <[email protected]> * fix test * fix test --------- Co-authored-by: tclemos <[email protected]> Co-authored-by: tclemos <[email protected]> Co-authored-by: Toni Ramírez <[email protected]> Co-authored-by: agnusmor <[email protected]> Co-authored-by: agnusmor <[email protected]> Co-authored-by: Thiago Coimbra Lemos <[email protected]> * Effective GasPrice refactor+fixes (0xPolygonHermez#2247) * effective GasPrice refactor * bugs fixes and finalizer tests fixes * fix typo * fix calculate effective gasprice percentage * fix test gas price * Fix/0xPolygonHermez#2257 effective gas price receipt (0xPolygonHermez#2258) * effective gas price returned by the rpc in the receipt * linter * bugfix: fixing l2blocks timestamp for the fist batch (0xPolygonHermez#2260) * bugfix: fixing l2blocks timestamp for the fist batch Signed-off-by: Nikolay Nedkov <[email protected]> * fix finalizer unit test --------- Signed-off-by: Nikolay Nedkov <[email protected]> * add more comments, and removed fields PrivateKeyPath and PrivateKeyPassword from etherman.Config that are not in use * add info to git action * add info to git action * fix github action * updated comments * updated comments * Fix/0xPolygonHermez#2263 gas used (0xPolygonHermez#2264) * fix fea2scalar and gas used * suggestion * fix fea2scalar * suggestion * Fix pending tx when duplicate nonce (0xPolygonHermez#2270) * fix pending tx when duplicate nonce * set pool.transaction.failed_reason to NULL when updating an existing tx * add more log details when adding tx to AddrQueue * fix query to add tx to the pool. Fix lint errors * change failed_reason for tx discarded due duplicate nonce * Only return a tx from the pool if tx is in pending status (0xPolygonHermez#2273) * Return a tx from the pool only if it is * fix TestGetTransactionByHash --------- Co-authored-by: agnusmor <[email protected]> * fix documentation with config file * improve: adding check to skip appending effectivePercentage if current forkId is under 5. Signed-off-by: Nikolay Nedkov <[email protected]> * Fiex effectiveGasprice unsigned txs with forkId lower than 5 (0xPolygonHermez#2278) * feat: adding functionality to stop sequencer on specific batch num from config param. Signed-off-by: Nikolay Nedkov <[email protected]> * patch: adding print for X-Real-IP in JSON-RPC Signed-off-by: Nikolay Nedkov <[email protected]> * Fix checkIfSynced (0xPolygonHermez#2289) * [Rehashing] Check logs order and fix blockhash and blockNumber in the log conversion (0xPolygonHermez#2280) * fix and check order * linter * flushID synchronizer (0xPolygonHermez#2287) * FlushID in synchronizer * linter * fix logs * commnets * executor error refactor (0xPolygonHermez#2299) * handle invalid rlp ROM error (0xPolygonHermez#2297) * add maxL2GasPrice (0xPolygonHermez#2294) * add maxL2GasPrice * fix * fix * add test * document parameter * update description * Error refactor (0xPolygonHermez#2302) * error refactor * refactor * Fix replaced tx as failed when duplicated nonce (0xPolygonHermez#2308) * Fix UpdateTxStatus for replacedTx * Fix adding tx with same nonce on AddrQueue * log reprocess need (0xPolygonHermez#2309) * log reprocess need * Update finalizer.go * Feature/2300 synchronizer detect if executor restart (0xPolygonHermez#2306) * detect if executor restarts and stop synchonizer * Update prover images (0xPolygonHermez#2311) * update prover image * update prover images * change executor param * Update testnet.prover.config.json * Update test.permissionless.prover.config.json * Update test.prover.config.json * Update public.prover.config.json * prover params * prover params * prover params * update prover images * add doc, and fix dockers to be able to use snap/restore feature (0xPolygonHermez#2315) * add doc, and fix dockers to be able to use snap/restore feature * add doc for snap/restore feature --------- Co-authored-by: Toni Ramírez <[email protected]> * Update docker-compose.yml * Update docker-compose.yml * do not add tx to the pool in case err != nil * do not add tx into the pool if a fatal error in the executor happens during pre execution * fix dbMultiWriteSinglePosition config value * workarround for the error error closing batch * workarround for the error error closing batch * workarround for the error error closing batch * workaround for the error of closing batch, another case * `Worker`'s `AddTxTracker` Bug Fix (0xPolygonHermez#2343) * bugfix: Resolve Function Bug in Worker Module Signed-off-by: Nikolay Nedkov <[email protected]> * improve: improving the wait for pending txs to be for only the txs for the current address. Signed-off-by: Nikolay Nedkov <[email protected]> --------- Signed-off-by: Nikolay Nedkov <[email protected]> * rename config files (0xPolygonHermez#2349) * fix closing batch + logs (0xPolygonHermez#2348) * fix closing batch + logs * fix * log description * typo errors * fix error: failed to store transactions for batch due to duplicate key * test * typo * Update README.md * Update release.yml * bugfix: fixing place where we need to increment the wg per address for pending txs Signed-off-by: Nikolay Nedkov <[email protected]> * add GasPriceMarginFactor and MaxGasPrice to eth-tx-manager * add logs, fix config * update config file documentation * refactoring how eth tx manager monitor txs to increase the throughput * monitoredTx log improvements * merge clean-up * merge clean-up * fix * ethTxManager gasOffset refactoring * update docs * add recover to monitor tx flow and unit test to failed gas estimation with forced gas --------- Signed-off-by: Nikolay Nedkov <[email protected]> Co-authored-by: Toni Ramírez <[email protected]> Co-authored-by: joanestebanr <[email protected]> Co-authored-by: Alonso Rodriguez <[email protected]> Co-authored-by: Toni Ramírez <[email protected]> Co-authored-by: agnusmor <[email protected]> Co-authored-by: agnusmor <[email protected]> Co-authored-by: joanestebanr <[email protected]> Co-authored-by: Nikolay Nedkov <[email protected]>
giskook · Oct 30, 2023 · 49a734a · 49a734a
1 parent 3335bdf
commit 49a734a
Show file tree

Hide file tree

Showing 23 changed files with 745 additions and 324 deletions.
diff --git a/aggregator/aggregator.go b/aggregator/aggregator.go
@@ -275,10 +275,10 @@ func (a *Aggregator) sendFinalProof() {
 				continue
 			}
 			monitoredTxID := buildMonitoredTxID(proof.BatchNumber, proof.BatchNumberFinal)
-			err = a.EthTxManager.Add(ctx, ethTxManagerOwner, monitoredTxID, sender, to, nil, data, nil)
+			err = a.EthTxManager.Add(ctx, ethTxManagerOwner, monitoredTxID, sender, to, nil, data, a.cfg.GasOffset, nil)
 			if err != nil {
-				log := log.WithFields("tx", monitoredTxID)
-				log.Errorf("Error to add batch verification tx to eth tx manager: %v", err)
+				mTxLogger := ethtxmanager.CreateLogger(ethTxManagerOwner, monitoredTxID, sender, to)
+				mTxLogger.Errorf("Error to add batch verification tx to eth tx manager: %v", err)
 				a.handleFailureToAddVerifyBatchToBeMonitored(ctx, proof)
 				continue
 			}
@@ -1027,23 +1027,23 @@ func (hc *healthChecker) Watch(req *grpchealth.HealthCheckRequest, server grpche
 }
 
 func (a *Aggregator) handleMonitoredTxResult(result ethtxmanager.MonitoredTxResult) {
-	resLog := log.WithFields("owner", ethTxManagerOwner, "txId", result.ID)
+	mTxResultLogger := ethtxmanager.CreateMonitoredTxResultLogger(ethTxManagerOwner, result)
 	if result.Status == ethtxmanager.MonitoredTxStatusFailed {
-		resLog.Fatal("failed to send batch verification, TODO: review this fatal and define what to do in this case")
+		mTxResultLogger.Fatal("failed to send batch verification, TODO: review this fatal and define what to do in this case")
 	}
 
 	// monitoredIDFormat: "proof-from-%v-to-%v"
 	idSlice := strings.Split(result.ID, "-")
 	proofBatchNumberStr := idSlice[2]
 	proofBatchNumber, err := strconv.ParseUint(proofBatchNumberStr, encoding.Base10, 0)
 	if err != nil {
-		resLog.Errorf("failed to read final proof batch number from monitored tx: %v", err)
+		mTxResultLogger.Errorf("failed to read final proof batch number from monitored tx: %v", err)
 	}
 
 	proofBatchNumberFinalStr := idSlice[4]
 	proofBatchNumberFinal, err := strconv.ParseUint(proofBatchNumberFinalStr, encoding.Base10, 0)
 	if err != nil {
-		resLog.Errorf("failed to read final proof batch number final from monitored tx: %v", err)
+		mTxResultLogger.Errorf("failed to read final proof batch number final from monitored tx: %v", err)
 	}
 
 	log := log.WithFields("txId", result.ID, "batches", fmt.Sprintf("%d-%d", proofBatchNumber, proofBatchNumberFinal))

diff --git a/aggregator/aggregator_test.go b/aggregator/aggregator_test.go
@@ -54,7 +54,7 @@ func TestSendFinalProof(t *testing.T) {
 		BatchNumberFinal: batchNumFinal,
 	}
 	finalProof := &prover.FinalProof{}
-	cfg := Config{SenderAddress: from.Hex()}
+	cfg := Config{SenderAddress: from.Hex(), GasOffset: uint64(10)}
 
 	testCases := []struct {
 		name    string
@@ -135,7 +135,7 @@ func TestSendFinalProof(t *testing.T) {
 					assert.True(a.verifyingProof)
 				}).Return(&to, data, nil).Once()
 				monitoredTxID := buildMonitoredTxID(batchNum, batchNumFinal)
-				m.ethTxManager.On("Add", mock.Anything, ethTxManagerOwner, monitoredTxID, from, &to, value, data, nil).Return(errBanana).Once()
+				m.ethTxManager.On("Add", mock.Anything, ethTxManagerOwner, monitoredTxID, from, &to, value, data, cfg.GasOffset, nil).Return(errBanana).Once()
 				m.stateMock.On("UpdateGeneratedProof", mock.Anything, recursiveProof, nil).Run(func(args mock.Arguments) {
 					// test is done, stop the sendFinalProof method
 					a.exit()
@@ -160,7 +160,7 @@ func TestSendFinalProof(t *testing.T) {
 					assert.True(a.verifyingProof)
 				}).Return(&to, data, nil).Once()
 				monitoredTxID := buildMonitoredTxID(batchNum, batchNumFinal)
-				m.ethTxManager.On("Add", mock.Anything, ethTxManagerOwner, monitoredTxID, from, &to, value, data, nil).Return(nil).Once()
+				m.ethTxManager.On("Add", mock.Anything, ethTxManagerOwner, monitoredTxID, from, &to, value, data, cfg.GasOffset, nil).Return(nil).Once()
 				ethTxManResult := ethtxmanager.MonitoredTxResult{
 					ID:     monitoredTxID,
 					Status: ethtxmanager.MonitoredTxStatusConfirmed,

diff --git a/aggregator/config.go b/aggregator/config.go
@@ -73,4 +73,16 @@ type Config struct {
 	// which a proof in generating state is considered to be stuck and
 	// allowed to be cleared.
 	GeneratingProofCleanupThreshold string `mapstructure:"GeneratingProofCleanupThreshold"`
+
+	// GasOffset is the amount of gas to be added to the gas estimation in order
+	// to provide an amount that is higher than the estimated one. This is used
+	// to avoid the TX getting reverted in case something has changed in the network
+	// state after the estimation which can cause the TX to require more gas to be
+	// executed.
+	//
+	// ex:
+	// gas estimation: 1000
+	// gas offset: 100
+	// final gas: 1100
+	GasOffset uint64 `mapstructure:"GasOffset"`
 }
diff --git a/aggregator/interfaces.go b/aggregator/interfaces.go
@@ -29,7 +29,7 @@ type proverInterface interface {
 // ethTxManager contains the methods required to send txs to
 // ethereum.
 type ethTxManager interface {
-	Add(ctx context.Context, owner, id string, from common.Address, to *common.Address, value *big.Int, data []byte, dbTx pgx.Tx) error
+	Add(ctx context.Context, owner, id string, from common.Address, to *common.Address, value *big.Int, data []byte, gasOffset uint64, dbTx pgx.Tx) error
 	Result(ctx context.Context, owner, id string, dbTx pgx.Tx) (ethtxmanager.MonitoredTxResult, error)
 	ResultsByStatus(ctx context.Context, owner string, statuses []ethtxmanager.MonitoredTxStatus, dbTx pgx.Tx) ([]ethtxmanager.MonitoredTxResult, error)
 	ProcessPendingMonitoredTxs(ctx context.Context, owner string, failedResultHandler ethtxmanager.ResultHandler, dbTx pgx.Tx)

diff --git a/aggregator/mocks/mock_ethtxmanager.go b/aggregator/mocks/mock_ethtxmanager.go
diff --git a/cmd/run.go b/cmd/run.go
@@ -173,7 +173,7 @@ func start(cliCtx *cli.Context) error {
 			if poolInstance == nil {
 				poolInstance = createPool(c.Pool, c.State.Batch.Constraints, l2ChainID, st, eventLog)
 			}
-			seq := createSequencer(*c, poolInstance, ethTxManagerStorage, st, eventLog)
+			seq := createSequencer(*c, poolInstance, st, eventLog)
 			go seq.Start(cliCtx.Context)
 		case SEQUENCE_SENDER:
 			ev.Component = event.Component_Sequence_Sender
@@ -217,7 +217,7 @@ func start(cliCtx *cli.Context) error {
 			if poolInstance == nil {
 				poolInstance = createPool(c.Pool, c.State.Batch.Constraints, l2ChainID, st, eventLog)
 			}
-			go runSynchronizer(*c, etherman, etm, st, poolInstance, eventLog)
+			go runSynchronizer(*c, etherman, ethTxManagerStorage, st, poolInstance, eventLog)
 		case ETHTXMANAGER:
 			ev.Component = event.Component_EthTxManager
 			ev.Description = "Running eth tx manager service"
@@ -285,7 +285,7 @@ func newEtherman(c config.Config) (*etherman.Client, error) {
 	return etherman, nil
 }
 
-func runSynchronizer(cfg config.Config, etherman *etherman.Client, ethTxManager *ethtxmanager.Client, st *state.State, pool *pool.Pool, eventLog *event.EventLog) {
+func runSynchronizer(cfg config.Config, etherman *etherman.Client, ethTxManagerStorage *ethtxmanager.PostgresStorage, st *state.State, pool *pool.Pool, eventLog *event.EventLog) {
 	var trustedSequencerURL string
 	var err error
 	if !cfg.IsTrustedSequencer {
@@ -313,8 +313,9 @@ func runSynchronizer(cfg config.Config, etherman *etherman.Client, ethTxManager
 			etherManForL1 = append(etherManForL1, eth)
 		}
 	}
+	etm := ethtxmanager.New(cfg.EthTxManager, etherman, ethTxManagerStorage, st)
 	sy, err := synchronizer.NewSynchronizer(
-		cfg.IsTrustedSequencer, etherman, etherManForL1, st, pool, ethTxManager,
+		cfg.IsTrustedSequencer, etherman, etherManForL1, st, pool, etm,
 		zkEVMClient, eventLog, cfg.NetworkConfig.Genesis, cfg.Synchronizer, cfg.Log.Environment == "development",
 	)
 	if err != nil {
@@ -389,15 +390,13 @@ func runJSONRPCServer(c config.Config, etherman *etherman.Client, chainID uint64
 	}
 }
 
-func createSequencer(cfg config.Config, pool *pool.Pool, etmStorage *ethtxmanager.PostgresStorage, st *state.State, eventLog *event.EventLog) *sequencer.Sequencer {
+func createSequencer(cfg config.Config, pool *pool.Pool, st *state.State, eventLog *event.EventLog) *sequencer.Sequencer {
 	etherman, err := newEtherman(cfg)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	ethTxManager := ethtxmanager.New(cfg.EthTxManager, etherman, etmStorage, st)
-
-	seq, err := sequencer.New(cfg.Sequencer, cfg.State.Batch, pool, st, etherman, ethTxManager, eventLog)
+	seq, err := sequencer.New(cfg.Sequencer, cfg.State.Batch, pool, st, etherman, eventLog)
 	if err != nil {
 		log.Fatal(err)
 	}

diff --git a/config/config_test.go b/config/config_test.go
@@ -156,6 +156,10 @@ func Test_Defaults(t *testing.T) {
 			path:          "SequenceSender.MaxTxSizeForL1",
 			expectedValue: uint64(131072),
 		},
+		{
+			path:          "SequenceSender.GasOffset",
+			expectedValue: uint64(80000),
+		},
 		{
 			path:          "Etherman.URL",
 			expectedValue: "http://localhost:8545",
@@ -437,7 +441,10 @@ func Test_Defaults(t *testing.T) {
 			path:          "Aggregator.GeneratingProofCleanupThreshold",
 			expectedValue: "10m",
 		},
-
+		{
+			path:          "Aggregator.GasOffset",
+			expectedValue: uint64(0),
+		},
 		{
 			path:          "State.Batch.Constraints.MaxTxsPerBatch",
 			expectedValue: uint64(300),

diff --git a/config/default.go b/config/default.go
@@ -145,6 +145,7 @@ LastBatchVirtualizationTimeMaxWaitPeriod = "5s"
 MaxTxSizeForL1 = 131072
 L2Coinbase = "0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266"
 PrivateKey = {Path = "/pk/sequencer.keystore", Password = "testonly"}
+GasOffset = 80000
 
 [Aggregator]
 Host = "0.0.0.0"
@@ -156,6 +157,7 @@ TxProfitabilityMinReward = "1.1"
 ProofStatePollingInterval = "5s"
 CleanupLockedProofsInterval = "2m"
 GeneratingProofCleanupThreshold = "10m"
+GasOffset = 0
 
 [L2GasPriceSuggester]
 Type = "follower"

diff --git a/db/migrations/state/0012.sql b/db/migrations/state/0012.sql
@@ -0,0 +1,8 @@
+-- +migrate Up
+ALTER TABLE state.monitored_txs
+    ADD COLUMN gas_offset DECIMAL(78, 0) NOT NULL DEFAULT 0;
+ALTER TABLE state.monitored_txs ALTER COLUMN gas_offset DROP DEFAULT;
+
+-- +migrate Down
+ALTER TABLE state.monitored_txs
+    DROP COLUMN gas_offset;
diff --git a/db/migrations/state/0012_test.go b/db/migrations/state/0012_test.go
@@ -0,0 +1,62 @@
+package migrations_test
+
+import (
+	"database/sql"
+	"testing"
+	"time"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/assert"
+)
+
+// this migration changes length of the token name
+type migrationTest0012 struct{}
+
+func (m migrationTest0012) InsertData(db *sql.DB) error {
+	addMonitoredTx := `
+        INSERT INTO state.monitored_txs (owner, id, from_addr, to_addr, nonce, value, data, gas, gas_price, status, history, block_num, created_at, updated_at) 
+                                VALUES (   $1, $2,        $3,      $4,    $5,    $6,   $7,  $8,        $9,    $10,     $11,       $12,        $13,        $14);`
+
+	args := []interface{}{
+		"owner", "id1", common.HexToAddress("0x111").String(), common.HexToAddress("0x222").String(), 333, 444,
+		[]byte{5, 5, 5}, 666, 777, "status", []string{common.HexToHash("0x888").String()}, 999, time.Now(), time.Now(),
+	}
+	if _, err := db.Exec(addMonitoredTx, args...); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (m migrationTest0012) RunAssertsAfterMigrationUp(t *testing.T, db *sql.DB) {
+	addMonitoredTx := `
+	INSERT INTO state.monitored_txs (owner, id, from_addr, to_addr, nonce, value, data, gas, gas_price, status, history, block_num, created_at, updated_at, gas_offset) 
+                            VALUES (   $1, $2,        $3,      $4,    $5,    $6,   $7,  $8,        $9,    $10,     $11,       $12,        $13,        $14,        $15);`
+
+	args := []interface{}{
+		"owner", "id2", common.HexToAddress("0x111").String(), common.HexToAddress("0x222").String(), 333, 444,
+		[]byte{5, 5, 5}, 666, 777, "status", []string{common.HexToHash("0x888").String()}, 999, time.Now(), time.Now(),
+		101010,
+	}
+	_, err := db.Exec(addMonitoredTx, args...)
+	assert.NoError(t, err)
+
+	gasOffset := 999
+
+	getGasOffsetQuery := `SELECT gas_offset FROM state.monitored_txs WHERE id = $1`
+	err = db.QueryRow(getGasOffsetQuery, "id1").Scan(&gasOffset)
+	assert.NoError(t, err)
+	assert.Equal(t, 0, gasOffset)
+
+	err = db.QueryRow(getGasOffsetQuery, "id2").Scan(&gasOffset)
+	assert.NoError(t, err)
+	assert.Equal(t, 101010, gasOffset)
+}
+
+func (m migrationTest0012) RunAssertsAfterMigrationDown(t *testing.T, db *sql.DB) {
+
+}
+
+func TestMigration0012(t *testing.T) {
+	runMigrationTest(t, 12, migrationTest0012{})
+}