From 9bb48689eca1660fa07e42c1345208a5368181eb Mon Sep 17 00:00:00 2001 From: colin <102356659+colinlyguo@users.noreply.github.com> Date: Fri, 19 Jan 2024 14:44:53 +0800 Subject: [PATCH] feat(sender): optimize access list tx (#1080) Co-authored-by: zzq0826 <770166635@qq.com> --- common/version/version.go | 2 +- .../internal/controller/sender/estimategas.go | 57 +++++++++++------ rollup/internal/controller/sender/sender.go | 24 +++---- .../internal/controller/sender/sender_test.go | 62 ++++++++++++++++--- rollup/mock_bridge/MockBridgeL1.sol | 6 +- 5 files changed, 106 insertions(+), 45 deletions(-) diff --git a/common/version/version.go b/common/version/version.go index e3a129a045..c101276330 100644 --- a/common/version/version.go +++ b/common/version/version.go @@ -5,7 +5,7 @@ import ( "runtime/debug" ) -var tag = "v4.3.54" +var tag = "v4.3.55" var commit = func() string { if info, ok := debug.ReadBuildInfo(); ok { diff --git a/rollup/internal/controller/sender/estimategas.go b/rollup/internal/controller/sender/estimategas.go index ef66ef412c..0edab916de 100644 --- a/rollup/internal/controller/sender/estimategas.go +++ b/rollup/internal/controller/sender/estimategas.go @@ -6,22 +6,21 @@ import ( "sync/atomic" "github.com/scroll-tech/go-ethereum" - "github.com/scroll-tech/go-ethereum/accounts/abi/bind" "github.com/scroll-tech/go-ethereum/common" "github.com/scroll-tech/go-ethereum/core/types" "github.com/scroll-tech/go-ethereum/log" ) -func (s *Sender) estimateLegacyGas(auth *bind.TransactOpts, contract *common.Address, value *big.Int, input []byte, fallbackGasLimit uint64) (*FeeData, error) { +func (s *Sender) estimateLegacyGas(contract *common.Address, value *big.Int, data []byte, fallbackGasLimit uint64) (*FeeData, error) { gasPrice, err := s.client.SuggestGasPrice(s.ctx) if err != nil { log.Error("estimateLegacyGas SuggestGasPrice failure", "error", err) return nil, err } - gasLimit, _, err := s.estimateGasLimit(auth, contract, input, gasPrice, nil, nil, value, false) + gasLimit, _, err := s.estimateGasLimit(contract, data, gasPrice, nil, nil, value, false) if err != nil { - log.Error("estimateLegacyGas estimateGasLimit failure", "gas price", gasPrice, "from", auth.From.Hex(), - "nonce", auth.Nonce.Uint64(), "contract address", contract.Hex(), "fallback gas limit", fallbackGasLimit, "error", err) + log.Error("estimateLegacyGas estimateGasLimit failure", "gas price", gasPrice, "from", s.auth.From.Hex(), + "nonce", s.auth.Nonce.Uint64(), "contract address", contract.Hex(), "fallback gas limit", fallbackGasLimit, "error", err) if fallbackGasLimit == 0 { return nil, err } @@ -35,7 +34,7 @@ func (s *Sender) estimateLegacyGas(auth *bind.TransactOpts, contract *common.Add }, nil } -func (s *Sender) estimateDynamicGas(auth *bind.TransactOpts, contract *common.Address, value *big.Int, input []byte, fallbackGasLimit uint64) (*FeeData, error) { +func (s *Sender) estimateDynamicGas(contract *common.Address, value *big.Int, data []byte, fallbackGasLimit uint64) (*FeeData, error) { gasTipCap, err := s.client.SuggestGasTipCap(s.ctx) if err != nil { log.Error("estimateDynamicGas SuggestGasTipCap failure", "error", err) @@ -50,10 +49,10 @@ func (s *Sender) estimateDynamicGas(auth *bind.TransactOpts, contract *common.Ad gasTipCap, new(big.Int).Mul(baseFee, big.NewInt(2)), ) - gasLimit, accessList, err := s.estimateGasLimit(auth, contract, input, nil, gasTipCap, gasFeeCap, value, true) + gasLimit, accessList, err := s.estimateGasLimit(contract, data, nil, gasTipCap, gasFeeCap, value, true) if err != nil { log.Error("estimateDynamicGas estimateGasLimit failure", - "from", auth.From.Hex(), "nonce", auth.Nonce.Uint64(), "contract address", contract.Hex(), + "from", s.auth.From.Hex(), "nonce", s.auth.Nonce.Uint64(), "contract address", contract.Hex(), "fallback gas limit", fallbackGasLimit, "error", err) if fallbackGasLimit == 0 { return nil, err @@ -73,15 +72,15 @@ func (s *Sender) estimateDynamicGas(auth *bind.TransactOpts, contract *common.Ad return feeData, nil } -func (s *Sender) estimateGasLimit(opts *bind.TransactOpts, contract *common.Address, input []byte, gasPrice, gasTipCap, gasFeeCap, value *big.Int, useAccessList bool) (uint64, *types.AccessList, error) { +func (s *Sender) estimateGasLimit(contract *common.Address, data []byte, gasPrice, gasTipCap, gasFeeCap, value *big.Int, useAccessList bool) (uint64, *types.AccessList, error) { msg := ethereum.CallMsg{ - From: opts.From, + From: s.auth.From, To: contract, GasPrice: gasPrice, GasTipCap: gasTipCap, GasFeeCap: gasFeeCap, Value: value, - Data: input, + Data: data, } gasLimitWithoutAccessList, err := s.client.EstimateGas(s.ctx, msg) if err != nil { @@ -93,8 +92,7 @@ func (s *Sender) estimateGasLimit(opts *bind.TransactOpts, contract *common.Addr return gasLimitWithoutAccessList, nil, nil } - var gasLimitWithAccessList uint64 - accessList, _, errStr, rpcErr := s.gethClient.CreateAccessList(s.ctx, msg) + accessList, gasLimitWithAccessList, errStr, rpcErr := s.gethClient.CreateAccessList(s.ctx, msg) if rpcErr != nil { log.Error("CreateAccessList RPC error", "error", rpcErr) return gasLimitWithoutAccessList, nil, rpcErr @@ -104,17 +102,36 @@ func (s *Sender) estimateGasLimit(opts *bind.TransactOpts, contract *common.Addr return gasLimitWithoutAccessList, nil, fmt.Errorf(errStr) } - msg.AccessList = *accessList - gasLimitWithAccessList, err = s.client.EstimateGas(s.ctx, msg) - if err != nil { - log.Error("estimateGasLimit EstimateGas failure with access list", "error", err) - return gasLimitWithoutAccessList, nil, err - } + // Heuristically fine-tune accessList because 'to' address is automatically included in the access list by the Ethereum protocol: https://github.com/ethereum/go-ethereum/blob/v1.13.10/core/state/statedb.go#L1322 + // This function returns a gas estimation because GO SDK does not support access list: https://github.com/ethereum/go-ethereum/blob/v1.13.10/ethclient/ethclient.go#L642 + accessList, gasLimitWithAccessList = finetuneAccessList(accessList, gasLimitWithAccessList, msg.To) - log.Info("gas", "gasLimitWithAccessList", gasLimitWithAccessList, "gasLimitWithoutAccessList", gasLimitWithoutAccessList) + log.Info("gas", "senderName", s.name, "senderService", s.service, "gasLimitWithAccessList", gasLimitWithAccessList, "gasLimitWithoutAccessList", gasLimitWithoutAccessList, "accessList", accessList) if gasLimitWithAccessList < gasLimitWithoutAccessList { return gasLimitWithAccessList, accessList, nil } return gasLimitWithoutAccessList, nil, nil } + +func finetuneAccessList(accessList *types.AccessList, gasLimitWithAccessList uint64, to *common.Address) (*types.AccessList, uint64) { + if accessList == nil || to == nil { + return accessList, gasLimitWithAccessList + } + + var newAccessList types.AccessList + for _, entry := range *accessList { + if entry.Address == *to && len(entry.StorageKeys) < 24 { + // This is a heuristic method, we check if number of slots is < 24. + // If so, we remove it and adjust the gas limit estimate accordingly. + // This removal helps in preventing double-counting of the 'to' address in access list calculations. + gasLimitWithAccessList -= 2400 + // Assuming each slot saves 100 gas units as access list storage key cost (1900 gas) plus warm sload (100 gas) is cheaper than cold sload (2100 gas). + gasLimitWithAccessList += uint64(100 * len(entry.StorageKeys)) + } else { + // Otherwise, keep the entry in the new access list. + newAccessList = append(newAccessList, entry) + } + } + return &newAccessList, gasLimitWithAccessList +} diff --git a/rollup/internal/controller/sender/sender.go b/rollup/internal/controller/sender/sender.go index 110076ec6c..a05b49dfc0 100644 --- a/rollup/internal/controller/sender/sender.go +++ b/rollup/internal/controller/sender/sender.go @@ -193,11 +193,11 @@ func (s *Sender) SendConfirmation(cfm *Confirmation) { s.confirmCh <- cfm } -func (s *Sender) getFeeData(auth *bind.TransactOpts, target *common.Address, value *big.Int, data []byte, fallbackGasLimit uint64) (*FeeData, error) { +func (s *Sender) getFeeData(target *common.Address, value *big.Int, data []byte, fallbackGasLimit uint64) (*FeeData, error) { if s.config.TxType == DynamicFeeTxType { - return s.estimateDynamicGas(auth, target, value, data, fallbackGasLimit) + return s.estimateDynamicGas(target, value, data, fallbackGasLimit) } - return s.estimateLegacyGas(auth, target, value, data, fallbackGasLimit) + return s.estimateLegacyGas(target, value, data, fallbackGasLimit) } // SendTransaction send a signed L2tL1 transaction. @@ -226,13 +226,13 @@ func (s *Sender) SendTransaction(ID string, target *common.Address, value *big.I } }() - if feeData, err = s.getFeeData(s.auth, target, value, data, fallbackGasLimit); err != nil { + if feeData, err = s.getFeeData(target, value, data, fallbackGasLimit); err != nil { s.metrics.sendTransactionFailureGetFee.WithLabelValues(s.service, s.name).Inc() log.Error("failed to get fee data", "from", s.auth.From.String(), "nonce", s.auth.Nonce.Uint64(), "fallback gas limit", fallbackGasLimit, "err", err) return common.Hash{}, fmt.Errorf("failed to get fee data, err: %w", err) } - if tx, err = s.createAndSendTx(s.auth, feeData, target, value, data, nil); err != nil { + if tx, err = s.createAndSendTx(feeData, target, value, data, nil); err != nil { s.metrics.sendTransactionFailureSendTx.WithLabelValues(s.service, s.name).Inc() log.Error("failed to create and send tx (non-resubmit case)", "from", s.auth.From.String(), "nonce", s.auth.Nonce.Uint64(), "err", err) return common.Hash{}, fmt.Errorf("failed to create and send transaction, err: %w", err) @@ -250,9 +250,9 @@ func (s *Sender) SendTransaction(ID string, target *common.Address, value *big.I return tx.Hash(), nil } -func (s *Sender) createAndSendTx(auth *bind.TransactOpts, feeData *FeeData, target *common.Address, value *big.Int, data []byte, overrideNonce *uint64) (*types.Transaction, error) { +func (s *Sender) createAndSendTx(feeData *FeeData, target *common.Address, value *big.Int, data []byte, overrideNonce *uint64) (*types.Transaction, error) { var ( - nonce = auth.Nonce.Uint64() + nonce = s.auth.Nonce.Uint64() txData types.TxData ) @@ -308,13 +308,13 @@ func (s *Sender) createAndSendTx(auth *bind.TransactOpts, feeData *FeeData, targ } // sign and send - tx, err := auth.Signer(auth.From, types.NewTx(txData)) + tx, err := s.auth.Signer(s.auth.From, types.NewTx(txData)) if err != nil { - log.Error("failed to sign tx", "address", auth.From.String(), "err", err) + log.Error("failed to sign tx", "address", s.auth.From.String(), "err", err) return nil, err } if err = s.client.SendTransaction(s.ctx, tx); err != nil { - log.Error("failed to send tx", "tx hash", tx.Hash().String(), "from", auth.From.String(), "nonce", tx.Nonce(), "err", err) + log.Error("failed to send tx", "tx hash", tx.Hash().String(), "from", s.auth.From.String(), "nonce", tx.Nonce(), "err", err) // Check if contain nonce, and reset nonce // only reset nonce when it is not from resubmit if strings.Contains(err.Error(), "nonce") && overrideNonce == nil { @@ -339,7 +339,7 @@ func (s *Sender) createAndSendTx(auth *bind.TransactOpts, feeData *FeeData, targ // update nonce when it is not from resubmit if overrideNonce == nil { - auth.Nonce = big.NewInt(int64(nonce + 1)) + s.auth.Nonce = big.NewInt(int64(nonce + 1)) } return tx, nil } @@ -442,7 +442,7 @@ func (s *Sender) resubmitTransaction(feeData *FeeData, auth *bind.TransactOpts, nonce := tx.Nonce() s.metrics.resubmitTransactionTotal.WithLabelValues(s.service, s.name).Inc() - tx, err := s.createAndSendTx(auth, feeData, tx.To(), tx.Value(), tx.Data(), &nonce) + tx, err := s.createAndSendTx(feeData, tx.To(), tx.Value(), tx.Data(), &nonce) if err != nil { log.Error("failed to create and send tx (resubmit case)", "from", s.auth.From.String(), "nonce", nonce, "err", err) return nil, err diff --git a/rollup/internal/controller/sender/sender_test.go b/rollup/internal/controller/sender/sender_test.go index 57386ec671..f79826ffeb 100644 --- a/rollup/internal/controller/sender/sender_test.go +++ b/rollup/internal/controller/sender/sender_test.go @@ -19,16 +19,20 @@ import ( "scroll-tech/common/docker" + bridgeAbi "scroll-tech/rollup/abi" + "scroll-tech/rollup/mock_bridge" + "scroll-tech/rollup/internal/config" ) const TXBatch = 50 var ( - privateKey *ecdsa.PrivateKey - cfg *config.Config - base *docker.App - txTypes = []string{"LegacyTx", "AccessListTx", "DynamicFeeTx"} + privateKey *ecdsa.PrivateKey + cfg *config.Config + base *docker.App + txTypes = []string{"LegacyTx", "AccessListTx", "DynamicFeeTx"} + scrollChainAddress common.Address ) func TestMain(m *testing.M) { @@ -51,6 +55,18 @@ func setupEnv(t *testing.T) { cfg.L1Config.RelayerConfig.SenderConfig.Endpoint = base.L1gethImg.Endpoint() cfg.L1Config.RelayerConfig.SenderConfig.CheckBalanceTime = 1 + + auth, err := bind.NewKeyedTransactorWithChainID(privateKey, base.L1gethImg.ChainID()) + assert.NoError(t, err) + + l1Client, err := base.L1Client() + assert.NoError(t, err) + + _, tx, _, err := mock_bridge.DeployMockBridgeL1(auth, l1Client) + assert.NoError(t, err) + + scrollChainAddress, err = bind.WaitDeployed(context.Background(), l1Client, tx) + assert.NoError(t, err) } func TestSender(t *testing.T) { @@ -60,6 +76,7 @@ func TestSender(t *testing.T) { t.Run("test new sender", testNewSender) t.Run("test pending limit", testPendLimit) t.Run("test fallback gas limit", testFallbackGasLimit) + t.Run("test access list transaction gas limit", testAccessListTransactionGasLimit) t.Run("test resubmit zero gas price transaction", testResubmitZeroGasPriceTransaction) t.Run("test resubmit non-zero gas price transaction", testResubmitNonZeroGasPriceTransaction) t.Run("test resubmit under priced transaction", testResubmitUnderpricedTransaction) @@ -124,7 +141,7 @@ func testFallbackGasLimit(t *testing.T) { // FallbackGasLimit = 100000 patchGuard := gomonkey.ApplyPrivateMethod(s, "estimateGasLimit", - func(opts *bind.TransactOpts, contract *common.Address, input []byte, gasPrice, gasTipCap, gasFeeCap, value *big.Int) (uint64, *types.AccessList, error) { + func(contract *common.Address, input []byte, gasPrice, gasTipCap, gasFeeCap, value *big.Int) (uint64, *types.AccessList, error) { return 0, nil, errors.New("estimateGasLimit error") }, ) @@ -152,7 +169,7 @@ func testResubmitZeroGasPriceTransaction(t *testing.T) { gasFeeCap: big.NewInt(0), gasLimit: 50000, } - tx, err := s.createAndSendTx(s.auth, feeData, &common.Address{}, big.NewInt(0), nil, nil) + tx, err := s.createAndSendTx(feeData, &common.Address{}, big.NewInt(0), nil, nil) assert.NoError(t, err) assert.NotNil(t, tx) // Increase at least 1 wei in gas price, gas tip cap and gas fee cap. @@ -162,6 +179,33 @@ func testResubmitZeroGasPriceTransaction(t *testing.T) { } } +func testAccessListTransactionGasLimit(t *testing.T) { + for _, txType := range txTypes { + cfgCopy := *cfg.L1Config.RelayerConfig.SenderConfig + cfgCopy.TxType = txType + s, err := NewSender(context.Background(), &cfgCopy, privateKey, "test", "test", nil) + assert.NoError(t, err) + + l2GasOracleABI, err := bridgeAbi.L2GasPriceOracleMetaData.GetAbi() + assert.NoError(t, err) + + data, err := l2GasOracleABI.Pack("setL2BaseFee", big.NewInt(2333)) + assert.NoError(t, err) + + gasLimit, accessList, err := s.estimateGasLimit(&scrollChainAddress, data, big.NewInt(100000000000), big.NewInt(100000000000), big.NewInt(100000000000), big.NewInt(0), true) + assert.NoError(t, err) + assert.Equal(t, uint64(43450), gasLimit) + assert.NotNil(t, accessList) + + gasLimit, accessList, err = s.estimateGasLimit(&scrollChainAddress, data, big.NewInt(100000000000), big.NewInt(100000000000), big.NewInt(100000000000), big.NewInt(0), false) + assert.NoError(t, err) + assert.Equal(t, uint64(43927), gasLimit) + assert.Nil(t, accessList) + + s.Stop() + } +} + func testResubmitNonZeroGasPriceTransaction(t *testing.T) { for _, txType := range txTypes { cfgCopy := *cfg.L1Config.RelayerConfig.SenderConfig @@ -177,7 +221,7 @@ func testResubmitNonZeroGasPriceTransaction(t *testing.T) { gasFeeCap: big.NewInt(100000), gasLimit: 50000, } - tx, err := s.createAndSendTx(s.auth, feeData, &common.Address{}, big.NewInt(0), nil, nil) + tx, err := s.createAndSendTx(feeData, &common.Address{}, big.NewInt(0), nil, nil) assert.NoError(t, err) assert.NotNil(t, tx) _, err = s.resubmitTransaction(feeData, s.auth, tx) @@ -201,7 +245,7 @@ func testResubmitUnderpricedTransaction(t *testing.T) { gasFeeCap: big.NewInt(100000), gasLimit: 50000, } - tx, err := s.createAndSendTx(s.auth, feeData, &common.Address{}, big.NewInt(0), nil, nil) + tx, err := s.createAndSendTx(feeData, &common.Address{}, big.NewInt(0), nil, nil) assert.NoError(t, err) assert.NotNil(t, tx) _, err = s.resubmitTransaction(feeData, s.auth, tx) @@ -219,7 +263,7 @@ func testResubmitTransactionWithRisingBaseFee(t *testing.T) { assert.NoError(t, err) tx := types.NewTransaction(s.auth.Nonce.Uint64(), common.Address{}, big.NewInt(0), 0, big.NewInt(0), nil) s.baseFeePerGas = 1000 - feeData, err := s.getFeeData(s.auth, &common.Address{}, big.NewInt(0), nil, 0) + feeData, err := s.getFeeData(&common.Address{}, big.NewInt(0), nil, 0) assert.NoError(t, err) // bump the basefee by 10x s.baseFeePerGas *= 10 diff --git a/rollup/mock_bridge/MockBridgeL1.sol b/rollup/mock_bridge/MockBridgeL1.sol index 06dbbed1ea..9824e40cf8 100644 --- a/rollup/mock_bridge/MockBridgeL1.sol +++ b/rollup/mock_bridge/MockBridgeL1.sol @@ -79,16 +79,16 @@ contract MockBridgeL1 { * Variables * *************/ - /// @notice Message nonce, used to avoid relay attack. uint256 public messageNonce; - mapping(uint256 => bytes32) public committedBatches; + uint256 public l2BaseFee; /*********************************** * Functions from L2GasPriceOracle * ***********************************/ - function setL2BaseFee(uint256) external { + function setL2BaseFee(uint256 _newL2BaseFee) external { + l2BaseFee = _newL2BaseFee; } /************************************