From 52272aef241c790e6d21f52e0b4442915797d1ed Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Fri, 22 Mar 2024 16:23:07 -0500 Subject: [PATCH 1/5] Use raw JSON RPCs to look for batch reverts [NIT-1279] --- arbnode/batch_poster.go | 49 ++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 438c3e4281..1b3d226280 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -431,6 +431,35 @@ func AccessList(opts *AccessListOpts) types.AccessList { return l } +type txData struct { + Hash common.Hash `json:"hash"` + Nonce hexutil.Uint64 `json:"nonce"` + From common.Address `json:"from"` +} + +// getTxsDataByBlock fetches all the transactions inside block of id 'number' using json rpc +// and returns an array of txData which has fields that are necessary in checking for batch reverts +func (b *BatchPoster) getTxsDataByBlock(ctx context.Context, number int64) ([]txData, error) { + blockNrStr := rpc.BlockNumber(number).String() + rawRpcClient := b.l1Reader.Client().Client() + + var numTxs hexutil.Uint + err := rawRpcClient.CallContext(ctx, &numTxs, "eth_getBlockTransactionCountByNumber", blockNrStr) + if err != nil { + return nil, fmt.Errorf("error fetching number of transactions in the block %d : %w", number, err) + } + + var result []txData + for i := hexutil.Uint64(0); i < hexutil.Uint64(numTxs); i++ { + var meta txData + if err = rawRpcClient.CallContext(ctx, &meta, "eth_getTransactionByBlockNumberAndIndex", blockNrStr, i); err != nil { + return nil, fmt.Errorf("error fetching transaction of index %d from block %d: %w", i, number, err) + } + result = append(result, meta) + } + return result, nil +} + // checkRevert checks blocks with number in range [from, to] whether they // contain reverted batch_poster transaction. // It returns true if it finds batch posting needs to halt, which is true if a batch reverts @@ -440,20 +469,15 @@ func (b *BatchPoster) checkReverts(ctx context.Context, to int64) (bool, error) return false, fmt.Errorf("wrong range, from: %d > to: %d", b.nextRevertCheckBlock, to) } for ; b.nextRevertCheckBlock <= to; b.nextRevertCheckBlock++ { - number := big.NewInt(b.nextRevertCheckBlock) - block, err := b.l1Reader.Client().BlockByNumber(ctx, number) + txs, err := b.getTxsDataByBlock(ctx, b.nextRevertCheckBlock) if err != nil { - return false, fmt.Errorf("getting block: %v by number: %w", number, err) + return false, fmt.Errorf("error getting transactions data of block %d: %w", b.nextRevertCheckBlock, err) } - for idx, tx := range block.Transactions() { - from, err := b.l1Reader.Client().TransactionSender(ctx, tx, block.Hash(), uint(idx)) - if err != nil { - return false, fmt.Errorf("getting sender of transaction tx: %v, %w", tx.Hash(), err) - } - if from == b.dataPoster.Sender() { - r, err := b.l1Reader.Client().TransactionReceipt(ctx, tx.Hash()) + for _, tx := range txs { + if tx.From == b.dataPoster.Sender() { + r, err := b.l1Reader.Client().TransactionReceipt(ctx, tx.Hash) if err != nil { - return false, fmt.Errorf("getting a receipt for transaction: %v, %w", tx.Hash(), err) + return false, fmt.Errorf("getting a receipt for transaction: %v, %w", tx.Hash, err) } if r.Status == types.ReceiptStatusFailed { shouldHalt := !b.config().DataPoster.UseNoOpStorage @@ -461,8 +485,7 @@ func (b *BatchPoster) checkReverts(ctx context.Context, to int64) (bool, error) if shouldHalt { logLevel = log.Error } - txErr := arbutil.DetailTxError(ctx, b.l1Reader.Client(), tx, r) - logLevel("Transaction from batch poster reverted", "nonce", tx.Nonce(), "txHash", tx.Hash(), "blockNumber", r.BlockNumber, "blockHash", r.BlockHash, "txErr", txErr) + logLevel("Transaction from batch poster reverted", "nonce", tx.Nonce, "txHash", tx.Hash, "blockNumber", r.BlockNumber, "blockHash", r.BlockHash) return shouldHalt, nil } } From 6d5ab86674bb0ddd8709e0e751926043a8c0f0f9 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Fri, 22 Mar 2024 16:37:10 -0500 Subject: [PATCH 2/5] address PR comments --- arbnode/batch_poster.go | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 1b3d226280..1117317ab6 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -442,22 +442,14 @@ type txData struct { func (b *BatchPoster) getTxsDataByBlock(ctx context.Context, number int64) ([]txData, error) { blockNrStr := rpc.BlockNumber(number).String() rawRpcClient := b.l1Reader.Client().Client() - - var numTxs hexutil.Uint - err := rawRpcClient.CallContext(ctx, &numTxs, "eth_getBlockTransactionCountByNumber", blockNrStr) - if err != nil { - return nil, fmt.Errorf("error fetching number of transactions in the block %d : %w", number, err) + var blk struct { + Transactions []txData `json:"transactions"` } - - var result []txData - for i := hexutil.Uint64(0); i < hexutil.Uint64(numTxs); i++ { - var meta txData - if err = rawRpcClient.CallContext(ctx, &meta, "eth_getTransactionByBlockNumberAndIndex", blockNrStr, i); err != nil { - return nil, fmt.Errorf("error fetching transaction of index %d from block %d: %w", i, number, err) - } - result = append(result, meta) + err := rawRpcClient.CallContext(ctx, &blk, "eth_getBlockByNumber", blockNrStr, true) + if err != nil { + return nil, fmt.Errorf("error fetching block %d : %w", number, err) } - return result, nil + return blk.Transactions, nil } // checkRevert checks blocks with number in range [from, to] whether they From cc5ec18656cbbafd604f876d071337c891e7bee7 Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Mon, 25 Mar 2024 14:34:06 -0500 Subject: [PATCH 3/5] modify DetailTxError impl to be usable for batchposter checkReverts --- arbnode/batch_poster.go | 29 +++++++++++++++++--------- arbutil/wait_for_l1.go | 46 ++++++++++++++++++++--------------------- 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 1117317ab6..b5f8558f59 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -431,19 +431,27 @@ func AccessList(opts *AccessListOpts) types.AccessList { return l } -type txData struct { - Hash common.Hash `json:"hash"` - Nonce hexutil.Uint64 `json:"nonce"` - From common.Address `json:"from"` +type txInfo struct { + Hash common.Hash `json:"hash"` + Nonce hexutil.Uint64 `json:"nonce"` + From common.Address `json:"from"` + To *common.Address `json:"to"` + Gas hexutil.Uint64 `json:"gas"` + GasPrice *hexutil.Big `json:"gasPrice"` + GasFeeCap *hexutil.Big `json:"maxFeePerGas,omitempty"` + GasTipCap *hexutil.Big `json:"maxPriorityFeePerGas,omitempty"` + Input hexutil.Bytes `json:"input"` + Value *hexutil.Big `json:"value"` + Accesses *types.AccessList `json:"accessList,omitempty"` } -// getTxsDataByBlock fetches all the transactions inside block of id 'number' using json rpc -// and returns an array of txData which has fields that are necessary in checking for batch reverts -func (b *BatchPoster) getTxsDataByBlock(ctx context.Context, number int64) ([]txData, error) { +// getTxsInfoByBlock fetches all the transactions inside block of id 'number' using json rpc +// and returns an array of txInfo which has fields that are necessary in checking for batch reverts +func (b *BatchPoster) getTxsInfoByBlock(ctx context.Context, number int64) ([]txInfo, error) { blockNrStr := rpc.BlockNumber(number).String() rawRpcClient := b.l1Reader.Client().Client() var blk struct { - Transactions []txData `json:"transactions"` + Transactions []txInfo `json:"transactions"` } err := rawRpcClient.CallContext(ctx, &blk, "eth_getBlockByNumber", blockNrStr, true) if err != nil { @@ -461,7 +469,7 @@ func (b *BatchPoster) checkReverts(ctx context.Context, to int64) (bool, error) return false, fmt.Errorf("wrong range, from: %d > to: %d", b.nextRevertCheckBlock, to) } for ; b.nextRevertCheckBlock <= to; b.nextRevertCheckBlock++ { - txs, err := b.getTxsDataByBlock(ctx, b.nextRevertCheckBlock) + txs, err := b.getTxsInfoByBlock(ctx, b.nextRevertCheckBlock) if err != nil { return false, fmt.Errorf("error getting transactions data of block %d: %w", b.nextRevertCheckBlock, err) } @@ -477,7 +485,8 @@ func (b *BatchPoster) checkReverts(ctx context.Context, to int64) (bool, error) if shouldHalt { logLevel = log.Error } - logLevel("Transaction from batch poster reverted", "nonce", tx.Nonce, "txHash", tx.Hash, "blockNumber", r.BlockNumber, "blockHash", r.BlockHash) + txErr := arbutil.DetailTxErrorForTxInfo(ctx, b.l1Reader.Client(), tx.Hash, r, tx.From, tx.To, tx.GasPrice.ToInt(), tx.GasFeeCap.ToInt(), tx.GasTipCap.ToInt(), tx.Value.ToInt(), tx.Input, *tx.Accesses, uint64(tx.Gas)) + logLevel("Transaction from batch poster reverted", "nonce", tx.Nonce, "txHash", tx.Hash, "blockNumber", r.BlockNumber, "blockHash", r.BlockHash, "txErr", txErr) return shouldHalt, nil } } diff --git a/arbutil/wait_for_l1.go b/arbutil/wait_for_l1.go index 180ce1c67e..8fbaaed4d4 100644 --- a/arbutil/wait_for_l1.go +++ b/arbutil/wait_for_l1.go @@ -13,7 +13,6 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/rpc" ) @@ -30,23 +29,20 @@ type L1Interface interface { Client() rpc.ClientInterface } -func SendTxAsCall(ctx context.Context, client L1Interface, tx *types.Transaction, from common.Address, blockNum *big.Int, unlimitedGas bool) ([]byte, error) { - var gas uint64 +func SendTxAsCall(ctx context.Context, client L1Interface, from common.Address, to *common.Address, gasPrice, gasFeeCap, gasTipCap, value, blockNum *big.Int, data []byte, accessList types.AccessList, gas uint64, unlimitedGas bool) ([]byte, error) { if unlimitedGas { gas = 0 - } else { - gas = tx.Gas() } callMsg := ethereum.CallMsg{ From: from, - To: tx.To(), + To: to, Gas: gas, - GasPrice: tx.GasPrice(), - GasFeeCap: tx.GasFeeCap(), - GasTipCap: tx.GasTipCap(), - Value: tx.Value(), - Data: tx.Data(), - AccessList: tx.AccessList(), + GasPrice: gasPrice, + GasFeeCap: gasFeeCap, + GasTipCap: gasTipCap, + Value: value, + Data: data, + AccessList: accessList, } return client.CallContract(ctx, callMsg, blockNum) } @@ -72,6 +68,15 @@ func GetPendingCallBlockNumber(ctx context.Context, client L1Interface) (*big.In } func DetailTxError(ctx context.Context, client L1Interface, tx *types.Transaction, txRes *types.Receipt) error { + // Re-execute the transaction as a call to get a better error + from, err := client.TransactionSender(ctx, tx, txRes.BlockHash, txRes.TransactionIndex) + if err != nil { + return fmt.Errorf("TransactionSender got: %w for tx %v", err, tx.Hash()) + } + return DetailTxErrorForTxInfo(ctx, client, tx.Hash(), txRes, from, tx.To(), tx.GasPrice(), tx.GasFeeCap(), tx.GasTipCap(), tx.Value(), tx.Data(), tx.AccessList(), tx.Gas()) +} + +func DetailTxErrorForTxInfo(ctx context.Context, client L1Interface, txHash common.Hash, txRes *types.Receipt, from common.Address, to *common.Address, gasPrice, gasFeeCap, gasTipCap, value *big.Int, data []byte, accessList types.AccessList, gas uint64) error { // Re-execute the transaction as a call to get a better error if ctx.Err() != nil { return ctx.Err() @@ -82,17 +87,12 @@ func DetailTxError(ctx context.Context, client L1Interface, tx *types.Transactio if txRes.Status == types.ReceiptStatusSuccessful { return nil } - from, err := client.TransactionSender(ctx, tx, txRes.BlockHash, txRes.TransactionIndex) - if err != nil { - return fmt.Errorf("TransactionSender got: %w for tx %v", err, tx.Hash()) - } - _, err = SendTxAsCall(ctx, client, tx, from, txRes.BlockNumber, false) - if err == nil { - return fmt.Errorf("tx failed but call succeeded for tx hash %v", tx.Hash()) + var err error + if _, err = SendTxAsCall(ctx, client, from, to, gasPrice, gasFeeCap, gasTipCap, value, txRes.BlockNumber, data, accessList, gas, false); err == nil { + return fmt.Errorf("tx failed but call succeeded for tx hash %v", txHash) } - _, err = SendTxAsCall(ctx, client, tx, from, txRes.BlockNumber, true) - if err == nil { - return fmt.Errorf("%w for tx hash %v", vm.ErrOutOfGas, tx.Hash()) + if _, err = SendTxAsCall(ctx, client, from, to, gasPrice, gasFeeCap, gasTipCap, value, txRes.BlockNumber, data, accessList, gas, true); err == nil { + return fmt.Errorf("tx failed but call succeeded for tx hash %v", txHash) } - return fmt.Errorf("SendTxAsCall got: %w for tx hash %v", err, tx.Hash()) + return fmt.Errorf("SendTxAsCall got: %w for tx hash %v", err, txHash) } From 054e8958976450d6ced55cb9cdec171afa68040b Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Mon, 25 Mar 2024 14:48:31 -0500 Subject: [PATCH 4/5] refactor --- arbnode/batch_poster.go | 13 ++++++++++- arbutil/wait_for_l1.go | 48 ++++++++++++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index b5f8558f59..b6c62d60a2 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -18,6 +18,7 @@ import ( "github.com/andybalholm/brotli" "github.com/spf13/pflag" + "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" @@ -485,7 +486,17 @@ func (b *BatchPoster) checkReverts(ctx context.Context, to int64) (bool, error) if shouldHalt { logLevel = log.Error } - txErr := arbutil.DetailTxErrorForTxInfo(ctx, b.l1Reader.Client(), tx.Hash, r, tx.From, tx.To, tx.GasPrice.ToInt(), tx.GasFeeCap.ToInt(), tx.GasTipCap.ToInt(), tx.Value.ToInt(), tx.Input, *tx.Accesses, uint64(tx.Gas)) + txErr := arbutil.DetailTxErrorUsingCallMsg(ctx, b.l1Reader.Client(), tx.Hash, r, ethereum.CallMsg{ + From: tx.From, + To: tx.To, + Gas: uint64(tx.Gas), + GasPrice: tx.GasPrice.ToInt(), + GasFeeCap: tx.GasFeeCap.ToInt(), + GasTipCap: tx.GasTipCap.ToInt(), + Value: tx.Value.ToInt(), + Data: tx.Input, + AccessList: *tx.Accesses, + }) logLevel("Transaction from batch poster reverted", "nonce", tx.Nonce, "txHash", tx.Hash, "blockNumber", r.BlockNumber, "blockHash", r.BlockHash, "txErr", txErr) return shouldHalt, nil } diff --git a/arbutil/wait_for_l1.go b/arbutil/wait_for_l1.go index 8fbaaed4d4..cfe24cf636 100644 --- a/arbutil/wait_for_l1.go +++ b/arbutil/wait_for_l1.go @@ -13,6 +13,7 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/rpc" ) @@ -29,20 +30,23 @@ type L1Interface interface { Client() rpc.ClientInterface } -func SendTxAsCall(ctx context.Context, client L1Interface, from common.Address, to *common.Address, gasPrice, gasFeeCap, gasTipCap, value, blockNum *big.Int, data []byte, accessList types.AccessList, gas uint64, unlimitedGas bool) ([]byte, error) { +func SendTxAsCall(ctx context.Context, client L1Interface, tx *types.Transaction, from common.Address, blockNum *big.Int, unlimitedGas bool) ([]byte, error) { + var gas uint64 if unlimitedGas { gas = 0 + } else { + gas = tx.Gas() } callMsg := ethereum.CallMsg{ From: from, - To: to, + To: tx.To(), Gas: gas, - GasPrice: gasPrice, - GasFeeCap: gasFeeCap, - GasTipCap: gasTipCap, - Value: value, - Data: data, - AccessList: accessList, + GasPrice: tx.GasPrice(), + GasFeeCap: tx.GasFeeCap(), + GasTipCap: tx.GasTipCap(), + Value: tx.Value(), + Data: tx.Data(), + AccessList: tx.AccessList(), } return client.CallContract(ctx, callMsg, blockNum) } @@ -69,14 +73,31 @@ func GetPendingCallBlockNumber(ctx context.Context, client L1Interface) (*big.In func DetailTxError(ctx context.Context, client L1Interface, tx *types.Transaction, txRes *types.Receipt) error { // Re-execute the transaction as a call to get a better error + if ctx.Err() != nil { + return ctx.Err() + } + if txRes == nil { + return errors.New("expected receipt") + } + if txRes.Status == types.ReceiptStatusSuccessful { + return nil + } from, err := client.TransactionSender(ctx, tx, txRes.BlockHash, txRes.TransactionIndex) if err != nil { return fmt.Errorf("TransactionSender got: %w for tx %v", err, tx.Hash()) } - return DetailTxErrorForTxInfo(ctx, client, tx.Hash(), txRes, from, tx.To(), tx.GasPrice(), tx.GasFeeCap(), tx.GasTipCap(), tx.Value(), tx.Data(), tx.AccessList(), tx.Gas()) + _, err = SendTxAsCall(ctx, client, tx, from, txRes.BlockNumber, false) + if err == nil { + return fmt.Errorf("tx failed but call succeeded for tx hash %v", tx.Hash()) + } + _, err = SendTxAsCall(ctx, client, tx, from, txRes.BlockNumber, true) + if err == nil { + return fmt.Errorf("%w for tx hash %v", vm.ErrOutOfGas, tx.Hash()) + } + return fmt.Errorf("SendTxAsCall got: %w for tx hash %v", err, tx.Hash()) } -func DetailTxErrorForTxInfo(ctx context.Context, client L1Interface, txHash common.Hash, txRes *types.Receipt, from common.Address, to *common.Address, gasPrice, gasFeeCap, gasTipCap, value *big.Int, data []byte, accessList types.AccessList, gas uint64) error { +func DetailTxErrorUsingCallMsg(ctx context.Context, client L1Interface, txHash common.Hash, txRes *types.Receipt, callMsg ethereum.CallMsg) error { // Re-execute the transaction as a call to get a better error if ctx.Err() != nil { return ctx.Err() @@ -88,11 +109,12 @@ func DetailTxErrorForTxInfo(ctx context.Context, client L1Interface, txHash comm return nil } var err error - if _, err = SendTxAsCall(ctx, client, from, to, gasPrice, gasFeeCap, gasTipCap, value, txRes.BlockNumber, data, accessList, gas, false); err == nil { + if _, err = client.CallContract(ctx, callMsg, txRes.BlockNumber); err == nil { return fmt.Errorf("tx failed but call succeeded for tx hash %v", txHash) } - if _, err = SendTxAsCall(ctx, client, from, to, gasPrice, gasFeeCap, gasTipCap, value, txRes.BlockNumber, data, accessList, gas, true); err == nil { - return fmt.Errorf("tx failed but call succeeded for tx hash %v", txHash) + callMsg.Gas = 0 + if _, err = client.CallContract(ctx, callMsg, txRes.BlockNumber); err == nil { + return fmt.Errorf("%w for tx hash %v", vm.ErrOutOfGas, txHash) } return fmt.Errorf("SendTxAsCall got: %w for tx hash %v", err, txHash) } From 329edf1d0a24f0b6bf0406749157aab7c7a0970a Mon Sep 17 00:00:00 2001 From: Ganesh Vanahalli Date: Tue, 26 Mar 2024 08:56:09 -0500 Subject: [PATCH 5/5] address PR comments --- arbnode/batch_poster.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index b6c62d60a2..e9cfe1dd3a 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -486,6 +486,10 @@ func (b *BatchPoster) checkReverts(ctx context.Context, to int64) (bool, error) if shouldHalt { logLevel = log.Error } + al := types.AccessList{} + if tx.Accesses != nil { + al = *tx.Accesses + } txErr := arbutil.DetailTxErrorUsingCallMsg(ctx, b.l1Reader.Client(), tx.Hash, r, ethereum.CallMsg{ From: tx.From, To: tx.To, @@ -495,7 +499,7 @@ func (b *BatchPoster) checkReverts(ctx context.Context, to int64) (bool, error) GasTipCap: tx.GasTipCap.ToInt(), Value: tx.Value.ToInt(), Data: tx.Input, - AccessList: *tx.Accesses, + AccessList: al, }) logLevel("Transaction from batch poster reverted", "nonce", tx.Nonce, "txHash", tx.Hash, "blockNumber", r.BlockNumber, "blockHash", r.BlockHash, "txErr", txErr) return shouldHalt, nil