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.

Sign up for GitHub

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

refactor: move hardcoded value to config #270

Merged
merged 13 commits into from
Dec 10, 2024
Merged

refactor: move hardcoded value to config #270

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1d091e2
refactor: move hardcoded value to config
gzeoneth Nov 26, 2024
fa91157
refactor: minimumAssertionPeriod and validatorAfkBlocks in bold upgra…
gzeoneth Nov 26, 2024
9e6fa38
perf: remove onchain verification
gzeoneth Dec 2, 2024
d709ef5
chore: update sigs
gzeoneth Dec 2, 2024
19c507d
Merge branch 'bold-merge' into move-to-config
gzeoneth Dec 9, 2024
7cafdae
refactor: remove hardcode in rollup initialize
gzeoneth Dec 9, 2024
f4d06c0
chore: update example
gzeoneth Dec 9, 2024
ac1fdda
Merge branch 'bold-merge' into move-to-config
gzeoneth Dec 9, 2024
da85e5a
test: new config param
gzeoneth Dec 9, 2024
8715541
fix: format test
gzeoneth Dec 9, 2024
be1a90b
Merge branch 'bold-merge' into move-to-config
gzeoneth Dec 9, 2024
760bca6
test: fix config
gzeoneth Dec 9, 2024
3782696
Apply suggestions from code review
gzeoneth Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions scripts/boldUpgradeCommon.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ export interface Config {
stakeAmt: BigNumber
miniStakeAmounts: BigNumber[]
chainId: number
minimumAssertionPeriod: number
validatorAfkBlocks: number
disableValidatorWhitelist: boolean
maxDataSize: number
blockLeafSize: number
Expand Down
4 changes: 4 additions & 0 deletions scripts/config.ts.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ export const config = {
owner: '0x1234123412341234123412341234123412341234',
loserStakeEscrow: ethers.constants.AddressZero,
chainId: ethers.BigNumber.from('13331370'),
minimumAssertionPeriod: 75,
validatorAfkBlocks: 201600,
chainConfig:
'{"chainId":13331370,"homesteadBlock":0,"daoForkBlock":null,"daoForkSupport":true,"eip150Block":0,"eip150Hash":"0x0000000000000000000000000000000000000000000000000000000000000000","eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"berlinBlock":0,"londonBlock":0,"clique":{"period":0,"epoch":0},"arbitrum":{"EnableArbOS":true,"AllowDebugPrecompiles":false,"DataAvailabilityCommittee":false,"InitialArbOSVersion":10,"InitialChainOwner":"0x1234123412341234123412341234123412341234","GenesisBlockNum":0}}',
minimumAssertionPeriod: ethers.BigNumber.from('75'),
validatorAfkBlocks: ethers.BigNumber.from('201600'),
gzeoneth marked this conversation as resolved.
Show resolved Hide resolved
genesisBlockNum: ethers.BigNumber.from('0'),
sequencerInboxMaxTimeVariation: {
delayBlocks: ethers.BigNumber.from('7200'),
Expand Down
2 changes: 2 additions & 0 deletions scripts/files/configs/arb1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export const arb1: Config = {
stakeAmt: parseEther('3600'),
miniStakeAmounts: [parseEther('0'), parseEther('555'), parseEther('79')],
chainId: 42161,
minimumAssertionPeriod: 75,
validatorAfkBlocks: 201600,
disableValidatorWhitelist: true,
blockLeafSize: 2 ** 26,
bigStepLeafSize: 2 ** 19,
Expand Down
2 changes: 2 additions & 0 deletions scripts/files/configs/local.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export const local: Config = {
parseEther('1'),
],
chainId: 412346,
minimumAssertionPeriod: 0,
validatorAfkBlocks: 201600,
disableValidatorWhitelist: true,
blockLeafSize: 1048576,
bigStepLeafSize: 512,
Expand Down
2 changes: 2 additions & 0 deletions scripts/files/configs/nova.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export const nova: Config = {
stakeAmt: parseEther('1'),
miniStakeAmounts: [parseEther('0'), parseEther('1'), parseEther('1')],
chainId: 42170,
minimumAssertionPeriod: 75,
validatorAfkBlocks: 201600,
disableValidatorWhitelist: false,
blockLeafSize: 2 ** 26, // leaf sizes same as arb1
bigStepLeafSize: 2 ** 19,
Expand Down
2 changes: 2 additions & 0 deletions scripts/files/configs/sepolia.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export const sepolia: Config = {
stakeAmt: parseEther('36'), // 1/100th of arb1, same for mini stakes
miniStakeAmounts: [parseEther('0'), parseEther('5.5'), parseEther('0.79')],
chainId: 421614,
minimumAssertionPeriod: 75,
validatorAfkBlocks: 201600,
disableValidatorWhitelist: false,
blockLeafSize: 2 ** 26, // leaf sizes same as arb1
bigStepLeafSize: 2 ** 19,
Expand Down
2 changes: 2 additions & 0 deletions scripts/rollupCreation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ async function _getDevRollupConfig(
loserStakeEscrow: ethers.constants.AddressZero,
chainId: JSON.parse(chainConfig)['chainId'],
chainConfig: chainConfig,
minimumAssertionPeriod: 75,
gzeoneth marked this conversation as resolved.
Show resolved Hide resolved
validatorAfkBlocks: 201600,
genesisAssertionState: {}, // AssertionState
genesisInboxCount: 0,
miniStakeValues: [
Expand Down
28 changes: 9 additions & 19 deletions src/rollup/BOLDUpgradeAction.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,9 @@ contract BOLDUpgradeAction {
address public immutable STAKE_TOKEN;
uint256 public immutable STAKE_AMOUNT;
uint256 public immutable CHAIN_ID;
uint256 public immutable MINIMUM_ASSERTION_PERIOD;
uint64 public immutable VALIDATOR_AFK_BLOCKS;

bool public immutable DISABLE_VALIDATOR_WHITELIST;
uint64 public immutable CHALLENGE_GRACE_PERIOD_BLOCKS;
address public immutable MINI_STAKE_AMOUNTS_STORAGE;
Expand Down Expand Up @@ -223,6 +226,8 @@ contract BOLDUpgradeAction {
uint256 stakeAmt;
uint256[] miniStakeAmounts;
uint256 chainId;
uint256 minimumAssertionPeriod;
uint64 validatorAfkBlocks;
bool disableValidatorWhitelist;
uint256 blockLeafSize;
uint256 bigStepLeafSize;
Expand Down Expand Up @@ -296,6 +301,8 @@ contract BOLDUpgradeAction {
IMPL_CHALLENGE_MANAGER = implementations.challengeManager;

CHAIN_ID = settings.chainId;
MINIMUM_ASSERTION_PERIOD = settings.minimumAssertionPeriod;
VALIDATOR_AFK_BLOCKS = settings.validatorAfkBlocks;
CONFIRM_PERIOD_BLOCKS = settings.confirmPeriodBlocks;
CHALLENGE_PERIOD_BLOCKS = settings.challengePeriodBlocks;
STAKE_TOKEN = settings.stakeToken;
Expand Down Expand Up @@ -373,6 +380,8 @@ contract BOLDUpgradeAction {
loserStakeEscrow: EXCESS_STAKE_RECEIVER, // additional funds get sent to the l1 timelock
chainId: CHAIN_ID,
chainConfig: "", // we can use an empty chain config it wont be used in the rollup initialization because we check if the rei is already connected there
minimumAssertionPeriod: MINIMUM_ASSERTION_PERIOD,
validatorAfkBlocks: VALIDATOR_AFK_BLOCKS,
miniStakeValues: ConstantArrayStorage(MINI_STAKE_AMOUNTS_STORAGE).array(),
sequencerInboxMaxTimeVariation: maxTimeVariation,
layerZeroBlockEdgeHeight: BLOCK_LEAF_SIZE,
Expand Down Expand Up @@ -433,17 +442,6 @@ contract BOLDUpgradeAction {
PROXY_ADMIN_SEQUENCER_INBOX.upgrade(sequencerInbox, IMPL_SEQUENCER_INBOX);
}

// verify
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there manual checklist somewhere that we could add these to?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not yet but we can make something

require(
PROXY_ADMIN_SEQUENCER_INBOX.getProxyImplementation(sequencerInbox)
== IMPL_SEQUENCER_INBOX,
"DelayBuffer: new seq inbox implementation not set"
);
require(
ISequencerInbox(SEQ_INBOX).isDelayBufferable() == IS_DELAY_BUFFERABLE,
"DelayBuffer: isDelayBufferable not set"
);

(uint256 delayBlocks, uint256 futureBlocks, uint256 delaySeconds, uint256 futureSeconds) =
ISequencerInbox(SEQ_INBOX).maxTimeVariation();

Expand All @@ -465,14 +463,6 @@ contract BOLDUpgradeAction {
})
);

// verify
(uint256 _delayBlocks, uint256 _futureBlocks, uint256 _delaySeconds, uint256 _futureSeconds)
= ISequencerInbox(SEQ_INBOX).maxTimeVariation();
require(_delayBlocks == delayBlocks, "DelayBuffer: delayBlocks");
require(_delaySeconds == delaySeconds, "DelayBuffer: delaySeconds");
require(_futureBlocks == futureBlocks, "DelayBuffer: futureBlocks");
require(_futureSeconds == futureSeconds, "DelayBuffer: futureSeconds");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have to remove these checks for contract code size, I don't think there are too much risk but arguable

ISequencerInbox(SEQ_INBOX).updateRollupAddress();
}

Expand Down
2 changes: 2 additions & 0 deletions src/rollup/Config.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ struct Config {
address loserStakeEscrow;
uint256 chainId;
string chainConfig;
uint256 minimumAssertionPeriod;
uint64 validatorAfkBlocks;
uint256[] miniStakeValues;
ISequencerInbox.MaxTimeVariation sequencerInboxMaxTimeVariation;
uint256 layerZeroBlockEdgeHeight;
Expand Down
8 changes: 4 additions & 4 deletions src/rollup/RollupAdminLogic.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ contract RollupAdminLogic is RollupCore, IRollupAdmin, DoubleLogicUUPSUpgradeabl
chainId = config.chainId;
baseStake = config.baseStake;
wasmModuleRoot = config.wasmModuleRoot;
// A little over 15 minutes
minimumAssertionPeriod = 75;
// ValidatorAfkBlocks is defaulted to 28 days assuming a 12 seconds block time.
// minimumAssertionPeriod was defaulted to 75 which is a little over 15 minutes
minimumAssertionPeriod = config.minimumAssertionPeriod;
// ValidatorAfkBlocks was defaulted to 201600 which is 28 days assuming a 12 seconds block time.
// Since it can take 14 days under normal circumstances to confirm an assertion, this means
// the validators will have been inactive for a further 14 days before the whitelist is removed.
validatorAfkBlocks = 201600;
validatorAfkBlocks = config.validatorAfkBlocks;
challengeGracePeriodBlocks = config.challengeGracePeriodBlocks;

// loser stake is now sent directly to loserStakeEscrow, it must not
Expand Down
2 changes: 2 additions & 0 deletions test/Rollup.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ contract RollupTest is Test {
baseStake: BASE_STAKE,
chainId: 0,
chainConfig: "{}",
minimumAssertionPeriod: 75,
validatorAfkBlocks: 201600,
confirmPeriodBlocks: uint64(CONFIRM_PERIOD_BLOCKS),
owner: owner,
sequencerInboxMaxTimeVariation: ISequencerInbox.MaxTimeVariation({
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/orbitChain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,8 @@ describe('Orbit Chain', () => {
chainId: ethers.BigNumber.from('433333'),
chainConfig:
'{"chainId":433333,"homesteadBlock":0,"daoForkBlock":null,"daoForkSupport":true,"eip150Block":0,"eip150Hash":"0x0000000000000000000000000000000000000000000000000000000000000000","eip155Block":0,"eip158Block":0,"byzantiumBlock":0,"constantinopleBlock":0,"petersburgBlock":0,"istanbulBlock":0,"muirGlacierBlock":0,"berlinBlock":0,"londonBlock":0,"clique":{"period":0,"epoch":0},"arbitrum":{"EnableArbOS":true,"AllowDebugPrecompiles":false,"DataAvailabilityCommittee":false,"InitialArbOSVersion":10,"InitialChainOwner":"0x72f7EEedF02C522242a4D3Bdc8aE6A8583aD7c5e","GenesisBlockNum":0}}',
minimumAssertionPeriod: 75,
validatorAfkBlocks: 201600,
genesisAssertionState: genesisAssertionState, // AssertionState
genesisInboxCount: 0,
miniStakeValues: [
Expand Down
27 changes: 23 additions & 4 deletions test/foundry/RollupCreator.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ contract RollupCreatorTest is Test {
baseStake: 1000,
chainId: 1337,
chainConfig: "abc",
confirmPeriodBlocks: 20,
minimumAssertionPeriod: 75,
validatorAfkBlocks: 1234,
confirmPeriodBlocks: 567,
owner: rollupOwner,
sequencerInboxMaxTimeVariation: timeVars,
stakeToken: address(token),
Expand Down Expand Up @@ -184,6 +186,12 @@ contract RollupCreatorTest is Test {
batchPosterManager,
"Invalid batch poster manager"
);
assertEq(
rollup.validatorAfkBlocks(), config.validatorAfkBlocks, "Invalid validatorAfkBlocks"
);
assertEq(
rollup.confirmPeriodBlocks(), config.confirmPeriodBlocks, "Invalid confirmPeriodBlocks"
);

// check proxy admin for non-rollup contracts
address proxyAdminExpectedAddress = computeCreateAddress(address(rollupCreator), 1);
Expand Down Expand Up @@ -285,7 +293,9 @@ contract RollupCreatorTest is Test {
baseStake: 1000,
chainId: 1337,
chainConfig: "abc",
confirmPeriodBlocks: 20,
minimumAssertionPeriod: 75,
validatorAfkBlocks: 1234,
confirmPeriodBlocks: 567,
owner: rollupOwner,
sequencerInboxMaxTimeVariation: timeVars,
stakeToken: address(token),
Expand Down Expand Up @@ -332,11 +342,12 @@ contract RollupCreatorTest is Test {
vm.stopPrank();

_postCreateERC20RollupChecks(
rollupAddress, batchPosterManager, nativeToken, validators, batchPosters
config, rollupAddress, batchPosterManager, nativeToken, validators, batchPosters
);
}

function _postCreateERC20RollupChecks(
Config memory config,
address rollupAddress,
address batchPosterManager,
address nativeToken,
Expand Down Expand Up @@ -371,6 +382,12 @@ contract RollupCreatorTest is Test {
batchPosterManager,
"Invalid batch poster manager"
);
assertEq(
rollup.validatorAfkBlocks(), config.validatorAfkBlocks, "Invalid validatorAfkBlocks"
);
assertEq(
rollup.confirmPeriodBlocks(), config.confirmPeriodBlocks, "Invalid confirmPeriodBlocks"
);

// native token check
IBridge bridge = RollupCore(address(rollupAddress)).bridge();
Expand Down Expand Up @@ -456,7 +473,9 @@ contract RollupCreatorTest is Test {
baseStake: 1000,
chainId: 1337,
chainConfig: "abc",
confirmPeriodBlocks: 20,
minimumAssertionPeriod: 75,
validatorAfkBlocks: 1234,
confirmPeriodBlocks: 567,
owner: rollupOwner,
sequencerInboxMaxTimeVariation: timeVars,
stakeToken: address(token),
Expand Down
2 changes: 1 addition & 1 deletion test/signatures/RollupAdminLogic
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"getStakerAddress(uint64)": "6ddd3744",
"getValidators()": "b7ab4db5",
"inbox()": "fb0e722b",
"initialize((uint64,address,uint256,bytes32,address,address,uint256,string,uint256[],(uint256,uint256,uint256,uint256),uint256,uint256,uint256,((bytes32[2],uint64[2]),uint8,bytes32),uint256,address,uint8,uint64,(uint64,uint64,uint64)),(address,address,address,address,address,address,address,address,address))": "9e7e6aa7",
"initialize((uint64,address,uint256,bytes32,address,address,uint256,string,uint256,uint64,uint256[],(uint256,uint256,uint256,uint256),uint256,uint256,uint256,((bytes32[2],uint64[2]),uint8,bytes32),uint256,address,uint8,uint64,(uint64,uint64,uint64)),(address,address,address,address,address,address,address,address,address))": "0ee5ef0c",
"isFirstChild(bytes32)": "30836228",
"isPending(bytes32)": "e531d8c7",
"isStaked(address)": "6177fd18",
Expand Down
2 changes: 1 addition & 1 deletion test/signatures/RollupCreator
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"bridgeCreator()": "f860cefa",
"challengeManagerTemplate()": "9c683d10",
"createRollup(((uint64,address,uint256,bytes32,address,address,uint256,string,uint256[],(uint256,uint256,uint256,uint256),uint256,uint256,uint256,((bytes32[2],uint64[2]),uint8,bytes32),uint256,address,uint8,uint64,(uint64,uint64,uint64)),address[],uint256,address,bool,uint256,address[],address))": "5491abea",
"createRollup(((uint64,address,uint256,bytes32,address,address,uint256,string,uint256,uint64,uint256[],(uint256,uint256,uint256,uint256),uint256,uint256,uint256,((bytes32[2],uint64[2]),uint8,bytes32),uint256,address,uint8,uint64,(uint64,uint64,uint64)),address[],uint256,address,bool,uint256,address[],address))": "a2f454fc",
"l2FactoriesDeployer()": "ac0425bc",
"osp()": "f26a62c6",
"owner()": "8da5cb5b",
Expand Down
2 changes: 2 additions & 0 deletions test/stakingPool/AssertionStakingPool.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ contract AssertionPoolTest is Test {
baseStake: BASE_STAKE,
chainId: 0,
chainConfig: "{}",
minimumAssertionPeriod: 75,
validatorAfkBlocks: 201600,
confirmPeriodBlocks: uint64(CONFIRM_PERIOD_BLOCKS),
owner: owner,
sequencerInboxMaxTimeVariation: ISequencerInbox.MaxTimeVariation({
Expand Down
Loading