Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(protocol): address miscellaneous feedbacks from Sigma Prime (TKO26) #15600

Merged
merged 11 commits into from
Jan 30, 2024
8 changes: 3 additions & 5 deletions packages/protocol/contracts/L1/TaikoErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@ abstract contract TaikoErrors {
error L1_BLOB_FOR_DA_DISABLED();
error L1_BLOB_NOT_FOUND();
error L1_BLOB_NOT_REUSEABLE();
error L1_BLOB_NOT_USED();
error L1_BLOCK_MISMATCH();
error L1_INSUFFICIENT_TOKEN();
error L1_INVALID_ADDRESS();
error L1_INVALID_AMOUNT();
error L1_INVALID_BLOCK_ID();
error L1_INVALID_CONFIG();
error L1_INVALID_ETH_DEPOSIT();
Expand All @@ -46,12 +44,12 @@ abstract contract TaikoErrors {
error L1_PROPOSER_NOT_EOA();
error L1_PROVING_PAUSED();
error L1_RECEIVE_DISABLED();
error L1_MISSING_VERIFIER();
error L1_TOO_MANY_BLOCKS();
error L1_TOO_MANY_TIERS();
error L1_TRANSITION_ID_ZERO();
error L1_TRANSITION_NOT_FOUND();
error L1_TXLIST_OFFSET_SIZE();
error L1_TXLIST_TOO_LARGE();
error L1_TXLIST_SIZE();
error L1_UNAUTHORIZED();
error L1_UNEXPECTED_PARENT();
error L1_UNEXPECTED_TRANSITION_ID();
Expand Down
14 changes: 0 additions & 14 deletions packages/protocol/contracts/L1/TaikoEvents.sol
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,4 @@ abstract contract TaikoEvents {
/// @param deposit The Ethereum deposit information including recipient,
/// amount, and ID.
event EthDeposited(TaikoData.EthDeposit deposit);

/// @dev Emitted when a user deposited Taiko token into this contract.
event TokenDeposited(uint256 amount);

/// @dev Emitted when a user withdrawed Taiko token from this contract.
event TokenWithdrawn(uint256 amount);

/// @dev Emitted when Taiko token are credited to the user's in-protocol
/// balance.
event TokenCredited(address to, uint256 amount);

/// @dev Emitted when Taiko token are debited from the user's in-protocol
/// balance.
event TokenDebited(address from, uint256 amount);
}
2 changes: 1 addition & 1 deletion packages/protocol/contracts/L1/gov/TaikoGovernor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ contract TaikoGovernor is
return 50_400; // 1 week
}

// The number of votes required in order for a voter to become a proposer
// The number of votes required in order for a voter to become a vote proposer
dantaik marked this conversation as resolved.
Show resolved Hide resolved
dantaik marked this conversation as resolved.
Show resolved Hide resolved
function proposalThreshold() public pure override returns (uint256) {
return 1_000_000_000 ether / 10_000; // 0.01% of Taiko Token
}
Expand Down
16 changes: 11 additions & 5 deletions packages/protocol/contracts/L1/libs/LibProving.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ library LibProving {
error L1_INVALID_TIER();
error L1_INVALID_TRANSITION();
error L1_NOT_ASSIGNED_PROVER();
error L1_MISSING_VERIFIER();
error L1_UNEXPECTED_TRANSITION_TIER();

function pauseProving(TaikoData.State storage state, bool pause) external {
Expand Down Expand Up @@ -91,9 +92,7 @@ library LibProving {
uint64 slot = meta.id % config.blockRingBufferSize;
TaikoData.Block storage blk = state.blocks[slot];

// Check the integrity of the block data. It's worth noting that in
// theory, this check may be skipped, but it's included for added
// caution.
// Check the integrity of the block data.
if (blk.blockId != meta.id || blk.metaHash != keccak256(abi.encode(meta))) {
revert L1_BLOCK_MISMATCH();
}
Expand Down Expand Up @@ -176,7 +175,7 @@ library LibProving {

// The new proof must meet or exceed the minimum tier required by the
// block or the previous proof; it cannot be on a lower tier.
if (proof.tier == 0 || proof.tier < meta.minTier || proof.tier < ts.tier) {
if (proof.tier == 0 || proof.tier < ts.tier) {
dantaik marked this conversation as resolved.
Show resolved Hide resolved
revert L1_INVALID_TIER();
}

Expand All @@ -185,7 +184,7 @@ library LibProving {
ITierProvider.Tier memory tier =
ITierProvider(resolver.resolve("tier_provider", false)).getTier(proof.tier);

maxBlocksToVerify = tier.maxBlocksToVerify;
maxBlocksToVerify = tier.maxBlocksToVerifyPerProof;

// We must verify the proof, and any failure in proof verification will
// result in a revert.
Expand All @@ -206,6 +205,13 @@ library LibProving {
// 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)
&& keccak256(abi.encodePacked(tier.verifierName))
!= keccak256(abi.encodePacked("tier_optimistic"))
dantaik marked this conversation as resolved.
Show resolved Hide resolved
) {
revert L1_MISSING_VERIFIER();
}
if (verifier != address(0)) {
bool isContesting = proof.tier == ts.tier && tier.contestBond != 0;

Expand Down
4 changes: 2 additions & 2 deletions packages/protocol/contracts/L1/libs/LibProvingAlt.sol
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ library LibProvingAlt {

// The new proof must meet or exceed the minimum tier required by the
// block or the previous proof; it cannot be on a lower tier.
if (proof.tier == 0 || proof.tier < meta.minTier || proof.tier < ts.tier) {
if (proof.tier == 0 || proof.tier < ts.tier) {
dantaik marked this conversation as resolved.
Show resolved Hide resolved
revert L1_INVALID_TIER();
}

Expand Down Expand Up @@ -234,7 +234,7 @@ library LibProvingAlt {
}

ts.timestamp = uint64(block.timestamp);
return tier.maxBlocksToVerify;
return tier.maxBlocksToVerifyPerProof;
}

/// @dev Handle the transition initialization logic
Expand Down
8 changes: 5 additions & 3 deletions packages/protocol/contracts/L1/libs/LibVerifying.sol
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,14 @@ library LibVerifying {
|| config.blockMaxTxListBytes > 128 * 1024 // calldata up to 128K
|| config.livenessBond == 0 || config.ethDepositRingBufferSize <= 1
|| config.ethDepositMinCountPerBlock == 0
// Audit recommendation, and gas tested. Processing 32 deposits (as initially set in
// TaikoL1.sol) costs 72_502 gas.
|| config.ethDepositMaxCountPerBlock > 32
|| config.ethDepositMaxCountPerBlock < config.ethDepositMinCountPerBlock
|| config.ethDepositMinAmount == 0
|| config.ethDepositMaxAmount <= config.ethDepositMinAmount
|| config.ethDepositMaxAmount >= type(uint96).max || config.ethDepositGas == 0
|| config.ethDepositMaxFee == 0 || config.ethDepositMaxFee >= type(uint96).max
|| config.ethDepositMaxFee == 0
|| config.ethDepositMaxFee >= type(uint96).max / config.ethDepositMaxCountPerBlock
) return false;

Expand Down Expand Up @@ -209,8 +212,7 @@ library LibVerifying {
// other transitions for this block, we disregard them entirely.
// The bonds for these other transitions are burned either when
// the transitions are generated or proven. In such cases, both
// the provers and contesters of of those transitions forfeit
// their bonds.
// the provers and contesters of those transitions forfeit their bonds.

emit BlockVerified({
blockId: blockId,
Expand Down
20 changes: 10 additions & 10 deletions packages/protocol/contracts/L1/provers/Guardians.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,22 @@ abstract contract Guardians is EssentialContract {
error INVALID_PROOF();

/// @notice Set the set of guardians
/// @param _guardians The new set of guardians
/// @param _newGuardians The new set of guardians
/// @param _minGuardians The minimum required to sign
function setGuardians(
address[] memory _guardians,
address[] memory _newGuardians,
uint8 _minGuardians
)
external
onlyOwner
nonReentrant
{
if (_guardians.length < MIN_NUM_GUARDIANS || _guardians.length > type(uint8).max) {
if (_newGuardians.length < MIN_NUM_GUARDIANS || _newGuardians.length > type(uint8).max) {
revert INVALID_GUARDIAN_SET();
}
if (
_minGuardians == 0 || _minGuardians < _guardians.length >> 1
|| _minGuardians > _guardians.length
) revert INVALID_MIN_GUARDIANS();
if (_minGuardians < _newGuardians.length >> 1 || _minGuardians > _newGuardians.length) {
revert INVALID_MIN_GUARDIANS();
}

// Delete current guardians data
uint256 guardiansLength = guardians.length;
Expand All @@ -64,8 +64,8 @@ abstract contract Guardians is EssentialContract {
sstore(guardians.slot, 0)
}

for (uint256 i = 0; i < _guardians.length;) {
address guardian = _guardians[i];
for (uint256 i = 0; i < _newGuardians.length;) {
address guardian = _newGuardians[i];
if (guardian == address(0)) revert INVALID_GUARDIAN();
if (guardianIds[guardian] != 0) revert INVALID_GUARDIAN_SET();

Expand All @@ -75,7 +75,7 @@ abstract contract Guardians is EssentialContract {
}

minGuardians = _minGuardians;
emit GuardiansUpdated(++version, _guardians);
emit GuardiansUpdated(++version, _newGuardians);
}

function isApproved(bytes32 hash) public view returns (bool) {
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/L1/tiers/ITierProvider.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ interface ITierProvider {
uint96 contestBond;
uint24 cooldownWindow;
uint16 provingWindow;
uint8 maxBlocksToVerify;
uint8 maxBlocksToVerifyPerProof;
}

/// @dev Retrieves the configuration for a specified tier.
Expand Down
8 changes: 4 additions & 4 deletions packages/protocol/contracts/L1/tiers/TaikoA6TierProvider.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ contract TaikoA6TierProvider is EssentialContract, ITierProvider {
contestBond: 1000 ether, // TKO
cooldownWindow: 24 hours,
provingWindow: 2 hours,
maxBlocksToVerify: 10
maxBlocksToVerifyPerProof: 10
});
}

Expand All @@ -52,7 +52,7 @@ contract TaikoA6TierProvider is EssentialContract, ITierProvider {
contestBond: 500 ether, // TKO
cooldownWindow: 24 hours,
provingWindow: 4 hours,
maxBlocksToVerify: 8
maxBlocksToVerifyPerProof: 8
});
}

Expand All @@ -63,7 +63,7 @@ contract TaikoA6TierProvider is EssentialContract, ITierProvider {
contestBond: 250 ether, // TKO
cooldownWindow: 24 hours,
provingWindow: 6 hours,
maxBlocksToVerify: 6
maxBlocksToVerifyPerProof: 6
});
}

Expand All @@ -74,7 +74,7 @@ contract TaikoA6TierProvider is EssentialContract, ITierProvider {
contestBond: 0, // not contestable
cooldownWindow: 24 hours,
provingWindow: 8 hours,
maxBlocksToVerify: 4
maxBlocksToVerifyPerProof: 4
});
}

Expand Down
3 changes: 2 additions & 1 deletion packages/protocol/contracts/L1/verifiers/PseZkVerifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ contract PseZkVerifier is EssentialContract, IVerifier {

uint256[50] private __gap;

error L1_BLOB_NOT_USED();
error L1_INVALID_PROOF();

/// @notice Initializes the contract with the provided address manager.
Expand Down Expand Up @@ -80,7 +81,7 @@ contract PseZkVerifier is EssentialContract, IVerifier {
pointProof: pf.pointProof
});
} else {
assert(zkProof.pointProof.length == 0);
if (zkProof.pointProof.length != 0) revert L1_BLOB_NOT_USED();
dantaik marked this conversation as resolved.
Show resolved Hide resolved
instance = calcInstance({
tran: tran,
prover: ctx.prover,
Expand Down
6 changes: 4 additions & 2 deletions packages/protocol/contracts/L2/TaikoL2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,21 @@ import "../libs/LibAddress.sol";
import "../libs/LibMath.sol";
import "./Lib1559Math.sol";
import "./CrossChainOwned.sol";
import "./TaikoL2Signer.sol";

/// @title TaikoL2
/// @notice Taiko L2 is a smart contract that handles cross-layer message
/// verification and manages EIP-1559 gas pricing for Layer 2 (L2) operations.
/// It is used to anchor the latest L1 block details to L2 for cross-layer
/// communication, manage EIP-1559 parameters for gas pricing, and store
/// verified L1 block information.
contract TaikoL2 is CrossChainOwned, TaikoL2Signer, ICrossChainSync {
contract TaikoL2 is CrossChainOwned, ICrossChainSync {
using LibAddress for address;
using LibMath for uint256;
using SafeERC20 for IERC20;

// Golden touch address
address public constant GOLDEN_TOUCH_ADDRESS = 0x0000777735367b36bC9B61C50022d9D0700dB4Ec;

struct Config {
uint32 gasTargetPerL1Block;
uint8 basefeeAdjustmentQuotient;
Expand Down
2 changes: 0 additions & 2 deletions packages/protocol/contracts/bridge/Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ contract Bridge is EssentialContract, IBridge {
mapping(address => bool) public addressBanned;
uint256[43] private __gap;

event SignalSent(address indexed sender, bytes32 msgHash);
event MessageSent(bytes32 indexed msgHash, Message message);
event MessageRecalled(bytes32 indexed msgHash);
event DestChainEnabled(uint64 indexed chainId, bool enabled);
event MessageStatusChanged(bytes32 indexed msgHash, Status status);
event AddressBanned(address indexed addr, bool banned);

Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/thirdparty/LibBytesUtils.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: MIT
// Taken from
// https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts/contracts/libraries/utils/LibBytesUtils.sol
// https://github.com/ethereum-optimism/optimism-legacy/blob/develop/packages/contracts/contracts/libraries/utils/Lib_BytesUtils.sol
// (The MIT License)
//
// Copyright 2020-2021 Optimism
Expand Down
7 changes: 0 additions & 7 deletions packages/protocol/contracts/thirdparty/LibMerkleTrie.sol
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,6 @@ library LibMerkleTrie {
return LibRLPReader.readBytes(_node.decoded[_node.decoded.length - 1]);
}

/**
* @notice Utility; determines the number of nibbles shared between two
* nibble arrays.
* @param _a First nibble array.
* @param _b Second nibble array.
* @return _shared Number of shared nibbles.
*/
/**
* @notice Utility; determines the number of nibbles shared between two
* nibble arrays.
Expand Down
2 changes: 2 additions & 0 deletions packages/protocol/contracts/thirdparty/LibRLPReader.sol
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ library LibRLPReader {
out := mload(ptr)

// Shift the bytes over to match the item size.
// Note: Will align to the right for an input smaller than 32 bytes.
if lt(itemLength, 32) { out := div(out, exp(256, sub(32, itemLength))) }
}

Expand Down Expand Up @@ -235,6 +236,7 @@ library LibRLPReader {
*/
function readAddress(RLPItem memory _in) internal pure returns (address) {
if (_in.length == 1) {
// Note: regardless of value of _in, it returns address(0) if length is 1.
return address(0);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/protocol/contracts/tokenvault/ERC1155Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,9 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable {
}
}
// Send back Ether
message.owner.sendEther(message.value);
if (message.value > 0) {
dantaik marked this conversation as resolved.
Show resolved Hide resolved
message.owner.sendEther(message.value);
}

// Emit TokenReleased event
emit TokenReleased({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ library LibUint512Math {
pure
returns (uint256 r0, uint256 r1)
{
// Library code itself dies not revert on overflow.
// (Taiko's static signAnchor() usage will not cause any overrun!)
assembly {
// Standard 256-bit addition for lower bits.
r0 := add(a0, b0)
Expand Down
11 changes: 7 additions & 4 deletions packages/protocol/test/L2/TaikoL2.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ contract TestTaikoL2 is TaikoTest {
address public addressManager;
TaikoL2EIP1559Configurable public L2;
SkipBasefeeCheckL2 public L2skip;
TaikoL2Signer public l2Signer;

function setUp() public {
addressManager = deployProxy({
Expand Down Expand Up @@ -68,6 +69,8 @@ contract TestTaikoL2 is TaikoTest {

vm.roll(block.number + 1);
vm.warp(block.timestamp + 30);

l2Signer = new TaikoL2Signer();
}

function test_L2_AnchorTx_with_constant_block_time() external {
Expand Down Expand Up @@ -227,19 +230,19 @@ contract TestTaikoL2 is TaikoTest {
}

function test_L2_AnchorTx_signing(bytes32 digest) external {
(uint8 v, uint256 r, uint256 s) = L2.signAnchor(digest, uint8(1));
(uint8 v, uint256 r, uint256 s) = l2Signer.signAnchor(digest, uint8(1));
address signer = ecrecover(digest, v + 27, bytes32(r), bytes32(s));
assertEq(signer, L2.GOLDEN_TOUCH_ADDRESS());

(v, r, s) = L2.signAnchor(digest, uint8(2));
(v, r, s) = l2Signer.signAnchor(digest, uint8(2));
signer = ecrecover(digest, v + 27, bytes32(r), bytes32(s));
assertEq(signer, L2.GOLDEN_TOUCH_ADDRESS());

vm.expectRevert();
L2.signAnchor(digest, uint8(0));
l2Signer.signAnchor(digest, uint8(0));

vm.expectRevert();
L2.signAnchor(digest, uint8(3));
l2Signer.signAnchor(digest, uint8(3));
}

function _anchor(uint32 parentGasLimit) private {
Expand Down
Loading
Loading