Skip to content

Commit

Permalink
feat(sender): optimize access list tx (#1080)
Browse files Browse the repository at this point in the history
Co-authored-by: zzq0826 <[email protected]>
  • Loading branch information
colinlyguo and zzq0826 authored Jan 19, 2024
1 parent 352aea4 commit 9bb4868
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 45 deletions.
2 changes: 1 addition & 1 deletion common/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
57 changes: 37 additions & 20 deletions rollup/internal/controller/sender/estimategas.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
}
24 changes: 12 additions & 12 deletions rollup/internal/controller/sender/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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
)

Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
62 changes: 53 additions & 9 deletions rollup/internal/controller/sender/sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -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")
},
)
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions rollup/mock_bridge/MockBridgeL1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/************************************
Expand Down

0 comments on commit 9bb4868

Please sign in to comment.