feat(protocol)!: improve protocol based on Brecht's internal review

packages/protocol/contracts/L1/TaikoData.sol

@@ -137,7 +137,6 @@ library TaikoData {
         uint64 timestamp; // slot 6 (90 bits)
         uint16 tier;
         uint8 contestations;
-        bytes32[4] __reserved;
     }
 
     /// @dev Struct containing data required for verifying a block.
@@ -151,7 +150,6 @@ library TaikoData {
         uint64 proposedIn; // L1 block number
         uint32 nextTransitionId;
         uint32 verifiedTransitionId;
-        bytes32[7] __reserved;
     }
 
     /// @dev Struct representing an Ethereum deposit.

packages/protocol/contracts/L1/TaikoL1.sol

@@ -43,6 +43,11 @@ contract TaikoL1 is
     TaikoData.State public state;
     uint256[100] private __gap;
 
+    modifier whenProvingNotPaused() {
+        if (state.slotB.provingPaused) revert L1_PROVING_PAUSED();
+        _;
+    }
+
     /// @dev Fallback function to receive Ether from Hooks
     receive() external payable {
         if (!_inNonReentrant()) revert L1_RECEIVE_DISABLED();
@@ -75,17 +80,19 @@ contract TaikoL1 is
         (meta, depositsProcessed) =
             LibProposing.proposeBlock(state, config, AddressResolver(this), params, txList);
 
-        if (!state.slotB.provingPaused && config.maxBlocksToVerifyPerProposal > 0) {
-            LibVerifying.verifyBlocks(
-                state, config, AddressResolver(this), config.maxBlocksToVerifyPerProposal
-            );
-        }
+        _verifyBlocks(config, config.maxBlocksToVerifyPerProposal);
     }
 
     /// @inheritdoc ITaikoL1
-    function proveBlock(uint64 blockId, bytes calldata input) external nonReentrant whenNotPaused {
-        if (state.slotB.provingPaused) revert L1_PROVING_PAUSED();
-
+    function proveBlock(
+        uint64 blockId,
+        bytes calldata input
+    )
+        external
+        nonReentrant
+        whenNotPaused
+        whenProvingNotPaused
+    {
         (
             TaikoData.BlockMetadata memory meta,
             TaikoData.Transition memory tran,
@@ -99,17 +106,12 @@ contract TaikoL1 is
         uint8 maxBlocksToVerify =
             LibProving.proveBlock(state, config, AddressResolver(this), meta, tran, proof);
 
-        if (maxBlocksToVerify > 0) {
-            LibVerifying.verifyBlocks(state, config, AddressResolver(this), maxBlocksToVerify);
-        }
+        _verifyBlocks(config, maxBlocksToVerify);
     }
 
     /// @inheritdoc ITaikoL1
     function verifyBlocks(uint64 maxBlocksToVerify) external nonReentrant whenNotPaused {
-        if (maxBlocksToVerify == 0) revert L1_INVALID_PARAM();
-        if (state.slotB.provingPaused) revert L1_PROVING_PAUSED();
-
-        LibVerifying.verifyBlocks(state, getConfig(), AddressResolver(this), maxBlocksToVerify);
+        _verifyBlocks(getConfig(), maxBlocksToVerify);
     }
 
     /// @notice Pause block proving.
@@ -250,6 +252,16 @@ contract TaikoL1 is
         return LibVerifying.isConfigValid(getConfig());
     }
 
+    function _verifyBlocks(
+        TaikoData.Config memory config,
+        uint64 maxBlocksToVerify
+    )
+        internal
+        whenProvingNotPaused
+    {
+        LibVerifying.verifyBlocks(state, config, AddressResolver(this), maxBlocksToVerify);
+    }
+
     function _authorizePause(address)
         internal
         view

packages/protocol/contracts/L1/gov/TaikoGovernor.sol

@@ -44,6 +44,7 @@ contract TaikoGovernor is
     {
         __OwnerUUPSUpgradable_init();
         __Governor_init("TaikoGovernor");
+        __GovernorCompatibilityBravo_init();
         __GovernorVotes_init(_token);
         __GovernorVotesQuorumFraction_init(4);
         __GovernorTimelockControl_init(_timelock);

packages/protocol/contracts/L1/hooks/AssignmentHook.sol

@@ -44,9 +44,10 @@ contract AssignmentHook is EssentialContract, IHook {
     }
 
     // Max gas paying the prover. This should be large enough to prevent the
-    // worst cases, usually block proposer shall be aware the risks and only
-    // choose provers that cannot consume too much gas when receiving Ether.
-    uint256 public constant MAX_GAS_PAYING_PROVER = 200_000;
+    // worst cases for the prover. To assure a trustless relationship between
+    // the proposer and the prover it's the prover's job to make sure it can
+    // get paid within this limit.
+    uint256 public constant MAX_GAS_PAYING_PROVER = 50_000;
 
     event BlockAssigned(
         address indexed assignedProver, TaikoData.BlockMetadata meta, ProverAssignment assignment
@@ -108,26 +109,13 @@ contract AssignmentHook is EssentialContract, IHook {
 
         // The proposer irrevocably pays a fee to the assigned prover, either in
         // Ether or ERC20 tokens.
-        uint256 refund;
+        uint256 totalFeeETH = input.tip;
         if (assignment.feeToken == address(0)) {
-            uint256 totalFee = proverFee + input.tip;
-            if (msg.value < totalFee) {
-                revert HOOK_ASSIGNMENT_INSUFFICIENT_FEE();
-            }
-
-            unchecked {
-                refund = msg.value - totalFee;
-            }
+            totalFeeETH += proverFee;
 
             // Paying Ether
             blk.assignedProver.sendEther(proverFee, MAX_GAS_PAYING_PROVER);
         } else {
-            if (msg.value < input.tip) {
-                revert HOOK_ASSIGNMENT_INSUFFICIENT_FEE();
-            }
-            unchecked {
-                refund = msg.value - input.tip;
-            }
             // Paying ERC20 tokens
             IERC20(assignment.feeToken).safeTransferFrom(
                 meta.coinbase, blk.assignedProver, proverFee
@@ -139,9 +127,9 @@ contract AssignmentHook is EssentialContract, IHook {
             address(block.coinbase).sendEther(input.tip);
         }
 
-        if (refund != 0) {
-            // Send all remaininger Ether back to TaikoL1 contract
-            taikoL1Address.sendEther(refund);
+        // Send all remaining Ether back to TaikoL1 contract
+        if (address(this).balance > 0) {
+            taikoL1Address.sendEther(address(this).balance);
         }
 
         emit BlockAssigned(blk.assignedProver, meta, assignment);

packages/protocol/contracts/L1/libs/LibProposing.sol

@@ -73,6 +73,7 @@ library LibProposing {
     {
         TaikoData.BlockParams memory params = abi.decode(data, (TaikoData.BlockParams));
 
+        // We need a prover that will submit proofs after the block has been submitted
         if (params.assignedProver == address(0)) {
             revert L1_INVALID_PROVER();
         }
@@ -95,6 +96,8 @@ library LibProposing {
             state.blocks[(b.numBlocks - 1) % config.blockRingBufferSize].metaHash;
 
         // Check if parent block has the right meta hash
+        // This is to allow the proposer to make sure the block builds on the expected latest chain
+        // state
         if (params.parentMetaHash != 0 && parentMetaHash != params.parentMetaHash) {
             revert L1_UNEXPECTED_PARENT();
         }
@@ -114,6 +117,12 @@ library LibProposing {
                 blobHash: 0, // to be initialized below
                 extraData: params.extraData,
                 depositsHash: keccak256(abi.encode(depositsProcessed)),
+                // TODO(Brecht): A bit limiting this is required to be the same address as
+                // msg.sender
+                // And fees will be collected on L2 so if a smart contract is used as the proposer
+                // it requires
+                // the same contract to be deployed on L2 at the same address owned by the same
+                // entity.
                 coinbase: msg.sender,
                 id: b.numBlocks,
                 gasLimit: config.blockMaxGasLimit,
@@ -155,14 +164,11 @@ library LibProposing {
                 }
             }
 
+            // Check that the txList data range is within the max size of a blob
             if (uint256(params.txListByteOffset) + params.txListByteSize > MAX_BYTES_PER_BLOB) {
                 revert L1_TXLIST_OFFSET();
             }
 
-            if (params.txListByteSize == 0 || params.txListByteSize > config.blockMaxTxListBytes) {
-                revert L1_TXLIST_SIZE();
-            }
-
             meta.txListByteOffset = params.txListByteOffset;
             meta.txListByteSize = params.txListByteSize;
         } else {
@@ -172,24 +178,25 @@ library LibProposing {
             // Taiko node software.
             if (!LibAddress.isSenderEOA()) revert L1_PROPOSER_NOT_EOA();
 
-            if (params.txListByteOffset != 0 || params.txListByteSize != 0) {
+            // The txList is the full byte array without any offset
+            if (params.txListByteOffset != 0) {
                 revert L1_INVALID_PARAM();
             }
 
-            // blockMaxTxListBytes is a uint24
-            if (txList.length > config.blockMaxTxListBytes) {
-                revert L1_TXLIST_SIZE();
-            }
-
             meta.blobHash = keccak256(txList);
             meta.txListByteOffset = 0;
             meta.txListByteSize = uint24(txList.length);
         }
 
+        // Check that the tx length is non-zero and within the supported range
+        if (meta.txListByteSize == 0 || meta.txListByteSize > config.blockMaxTxListBytes) {
+            revert L1_TXLIST_SIZE();
+        }
+
         // Following the Merge, the L1 mixHash incorporates the
         // prevrandao value from the beacon chain. Given the possibility
         // of multiple Taiko blocks being proposed within a single
-        // Ethereum block, we must introduce a salt to this random
+        // Ethereum block, we choose to introduce a salt to this random
         // number as the L2 mixHash.
         meta.difficulty = keccak256(abi.encodePacked(block.prevrandao, b.numBlocks, block.number));
 
@@ -198,38 +205,25 @@ library LibProposing {
             uint256(meta.difficulty)
         );
 
-        // Now, it's essential to initialize the block that will be stored
-        // on L1. We should aim to utilize as few storage slots as possible,
-        // alghouth using a ring buffer can minimize storage writes once
-        // the buffer reaches its capacity.
-        TaikoData.Block storage blk = state.blocks[b.numBlocks % config.blockRingBufferSize];
-
-        // Please note that all fields must be re-initialized since we are
-        // utilizing an existing ring buffer slot, not creating a new storage
-        // slot.
-        blk.metaHash = keccak256(abi.encode(meta));
-
-        // Safeguard the liveness bond to ensure its preservation,
-        // particularly in scenarios where it might be altered after the
-        // block's proposal but before it has been proven or verified.
-        blk.livenessBond = config.livenessBond;
-        blk.blockId = b.numBlocks;
-
-        blk.proposedAt = meta.timestamp;
-        blk.proposedIn = uint64(block.number);
-
-        // For a new block, the next transition ID is always 1, not 0.
-        blk.nextTransitionId = 1;
-
-        // For unverified block, its verifiedTransitionId is always 0.
-        blk.verifiedTransitionId = 0;
-
-        // Verify assignment authorization; if prover's address is an IProver
-        // contract, transfer Ether and call "validateAssignment" for
-        // verification.
-        // Prover can charge ERC20/NFT as fees; msg.value can be zero. Taiko
-        // doesn't mandate Ether as the only proofing fee.
-        blk.assignedProver = params.assignedProver;
+        // Create the block that will be stored onchain
+        TaikoData.Block memory blk = TaikoData.Block({
+            metaHash: keccak256(abi.encode(meta)),
+            // Safeguard the liveness bond to ensure its preservation,
+            // particularly in scenarios where it might be altered after the
+            // block's proposal but before it has been proven or verified.
+            livenessBond: config.livenessBond,
+            blockId: b.numBlocks,
+            proposedAt: meta.timestamp,
+            proposedIn: uint64(block.number),
+            // For a new block, the next transition ID is always 1, not 0.
+            nextTransitionId: 1,
+            // For unverified block, its verifiedTransitionId is always 0.
+            verifiedTransitionId: 0,
+            assignedProver: params.assignedProver
+        });
+
+        // Store the block in the ring buffer
+        state.blocks[b.numBlocks % config.blockRingBufferSize] = blk;
 
         // Increment the counter (cursor) by 1.
         unchecked {

packages/protocol/contracts/L1/libs/LibProving.sol

@@ -127,14 +127,15 @@ library LibProving {
         ITierProvider.Tier memory tier =
             ITierProvider(resolver.resolve("tier_provider", false)).getTier(proof.tier);
 
+        // Check if this prover is allowed to submit a proof for this block
         _checkProverPermission(state, blk, ts, tid, tier);
 
         // We must verify the proof, and any failure in proof verification will
         // result in a revert.
         //
         // It's crucial to emphasize that the proof can be assessed in two
         // potential modes: "proving mode" and "contesting mode." However, the
-        // precise verification logic is defined within each tier'IVerifier
+        // precise verification logic is defined within each tier's IVerifier
         // contract implementation. We simply specify to the verifier contract
         // which mode it should utilize - if the new tier is higher than the
         // previous tier, we employ the proving mode; otherwise, we employ the
@@ -145,26 +146,32 @@ library LibProving {
         // Taiko's core protocol.
         {
             address verifier = resolver.resolve(tier.verifierName, true);
-            // The verifier can be address-zero, signifying that there are no
-            // proof checks for the tier. In practice, this only applies to
-            // optimistic proofs.
-            if (verifier == address(0) && tier.verifierName != TIER_OP) {
-                revert L1_MISSING_VERIFIER();
-            }
 
             if (verifier != address(0)) {
                 bool isContesting = proof.tier == ts.tier && tier.contestBond != 0;
 
                 IVerifier.Context memory ctx = IVerifier.Context({
                     metaHash: blk.metaHash,
                     blobHash: meta.blobHash,
+                    // TODO(Brecht): Quite limiting this is required to be the same address as
+                    // msg.sender,
+                    // less flexibility on the prover's side for proof generation/proof submission
+                    // using
+                    // multiple accounts.
+                    // Added msgSender to allow the prover to be any address in the future.
                     prover: msg.sender,
+                    msgSender: msg.sender,
                     blockId: blk.blockId,
                     isContesting: isContesting,
                     blobUsed: meta.blobUsed
                 });
 
                 IVerifier(verifier).verifyProof(ctx, tran, proof);
+            } else if (tier.verifierName != TIER_OP) {
+                // The verifier can be address-zero, signifying that there are no
+                // proof checks for the tier. In practice, this only applies to
+                // optimistic proofs.
+                revert L1_MISSING_VERIFIER();
             }
         }
 
@@ -314,6 +321,8 @@ library LibProving {
                 // In scenarios where this transition is not the first one, we
                 // straightforwardly reset the transition prover to address
                 // zero.
+                // TODO(Brecht): Is it sure that in all cases all the neccessary data is stored
+                // in the transition in this case after this code?
                 ts.prover = address(0);
 
                 // Furthermore, we index the transition for future retrieval.
@@ -322,6 +331,8 @@ library LibProving {
                 // only possess one transition — the correct one — we don't need
                 // to be concerned about the cost in this case.
                 state.transitionIds[blk.blockId][tran.parentHash] = tid;
+
+                // There is no need to initialize ts.key here because it's only used when tid == 1
             }
         } else {
             // A transition with the provided parentHash has been located.
@@ -402,6 +413,8 @@ library LibProving {
         if (tid == 1 && ts.tier == 0 && inProvingWindow) {
             if (!isAssignedPover) revert L1_NOT_ASSIGNED_PROVER();
         } else {
+            // Disallow the same address to prove the block so that we can detect that the
+            // assigned prover should not receive his liveness bond back
             if (isAssignedPover) revert L1_ASSIGNED_PROVER_NOT_ALLOWED();
         }
     }

packages/protocol/contracts/L1/libs/LibUtils.sol

@@ -74,8 +74,8 @@ library LibUtils {
             state.transitions[slot][blk.verifiedTransitionId];
 
         return ICrossChainSync.Snippet({
-            remoteBlockId: blockId,
             syncedInBlock: blk.proposedIn,
+            blockId: blockId,
             blockHash: transition.blockHash,
             stateRoot: transition.stateRoot
         });