-
Notifications
You must be signed in to change notification settings - Fork 42
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
[INT-104] feat: add WasmModuleRoot verification #37
Changes from 4 commits
9758aa7
0a06b2c
a9112db
b4036bc
9ff6a0c
6417a03
f3acabc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
export const WASMModuleRoots = [ | ||
'0xbb9d58e9527566138b682f3a207c0976d5359837f6e330f4017434cca983ff41', // consensus-v1-rc1 | ||
'0x9d68e40c47e3b87a8a7e6368cc52915720a6484bb2f47ceabad7e573e3a11232', // consensus-v2.1 | ||
'0x53c288a0ca7100c0f2db8ab19508763a51c7fd1be125d376d940a65378acaee7', // consensus-v3 | ||
'0x588762be2f364be15d323df2aa60ffff60f2b14103b34823b6f7319acd1ae7a3', // consensus-v3.1 | ||
'0xcfba6a883c50a1b4475ab909600fa88fc9cceed9e3ff6f43dccd2d27f6bd57cf', // consensus-v3.2 | ||
'0xa24ccdb052d92c5847e8ea3ce722442358db4b00985a9ee737c4e601b6ed9876', // consensus-v4 | ||
'0x1e09e6d9e35b93f33ed22b2bc8dc10bbcf63fdde5e8a1fb8cc1bcd1a52f14bd0', // consensus-v5 | ||
'0x3848eff5e0356faf1fc9cafecb789584c5e7f4f8f817694d842ada96613d8bab', // consensus-v6 | ||
'0x53dd4b9a3d807a8cbb4d58fbfc6a0857c3846d46956848cae0a1cc7eca2bb5a8', // consensus-v7 | ||
'0x2b20e1490d1b06299b222f3239b0ae07e750d8f3b4dedd19f500a815c1548bbc', // consensus-v7.1 | ||
'0xd1842bfbe047322b3f3b3635b5fe62eb611557784d17ac1d2b1ce9c170af6544', // consensus-v9 | ||
'0x6b94a7fc388fd8ef3def759297828dc311761e88d8179c7ee8d3887dc554f3c3', // consensus-v10 | ||
'0xda4e3ad5e7feacb817c21c8d0220da7650fe9051ece68a3f0b1c5d38bbb27b21', // consensus-v10.1 | ||
'0x0754e09320c381566cc0449904c377a52bd34a6b9404432e80afd573b67f7b17', // consensus-v10.2 | ||
'0xf559b6d4fa869472dabce70fe1c15221bdda837533dfd891916836975b434dec', // consensus-v10.3 | ||
'0xf4389b835497a910d7ba3ebfb77aa93da985634f3c052de1290360635be40c4a', // consensus-v11 | ||
]; | ||
|
||
export const currentMainnetWasmModuleRootIndex = WASMModuleRoots.indexOf( | ||
'0x5eF0D09d1E6204141B4d37530808eD19f60FBa35', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see we also use the wasmModuleRoot at src/createRollupPrepareConfig.ts, should we create a global constant file for this? @spsjvc |
||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,40 @@ import { | |
} from '../lib/utils'; | ||
import { zeroAddress } from 'viem'; | ||
import { SequencerInbox__factory } from '@arbitrum/sdk/dist/lib/abi/factories/SequencerInbox__factory'; | ||
import { currentMainnetWasmModuleRootIndex, WASMModuleRoots } from '../lib/constants'; | ||
|
||
// Constants | ||
const minConfirmPeriodBlocks = BigInt((24 * 60 * 60) / 12.5); // 1 day | ||
|
||
export const wasmModuleRootHandler = async ( | ||
orbitHandler: OrbitHandler, | ||
rollupAddress: `0x${string}`, | ||
): Promise<string> => { | ||
let warningMessage: string = ''; | ||
|
||
// get the wasmModuleRoot of the given orbit chain | ||
const moduleRoot = (await orbitHandler.readContract( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can add this check inside the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering we can have a separate command to just verify the wasm module root, so I created a new handler here, do you think we don't need to have a separate one? (Because we can also use this to check if they upgrade to our latest arbos version in the future, this is the reason why I created There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a clear reason to have it in a separate command for now. I could be wrong though. If we want to check if a team upgraded to the latest ArbOS version we can run the whole rollup verifier to see if the rest of the parameters are properly set too. In any case, this is not very important now, since there will be changes while we integrate the script in the Orbit SDK, so not 100% sure about this, haha. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, so I will add it to |
||
'parent', | ||
rollupAddress, | ||
[...RollupCore__factory.abi, ...Ownable__factory.abi] as Abi, | ||
'wasmModuleRoot', | ||
)) as `0x${string}`; | ||
|
||
const index = WASMModuleRoots.indexOf(moduleRoot); | ||
|
||
// check if this wasmModuleRoot belongs to one of mainnet version | ||
if (0 <= index) { | ||
console.log('The state transition function is standard version'); | ||
// check if the rollups' arbos version is latest or not | ||
if (index < currentMainnetWasmModuleRootIndex) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following with the last comment, this will never be true since I think currentMainnetWasmModuleRootIndex will always be -1. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sometimes is not -1 is because we just schedule to upgrade but hasn't upgraded |
||
warningMessage = 'Arbos version is old'; | ||
} | ||
} else { | ||
warningMessage = `The node is using a customized state transition function, the wasmModule root is ${moduleRoot}`; | ||
} | ||
return warningMessage; | ||
}; | ||
|
||
export const rollupHandler = async ( | ||
orbitHandler: OrbitHandler, | ||
rollupAddress: `0x${string}`, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import { OrbitHandler } from './lib/client'; | ||
import { wasmModuleRootHandler } from './partial-handlers/rollup'; | ||
import 'dotenv/config'; | ||
|
||
if (!process.env.PARENT_CHAIN_ID || !process.env.ROLLUP_ADDRESS) { | ||
throw new Error( | ||
`The following environmental variables are required: PARENT_CHAIN_ID, ROLLUP_ADDRESS`, | ||
); | ||
} | ||
|
||
// Get the orbit handler | ||
const orbitHandler = new OrbitHandler( | ||
Number(process.env.PARENT_CHAIN_ID), | ||
process.env.ORBIT_CHAIN_ID ? Number(process.env.ORBIT_CHAIN_ID) : undefined, | ||
process.env.ORBIT_CHAIN_RPC ?? undefined, | ||
); | ||
|
||
const main = async () => { | ||
const wasmModuleRootMessage = await wasmModuleRootHandler( | ||
orbitHandler, | ||
process.env.ROLLUP_ADDRESS as `0x${string}`, | ||
); | ||
|
||
console.log(`*****************`); | ||
console.log(`Warning messages:`); | ||
console.log(`*****************`); | ||
if (wasmModuleRootMessage) { | ||
console.log(wasmModuleRootMessage); | ||
} else { | ||
console.log(`No messages`); | ||
} | ||
}; | ||
|
||
// Calling main | ||
main() | ||
.then(() => process.exit(0)) | ||
.catch((error) => { | ||
console.error(error); | ||
process.exit(1); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this hash correct? I don't see it in WASMModuleRoots, so currentMainnetWasmModuleRootIndex will always be
-1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it is v10 now, sometimes is not -1 is because we just schedule to upgrade but hasn't upgraded: https://github.com/OffchainLabs/nitro/blob/2142d5f4d2e3d967535da8edd0f071803caa0375/Dockerfile#L152
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the hash is not correct, ty, will change it