Revert "feat(rollup_relayer): add chain_monitor client (#885)"

This reverts commit 3958e8bd86.

bridge/internal/controller/relayer/l2_relayer.go

@@ -388,6 +388,13 @@ func (r *Layer2Relayer) ProcessPendingBatches() {
 		txHash, err := r.commitSender.SendTransaction(txID, &r.cfg.RollupContractAddress, big.NewInt(0), calldata, 0)
 		if err != nil {
 			log.Error(
+				"Failed to send commitBatch tx to layer1",
+				"index", batch.Index,
+				"hash", batch.Hash,
+				"RollupContractAddress", r.cfg.RollupContractAddress,
+				"err", err,
+			)
+			log.Debug(
 				"Failed to send commitBatch tx to layer1",
 				"index", batch.Index,
 				"hash", batch.Hash,
@@ -487,6 +494,14 @@ func (r *Layer2Relayer) ProcessCommittedBatches() {
 				// the client does not see the 1st tx's updates at this point.
 				// TODO: add more fine-grained error handling
 				log.Error(
+					"finalizeBatchWithProof in layer1 failed",
+					"index", batch.Index,
+					"hash", batch.Hash,
+					"RollupContractAddress", r.cfg.RollupContractAddress,
+					"err", err,
+				)
+
+				log.Debug(
 					"finalizeBatchWithProof in layer1 failed",
 					"index", batch.Index,
 					"hash", batch.Hash,

bridge/internal/controller/sender/estimategas.go

@@ -7,15 +7,19 @@ import (
 	"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/log"
 )
 
 func (s *Sender) estimateLegacyGas(auth *bind.TransactOpts, contract *common.Address, value *big.Int, input []byte, minGasLimit 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, minGasLimit)
 	if err != nil {
+		log.Error("estimateLegacyGas estimateGasLimit failure", "gasPrice", gasPrice, "error", err)
 		return nil, err
 	}
 	return &FeeData{
@@ -27,6 +31,7 @@ func (s *Sender) estimateLegacyGas(auth *bind.TransactOpts, contract *common.Add
 func (s *Sender) estimateDynamicGas(auth *bind.TransactOpts, contract *common.Address, value *big.Int, input []byte, minGasLimit uint64) (*FeeData, error) {
 	gasTipCap, err := s.client.SuggestGasTipCap(s.ctx)
 	if err != nil {
+		log.Error("estimateDynamicGas SuggestGasTipCap failure", "error", err)
 		return nil, err
 	}
 
@@ -40,6 +45,7 @@ func (s *Sender) estimateDynamicGas(auth *bind.TransactOpts, contract *common.Ad
 	)
 	gasLimit, err := s.estimateGasLimit(auth, contract, input, nil, gasTipCap, gasFeeCap, value, minGasLimit)
 	if err != nil {
+		log.Error("estimateDynamicGas estimateGasLimit failure", "error", err)
 		return nil, err
 	}
 	return &FeeData{
@@ -61,13 +67,14 @@ func (s *Sender) estimateGasLimit(opts *bind.TransactOpts, contract *common.Addr
 	}
 	gasLimit, err := s.client.EstimateGas(s.ctx, msg)
 	if err != nil {
+		log.Error("estimateGasLimit EstimateGas failure", "msg", msg, "error", err)
 		return 0, err
 	}
 	if minGasLimit > gasLimit {
 		gasLimit = minGasLimit
 	}
 
-	gasLimit = gasLimit * 15 / 10 // 50% extra gas to void out of gas error
+	gasLimit = gasLimit * 15 / 10 // 50% extra gas to avoid out of gas error
 
 	return gasLimit, nil
 }

bridge/internal/controller/sender/sender.go

@@ -56,6 +56,15 @@ type FeeData struct {
 	gasLimit uint64
 }
 
+func newEmptyFeeData() *FeeData {
+	return &FeeData{
+		gasFeeCap: big.NewInt(0),
+		gasTipCap: big.NewInt(0),
+		gasPrice:  big.NewInt(0),
+		gasLimit:  0,
+	}
+}
+
 // PendingTransaction submitted but pending transactions
 type PendingTransaction struct {
 	submitAt uint64
@@ -85,6 +94,8 @@ type Sender struct {
 	stopCh chan struct{}
 
 	metrics *senderMetrics
+
+	cachedMaxFeeData *FeeData // hacky way to avoid getFeeData error
 }
 
 // NewSender returns a new instance of transaction sender
@@ -129,19 +140,20 @@ func NewSender(ctx context.Context, config *config.SenderConfig, priv *ecdsa.Pri
 	}
 
 	sender := &Sender{
-		ctx:           ctx,
-		config:        config,
-		client:        client,
-		chainID:       chainID,
-		auth:          auth,
-		minBalance:    config.MinBalance,
-		confirmCh:     make(chan *Confirmation, 128),
-		blockNumber:   header.Number.Uint64(),
-		baseFeePerGas: baseFeePerGas,
-		pendingTxs:    cmapV2.New[*PendingTransaction](),
-		stopCh:        make(chan struct{}),
-		name:          name,
-		service:       service,
+		ctx:              ctx,
+		config:           config,
+		client:           client,
+		chainID:          chainID,
+		auth:             auth,
+		minBalance:       config.MinBalance,
+		confirmCh:        make(chan *Confirmation, 128),
+		blockNumber:      header.Number.Uint64(),
+		baseFeePerGas:    baseFeePerGas,
+		pendingTxs:       cmapV2.New[*PendingTransaction](),
+		stopCh:           make(chan struct{}),
+		name:             name,
+		service:          service,
+		cachedMaxFeeData: newEmptyFeeData(),
 	}
 	sender.metrics = initSenderMetrics(reg)
 
@@ -189,6 +201,26 @@ func (s *Sender) getFeeData(auth *bind.TransactOpts, target *common.Address, val
 	return s.estimateLegacyGas(auth, target, value, data, minGasLimit)
 }
 
+func (s *Sender) cacheMaxFeeData(feeData *FeeData) {
+	if feeData == nil {
+		log.Error("cacheMaxFeeData", "err", "feeData must not be nil")
+		return
+	}
+
+	if feeData.gasFeeCap != nil && feeData.gasFeeCap.Cmp(s.cachedMaxFeeData.gasFeeCap) > 0 {
+		s.cachedMaxFeeData.gasFeeCap = feeData.gasFeeCap
+	}
+	if feeData.gasTipCap != nil && feeData.gasTipCap.Cmp(s.cachedMaxFeeData.gasTipCap) > 0 {
+		s.cachedMaxFeeData.gasTipCap = feeData.gasTipCap
+	}
+	if feeData.gasPrice != nil && feeData.gasPrice.Cmp(s.cachedMaxFeeData.gasPrice) > 0 {
+		s.cachedMaxFeeData.gasPrice = feeData.gasPrice
+	}
+	if feeData.gasLimit > s.cachedMaxFeeData.gasLimit {
+		s.cachedMaxFeeData.gasLimit = feeData.gasLimit
+	}
+}
+
 // SendTransaction send a signed L2tL1 transaction.
 func (s *Sender) SendTransaction(ID string, target *common.Address, value *big.Int, data []byte, minGasLimit uint64) (common.Hash, error) {
 	s.metrics.sendTransactionTotal.WithLabelValues(s.service, s.name).Inc()
@@ -217,7 +249,13 @@ func (s *Sender) SendTransaction(ID string, target *common.Address, value *big.I
 
 	if feeData, err = s.getFeeData(s.auth, target, value, data, minGasLimit); err != nil {
 		s.metrics.sendTransactionFailureGetFee.WithLabelValues(s.service, s.name).Inc()
-		return common.Hash{}, fmt.Errorf("failed to get fee data, err: %w", err)
+		log.Error("failed to get fee data", "err", err)
+		if s.cachedMaxFeeData.gasLimit == 0 { // if no MaxFeeData cached, and getFeeData fails
+			return common.Hash{}, fmt.Errorf("failed to get fee data for the first time, err: %w", err)
+		}
+		feeData = s.cachedMaxFeeData
+	} else {
+		s.cacheMaxFeeData(feeData)
 	}
 
 	if tx, err = s.createAndSendTx(s.auth, feeData, target, value, data, nil); err != nil {
@@ -410,6 +448,7 @@ func (s *Sender) resubmitTransaction(feeData *FeeData, auth *bind.TransactOpts,
 	}
 
 	log.Debug("Transaction gas adjustment details", txInfo)
+	s.cacheMaxFeeData(feeData)
 
 	nonce := tx.Nonce()
 	s.metrics.resubmitTransactionTotal.WithLabelValues(s.service, s.name).Inc()

common/version/version.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 )
 
-var tag = "v4.2.8"
+var tag = "v4.2.13"
 
 var commit = func() string {
 	if info, ok := debug.ReadBuildInfo(); ok {

contracts/docs/apis/L2ScrollMessenger.md

@@ -104,45 +104,6 @@ Mapping from L2 message hash to sent status.
 |---|---|---|
 | _0 | bool | undefined |
 
-### l1MessageFailedTimes
-
-```solidity
-function l1MessageFailedTimes(bytes32) external view returns (uint256)
-```
-
-Mapping from L1 message hash to the number of failure times.
-
-
-
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | bytes32 | undefined |
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | uint256 | undefined |
-
-### maxFailedExecutionTimes
-
-```solidity
-function maxFailedExecutionTimes() external view returns (uint256)
-```
-
-The maximum number of times each L1 message can fail on L2.
-
-
-
-
-#### Returns
-
-| Name | Type | Description |
-|---|---|---|
-| _0 | uint256 | undefined |
-
 ### messageQueue
 
 ```solidity
@@ -329,22 +290,6 @@ Update fee vault contract.
 |---|---|---|
 | _newFeeVault | address | The address of new fee vault contract. |
 
-### updateMaxFailedExecutionTimes
-
-```solidity
-function updateMaxFailedExecutionTimes(uint256 _newMaxFailedExecutionTimes) external nonpayable
-```
-
-Update max failed execution times.
-
-*This function can only called by contract owner.*
-
-#### Parameters
-
-| Name | Type | Description |
-|---|---|---|
-| _newMaxFailedExecutionTimes | uint256 | The new max failed execution times. |
-
 ### updateRateLimiter
 
 ```solidity

contracts/src/L1/rollup/ScrollChain.sol

@@ -36,10 +36,10 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
     /// @param newVerifier The address of new rollup verifier.
     event UpdateVerifier(address indexed oldVerifier, address indexed newVerifier);
 
-    /// @notice Emitted when the value of `maxNumL2TxInChunk` is updated.
-    /// @param oldMaxNumL2TxInChunk The old value of `maxNumL2TxInChunk`.
-    /// @param newMaxNumL2TxInChunk The new value of `maxNumL2TxInChunk`.
-    event UpdateMaxNumL2TxInChunk(uint256 oldMaxNumL2TxInChunk, uint256 newMaxNumL2TxInChunk);
+    /// @notice Emitted when the value of `maxNumTxInChunk` is updated.
+    /// @param oldMaxNumTxInChunk The old value of `maxNumTxInChunk`.
+    /// @param newMaxNumTxInChunk The new value of `maxNumTxInChunk`.
+    event UpdateMaxNumTxInChunk(uint256 oldMaxNumTxInChunk, uint256 newMaxNumTxInChunk);
 
     /*************
      * Constants *
@@ -53,7 +53,7 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
      *************/
 
     /// @notice The maximum number of transactions allowed in each chunk.
-    uint256 public maxNumL2TxInChunk;
+    uint256 public maxNumTxInChunk;
 
     /// @notice The address of L1MessageQueue.
     address public messageQueue;
@@ -107,16 +107,16 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
     function initialize(
         address _messageQueue,
         address _verifier,
-        uint256 _maxNumL2TxInChunk
+        uint256 _maxNumTxInChunk
     ) public initializer {
         OwnableUpgradeable.__Ownable_init();
 
         messageQueue = _messageQueue;
         verifier = _verifier;
-        maxNumL2TxInChunk = _maxNumL2TxInChunk;
+        maxNumTxInChunk = _maxNumTxInChunk;
 
         emit UpdateVerifier(address(0), _verifier);
-        emit UpdateMaxNumL2TxInChunk(0, _maxNumL2TxInChunk);
+        emit UpdateMaxNumTxInChunk(0, _maxNumTxInChunk);
     }
 
     /*************************
@@ -398,13 +398,13 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
         emit UpdateVerifier(_oldVerifier, _newVerifier);
     }
 
-    /// @notice Update the value of `maxNumL2TxInChunk`.
-    /// @param _maxNumL2TxInChunk The new value of `maxNumL2TxInChunk`.
-    function updateMaxNumL2TxInChunk(uint256 _maxNumL2TxInChunk) external onlyOwner {
-        uint256 _oldMaxNumL2TxInChunk = maxNumL2TxInChunk;
-        maxNumL2TxInChunk = _maxNumL2TxInChunk;
+    /// @notice Update the value of `maxNumTxInChunk`.
+    /// @param _maxNumTxInChunk The new value of `maxNumTxInChunk`.
+    function updateMaxNumTxInChunk(uint256 _maxNumTxInChunk) external onlyOwner {
+        uint256 _oldMaxNumTxInChunk = maxNumTxInChunk;
+        maxNumTxInChunk = _maxNumTxInChunk;
 
-        emit UpdateMaxNumL2TxInChunk(_oldMaxNumL2TxInChunk, _maxNumL2TxInChunk);
+        emit UpdateMaxNumTxInChunk(_oldMaxNumTxInChunk, _maxNumTxInChunk);
     }
 
     /// @notice Pause the contract
@@ -462,19 +462,26 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
 
         uint256 _numBlocks = ChunkCodec.validateChunkLength(chunkPtr, _chunk.length);
 
-        // concatenate block contexts
-        uint256 _totalTransactionsInChunk;
-        for (uint256 i = 0; i < _numBlocks; i++) {
-            dataPtr = ChunkCodec.copyBlockContext(chunkPtr, dataPtr, i);
-            uint256 _numTransactionsInBlock = ChunkCodec.numTransactions(blockPtr);
-            unchecked {
-                _totalTransactionsInChunk += _numTransactionsInBlock;
-                blockPtr += ChunkCodec.BLOCK_CONTEXT_LENGTH;
+        // concatenate block contexts, use scope to avoid stack too deep
+        {
+            uint256 _totalTransactionsInChunk;
+            for (uint256 i = 0; i < _numBlocks; i++) {
+                dataPtr = ChunkCodec.copyBlockContext(chunkPtr, dataPtr, i);
+                uint256 _numTransactionsInBlock = ChunkCodec.numTransactions(blockPtr);
+                unchecked {
+                    _totalTransactionsInChunk += _numTransactionsInBlock;
+                    blockPtr += ChunkCodec.BLOCK_CONTEXT_LENGTH;
+                }
+            }
+            assembly {
+                mstore(0x40, add(dataPtr, mul(_totalTransactionsInChunk, 0x20))) // reserve memory for tx hashes
             }
         }
 
+        // It is used to compute the actual number of transactions in chunk.
+        uint256 txHashStartDataPtr;
         assembly {
-            mstore(0x40, add(dataPtr, mul(_totalTransactionsInChunk, 0x20))) // reserve memory for tx hashes
+            txHashStartDataPtr := dataPtr
             blockPtr := add(chunkPtr, 1) // reset block ptr
         }
 
@@ -513,11 +520,8 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
             }
         }
 
-        // check the number of L2 transactions in the chunk
-        require(
-            _totalTransactionsInChunk - _totalNumL1MessagesInChunk <= maxNumL2TxInChunk,
-            "too many L2 txs in one chunk"
-        );
+        // check the actual number of transactions in the chunk
+        require((dataPtr - txHashStartDataPtr) / 32 <= maxNumTxInChunk, "too many txs in one chunk");
 
         // check chunk has correct length
         require(l2TxPtr - chunkPtr == _chunk.length, "incomplete l2 transaction data");
@@ -550,9 +554,10 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
 
         unchecked {
             uint256 _bitmap;
+            uint256 rem;
             for (uint256 i = 0; i < _numL1Messages; i++) {
                 uint256 quo = _totalL1MessagesPoppedInBatch >> 8;
-                uint256 rem = _totalL1MessagesPoppedInBatch & 0xff;
+                rem = _totalL1MessagesPoppedInBatch & 0xff;
 
                 // load bitmap every 256 bits
                 if (i == 0 || rem == 0) {
@@ -574,7 +579,7 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
             }
 
             // check last L1 message is not skipped, _totalL1MessagesPoppedInBatch must > 0
-            uint256 rem = (_totalL1MessagesPoppedInBatch - 1) & 0xff;
+            rem = (_totalL1MessagesPoppedInBatch - 1) & 0xff;
             require(((_bitmap >> rem) & 1) == 0, "cannot skip last L1 message");
         }
 

contracts/src/test/ScrollChain.t.sol

@@ -19,7 +19,7 @@ contract ScrollChainTest is DSTestPlus {
     event UpdateSequencer(address indexed account, bool status);
     event UpdateProver(address indexed account, bool status);
     event UpdateVerifier(address indexed oldVerifier, address indexed newVerifier);
-    event UpdateMaxNumL2TxInChunk(uint256 oldMaxNumL2TxInChunk, uint256 newMaxNumL2TxInChunk);
+    event UpdateMaxNumTxInChunk(uint256 oldMaxNumTxInChunk, uint256 newMaxNumTxInChunk);
 
     event CommitBatch(uint256 indexed batchIndex, bytes32 indexed batchHash);
     event FinalizeBatch(uint256 indexed batchIndex, bytes32 indexed batchHash, bytes32 stateRoot, bytes32 withdrawRoot);
@@ -429,6 +429,15 @@ contract ScrollChainTest is DSTestPlus {
             mstore(add(bitmap, add(0x20, 32)), 42) // bitmap1
         }
 
+        // too many txs in one chunk, revert
+        rollup.updateMaxNumTxInChunk(2); // 3 - 1
+        hevm.expectRevert("too many txs in one chunk");
+        rollup.commitBatch(0, batchHeader1, chunks, bitmap); // first chunk with too many txs
+        rollup.updateMaxNumTxInChunk(185); // 5+10+300 - 2 - 127
+        hevm.expectRevert("too many txs in one chunk");
+        rollup.commitBatch(0, batchHeader1, chunks, bitmap); // second chunk with too many txs
+
+        rollup.updateMaxNumTxInChunk(186);
         hevm.expectEmit(true, true, false, true);
         emit CommitBatch(2, bytes32(0x03a9cdcb9d582251acf60937db006ec99f3505fd4751b7c1f92c9a8ef413e873));
         rollup.commitBatch(0, batchHeader1, chunks, bitmap);
@@ -631,20 +640,20 @@ contract ScrollChainTest is DSTestPlus {
         assertEq(rollup.verifier(), _newVerifier);
     }
 
-    function testUpdateMaxNumL2TxInChunk(uint256 _maxNumL2TxInChunk) public {
+    function testUpdateMaxNumTxInChunk(uint256 _maxNumTxInChunk) public {
         // set by non-owner, should revert
         hevm.startPrank(address(1));
         hevm.expectRevert("Ownable: caller is not the owner");
-        rollup.updateMaxNumL2TxInChunk(_maxNumL2TxInChunk);
+        rollup.updateMaxNumTxInChunk(_maxNumTxInChunk);
         hevm.stopPrank();
 
         // change to random operator
         hevm.expectEmit(false, false, false, true);
-        emit UpdateMaxNumL2TxInChunk(100, _maxNumL2TxInChunk);
+        emit UpdateMaxNumTxInChunk(100, _maxNumTxInChunk);
 
-        assertEq(rollup.maxNumL2TxInChunk(), 100);
-        rollup.updateMaxNumL2TxInChunk(_maxNumL2TxInChunk);
-        assertEq(rollup.maxNumL2TxInChunk(), _maxNumL2TxInChunk);
+        assertEq(rollup.maxNumTxInChunk(), 100);
+        rollup.updateMaxNumTxInChunk(_maxNumTxInChunk);
+        assertEq(rollup.maxNumTxInChunk(), _maxNumTxInChunk);
     }
 
     function testImportGenesisBlock() public {