Skip to content

Commit

Permalink
access list bug fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@colinlyguo
colinlyguo committed Jan 18, 2024
1 parent 95034cd commit 5ae4f0a
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 50 deletions.
35 changes: 13 additions & 22 deletions rollup/internal/controller/sender/estimategas.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,21 @@ import (
"math/big"

"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(to *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(to, 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.String(),
"nonce", s.auth.Nonce.Uint64(), "contract address", to.String(), "fallback gas limit", fallbackGasLimit, "error", err)
if fallbackGasLimit == 0 {
return nil, err
}
Expand All @@ -34,18 +33,18 @@ 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, baseFee uint64) (*FeeData, error) {
func (s *Sender) estimateDynamicGas(to *common.Address, value *big.Int, input []byte, fallbackGasLimit uint64, baseFee uint64) (*FeeData, error) {
gasTipCap, err := s.client.SuggestGasTipCap(s.ctx)
if err != nil {
log.Error("estimateDynamicGas SuggestGasTipCap failure", "error", err)
return nil, err
}

gasFeeCap := new(big.Int).Add(gasTipCap, new(big.Int).Mul(new(big.Int).SetUint64(baseFee), big.NewInt(2)))
gasLimit, accessList, err := s.estimateGasLimit(auth, contract, input, nil, gasTipCap, gasFeeCap, value, true)
gasLimit, accessList, err := s.estimateGasLimit(to, input, 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.String(), "nonce", s.auth.Nonce.Uint64(), "to address", to.String(),
"fallback gas limit", fallbackGasLimit, "error", err)
if fallbackGasLimit == 0 {
return nil, err
Expand All @@ -65,15 +64,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(to *common.Address, data []byte, gasPrice, gasTipCap, gasFeeCap, value *big.Int, useAccessList bool) (uint64, *types.AccessList, error) {
msg := ethereum.CallMsg{
From: opts.From,
To: contract,
From: s.auth.From,
To: to,
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 @@ -85,8 +84,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 @@ -96,14 +94,7 @@ 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
}

log.Info("gas", "gasLimitWithAccessList", gasLimitWithAccessList, "gasLimitWithoutAccessList", gasLimitWithoutAccessList)
log.Info("gas comparison", "senderName", s.name, "senderService", s.service, "gasLimitWithAccessList", gasLimitWithAccessList, "gasLimitWithoutAccessList", gasLimitWithoutAccessList, "accessList", accessList)

if gasLimitWithAccessList < gasLimitWithoutAccessList {
return gasLimitWithAccessList, accessList, nil
Expand Down
24 changes: 12 additions & 12 deletions rollup/internal/controller/sender/sender.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,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, baseFee uint64) (*FeeData, error) {
func (s *Sender) getFeeData(target *common.Address, value *big.Int, data []byte, fallbackGasLimit uint64, baseFee uint64) (*FeeData, error) {
if s.config.TxType == DynamicFeeTxType {
return s.estimateDynamicGas(auth, target, value, data, fallbackGasLimit, baseFee)
return s.estimateDynamicGas(target, value, data, fallbackGasLimit, baseFee)
}
return s.estimateLegacyGas(auth, target, value, data, fallbackGasLimit)
return s.estimateLegacyGas(target, value, data, fallbackGasLimit)
}

// SendTransaction send a signed L2tL1 transaction.
Expand All @@ -176,13 +176,13 @@ func (s *Sender) SendTransaction(contextID string, target *common.Address, value
return common.Hash{}, fmt.Errorf("failed to get block number and base fee, err: %w", err)
}

if feeData, err = s.getFeeData(s.auth, target, value, data, fallbackGasLimit, baseFee); err != nil {
if feeData, err = s.getFeeData(target, value, data, fallbackGasLimit, baseFee); 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 @@ -195,9 +195,9 @@ func (s *Sender) SendTransaction(contextID string, target *common.Address, value
return tx.Hash(), nil
}

func (s *Sender) createAndSendTx(auth *bind.TransactOpts, feeData *FeeData, target *common.Address, value *big.Int, data []byte, overrideNonce *uint64) (*gethTypes.Transaction, error) {
func (s *Sender) createAndSendTx(feeData *FeeData, target *common.Address, value *big.Int, data []byte, overrideNonce *uint64) (*gethTypes.Transaction, error) {
var (
nonce = auth.Nonce.Uint64()
nonce = s.auth.Nonce.Uint64()
txData gethTypes.TxData
)

Expand Down Expand Up @@ -252,14 +252,14 @@ func (s *Sender) createAndSendTx(auth *bind.TransactOpts, feeData *FeeData, targ
}

// sign and send
tx, err := auth.Signer(auth.From, gethTypes.NewTx(txData))
tx, err := s.auth.Signer(s.auth.From, gethTypes.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 @@ -284,7 +284,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 @@ -380,7 +380,7 @@ func (s *Sender) resubmitTransaction(auth *bind.TransactOpts, tx *gethTypes.Tran

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
72 changes: 57 additions & 15 deletions rollup/internal/controller/sender/sender_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,21 @@ import (
"scroll-tech/common/docker"
"scroll-tech/common/types"
"scroll-tech/database/migrate"
bridgeAbi "scroll-tech/rollup/abi"

"scroll-tech/rollup/internal/config"
"scroll-tech/rollup/mock_bridge"
)

const TXBatch = 50

var (
privateKey *ecdsa.PrivateKey
cfg *config.Config
base *docker.App
txTypes = []string{"LegacyTx", "AccessListTx", "DynamicFeeTx"}
db *gorm.DB
privateKey *ecdsa.PrivateKey
cfg *config.Config
base *docker.App
txTypes = []string{"LegacyTx", "AccessListTx", "DynamicFeeTx"}
db *gorm.DB
scrollChainAddress common.Address
)

func TestMain(m *testing.M) {
Expand All @@ -50,7 +53,6 @@ func setupEnv(t *testing.T) {
assert.NoError(t, err)
priv, err := crypto.HexToECDSA("1212121212121212121212121212121212121212121212121212121212121212")
assert.NoError(t, err)
// Load default private key.
privateKey = priv

base.RunL1Geth(t)
Expand All @@ -69,6 +71,18 @@ func setupEnv(t *testing.T) {
sqlDB, err := db.DB()
assert.NoError(t, err)
assert.NoError(t, migrate.ResetDB(sqlDB))

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 @@ -78,6 +92,7 @@ func TestSender(t *testing.T) {
t.Run("test new sender", testNewSender)
t.Run("test fallback gas limit", testFallbackGasLimit)
t.Run("test send and retrieve transaction", testSendAndRetrieveTransaction)
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 @@ -121,7 +136,7 @@ func testSendAndRetrieveTransaction(t *testing.T) {
s, err := NewSender(context.Background(), &cfgCopy1, privateKey, "test", "test", types.SenderTypeUnknown, db, nil)
assert.NoError(t, err)

hash, err := s.SendTransaction("0", &common.Address{}, big.NewInt(1), nil, 0)
hash, err := s.SendTransaction("0", &common.Address{}, big.NewInt(0), nil, 0)
assert.NoError(t, err)
txs, err := s.pendingTransactionOrm.GetPendingOrReplacedTransactionsBySenderType(context.Background(), s.senderType, 1)
assert.NoError(t, err)
Expand All @@ -138,6 +153,33 @@ func testSendAndRetrieveTransaction(t *testing.T) {
}
}

func testAccessListTransactionGasLimit(t *testing.T) {
for _, txType := range txTypes {
cfgCopy1 := *cfg.L1Config.RelayerConfig.SenderConfig
cfgCopy1.TxType = txType
s, err := NewSender(context.Background(), &cfgCopy1, privateKey, "test", "test", types.SenderTypeUnknown, db, 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(43927), gasLimit)
assert.Nil(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 testFallbackGasLimit(t *testing.T) {
for _, txType := range txTypes {
sqlDB, err := db.DB()
Expand All @@ -154,7 +196,7 @@ func testFallbackGasLimit(t *testing.T) {
assert.NoError(t, err)

// FallbackGasLimit = 0
txHash0, err := s.SendTransaction("0", &common.Address{}, big.NewInt(1), nil, 0)
txHash0, err := s.SendTransaction("0", &common.Address{}, big.NewInt(0), nil, 0)
assert.NoError(t, err)
tx0, _, err := client.TransactionByHash(context.Background(), txHash0)
assert.NoError(t, err)
Expand All @@ -167,7 +209,7 @@ func testFallbackGasLimit(t *testing.T) {
},
)

txHash1, err := s.SendTransaction("1", &common.Address{}, big.NewInt(1), nil, 100000)
txHash1, err := s.SendTransaction("1", &common.Address{}, big.NewInt(0), nil, 100000)
assert.NoError(t, err)
tx1, _, err := client.TransactionByHash(context.Background(), txHash1)
assert.NoError(t, err)
Expand All @@ -194,7 +236,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 Down Expand Up @@ -223,7 +265,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(s.auth, tx, 0)
Expand Down Expand Up @@ -251,7 +293,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(s.auth, tx, 0)
Expand Down Expand Up @@ -307,7 +349,7 @@ func testCheckPendingTransactionTxConfirmed(t *testing.T) {
s, err := NewSender(context.Background(), &cfgCopy, privateKey, "test", "test", types.SenderTypeUnknown, db, nil)
assert.NoError(t, err)

_, err = s.SendTransaction("test", &common.Address{}, big.NewInt(1), nil, 0)
_, err = s.SendTransaction("test", &common.Address{}, big.NewInt(0), nil, 0)
assert.NoError(t, err)

txs, err := s.pendingTransactionOrm.GetPendingOrReplacedTransactionsBySenderType(context.Background(), s.senderType, 1)
Expand Down Expand Up @@ -342,7 +384,7 @@ func testCheckPendingTransactionResubmitTxConfirmed(t *testing.T) {
s, err := NewSender(context.Background(), &cfgCopy, privateKey, "test", "test", types.SenderTypeUnknown, db, nil)
assert.NoError(t, err)

originTxHash, err := s.SendTransaction("test", &common.Address{}, big.NewInt(1), nil, 0)
originTxHash, err := s.SendTransaction("test", &common.Address{}, big.NewInt(0), nil, 0)
assert.NoError(t, err)

txs, err := s.pendingTransactionOrm.GetPendingOrReplacedTransactionsBySenderType(context.Background(), s.senderType, 1)
Expand Down Expand Up @@ -385,7 +427,7 @@ func testCheckPendingTransactionReplacedTxConfirmed(t *testing.T) {
s, err := NewSender(context.Background(), &cfgCopy, privateKey, "test", "test", types.SenderTypeUnknown, db, nil)
assert.NoError(t, err)

_, err = s.SendTransaction("test", &common.Address{}, big.NewInt(1), nil, 0)
_, err = s.SendTransaction("test", &common.Address{}, big.NewInt(0), nil, 0)
assert.NoError(t, err)

txs, err := s.pendingTransactionOrm.GetPendingOrReplacedTransactionsBySenderType(context.Background(), s.senderType, 1)
Expand Down
6 changes: 5 additions & 1 deletion rollup/mock_bridge/MockBridgeL1.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,15 @@ contract MockBridgeL1 {

mapping(uint256 => bytes32) public committedBatches;

/// @notice The latest known l2 base fee.
uint256 public l2BaseFee;

/***********************************
* Functions from L2GasPriceOracle *
***********************************/

function setL2BaseFee(uint256) external {
function setL2BaseFee(uint256 _newL2BaseFee) external {
l2BaseFee = _newL2BaseFee;
}

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

0 comments on commit 5ae4f0a

Please sign in to comment.