Skip to content

Commit

Permalink
feat: adjust PR reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
seolaoh committed Dec 12, 2024
1 parent 146dd5c commit a57d6fd
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 66 deletions.
66 changes: 33 additions & 33 deletions kroma-bindings/bindings/gaspriceoracle.go

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion kroma-bindings/bindings/gaspriceoracle_more.go

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions op-node/rollup/derive/kromampt_upgrade_transactions.go

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions packages/contracts/.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -161,20 +161,20 @@ GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 421695
GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3467594)
GasBenchMark_L1StandardBridge_Finalize:test_finalizeBridgeETH_benchmark() (gas: 32252)
GasBenchMark_L2OutputOracle:test_submitL2Output_benchmark() (gas: 157518)
GasPriceOracleBedrock_Test:test_baseFee_succeeds() (gas: 8347)
GasPriceOracleBedrock_Test:test_baseFee_succeeds() (gas: 8325)
GasPriceOracleBedrock_Test:test_decimals_succeeds() (gas: 6256)
GasPriceOracleBedrock_Test:test_gasPrice_succeeds() (gas: 8316)
GasPriceOracleBedrock_Test:test_l1BaseFee_succeeds() (gas: 12858)
GasPriceOracleBedrock_Test:test_l1BaseFee_succeeds() (gas: 12836)
GasPriceOracleBedrock_Test:test_overhead_succeeds() (gas: 12806)
GasPriceOracleBedrock_Test:test_scalar_succeeds() (gas: 12759)
GasPriceOracleEcotone_Test:test_baseFeeScalar_succeeds() (gas: 12903)
GasPriceOracleEcotone_Test:test_baseFee_succeeds() (gas: 8347)
GasPriceOracleEcotone_Test:test_blobBaseFeeScalar_succeeds() (gas: 12903)
GasPriceOracleEcotone_Test:test_baseFee_succeeds() (gas: 8325)
GasPriceOracleEcotone_Test:test_blobBaseFeeScalar_succeeds() (gas: 12881)
GasPriceOracleEcotone_Test:test_blobBaseFee_succeeds() (gas: 12811)
GasPriceOracleEcotone_Test:test_decimals_succeeds() (gas: 6234)
GasPriceOracleEcotone_Test:test_gasPrice_succeeds() (gas: 8339)
GasPriceOracleEcotone_Test:test_getL1Fee_succeeds() (gas: 28049)
GasPriceOracleEcotone_Test:test_l1BaseFee_succeeds() (gas: 12880)
GasPriceOracleEcotone_Test:test_getL1Fee_succeeds() (gas: 28027)
GasPriceOracleEcotone_Test:test_l1BaseFee_succeeds() (gas: 12858)
GasPriceOracleEcotone_Test:test_overhead_legacyFunction_reverts() (gas: 10507)
GasPriceOracleEcotone_Test:test_scalar_legacyFunction_reverts() (gas: 10517)
GasPriceOracleEcotone_Test:test_setEcotone_wrongCaller_reverts() (gas: 11616)
Expand Down
18 changes: 10 additions & 8 deletions packages/contracts/contracts/L2/GasPriceOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ contract GasPriceOracle is ISemver {
msg.sender == L1Block(Predeploys.KROMA_L1_BLOCK_ATTRIBUTES).DEPOSITOR_ACCOUNT(),
"GasPriceOracle: only the depositor account can set isKromaMPT flag"
);
require(getKromaMPT() == false, "GasPriceOracle: Kroma MPT already active");
require(isKromaMPT() == false, "GasPriceOracle: Kroma MPT already active");

bytes32 slot = IS_KROMA_MPT_KEY;
assembly {
Expand All @@ -69,12 +69,14 @@ contract GasPriceOracle is ISemver {
}

/// @notice Retrieves the boolean indicates whether the network has gone through the Kroma MPT upgrade.
/// @return isKromaMPT The boolean indicates whether the network has gone through the Kroma MPT upgrade.
function getKromaMPT() public view returns (bool isKromaMPT) {
/// @return The boolean indicates whether the network has gone through the Kroma MPT upgrade.
function isKromaMPT() public view returns (bool) {
bytes32 slot = IS_KROMA_MPT_KEY;
bool kromaMPT;
assembly {
isKromaMPT := sload(slot)
kromaMPT := sload(slot)
}
return kromaMPT;
}

/// @notice Retrieves the current gas price (base fee).
Expand Down Expand Up @@ -108,7 +110,7 @@ contract GasPriceOracle is ISemver {
/// @notice Retrieves the latest known L1 base fee.
/// @return Latest known L1 base fee.
function l1BaseFee() public view returns (uint256) {
if (getKromaMPT()) {
if (isKromaMPT()) {
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).basefee();
} else {
return L1Block(Predeploys.KROMA_L1_BLOCK_ATTRIBUTES).basefee();
Expand All @@ -118,7 +120,7 @@ contract GasPriceOracle is ISemver {
/// @notice Retrieves the current blob base fee.
/// @return Current blob base fee.
function blobBaseFee() public view returns (uint256) {
if (getKromaMPT()) {
if (isKromaMPT()) {
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).blobBaseFee();
} else {
return L1Block(Predeploys.KROMA_L1_BLOCK_ATTRIBUTES).blobBaseFee();
Expand All @@ -128,7 +130,7 @@ contract GasPriceOracle is ISemver {
/// @notice Retrieves the current base fee scalar.
/// @return Current base fee scalar.
function baseFeeScalar() public view returns (uint32) {
if (getKromaMPT()) {
if (isKromaMPT()) {
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).baseFeeScalar();
} else {
return L1Block(Predeploys.KROMA_L1_BLOCK_ATTRIBUTES).baseFeeScalar();
Expand All @@ -138,7 +140,7 @@ contract GasPriceOracle is ISemver {
/// @notice Retrieves the current blob base fee scalar.
/// @return Current blob base fee scalar.
function blobBaseFeeScalar() public view returns (uint32) {
if (getKromaMPT()) {
if (isKromaMPT()) {
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).blobBaseFeeScalar();
} else {
return L1Block(Predeploys.KROMA_L1_BLOCK_ATTRIBUTES).blobBaseFeeScalar();
Expand Down
8 changes: 3 additions & 5 deletions packages/contracts/contracts/test/Encoding.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,10 @@ contract Encoding_Test is CommonTest {
assertEq(txn, _txn, "failed to assert deposit tx");

// assert KromaDepositTx
{
txn = Encoding.encodeDepositTransaction(t, true);
_txn = ffi.encodeDepositTransaction(t, true);
txn = Encoding.encodeDepositTransaction(t, true);
_txn = ffi.encodeDepositTransaction(t, true);

assertEq(txn, _txn, "failed to assert kroma deposit tx");
}
assertEq(txn, _txn, "failed to assert kroma deposit tx");
}
// [Kroma: END]
}
6 changes: 3 additions & 3 deletions packages/contracts/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ export const predeploys = {
ProtocolVault: '0x4200000000000000000000000000000000000006',
KromaL1FeeVault: '0x4200000000000000000000000000000000000007',
ValidatorRewardVault: '0x4200000000000000000000000000000000000008',
SequencerFeeVault: '0x4200000000000000000000000000000000000011',
BaseFeeVault: '0x4200000000000000000000000000000000000019',
L1FeeVault: '0x420000000000000000000000000000000000001A',
L2StandardBridge: '0x4200000000000000000000000000000000000009',
L2ERC721Bridge: '0x420000000000000000000000000000000000000A',
KromaMintableERC20Factory: '0x420000000000000000000000000000000000000B',
KromaMintableERC721Factory: '0x420000000000000000000000000000000000000C',
SequencerFeeVault: '0x4200000000000000000000000000000000000011',
BaseFeeVault: '0x4200000000000000000000000000000000000019',
L1FeeVault: '0x420000000000000000000000000000000000001A',
Create2Deployer: '0x13b0D85CcB8bf860b6b79AF3029fCA081AE9beF2',
}

Expand Down
86 changes: 78 additions & 8 deletions packages/contracts/tasks/check-l2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,28 +413,28 @@ const check = {
await checkProxy(hre, 'ProtocolVault', signer.provider)
await assertProxy(hre, 'ProtocolVault', signer.provider)
},
// L1FeeVault
// KromaL1FeeVault
// - check version
// - check MIN_WITHDRAWAL_AMOUNT
// - check RECIPIENT
L1FeeVault: async (hre: HardhatRuntimeEnvironment, signer: Signer) => {
const L1FeeVault = await hre.ethers.getContractAt(
KromaL1FeeVault: async (hre: HardhatRuntimeEnvironment, signer: Signer) => {
const KromaL1FeeVault = await hre.ethers.getContractAt(
'L1FeeVault',
predeploys.KromaL1FeeVault,
signer
)

await assertSemver(L1FeeVault, 'L1FeeVault')
await assertSemver(KromaL1FeeVault, 'KromaL1FeeVault')

const MIN_WITHDRAWAL_AMOUNT = await L1FeeVault.MIN_WITHDRAWAL_AMOUNT()
const MIN_WITHDRAWAL_AMOUNT = await KromaL1FeeVault.MIN_WITHDRAWAL_AMOUNT()
console.log(` - MIN_WITHDRAWAL_AMOUNT: ${MIN_WITHDRAWAL_AMOUNT}`)

const RECIPIENT = await L1FeeVault.RECIPIENT()
const RECIPIENT = await KromaL1FeeVault.RECIPIENT()
assert(RECIPIENT !== hre.ethers.constants.AddressZero)
yell(` - RECIPIENT: ${RECIPIENT}`)

await checkProxy(hre, 'L1FeeVault', signer.provider)
await assertProxy(hre, 'L1FeeVault', signer.provider)
await checkProxy(hre, 'KromaL1FeeVault', signer.provider)
await assertProxy(hre, 'KromaL1FeeVault', signer.provider)
},
// L2ToL1MessagePasser
// - check version
Expand All @@ -459,6 +459,76 @@ const check = {
await checkProxy(hre, 'L2ToL1MessagePasser', signer.provider)
await assertProxy(hre, 'L2ToL1MessagePasser', signer.provider)
},
// SequencerFeeVault
// - check version
// - check MIN_WITHDRAWAL_AMOUNT
// - check RECIPIENT
SequencerFeeVault: async (hre: HardhatRuntimeEnvironment, signer: Signer) => {
const SequencerFeeVault = await hre.ethers.getContractAt(
'ProtocolVault',
predeploys.SequencerFeeVault,
signer
)

await assertSemver(SequencerFeeVault, 'SequencerFeeVault')

const MIN_WITHDRAWAL_AMOUNT =
await SequencerFeeVault.MIN_WITHDRAWAL_AMOUNT()
console.log(` - MIN_WITHDRAWAL_AMOUNT: ${MIN_WITHDRAWAL_AMOUNT}`)

const RECIPIENT = await SequencerFeeVault.RECIPIENT()
assert(RECIPIENT !== hre.ethers.constants.AddressZero)
yell(` - RECIPIENT: ${RECIPIENT}`)

await checkProxy(hre, 'SequencerFeeVault', signer.provider)
await assertProxy(hre, 'SequencerFeeVault', signer.provider)
},
// BaseFeeVault
// - check version
// - check MIN_WITHDRAWAL_AMOUNT
// - check RECIPIENT
BaseFeeVault: async (hre: HardhatRuntimeEnvironment, signer: Signer) => {
const BaseFeeVault = await hre.ethers.getContractAt(
'ProtocolVault',
predeploys.BaseFeeVault,
signer
)

await assertSemver(BaseFeeVault, 'BaseFeeVault')

const MIN_WITHDRAWAL_AMOUNT = await BaseFeeVault.MIN_WITHDRAWAL_AMOUNT()
console.log(` - MIN_WITHDRAWAL_AMOUNT: ${MIN_WITHDRAWAL_AMOUNT}`)

const RECIPIENT = await BaseFeeVault.RECIPIENT()
assert(RECIPIENT !== hre.ethers.constants.AddressZero)
yell(` - RECIPIENT: ${RECIPIENT}`)

await checkProxy(hre, 'BaseFeeVault', signer.provider)
await assertProxy(hre, 'BaseFeeVault', signer.provider)
},
// L1FeeVault
// - check version
// - check MIN_WITHDRAWAL_AMOUNT
// - check RECIPIENT
L1FeeVault: async (hre: HardhatRuntimeEnvironment, signer: Signer) => {
const L1FeeVault = await hre.ethers.getContractAt(
'L1FeeVault',
predeploys.L1FeeVault,
signer
)

await assertSemver(L1FeeVault, 'L1FeeVault')

const MIN_WITHDRAWAL_AMOUNT = await L1FeeVault.MIN_WITHDRAWAL_AMOUNT()
console.log(` - MIN_WITHDRAWAL_AMOUNT: ${MIN_WITHDRAWAL_AMOUNT}`)

const RECIPIENT = await L1FeeVault.RECIPIENT()
assert(RECIPIENT !== hre.ethers.constants.AddressZero)
yell(` - RECIPIENT: ${RECIPIENT}`)

await checkProxy(hre, 'L1FeeVault', signer.provider)
await assertProxy(hre, 'L1FeeVault', signer.provider)
},
}

task('check-l2', 'Checks a freshly migrated L2 system for correct migration')
Expand Down

0 comments on commit a57d6fd

Please sign in to comment.