From a394abd70575f4472d11bd11ae6767663fb7b324 Mon Sep 17 00:00:00 2001 From: D <51912515+adaki2004@users.noreply.github.com> Date: Sat, 13 Jan 2024 21:19:53 +0530 Subject: [PATCH] feat(protocol): fix issues in AssignmentHook (#15486) Co-authored-by: David Co-authored-by: Daniel Wang Co-authored-by: Daniel Wang <99078276+dantaik@users.noreply.github.com> --- packages/protocol/contracts/L1/ITaikoL1.sol | 4 +++ packages/protocol/contracts/L1/TaikoL1.sol | 5 ++- .../contracts/L1/hooks/AssignmentHook.sol | 35 +++++++++++++------ .../contracts/L1/libs/LibProposing.sol | 6 ++-- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/packages/protocol/contracts/L1/ITaikoL1.sol b/packages/protocol/contracts/L1/ITaikoL1.sol index 46aac217bef..df94d42190e 100644 --- a/packages/protocol/contracts/L1/ITaikoL1.sol +++ b/packages/protocol/contracts/L1/ITaikoL1.sol @@ -43,4 +43,8 @@ interface ITaikoL1 { /// @notice Verifies up to a certain number of blocks. /// @param maxBlocksToVerify Max number of blocks to verify. function verifyBlocks(uint64 maxBlocksToVerify) external; + + /// @notice Gets the configuration of the TaikoL1 contract. + /// @return Config struct containing configuration parameters. + function getConfig() external view returns (TaikoData.Config memory); } diff --git a/packages/protocol/contracts/L1/TaikoL1.sol b/packages/protocol/contracts/L1/TaikoL1.sol index 948eb385547..2efee8cd8a4 100644 --- a/packages/protocol/contracts/L1/TaikoL1.sol +++ b/packages/protocol/contracts/L1/TaikoL1.sol @@ -203,9 +203,8 @@ contract TaikoL1 is return ITierProvider(resolve("tier_provider", false)).getMinTier(rand); } - /// @notice Gets the configuration of the TaikoL1 contract. - /// @return Config struct containing configuration parameters. - function getConfig() public view virtual returns (TaikoData.Config memory) { + /// @inheritdoc ITaikoL1 + function getConfig() public view virtual override returns (TaikoData.Config memory) { // All hard-coded configurations: // - treasury: the actual TaikoL2 address. // - blockMaxTxs: 150 (limited by the PSE zkEVM circuits) diff --git a/packages/protocol/contracts/L1/hooks/AssignmentHook.sol b/packages/protocol/contracts/L1/hooks/AssignmentHook.sol index 0c2637ddb4d..6a03d59c3f4 100644 --- a/packages/protocol/contracts/L1/hooks/AssignmentHook.sol +++ b/packages/protocol/contracts/L1/hooks/AssignmentHook.sol @@ -18,7 +18,7 @@ import "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; import "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; import "../../common/EssentialContract.sol"; import "../../libs/LibAddress.sol"; -import "../TaikoData.sol"; +import "../ITaikoL1.sol"; import "./IHook.sol"; /// @title AssignmentHook @@ -39,7 +39,7 @@ contract AssignmentHook is EssentialContract, IHook { struct Input { ProverAssignment assignment; - uint256 tip; + uint256 tip; // A tip to L1 block builder } // Max gas paying the prover. This should be large enough to prevent the @@ -70,6 +70,11 @@ contract AssignmentHook is EssentialContract, IHook { nonReentrant onlyFromNamed("taiko") { + // Note that + // - 'msg.sender' is the TaikoL1 contract address + // - 'block.coinbase' is the L1 block builder + // - 'meta.coinbase' is the L2 block proposer + Input memory input = abi.decode(data, (Input)); ProverAssignment memory assignment = input.assignment; @@ -85,7 +90,8 @@ contract AssignmentHook is EssentialContract, IHook { // Hash the assignment with the blobHash, this hash will be signed by // the prover, therefore, we add a string as a prefix. - bytes32 hash = hashAssignment(assignment, msg.sender, meta.blobHash); + address taikoL1Address = msg.sender; + bytes32 hash = hashAssignment(assignment, taikoL1Address, meta.blobHash); if (!blk.assignedProver.isValidSignature(hash, assignment.signature)) { revert HOOK_ASSIGNMENT_INVALID_SIG(); @@ -93,7 +99,7 @@ contract AssignmentHook is EssentialContract, IHook { // Send the liveness bond to the Taiko contract IERC20 tko = IERC20(resolve("taiko_token", false)); - tko.transferFrom(blk.assignedProver, msg.sender, blk.livenessBond); + tko.transferFrom(blk.assignedProver, taikoL1Address, blk.livenessBond); // Find the prover fee using the minimal tier uint256 proverFee = _getProverFee(assignment.tierFees, meta.minTier); @@ -102,12 +108,13 @@ contract AssignmentHook is EssentialContract, IHook { // Ether or ERC20 tokens. uint256 refund; if (assignment.feeToken == address(0)) { - if (msg.value < proverFee + input.tip) { + uint256 totalFee = proverFee + input.tip; + if (msg.value < totalFee) { revert HOOK_ASSIGNMENT_INSUFFICIENT_FEE(); } unchecked { - refund = msg.value - proverFee - input.tip; + refund = msg.value - totalFee; } // Paying Ether @@ -120,7 +127,9 @@ contract AssignmentHook is EssentialContract, IHook { refund = msg.value - input.tip; } // Paying ERC20 tokens - IERC20(assignment.feeToken).safeTransferFrom(msg.sender, blk.assignedProver, proverFee); + IERC20(assignment.feeToken).safeTransferFrom( + meta.coinbase, blk.assignedProver, proverFee + ); } // block.coinbase can be address(0) in tests @@ -129,7 +138,8 @@ contract AssignmentHook is EssentialContract, IHook { } if (refund != 0) { - msg.sender.sendEther(refund); + // Send all remaininger Ether back to TaikoL1 contract + taikoL1Address.sendEther(refund); } emit BlockAssigned(blk.assignedProver, meta, assignment); @@ -137,17 +147,20 @@ contract AssignmentHook is EssentialContract, IHook { function hashAssignment( ProverAssignment memory assignment, - address taikoAddress, + address taikoL1Address, bytes32 blobHash ) public - pure + view returns (bytes32) { return keccak256( abi.encode( "PROVER_ASSIGNMENT", - taikoAddress, + ITaikoL1(taikoL1Address).getConfig().chainId, + taikoL1Address, + address(this), + assignment.metaHash, blobHash, assignment.feeToken, assignment.expiry, diff --git a/packages/protocol/contracts/L1/libs/LibProposing.sol b/packages/protocol/contracts/L1/libs/LibProposing.sol index ada8cefa5a9..92f2cb6bc2e 100644 --- a/packages/protocol/contracts/L1/libs/LibProposing.sol +++ b/packages/protocol/contracts/L1/libs/LibProposing.sol @@ -261,8 +261,10 @@ library LibProposing { } // Check that after hooks, the Taiko Token balance of this contract - // have increased by at least config.livenessBond - if (tko.balanceOf(address(this)) < tkoBalance + config.livenessBond) { + // have increased by the same amount as config.livenessBond (to prevent) + // multiple draining payments by a malicious proposer nesting the same + // hook. + if (tko.balanceOf(address(this)) != tkoBalance + config.livenessBond) { revert L1_LIVENESS_BOND_NOT_RECEIVED(); } }