diff --git a/.github/workflows/protocol.yml b/.github/workflows/protocol.yml index 7add028e744..d02e089ef69 100644 --- a/.github/workflows/protocol.yml +++ b/.github/workflows/protocol.yml @@ -49,7 +49,7 @@ jobs: - name: protocol - Deploy L1 Contracts working-directory: ./packages/protocol run: | - anvil & + anvil --hardfork cancun & while ! nc -z localhost 8545; do sleep 1 done diff --git a/packages/protocol/contracts/L1/TaikoData.sol b/packages/protocol/contracts/L1/TaikoData.sol index 9d7c89b9c7a..05fd2257701 100644 --- a/packages/protocol/contracts/L1/TaikoData.sol +++ b/packages/protocol/contracts/L1/TaikoData.sol @@ -138,7 +138,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. @@ -152,7 +151,6 @@ library TaikoData { uint64 proposedIn; // L1 block number uint32 nextTransitionId; uint32 verifiedTransitionId; - bytes32[7] __reserved; } /// @dev Struct representing an Ethereum deposit. diff --git a/packages/protocol/contracts/L1/TaikoL1.sol b/packages/protocol/contracts/L1/TaikoL1.sol index 694de59b79a..610e178aa2e 100644 --- a/packages/protocol/contracts/L1/TaikoL1.sol +++ b/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 diff --git a/packages/protocol/contracts/L1/gov/TaikoGovernor.sol b/packages/protocol/contracts/L1/gov/TaikoGovernor.sol index 0f0b8549e09..8ff0db5c792 100644 --- a/packages/protocol/contracts/L1/gov/TaikoGovernor.sol +++ b/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); diff --git a/packages/protocol/contracts/L1/hooks/AssignmentHook.sol b/packages/protocol/contracts/L1/hooks/AssignmentHook.sol index 91e2c481b67..bfb86a5559c 100644 --- a/packages/protocol/contracts/L1/hooks/AssignmentHook.sol +++ b/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); diff --git a/packages/protocol/contracts/L1/libs/LibProposing.sol b/packages/protocol/contracts/L1/libs/LibProposing.sol index c71755b8811..40b1b8851a7 100644 --- a/packages/protocol/contracts/L1/libs/LibProposing.sol +++ b/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(); } @@ -99,6 +100,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(); } @@ -159,14 +162,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 { @@ -176,24 +176,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)); @@ -202,38 +203,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 { diff --git a/packages/protocol/contracts/L1/libs/LibProving.sol b/packages/protocol/contracts/L1/libs/LibProving.sol index 3f0e512e5b0..733781325a3 100644 --- a/packages/protocol/contracts/L1/libs/LibProving.sol +++ b/packages/protocol/contracts/L1/libs/LibProving.sol @@ -127,6 +127,7 @@ 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 @@ -134,7 +135,7 @@ library LibProving { // // 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,12 +146,6 @@ 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; @@ -158,13 +153,23 @@ library LibProving { 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 +319,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 +329,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 +411,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(); } } diff --git a/packages/protocol/contracts/L1/libs/LibUtils.sol b/packages/protocol/contracts/L1/libs/LibUtils.sol index 8824f4aa5b3..8568da953e6 100644 --- a/packages/protocol/contracts/L1/libs/LibUtils.sol +++ b/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 }); diff --git a/packages/protocol/contracts/L1/libs/LibVerifying.sol b/packages/protocol/contracts/L1/libs/LibVerifying.sol index 5b95e4d3d08..e32d0cb9b66 100644 --- a/packages/protocol/contracts/L1/libs/LibVerifying.sol +++ b/packages/protocol/contracts/L1/libs/LibVerifying.sol @@ -116,6 +116,10 @@ library LibVerifying { ) internal { + if (maxBlocksToVerify == 0) { + return; + } + // Retrieve the latest verified block and the associated transition used // for its verification. TaikoData.SlotB memory b = state.slotB; @@ -136,17 +140,17 @@ library LibVerifying { // blockHash on L2. bytes32 blockHash = state.transitions[slot][tid].blockHash; bytes32 stateRoot; - uint64 processed; + uint64 numBlocksVerified; address tierProvider; // Unchecked is safe: // - assignment is within ranges - // - blockId and processed values incremented will still be OK in the + // - blockId and numBlocksVerified values incremented will still be OK in the // next 584K years if we verifying one block per every second unchecked { ++blockId; - while (blockId < b.numBlocks && processed < maxBlocksToVerify) { + while (blockId < b.numBlocks && numBlocksVerified < maxBlocksToVerify) { slot = blockId % config.blockRingBufferSize; blk = state.blocks[slot]; @@ -228,11 +232,11 @@ library LibVerifying { }); ++blockId; - ++processed; + ++numBlocksVerified; } - if (processed > 0) { - uint64 lastVerifiedBlockId = b.lastVerifiedBlockId + processed; + if (numBlocksVerified > 0) { + uint64 lastVerifiedBlockId = b.lastVerifiedBlockId + numBlocksVerified; // Update protocol level state variables state.slotB.lastVerifiedBlockId = lastVerifiedBlockId; diff --git a/packages/protocol/contracts/L1/provers/GuardianProver.sol b/packages/protocol/contracts/L1/provers/GuardianProver.sol index 301dd38e82e..4d5e47b0fe9 100644 --- a/packages/protocol/contracts/L1/provers/GuardianProver.sol +++ b/packages/protocol/contracts/L1/provers/GuardianProver.sol @@ -20,8 +20,6 @@ import "./Guardians.sol"; /// @title GuardianProver contract GuardianProver is Guardians { - error PROVING_FAILED(); - /// @notice Initializes the contract with the provided address manager. /// @param _addressManager The address of the address manager contract. function init(address _addressManager) external initializer { @@ -45,10 +43,7 @@ contract GuardianProver is Guardians { if (approved) { deleteApproval(hash); - bytes memory data = - abi.encodeCall(ITaikoL1.proveBlock, (meta.id, abi.encode(meta, tran, proof))); - (bool success,) = resolve("taiko", false).call(data); - if (!success) revert PROVING_FAILED(); + ITaikoL1(resolve("taiko", false)).proveBlock(meta.id, abi.encode(meta, tran, proof)); } } } diff --git a/packages/protocol/contracts/L1/provers/Guardians.sol b/packages/protocol/contracts/L1/provers/Guardians.sol index beb553aea4d..1476ae04b00 100644 --- a/packages/protocol/contracts/L1/provers/Guardians.sol +++ b/packages/protocol/contracts/L1/provers/Guardians.sol @@ -21,6 +21,7 @@ import "../TaikoData.sol"; abstract contract Guardians is EssentialContract { uint256 public constant MIN_NUM_GUARDIANS = 5; + // Contains the index of the guardian in `guardians` plus one (zero means not a guardian) mapping(address guardian => uint256 id) public guardianIds; // slot 1 mapping(uint32 version => mapping(bytes32 => uint256 approvalBits)) internal _approvals; address[] public guardians; // slot 3 @@ -48,34 +49,41 @@ abstract contract Guardians is EssentialContract { onlyOwner nonReentrant { + // We need at least MIN_NUM_GUARDIANS and at most 255 guardians (so the approval bits fit in + // a uint256) if (_newGuardians.length < MIN_NUM_GUARDIANS || _newGuardians.length > type(uint8).max) { revert INVALID_GUARDIAN_SET(); } - if (_minGuardians < _newGuardians.length >> 1 || _minGuardians > _newGuardians.length) { + // Minimum number of guardians to approve is at least equal or greater than half the + // guardians (rounded up) and less or equal than the total number of guardians + if (_minGuardians < (_newGuardians.length + 1) >> 1 || _minGuardians > _newGuardians.length) + { revert INVALID_MIN_GUARDIANS(); } - // Delete current guardians data - uint256 guardiansLength = guardians.length; - for (uint256 i; i < guardiansLength; ++i) { + // Delete the current guardians + for (uint256 i; i < guardians.length; ++i) { delete guardianIds[guardians[i]]; } - assembly { - sstore(guardians.slot, 0) - } + delete guardians; - for (uint256 i = 0; i < _newGuardians.length;) { + // Set the new guardians + for (uint256 i = 0; i < _newGuardians.length; ++i) { address guardian = _newGuardians[i]; if (guardian == address(0)) revert INVALID_GUARDIAN(); + // This makes sure there are not duplicate addresses if (guardianIds[guardian] != 0) revert INVALID_GUARDIAN_SET(); // Save and index the guardian guardians.push(guardian); - guardianIds[guardian] = ++i; + guardianIds[guardian] = guardians.length; } + // Bump the version so previous approvals get invalidated + ++version; + minGuardians = _minGuardians; - emit GuardiansUpdated(++version, _newGuardians); + emit GuardiansUpdated(version, _newGuardians); } function isApproved(bytes32 hash) public view returns (bool) { diff --git a/packages/protocol/contracts/L2/CrossChainOwned.sol b/packages/protocol/contracts/L2/CrossChainOwned.sol index 19eba080b2f..af943249c31 100644 --- a/packages/protocol/contracts/L2/CrossChainOwned.sol +++ b/packages/protocol/contracts/L2/CrossChainOwned.sol @@ -22,8 +22,8 @@ import "../bridge/IBridge.sol"; /// @title CrossChainOwned /// @notice This contract's owner can be a local address or one that lives on another chain and uses /// signals for transaction approval. -/// @dev Notice that when send the message on the owner chain, the gas limit of the message must not -/// be zero, so on this chain, some EOA can help execute this transaction. +/// @dev Notice that when sending the message on the owner chain, the gas limit of the message must +/// not be zero, so on this chain, some EOA can help execute this transaction. abstract contract CrossChainOwned is EssentialContract { uint64 public ownerChainId; // slot 1 uint64 public nextTxId; diff --git a/packages/protocol/contracts/L2/TaikoL2.sol b/packages/protocol/contracts/L2/TaikoL2.sol index 2d620b7e66d..918e900a789 100644 --- a/packages/protocol/contracts/L2/TaikoL2.sol +++ b/packages/protocol/contracts/L2/TaikoL2.sol @@ -150,8 +150,8 @@ contract TaikoL2 is CrossChainOwned, ICrossChainSync { // Update state variables l2Hashes[parentId] = blockhash(parentId); snippets[l1Height] = ICrossChainSync.Snippet({ - remoteBlockId: l1Height, syncedInBlock: uint64(block.number), + blockId: l1Height, blockHash: l1BlockHash, stateRoot: l1StateRoot }); @@ -271,7 +271,10 @@ contract TaikoL2 is CrossChainOwned, ICrossChainSync { // Calculate how much more gas to issue to offset gas excess. // after each L1 block time, config.gasTarget more gas is issued, // the gas excess will be reduced accordingly. - // Note that when latestSyncedL1Height is zero, we skip this step. + // Note that when latestSyncedL1Height is zero, we skip this step + // because that means this is the first time calculating the basefee + // and the difference between the L1 height would be extremely big, + // reverting the initial gas excess value back to 0. uint256 numL1Blocks; if (latestSyncedL1Height > 0 && l1Height > latestSyncedL1Height) { numL1Blocks = l1Height - latestSyncedL1Height; @@ -286,7 +289,7 @@ contract TaikoL2 is CrossChainOwned, ICrossChainSync { // The base fee per gas used by this block is the spot price at the // bonding curve, regardless the actual amount of gas used by this - // block, however, the this block's gas used will affect the next + // block, however, this block's gas used will affect the next // block's base fee. _basefee = Lib1559Math.basefee( _gasExcess, uint256(config.basefeeAdjustmentQuotient) * config.gasTargetPerL1Block diff --git a/packages/protocol/contracts/L2/TaikoL2EIP1559Configurable.sol b/packages/protocol/contracts/L2/TaikoL2EIP1559Configurable.sol index 4302e1e7212..0d9a118dd4f 100644 --- a/packages/protocol/contracts/L2/TaikoL2EIP1559Configurable.sol +++ b/packages/protocol/contracts/L2/TaikoL2EIP1559Configurable.sol @@ -19,7 +19,7 @@ import "./TaikoL2.sol"; /// @title TaikoL2EIP1559Configurable /// @notice Taiko L2 with a setter to change EIP-1559 configurations and states. contract TaikoL2EIP1559Configurable is TaikoL2 { - Config private _config; + Config public customConfig; uint256[49] private __gap; event ConfigAndExcessChanged(Config config, uint64 gasExcess); @@ -27,26 +27,26 @@ contract TaikoL2EIP1559Configurable is TaikoL2 { error L2_INVALID_CONFIG(); /// @notice Sets EIP1559 configuration and gas excess. - /// @param config The new EIP1559 config. + /// @param newConfig The new EIP1559 config. /// @param newGasExcess The new gas excess function setConfigAndExcess( - Config memory config, + Config memory newConfig, uint64 newGasExcess ) external virtual onlyOwner { - if (config.gasTargetPerL1Block == 0) revert L2_INVALID_CONFIG(); - if (config.basefeeAdjustmentQuotient == 0) revert L2_INVALID_CONFIG(); + if (newConfig.gasTargetPerL1Block == 0) revert L2_INVALID_CONFIG(); + if (newConfig.basefeeAdjustmentQuotient == 0) revert L2_INVALID_CONFIG(); - _config = config; + customConfig = newConfig; gasExcess = newGasExcess; - emit ConfigAndExcessChanged(config, newGasExcess); + emit ConfigAndExcessChanged(newConfig, newGasExcess); } function getConfig() public view override returns (Config memory) { - return _config; + return customConfig; } } diff --git a/packages/protocol/contracts/bridge/Bridge.sol b/packages/protocol/contracts/bridge/Bridge.sol index 68600615e42..ead8b460929 100644 --- a/packages/protocol/contracts/bridge/Bridge.sol +++ b/packages/protocol/contracts/bridge/Bridge.sol @@ -46,7 +46,10 @@ contract Bridge is EssentialContract, IBridge { address preferredExecutor; } - uint256 internal constant PLACEHOLDER = type(uint256).max; + // The slot in transient storage of the call context + // This is the keccak256 hash of "bridge.ctx_slot" + bytes32 private constant _CTX_SLOT = + 0xe4ece82196de19aabe639620d7f716c433d1348f96ce727c9989a982dbadc2b9; uint128 public nextMessageId; // slot 1 mapping(bytes32 msgHash => bool recalled) private __isMessageRecalled; // slot 2, deprecated @@ -89,7 +92,6 @@ contract Bridge is EssentialContract, IBridge { /// @param _addressManager The address of the {AddressManager} contract. function init(address _addressManager) external initializer { __Essential_init(_addressManager); - _ctx.msgHash == bytes32(PLACEHOLDER); } /// @notice Suspend or unsuspend invocation for a list of messages. @@ -134,7 +136,7 @@ contract Bridge is EssentialContract, IBridge { whenNotPaused returns (bytes32 msgHash, Message memory _message) { - // Ensure the message user is not null. + // Ensure the message owner is not null. if (message.owner == address(0)) revert B_INVALID_USER(); // Check if the destination chain is enabled. @@ -211,7 +213,7 @@ contract Bridge is EssentialContract, IBridge { // Execute the recall logic based on the contract's support for the // IRecallableSender interface if (message.from.supportsInterface(type(IRecallableSender).interfaceId)) { - _ctx = Context({ + _storeContext({ msgHash: msgHash, from: address(this), srcChainId: message.srcChainId @@ -223,11 +225,7 @@ contract Bridge is EssentialContract, IBridge { ); // Reset the context after the message call - _ctx = Context({ - msgHash: bytes32(PLACEHOLDER), - from: address(uint160(PLACEHOLDER)), - srcChainId: uint64(PLACEHOLDER) - }); + _resetContext(); } else { message.owner.sendEther(message.value); } @@ -256,6 +254,10 @@ contract Bridge is EssentialContract, IBridge { whenNotPaused sameChain(message.destChainId) { + // TODO(Brecht): `message.owner`, but this is the `msg.sender` on the source chain. + // If the address is not owned by the same entity on the destination chain + // (e.g. can be the case for smart wallets/general contracts) this can give unexpected + // results (especially with refunding). bytes32 msgHash = hashMessage(message); if (messageStatus[msgHash] != Status.NEW) revert B_STATUS_MISMATCH(); @@ -280,8 +282,6 @@ contract Bridge is EssentialContract, IBridge { } } - // assert(receivedAt != 0); - if (invocationDelay != 0 && msg.sender != proofReceipt[msgHash].preferredExecutor) { // If msg.sender is not the one that proved the message, then there // is an extra delay. @@ -443,11 +443,11 @@ contract Bridge is EssentialContract, IBridge { /// @notice Gets the current context. /// @inheritdoc IBridge - function context() public view returns (Context memory) { - if (_ctx.msgHash == bytes32(PLACEHOLDER) || _ctx.msgHash == 0) { + function context() public view returns (Context memory ctx) { + ctx = _loadContext(); + if (ctx.msgHash == 0) { revert B_INVALID_CONTEXT(); } - return _ctx; } /// @notice Returns invocation delay values. @@ -516,17 +516,13 @@ contract Bridge is EssentialContract, IBridge { if (gasLimit == 0) revert B_INVALID_GAS_LIMIT(); assert(message.from != address(this)); - _ctx = Context({ msgHash: msgHash, from: message.from, srcChainId: message.srcChainId }); + _storeContext({ msgHash: msgHash, from: message.from, srcChainId: message.srcChainId }); // Perform the message call and capture the success value (success,) = message.to.call{ value: message.value, gas: gasLimit }(message.data); // Reset the context after the message call - _ctx = Context({ - msgHash: bytes32(PLACEHOLDER), - from: address(uint160(PLACEHOLDER)), - srcChainId: uint64(PLACEHOLDER) - }); + _resetContext(); } /// @notice Updates the status of a bridge message. @@ -547,16 +543,43 @@ contract Bridge is EssentialContract, IBridge { } } + /// @notice Resets the call context + function _resetContext() private { + _storeContext(bytes32(0), address(0), uint64(0)); + } + + /// @notice Stores the call context + function _storeContext(bytes32 msgHash, address from, uint64 srcChainId) private { + assembly { + tstore(_CTX_SLOT, msgHash) + tstore(add(_CTX_SLOT, 1), from) + tstore(add(_CTX_SLOT, 2), srcChainId) + } + } + + /// @notice Loads the call context + function _loadContext() private view returns (Context memory) { + bytes32 msgHash; + address from; + uint64 srcChainId; + assembly { + msgHash := tload(_CTX_SLOT) + from := tload(add(_CTX_SLOT, 1)) + srcChainId := tload(add(_CTX_SLOT, 2)) + } + return Context({ msgHash: msgHash, from: from, srcChainId: srcChainId }); + } + /// @notice Checks if the signal was received. /// @param signalService The signalService /// @param signal The signal. - /// @param srcChainId The ID of the source chain. + /// @param chainId The ID of the chain the signal is stored on /// @param proof The merkle inclusion proof. /// @return True if the message was received. function _proveSignalReceived( address signalService, bytes32 signal, - uint64 srcChainId, + uint64 chainId, bytes calldata proof ) private @@ -565,7 +588,7 @@ contract Bridge is EssentialContract, IBridge { { bytes memory data = abi.encodeCall( ISignalService.proveSignalReceived, - (srcChainId, resolve(srcChainId, "bridge", false), signal, proof) + (chainId, resolve(chainId, "bridge", false), signal, proof) ); (bool success, bytes memory ret) = signalService.staticcall(data); return success ? abi.decode(ret, (bool)) : false; diff --git a/packages/protocol/contracts/common/EssentialContract.sol b/packages/protocol/contracts/common/EssentialContract.sol index 0ac00c7e106..f522369da51 100644 --- a/packages/protocol/contracts/common/EssentialContract.sol +++ b/packages/protocol/contracts/common/EssentialContract.sol @@ -35,7 +35,7 @@ abstract contract EssentialContract is OwnerUUPSUpgradable, AddressResolver { __AddressResolver_init(_addressManager); } - /// @notice Initializes the contract with an address manager. + /// @notice Initializes the contract without an address manager. // solhint-disable-next-line func-name-mixedcase function __Essential_init() internal virtual { __Essential_init(address(0)); diff --git a/packages/protocol/contracts/common/ICrossChainSync.sol b/packages/protocol/contracts/common/ICrossChainSync.sol index 97d22e8af9a..357dd923051 100644 --- a/packages/protocol/contracts/common/ICrossChainSync.sol +++ b/packages/protocol/contracts/common/ICrossChainSync.sol @@ -22,8 +22,8 @@ pragma solidity 0.8.24; /// both chains remain consistent and can be cross-referenced with integrity. interface ICrossChainSync { struct Snippet { - uint64 remoteBlockId; uint64 syncedInBlock; + uint64 blockId; bytes32 blockHash; bytes32 stateRoot; } diff --git a/packages/protocol/contracts/common/OwnerUUPSUpgradable.sol b/packages/protocol/contracts/common/OwnerUUPSUpgradable.sol index 50c211c6c95..b6a9065b8f5 100644 --- a/packages/protocol/contracts/common/OwnerUUPSUpgradable.sol +++ b/packages/protocol/contracts/common/OwnerUUPSUpgradable.sol @@ -25,7 +25,12 @@ abstract contract OwnerUUPSUpgradable is UUPSUpgradeable, OwnableUpgradeable { uint8 private constant _FALSE = 1; uint8 private constant _TRUE = 2; - uint8 private _reentry; // slot 1 + // The slot in transient storage of the reentry lock + // This is the keccak256 hash of "ownerUUPS.reentry_slot" + bytes32 private constant _REENTRY_SLOT = + 0xa5054f728453d3dbe953bdc43e4d0cb97e662ea32d7958190f3dc2da31d9721a; + + uint8 private _reentryDeprecated; // slot 1 uint8 private _paused; uint256[49] private __gap; @@ -36,10 +41,10 @@ abstract contract OwnerUUPSUpgradable is UUPSUpgradeable, OwnableUpgradeable { error INVALID_PAUSE_STATUS(); modifier nonReentrant() { - if (_reentry == _TRUE) revert REENTRANT_CALL(); - _reentry = _TRUE; + if (_loadReentryLock() == _TRUE) revert REENTRANT_CALL(); + _storeReentryLock(_TRUE); _; - _reentry = _FALSE; + _storeReentryLock(_FALSE); } modifier whenPaused() { @@ -80,11 +85,24 @@ abstract contract OwnerUUPSUpgradable is UUPSUpgradeable, OwnableUpgradeable { // solhint-disable-next-line func-name-mixedcase function __OwnerUUPSUpgradable_init() internal virtual { __Ownable_init(); - _reentry = _FALSE; _paused = _FALSE; } function _inNonReentrant() internal view returns (bool) { - return _reentry == _TRUE; + return _loadReentryLock() == _TRUE; + } + + // Stores the reentry lock + function _storeReentryLock(uint8 reentry) private { + assembly { + tstore(_REENTRY_SLOT, reentry) + } + } + + // Loads the reentry lock + function _loadReentryLock() private view returns (uint8 reentry) { + assembly { + reentry := tload(_REENTRY_SLOT) + } } } diff --git a/packages/protocol/contracts/signal/ISignalService.sol b/packages/protocol/contracts/signal/ISignalService.sol index b307c083b02..ea4a63af0a2 100644 --- a/packages/protocol/contracts/signal/ISignalService.sol +++ b/packages/protocol/contracts/signal/ISignalService.sol @@ -24,7 +24,7 @@ interface ISignalService { /// @notice Verifies if a particular signal has already been sent. /// @param app The address that initiated the signal. - /// @param signal The signal (message) to send. + /// @param signal The signal (message) that was sent. /// @return True if the signal has been sent, otherwise false. function isSignalSent(address app, bytes32 signal) external view returns (bool); diff --git a/packages/protocol/contracts/tokenvault/BaseNFTVault.sol b/packages/protocol/contracts/tokenvault/BaseNFTVault.sol index 664c4eb589d..c1e5306aff4 100644 --- a/packages/protocol/contracts/tokenvault/BaseNFTVault.sol +++ b/packages/protocol/contracts/tokenvault/BaseNFTVault.sol @@ -108,12 +108,12 @@ abstract contract BaseNFTVault is BaseVault { error VAULT_INVALID_TOKEN(); error VAULT_INVALID_AMOUNT(); error VAULT_INVALID_USER(); - error VAULT_INVALID_SRC_CHAIN_ID(); + error VAULT_INVALID_TO(); error VAULT_INTERFACE_NOT_SUPPORTED(); error VAULT_TOKEN_ARRAY_MISMATCH(); error VAULT_MAX_TOKEN_PER_TXN_EXCEEDED(); - modifier withValidOperation(BridgeTransferOp calldata op) { + modifier withValidOperation(BridgeTransferOp memory op) { if (op.tokenIds.length != op.amounts.length) { revert VAULT_TOKEN_ARRAY_MISMATCH(); } diff --git a/packages/protocol/contracts/tokenvault/BaseVault.sol b/packages/protocol/contracts/tokenvault/BaseVault.sol index 1733dec2e23..d66c8850c63 100644 --- a/packages/protocol/contracts/tokenvault/BaseVault.sol +++ b/packages/protocol/contracts/tokenvault/BaseVault.sol @@ -52,8 +52,8 @@ abstract contract BaseVault is EssentialContract, IRecallableSender, IERC165Upgr returns (IBridge.Context memory ctx) { ctx = IBridge(msg.sender).context(); - address sender = resolve(ctx.srcChainId, name(), false); - if (ctx.from != sender) revert VAULT_PERMISSION_DENIED(); + address selfOnSourceChain = resolve(ctx.srcChainId, name(), false); + if (ctx.from != selfOnSourceChain) revert VAULT_PERMISSION_DENIED(); } function checkRecallMessageContext() diff --git a/packages/protocol/contracts/tokenvault/BridgedERC1155.sol b/packages/protocol/contracts/tokenvault/BridgedERC1155.sol index 86d60b09568..f5f5ddf9c3a 100644 --- a/packages/protocol/contracts/tokenvault/BridgedERC1155.sol +++ b/packages/protocol/contracts/tokenvault/BridgedERC1155.sol @@ -38,7 +38,6 @@ contract BridgedERC1155 is uint256[46] private __gap; error BTOKEN_CANNOT_RECEIVE(); - error BTOKEN_INVALID_PARAMS(); /// @dev Initializer function to be called after deployment. /// @param _addressManager The address of the address manager. @@ -56,11 +55,14 @@ contract BridgedERC1155 is external initializer { - if (_srcToken == address(0) || _srcChainId == 0 || _srcChainId == block.chainid) { - revert BTOKEN_INVALID_PARAMS(); - } + // Check if provided parameters are valid. + // The symbol and the name can be empty for ERC1155 tokens so we use some placeholder data + // for them instead. + LibBridgedToken.validateInputs(_srcToken, _srcChainId, "foo", "foo"); + __Essential_init(_addressManager); - __ERC1155_init(""); + __ERC1155_init(LibBridgedToken.buildURI(_srcToken, _srcChainId)); + srcToken = _srcToken; srcChainId = _srcChainId; symbol_ = _symbol; diff --git a/packages/protocol/contracts/tokenvault/BridgedERC20.sol b/packages/protocol/contracts/tokenvault/BridgedERC20.sol index 159021ef14e..97ac3642df4 100644 --- a/packages/protocol/contracts/tokenvault/BridgedERC20.sol +++ b/packages/protocol/contracts/tokenvault/BridgedERC20.sol @@ -41,7 +41,6 @@ contract BridgedERC20 is uint256[47] private __gap; error BTOKEN_CANNOT_RECEIVE(); - error BTOKEN_INVALID_PARAMS(); error BTOKEN_UNAUTHORIZED(); modifier onlyOwnerOrSnapshooter() { @@ -72,12 +71,7 @@ contract BridgedERC20 is initializer { // Check if provided parameters are valid - if ( - _srcToken == address(0) || _srcChainId == 0 || _srcChainId == block.chainid - || bytes(_symbol).length == 0 || bytes(_name).length == 0 - ) { - revert BTOKEN_INVALID_PARAMS(); - } + LibBridgedToken.validateInputs(_srcToken, _srcChainId, _symbol, _name); // Initialize OwnerUUPSUpgradable and ERC20Upgradeable __Essential_init(_addressManager); diff --git a/packages/protocol/contracts/tokenvault/BridgedERC20Base.sol b/packages/protocol/contracts/tokenvault/BridgedERC20Base.sol index 664c6484ed7..059388fe74f 100644 --- a/packages/protocol/contracts/tokenvault/BridgedERC20Base.sol +++ b/packages/protocol/contracts/tokenvault/BridgedERC20Base.sol @@ -33,25 +33,26 @@ abstract contract BridgedERC20Base is EssentialContract, IBridgedERC20 { error BB_MINT_DISALLOWED(); function changeMigrationStatus( - address addr, - bool inbound + address _migratingAddress, + bool _migratingInbound ) external + nonReentrant whenNotPaused onlyFromOwnerOrNamed("erc20_vault") { - if (addr == migratingAddress && inbound == migratingInbound) { + if (_migratingAddress == migratingAddress && _migratingInbound == migratingInbound) { revert BB_INVALID_PARAMS(); } - migratingAddress = addr; - migratingInbound = inbound; - emit MigrationStatusChanged(addr, inbound); + migratingAddress = _migratingAddress; + migratingInbound = _migratingInbound; + emit MigrationStatusChanged(_migratingAddress, _migratingInbound); } function mint(address account, uint256 amount) public nonReentrant whenNotPaused { // mint is disabled while migrating outbound. - if (migratingAddress != address(0) && !migratingInbound) revert BB_MINT_DISALLOWED(); + if (_isMigratingOut()) revert BB_MINT_DISALLOWED(); if (msg.sender == migratingAddress) { // Inbound migration @@ -65,14 +66,15 @@ abstract contract BridgedERC20Base is EssentialContract, IBridgedERC20 { } function burn(address account, uint256 amount) public nonReentrant whenNotPaused { - if (migratingAddress != address(0) && !migratingInbound) { + if (_isMigratingOut()) { + // Only the owner of the tokens himself can migrate out if (msg.sender != account) revert BB_PERMISSION_DENIED(); - // Outbond migration + // Outbound migration emit MigratedTo(migratingAddress, account, amount); // Ask the new bridged token to mint token for the user. IBridgedERC20(migratingAddress).mint(account, amount); } else if (msg.sender != resolve("erc20_vault", true)) { - // Bridging to vault + // Only the vault can burn tokens when not migrating out revert RESOLVER_DENIED(); } @@ -85,4 +87,8 @@ abstract contract BridgedERC20Base is EssentialContract, IBridgedERC20 { function _mintToken(address account, uint256 amount) internal virtual; function _burnToken(address from, uint256 amount) internal virtual; + + function _isMigratingOut() internal view returns (bool) { + return migratingAddress != address(0) && !migratingInbound; + } } diff --git a/packages/protocol/contracts/tokenvault/BridgedERC721.sol b/packages/protocol/contracts/tokenvault/BridgedERC721.sol index 99cc2a552b5..cf13c7660c4 100644 --- a/packages/protocol/contracts/tokenvault/BridgedERC721.sol +++ b/packages/protocol/contracts/tokenvault/BridgedERC721.sol @@ -28,7 +28,6 @@ contract BridgedERC721 is EssentialContract, ERC721Upgradeable { uint256[48] private __gap; error BTOKEN_CANNOT_RECEIVE(); - error BTOKEN_INVALID_PARAMS(); error BTOKEN_INVALID_BURN(); /// @dev Initializer function to be called after deployment. @@ -47,14 +46,12 @@ contract BridgedERC721 is EssentialContract, ERC721Upgradeable { external initializer { - if ( - _srcToken == address(0) || _srcChainId == 0 || _srcChainId == block.chainid - || bytes(_symbol).length == 0 || bytes(_name).length == 0 - ) { - revert BTOKEN_INVALID_PARAMS(); - } + // Check if provided parameters are valid + LibBridgedToken.validateInputs(_srcToken, _srcChainId, _symbol, _name); + __Essential_init(_addressManager); __ERC721_init(_name, _symbol); + srcToken = _srcToken; srcChainId = _srcChainId; } @@ -111,9 +108,9 @@ contract BridgedERC721 is EssentialContract, ERC721Upgradeable { return (srcToken, srcChainId); } - /// @notice Returns an empty token URI. - function tokenURI(uint256) public pure virtual override returns (string memory) { - return ""; + /// @notice Returns the token URI. + function tokenURI(uint256) public view virtual override returns (string memory) { + return LibBridgedToken.buildURI(srcToken, srcChainId); } function _beforeTokenTransfer( diff --git a/packages/protocol/contracts/tokenvault/ERC1155Vault.sol b/packages/protocol/contracts/tokenvault/ERC1155Vault.sol index c4bb44a36e8..2f9b5bbcd5d 100644 --- a/packages/protocol/contracts/tokenvault/ERC1155Vault.sol +++ b/packages/protocol/contracts/tokenvault/ERC1155Vault.sol @@ -24,8 +24,7 @@ import "./BridgedERC1155.sol"; /// @title ERC1155NameAndSymbol /// @notice Interface for ERC1155 contracts that provide name() and symbol() /// functions. These functions may not be part of the official interface but are -/// used by -/// some contracts. +/// used by some contracts. interface ERC1155NameAndSymbol { function name() external view returns (string memory); function symbol() external view returns (string memory); @@ -45,7 +44,7 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable { /// the destination chain so the user can receive the same (bridged) tokens /// by invoking the message call. /// @param op Option for sending the ERC1155 token. - function sendToken(BridgeTransferOp calldata op) + function sendToken(BridgeTransferOp memory op) external payable nonReentrant @@ -63,11 +62,6 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable { (bytes memory data, CanonicalNFT memory ctoken) = _handleMessage(msg.sender, op); - // Store variables in memory to avoid stack-too-deep error - uint256[] memory _amounts = op.amounts; - address _token = op.token; - uint256[] memory _tokenIds = op.tokenIds; - // Create a message to send to the destination chain IBridge.Message memory message; message.destChainId = op.destChainId; @@ -92,9 +86,9 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable { to: op.to, destChainId: _message.destChainId, ctoken: ctoken.addr, - token: _token, - tokenIds: _tokenIds, - amounts: _amounts + token: op.token, + tokenIds: op.tokenIds, + amounts: op.amounts }); } @@ -122,11 +116,13 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable { // Check context validity IBridge.Context memory ctx = checkProcessMessageContext(); - address _to = to == address(0) || to == address(this) ? from : to; + // Don't allow sending to disallowed addresses. + // Don't send the tokens back to `from` because `from` is on the source chain. + if (to == address(0) || to == address(this)) revert VAULT_INVALID_TO(); // Transfer the ETH and the tokens to the `to` address - address token = _transferTokens(ctoken, _to, tokenIds, amounts); - _to.sendEther(msg.value); + address token = _transferTokens(ctoken, to, tokenIds, amounts); + to.sendEther(msg.value); emit TokenReceived({ msgHash: ctx.msgHash, @@ -156,8 +152,6 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable { (CanonicalNFT memory ctoken,,, uint256[] memory tokenIds, uint256[] memory amounts) = abi.decode(message.data[4:], (CanonicalNFT, address, address, uint256[], uint256[])); - if (ctoken.addr == address(0)) revert VAULT_INVALID_TOKEN(); - // Transfer the ETH and tokens back to the owner address token = _transferTokens(ctoken, message.owner, tokenIds, amounts); message.owner.sendEther(message.value); diff --git a/packages/protocol/contracts/tokenvault/ERC20Vault.sol b/packages/protocol/contracts/tokenvault/ERC20Vault.sol index 9be02d87797..6e4895a5b0d 100644 --- a/packages/protocol/contracts/tokenvault/ERC20Vault.sol +++ b/packages/protocol/contracts/tokenvault/ERC20Vault.sol @@ -58,7 +58,7 @@ contract ERC20Vault is BaseVault { mapping(address btoken => bool blacklisted) public btokenBlacklist; - uint256[46] private __gap; + uint256[47] private __gap; event BridgedTokenDeployed( uint256 indexed srcChainId, @@ -106,6 +106,7 @@ contract ERC20Vault is BaseVault { error VAULT_INVALID_TOKEN(); error VAULT_INVALID_AMOUNT(); error VAULT_INVALID_NEW_BTOKEN(); + error VAULT_INVALID_TO(); error VAULT_NOT_SAME_OWNER(); function changeBridgedToken( @@ -133,7 +134,7 @@ contract ERC20Vault is BaseVault { if (btokenOld != address(0)) { CanonicalERC20 memory _ctoken = bridgedToCanonical[btokenOld]; - // Check that the ctoken must match the saved one. + // The ctoken must match the saved one. if ( _ctoken.decimals != ctoken.decimals || keccak256(bytes(_ctoken.symbol)) != keccak256(bytes(ctoken.symbol)) @@ -225,11 +226,14 @@ contract ERC20Vault is BaseVault { whenNotPaused { IBridge.Context memory ctx = checkProcessMessageContext(); - address _to = to == address(0) || to == address(this) ? from : to; + + // Don't allow sending to disallowed addresses. + // Don't send the tokens back to `from` because `from` is on the source chain. + if (to == address(0) || to == address(this)) revert VAULT_INVALID_TO(); // Transfer the ETH and the tokens to the `to` address - address token = _transferTokens(ctoken, _to, amount); - _to.sendEther(msg.value); + address token = _transferTokens(ctoken, to, amount); + to.sendEther(msg.value); emit TokenReceived({ msgHash: ctx.msgHash, @@ -257,8 +261,6 @@ contract ERC20Vault is BaseVault { (CanonicalERC20 memory ctoken,,, uint256 amount) = abi.decode(message.data[4:], (CanonicalERC20, address, address, uint256)); - if (ctoken.addr == address(0)) revert VAULT_INVALID_TOKEN(); - // Transfer the ETH and tokens back to the owner address token = _transferTokens(ctoken, message.owner, amount); message.owner.sendEther(message.value); diff --git a/packages/protocol/contracts/tokenvault/ERC721Vault.sol b/packages/protocol/contracts/tokenvault/ERC721Vault.sol index a300b526588..554e2e34ee6 100644 --- a/packages/protocol/contracts/tokenvault/ERC721Vault.sol +++ b/packages/protocol/contracts/tokenvault/ERC721Vault.sol @@ -35,7 +35,7 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable { /// destination chain so the user can receive the same (bridged) tokens /// by invoking the message call. /// @param op Option for sending the ERC721 token. - function sendToken(BridgeTransferOp calldata op) + function sendToken(BridgeTransferOp memory op) external payable nonReentrant @@ -53,12 +53,6 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable { (bytes memory data, CanonicalNFT memory ctoken) = _handleMessage(msg.sender, op); - // We need to save them into memory - because structs containing - // dynamic arrays will cause stack-too-deep error when passed - uint256[] memory _amounts = op.amounts; - address _token = op.token; - uint256[] memory _tokenIds = op.tokenIds; - IBridge.Message memory message; message.destChainId = op.destChainId; message.data = data; @@ -80,9 +74,9 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable { to: op.to, destChainId: _message.destChainId, ctoken: ctoken.addr, - token: _token, - tokenIds: _tokenIds, - amounts: _amounts + token: op.token, + tokenIds: op.tokenIds, + amounts: op.amounts }); } @@ -104,11 +98,13 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable { { IBridge.Context memory ctx = checkProcessMessageContext(); - address _to = to == address(0) || to == address(this) ? from : to; + // Don't allow sending to disallowed addresses. + // Don't send the tokens back to `from` because `from` is on the source chain. + if (to == address(0) || to == address(this)) revert VAULT_INVALID_TO(); // Transfer the ETH and the tokens to the `to` address - address token = _transferTokens(ctoken, _to, tokenIds); - _to.sendEther(msg.value); + address token = _transferTokens(ctoken, to, tokenIds); + to.sendEther(msg.value); emit TokenReceived({ msgHash: ctx.msgHash, @@ -134,16 +130,9 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable { { checkRecallMessageContext(); - if (message.owner == address(0)) revert VAULT_INVALID_USER(); - if (message.srcChainId != block.chainid) { - revert VAULT_INVALID_SRC_CHAIN_ID(); - } - (CanonicalNFT memory ctoken,,, uint256[] memory tokenIds) = abi.decode(message.data[4:], (CanonicalNFT, address, address, uint256[])); - if (ctoken.addr == address(0)) revert VAULT_INVALID_TOKEN(); - // Transfer the ETH and tokens back to the owner address token = _transferTokens(ctoken, message.owner, tokenIds); message.owner.sendEther(message.value); @@ -209,7 +198,7 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable { /// @return ctoken The canonical token. function _handleMessage( address user, - BridgeTransferOp calldata op + BridgeTransferOp memory op ) private returns (bytes memory msgData, CanonicalNFT memory ctoken) diff --git a/packages/protocol/contracts/tokenvault/LibBridgedToken.sol b/packages/protocol/contracts/tokenvault/LibBridgedToken.sol index 2e2ba18de52..f665de21f44 100644 --- a/packages/protocol/contracts/tokenvault/LibBridgedToken.sol +++ b/packages/protocol/contracts/tokenvault/LibBridgedToken.sol @@ -18,6 +18,25 @@ import "lib/openzeppelin-contracts/contracts/utils/Strings.sol"; /// @title LibBridgedToken library LibBridgedToken { + error BTOKEN_INVALID_PARAMS(); + + function validateInputs( + address _srcToken, + uint256 _srcChainId, + string memory _symbol, + string memory _name + ) + internal + view + { + if ( + _srcToken == address(0) || _srcChainId == 0 || _srcChainId == block.chainid + || bytes(_symbol).length == 0 || bytes(_name).length == 0 + ) { + revert BTOKEN_INVALID_PARAMS(); + } + } + function buildName( string memory name, uint256 srcChainId @@ -32,4 +51,18 @@ library LibBridgedToken { function buildSymbol(string memory symbol) internal pure returns (string memory) { return string.concat(symbol, ".t"); } + + function buildURI(address srcToken, uint256 srcChainId) internal pure returns (string memory) { + // Creates a base URI in the format specified by EIP-681: + // https://eips.ethereum.org/EIPS/eip-681 + return string( + abi.encodePacked( + "ethereum:", + Strings.toHexString(uint160(srcToken), 20), + "@", + Strings.toString(srcChainId), + "/tokenURI?uint256=" + ) + ); + } } diff --git a/packages/protocol/contracts/verifiers/GuardianVerifier.sol b/packages/protocol/contracts/verifiers/GuardianVerifier.sol index 9fff647727d..1bcec885355 100644 --- a/packages/protocol/contracts/verifiers/GuardianVerifier.sol +++ b/packages/protocol/contracts/verifiers/GuardianVerifier.sol @@ -39,7 +39,7 @@ contract GuardianVerifier is EssentialContract, IVerifier { external view { - if (ctx.prover != resolve("guardian_prover", false)) { + if (ctx.msgSender != resolve("guardian_prover", false)) { revert PERMISSION_DENIED(); } } diff --git a/packages/protocol/contracts/verifiers/IVerifier.sol b/packages/protocol/contracts/verifiers/IVerifier.sol index cf9165945c0..77d1b540a65 100644 --- a/packages/protocol/contracts/verifiers/IVerifier.sol +++ b/packages/protocol/contracts/verifiers/IVerifier.sol @@ -26,6 +26,7 @@ interface IVerifier { uint64 blockId; bool isContesting; bool blobUsed; + address msgSender; } function verifyProof( diff --git a/packages/protocol/contracts/verifiers/PseZkVerifier.sol b/packages/protocol/contracts/verifiers/PseZkVerifier.sol index 00320bb36ac..f31b083b404 100644 --- a/packages/protocol/contracts/verifiers/PseZkVerifier.sol +++ b/packages/protocol/contracts/verifiers/PseZkVerifier.sol @@ -61,17 +61,13 @@ contract PseZkVerifier is EssentialContract, IVerifier { ZkEvmProof memory zkProof = abi.decode(proof.data, (ZkEvmProof)); - bytes32 instance; + bytes32 txListHash; + uint256 pointValue; if (ctx.blobUsed) { PointProof memory pf = abi.decode(zkProof.pointProof, (PointProof)); - instance = calcInstance({ - tran: tran, - prover: ctx.prover, - metaHash: ctx.metaHash, - txListHash: pf.txListHash, - pointValue: pf.pointValue - }); + txListHash = pf.txListHash; + pointValue = pf.pointValue; Lib4844.evaluatePoint({ blobHash: ctx.blobHash, @@ -82,15 +78,19 @@ contract PseZkVerifier is EssentialContract, IVerifier { }); } else { if (zkProof.pointProof.length != 0) revert L1_BLOB_NOT_USED(); - instance = calcInstance({ - tran: tran, - prover: ctx.prover, - metaHash: ctx.metaHash, - txListHash: ctx.blobHash, - pointValue: 0 - }); + + txListHash = ctx.blobHash; + pointValue = 0; } + bytes32 instance = calcInstance({ + tran: tran, + prover: ctx.prover, + metaHash: ctx.metaHash, + txListHash: txListHash, + pointValue: pointValue + }); + // Validate the instance using bytes utilities. bool verified = Bytes.equal( Bytes.slice(zkProof.zkp, 0, 32), bytes.concat(bytes16(0), bytes16(instance)) diff --git a/packages/protocol/contracts/verifiers/SgxVerifier.sol b/packages/protocol/contracts/verifiers/SgxVerifier.sol index f78cc32153b..6b59de3d5a5 100644 --- a/packages/protocol/contracts/verifiers/SgxVerifier.sol +++ b/packages/protocol/contracts/verifiers/SgxVerifier.sol @@ -70,7 +70,6 @@ contract SgxVerifier is EssentialContract, IVerifier { error SGX_DELETE_NOT_AUTHORIZED(); error SGX_INVALID_ATTESTATION(); error SGX_INVALID_INSTANCE(); - error SGX_INVALID_INSTANCES(); error SGX_INVALID_PROOF(); error SGX_MISSING_ATTESTATION(); error SGX_RA_NOT_SUPPORTED(); @@ -89,7 +88,6 @@ contract SgxVerifier is EssentialContract, IVerifier { onlyOwner returns (uint256[] memory ids) { - if (_instances.length == 0) revert SGX_INVALID_INSTANCES(); ids = _addInstances(_instances, true); } @@ -99,13 +97,14 @@ contract SgxVerifier is EssentialContract, IVerifier { external onlyFromOwnerOrNamed("rollup_watchdog") { - if (_ids.length == 0) revert SGX_INVALID_INSTANCES(); for (uint256 i; i < _ids.length; ++i) { - if (instances[_ids[i]].addr == address(0)) revert SGX_INVALID_INSTANCE(); + uint256 idx = _ids[i]; - emit InstanceDeleted(_ids[i], instances[_ids[i]].addr); + if (instances[idx].addr == address(0)) revert SGX_INVALID_INSTANCE(); - delete instances[_ids[i]]; + emit InstanceDeleted(idx, instances[idx].addr); + + delete instances[idx]; } } diff --git a/packages/protocol/script/DeployOnL1.s.sol b/packages/protocol/script/DeployOnL1.s.sol index 48328bfd226..7c9a4c427cc 100644 --- a/packages/protocol/script/DeployOnL1.s.sol +++ b/packages/protocol/script/DeployOnL1.s.sol @@ -18,7 +18,6 @@ import "lib/openzeppelin-contracts/contracts/utils/Strings.sol"; import "../contracts/L1/TaikoToken.sol"; import "../contracts/L1/TaikoL1.sol"; -import "../contracts/L1/hooks/AssignmentHook.sol"; import "../contracts/L1/provers/GuardianProver.sol"; import "../contracts/L1/tiers/TaikoA6TierProvider.sol"; import "../contracts/L1/tiers/OptimisticTierProvider.sol"; @@ -367,7 +366,7 @@ contract DeployOnL1 is DeployCapability { }); address[] memory plonkVerifiers = new address[](1); - plonkVerifiers[0] = deployPseZkEvmVerifier("contracts/L1/verifiers/PlonkVerifier.yulp"); + plonkVerifiers[0] = deployPseZkEvmVerifier("contracts/verifiers/PlonkVerifier.yulp"); for (uint16 i = 0; i < plonkVerifiers.length; ++i) { register( diff --git a/packages/protocol/test/L1/Guardians.t.sol b/packages/protocol/test/L1/Guardians.t.sol index ad5e074d6dd..43cbb85ad2e 100644 --- a/packages/protocol/test/L1/Guardians.t.sol +++ b/packages/protocol/test/L1/Guardians.t.sol @@ -59,28 +59,28 @@ contract TestSignalService is TaikoTest { } function test_guardians_approve() public { - address[] memory signers = getSigners(5); - target.setGuardians(signers, 3); + address[] memory signers = getSigners(6); + target.setGuardians(signers, 4); bytes32 hash = keccak256("paris"); - for (uint256 i; i < 5; ++i) { + for (uint256 i; i < 6; ++i) { vm.prank(signers[0]); assertEq(target.approve(hash), false); assertEq(target.isApproved(hash), false); } hash = keccak256("singapore"); - for (uint256 i; i < 5; ++i) { + for (uint256 i; i < 6; ++i) { vm.startPrank(signers[i]); target.approve(hash); - assertEq(target.approve(hash), i >= 2); - assertEq(target.isApproved(hash), i >= 2); + assertEq(target.approve(hash), i >= 3); + assertEq(target.isApproved(hash), i >= 3); vm.stopPrank(); } // changing the settings will invalid all approval history - target.setGuardians(signers, 2); + target.setGuardians(signers, 3); assertEq(target.version(), 2); assertEq(target.isApproved(hash), false); } diff --git a/packages/protocol/test/verifiers/GuardianVerifier.t.sol b/packages/protocol/test/verifiers/GuardianVerifier.t.sol index f47fbea7162..d9fbc135f6a 100644 --- a/packages/protocol/test/verifiers/GuardianVerifier.t.sol +++ b/packages/protocol/test/verifiers/GuardianVerifier.t.sol @@ -22,6 +22,7 @@ contract TestGuardianVerifier is TaikoL1TestBase { metaHash: bytes32(0), blobHash: bytes32(0), prover: address(gp), + msgSender: address(gp), blockId: 10, isContesting: false, blobUsed: false @@ -50,6 +51,7 @@ contract TestGuardianVerifier is TaikoL1TestBase { metaHash: bytes32(0), blobHash: bytes32(0), prover: Alice, // invalid + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false diff --git a/packages/protocol/test/verifiers/PseZkVerifier.t.sol b/packages/protocol/test/verifiers/PseZkVerifier.t.sol index 03834b1ac47..ede71044301 100644 --- a/packages/protocol/test/verifiers/PseZkVerifier.t.sol +++ b/packages/protocol/test/verifiers/PseZkVerifier.t.sol @@ -40,6 +40,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: true, // skips all verification when true blobUsed: false @@ -68,6 +69,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: true @@ -120,6 +122,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -158,6 +161,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -188,6 +192,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -227,6 +232,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -265,6 +271,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -300,6 +307,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -342,6 +350,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -384,6 +393,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -423,6 +433,7 @@ contract TestPseZkVerifier is TaikoL1TestBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false diff --git a/packages/protocol/test/verifiers/SgxVerifier.t.sol b/packages/protocol/test/verifiers/SgxVerifier.t.sol index 5187eb33254..98dd8f8ec8e 100644 --- a/packages/protocol/test/verifiers/SgxVerifier.t.sol +++ b/packages/protocol/test/verifiers/SgxVerifier.t.sol @@ -185,6 +185,7 @@ contract TestSgxVerifier is TaikoL1TestBase, AttestationBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: KNOWN_ADDRESS, + msgSender: KNOWN_ADDRESS, blockId: 10, isContesting: false, blobUsed: false @@ -234,6 +235,7 @@ contract TestSgxVerifier is TaikoL1TestBase, AttestationBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: true, // skips all verification when true blobUsed: false @@ -266,6 +268,7 @@ contract TestSgxVerifier is TaikoL1TestBase, AttestationBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -300,6 +303,7 @@ contract TestSgxVerifier is TaikoL1TestBase, AttestationBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -339,6 +343,7 @@ contract TestSgxVerifier is TaikoL1TestBase, AttestationBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false @@ -379,6 +384,7 @@ contract TestSgxVerifier is TaikoL1TestBase, AttestationBase { metaHash: bytes32("ab"), blobHash: bytes32("cd"), prover: Alice, + msgSender: Alice, blockId: 10, isContesting: false, blobUsed: false diff --git a/packages/protocol/utils/generate_genesis/taikoL2.ts b/packages/protocol/utils/generate_genesis/taikoL2.ts index b025b69d922..dd4aa26aae9 100644 --- a/packages/protocol/utils/generate_genesis/taikoL2.ts +++ b/packages/protocol/utils/generate_genesis/taikoL2.ts @@ -296,7 +296,6 @@ async function generateContractConfigs( _initialized: 1, _initializing: false, // ReentrancyGuardUpgradeable - _reentry: 1, // _FALSE _paused: 1, // _FALSE // Ownable2Upgradeable _owner: ownerSecurityCouncil, @@ -331,7 +330,6 @@ async function generateContractConfigs( _initialized: 1, _initializing: false, // ReentrancyGuardUpgradeable - _reentry: 1, // _FALSE _paused: 1, // _FALSE // Ownable2Upgradeable _owner: ownerSecurityCouncil, @@ -366,7 +364,6 @@ async function generateContractConfigs( _initialized: 1, _initializing: false, // ReentrancyGuardUpgradeable - _reentry: 1, // _FALSE _paused: 1, // _FALSE // Ownable2Upgradeable _owner: ownerSecurityCouncil, @@ -401,7 +398,6 @@ async function generateContractConfigs( _initialized: 1, _initializing: false, // ReentrancyGuardUpgradeable - _reentry: 1, // _FALSE _paused: 1, // _FALSE // Ownable2Upgradeable _owner: ownerSecurityCouncil, @@ -460,7 +456,6 @@ async function generateContractConfigs( _initialized: 1, _initializing: false, // ReentrancyGuardUpgradeable - _reentry: 1, // _FALSE _paused: 1, // _FALSE // Ownable2Upgradeable _owner: ownerSecurityCouncil,