From 024739392bf0b7c07cadf769ee74a66e25e78f40 Mon Sep 17 00:00:00 2001 From: seolaoh Date: Fri, 12 Jul 2024 14:23:00 +0900 Subject: [PATCH 1/3] feat(contracts): apply audit results about vesting wallet (#342) --- packages/contracts/.gas-snapshot | 17 +++++++++-------- .../contracts/test/KromaVestingWallet.t.sol | 12 ++++++++++++ .../contracts/universal/KromaVestingWallet.sol | 7 ++++++- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/packages/contracts/.gas-snapshot b/packages/contracts/.gas-snapshot index 6a5915547..b58b937f1 100644 --- a/packages/contracts/.gas-snapshot +++ b/packages/contracts/.gas-snapshot @@ -164,14 +164,15 @@ KromaPortal_Test:test_receive_succeeds() (gas: 127638) KromaPortal_Test:test_simple_isOutputFinalized_succeeds() (gas: 35393) KromaPortal_Test:test_unpause_onlyGuardian_reverts() (gas: 46358) KromaPortal_Test:test_unpause_succeeds() (gas: 31932) -KromaVestingWalletTest:test_constructor_succeeds() (gas: 13938) -KromaVestingWalletTest:test_constructor_zeroValues_reverts() (gas: 72905) -KromaVestingWalletTest:test_initialize_succeeds() (gas: 21631) -KromaVestingWalletTest:test_release_afterFullyVested_succeeds() (gas: 79730) -KromaVestingWalletTest:test_release_notBeneficiary_reverts() (gas: 28574) -KromaVestingWalletTest:test_release_succeeds() (gas: 116326) -KromaVestingWalletTest:test_release_tokenAfterFullyVested_succeeds() (gas: 95049) -KromaVestingWalletTest:test_release_token_succeeds() (gas: 154458) +KromaVestingWalletTest:test_constructor_succeeds() (gas: 13916) +KromaVestingWalletTest:test_constructor_zeroValues_reverts() (gas: 72941) +KromaVestingWalletTest:test_initialize_durationNotMultiple_reverts() (gas: 1609487) +KromaVestingWalletTest:test_initialize_succeeds() (gas: 21609) +KromaVestingWalletTest:test_release_afterFullyVested_succeeds() (gas: 79771) +KromaVestingWalletTest:test_release_notBeneficiary_reverts() (gas: 28552) +KromaVestingWalletTest:test_release_succeeds() (gas: 115505) +KromaVestingWalletTest:test_release_tokenAfterFullyVested_succeeds() (gas: 95024) +KromaVestingWalletTest:test_release_token_succeeds() (gas: 153617) L1BlockBedrock_Test:test_updateValues_succeeds() (gas: 65631) L1BlockEcotone_Test:test_setL1BlockValuesEcotone_isDepositor_succeeds() (gas: 80519) L1BlockEcotone_Test:test_setL1BlockValuesEcotone_notDepositor_fails() (gas: 7621) diff --git a/packages/contracts/contracts/test/KromaVestingWallet.t.sol b/packages/contracts/contracts/test/KromaVestingWallet.t.sol index f3a7419fb..c7c37fdf2 100644 --- a/packages/contracts/contracts/test/KromaVestingWallet.t.sol +++ b/packages/contracts/contracts/test/KromaVestingWallet.t.sol @@ -76,6 +76,18 @@ contract KromaVestingWalletTest is CommonTest { assertEq(vestingWallet.duration(), durationSec); } + function test_initialize_durationNotMultiple_reverts() external { + vestingWallet = KromaVestingWallet(payable(address(new Proxy(multisig)))); + KromaVestingWallet vestingWalletImpl = new KromaVestingWallet(cliffDivider, vestingCycle); + + vm.prank(multisig); + vm.expectRevert("Proxy: delegatecall to new implementation contract failed"); + toProxy(address(vestingWallet)).upgradeToAndCall( + address(vestingWalletImpl), + abi.encodeCall(vestingWallet.initialize, (beneficiary, startTime, durationSec + 1)) + ); + } + function test_release_token_succeeds() external { // Ensure test env is set properly assertEq(token.balanceOf(beneficiary), 0); diff --git a/packages/contracts/contracts/universal/KromaVestingWallet.sol b/packages/contracts/contracts/universal/KromaVestingWallet.sol index 87a5fd810..0515fa0f8 100644 --- a/packages/contracts/contracts/universal/KromaVestingWallet.sol +++ b/packages/contracts/contracts/universal/KromaVestingWallet.sol @@ -59,6 +59,11 @@ contract KromaVestingWallet is VestingWalletUpgradeable { uint64 _startTimestamp, uint64 _durationSeconds ) public initializer { + require( + _durationSeconds % VESTING_CYCLE == 0, + "KromaVestingWallet: duration should be multiple of vesting cycle" + ); + __VestingWallet_init(_beneficiary, _startTimestamp, _durationSeconds); } @@ -94,7 +99,7 @@ contract KromaVestingWallet is VestingWalletUpgradeable { ) internal view override returns (uint256) { if (timestamp < start()) { return 0; - } else if (timestamp > start() + duration()) { + } else if (timestamp >= start() + duration()) { return totalAllocation; } else { // At the start date, cliff amount of the assets are immediately vested. From 2ec44039ee387f442716d386fa69f18d6a3ae247 Mon Sep 17 00:00:00 2001 From: Elias Rad <146735585+nnsW3@users.noreply.github.com> Date: Wed, 24 Jul 2024 02:04:06 +0300 Subject: [PATCH 2/3] docs: rectify typographical inaccuracies (#337) * fix crossdomainmessenger.go * fix types.go * fix README.md * fix withdrawal.go * fix status.go * fix flags_test.go * Update kroma-chain-ops/README.md Co-authored-by: seolaoh * Update types.go * Update withdrawal.go * Update kroma-validator/challenge/status.go Co-authored-by: Hansol Lee <38912532+0xHansLee@users.noreply.github.com> * Update flags_test.go * Update kroma-bindings/bindings/crossdomainmessenger.go --------- Co-authored-by: seolaoh Co-authored-by: Hansol Lee <38912532+0xHansLee@users.noreply.github.com> --- kroma-chain-ops/README.md | 4 ++-- kroma-validator/challenge/status.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/kroma-chain-ops/README.md b/kroma-chain-ops/README.md index e033b0d9c..77d540e56 100644 --- a/kroma-chain-ops/README.md +++ b/kroma-chain-ops/README.md @@ -9,8 +9,8 @@ has been configured correctly. It iterates over all 256 predeployed proxies to make sure they are configured correctly with the correct proxy admin address. After that, it checks that all [predeploys](../kroma-bindings/predeploys/addresses.go) are configured and aliased correctly. Additional contract-specific -checks ensure configuration like ownership, version, and storage -is set correctly for the predeploys. +checks ensure configurations like ownership, version, and storage +are set correctly for the predeploys. #### Usage diff --git a/kroma-validator/challenge/status.go b/kroma-validator/challenge/status.go index f2cd53ea5..37f85cde1 100644 --- a/kroma-validator/challenge/status.go +++ b/kroma-validator/challenge/status.go @@ -1,8 +1,8 @@ package challenge const ( - // StatusNone is regarded as a challenge is not in progress. - // The other status are regarded as a challenge is in progress. + // StatusNone is regarded as a challenge that is not in progress. + // The others are regarded as a challenge in progress. StatusNone uint8 = iota StatusChallengerTurn StatusAsserterTurn From bc896c5cd351beac5f4769518a6197e2a6a375ef Mon Sep 17 00:00:00 2001 From: seolaoh Date: Thu, 8 Aug 2024 13:01:19 +0900 Subject: [PATCH 3/3] fix(contracts): not upgrade already deployed contract (#371) --- packages/contracts/src/deploy-utils.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/contracts/src/deploy-utils.ts b/packages/contracts/src/deploy-utils.ts index 4e047ad47..be2cd3642 100644 --- a/packages/contracts/src/deploy-utils.ts +++ b/packages/contracts/src/deploy-utils.ts @@ -43,7 +43,10 @@ export const deploy = async ( name: string, opts: DeployOptions = {} ): Promise => { - const created = await deployImpl(hre, name, opts) + const [created, newlyDeployed] = await deployImpl(hre, name, opts) + if (!newlyDeployed) { + return created + } if (opts.isProxyImpl) { const { deployer } = await hre.getNamedAccounts() @@ -107,7 +110,10 @@ export const deployAndUpgradeByDeployer = async ( name: string, opts: DeployOptions = {} ): Promise => { - const created = await deployImpl(hre, name, opts) + const [created, newlyDeployed] = await deployImpl(hre, name, opts) + if (!newlyDeployed) { + return created + } const { deployer } = await hre.getNamedAccounts() const proxyName = name + 'Proxy' @@ -158,12 +164,13 @@ export const deployAndUpgradeByDeployer = async ( * @param opts.args Arguments to pass to the contract constructor. * @param opts.postDeployAction Action to perform after the contract is deployed. * @returns A deployed contract object. + * @returns If the contract is newly deployed or not. */ const deployImpl = async ( hre: HardhatRuntimeEnvironment, name: string, opts: DeployOptions = {} -): Promise => { +): Promise<[Contract | null, boolean]> => { const { deployer } = await hre.getNamedAccounts() // Wrap in a try/catch in case there is not a deployConfig for the current network. @@ -194,7 +201,7 @@ const deployImpl = async ( // If the contract is not newly deployed, do not proceed further. if (!result.newlyDeployed) { - return created + return [created, false] } // Always wait for the transaction to be mined, just in case. @@ -211,7 +218,7 @@ const deployImpl = async ( await opts.postDeployAction(created) } - return created + return [created, true] } /**