-
Notifications
You must be signed in to change notification settings - Fork 73
feat: update wasm module root for consensus v51.1 #644
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
Changes from 1 commit
380ba0e
c18bbd5
21f427c
31f8a6b
e8acc91
5171df0
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,15 @@ | ||
| import * as chains from './chains'; | ||
|
|
||
| export function chainIsArbitrum(chainId: bigint): boolean { | ||
| switch (chainId) { | ||
| case BigInt(chains.arbitrumOne.id): | ||
| case BigInt(chains.arbitrumNova.id): | ||
| case BigInt(chains.arbitrumSepolia.id): | ||
| case BigInt(chains.nitroTestnodeL2.id): | ||
| case BigInt(chains.nitroTestnodeL3.id): | ||
| return true; | ||
|
|
||
| default: | ||
| return false; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,8 @@ import { ChainConfig } from './types/ChainConfig'; | |
| import { getRollupCreatorAddress } from './utils/getRollupCreatorAddress'; | ||
| import { fetchDecimals } from './utils/erc20'; | ||
| import { TransactionRequestGasOverrides, applyPercentIncrease } from './utils/gasOverrides'; | ||
| import { isBlacklistedWasmModuleRoot } from './wasmModuleRoot'; | ||
| import { chainIsArbitrum } from './chainIsArbitrum'; | ||
|
|
||
| import { | ||
| CreateRollupParams, | ||
|
|
@@ -126,6 +128,8 @@ export async function createRollupPrepareTransactionRequest<TChain extends Chain | |
| ); | ||
| } | ||
|
|
||
| const chainId = params.config.chainId; | ||
|
|
||
| if (isKnownWasmModuleRoot(wasmModuleRoot)) { | ||
| const consensusRelease = getConsensusReleaseByWasmModuleRoot(wasmModuleRoot); | ||
|
|
||
|
|
@@ -136,6 +140,12 @@ export async function createRollupPrepareTransactionRequest<TChain extends Chain | |
| } | ||
| } | ||
|
|
||
| if (!chainIsArbitrum(chainId) && isBlacklistedWasmModuleRoot(wasmModuleRoot)) { | ||
|
||
| throw new Error( | ||
| `Wasm module root ${wasmModuleRoot} is not supported. Please update your "wasmModuleRoot" to that of a Consensus version compatible with ArbOS ${arbOSVersion}.`, | ||
| ); | ||
| } | ||
|
|
||
| const paramsWithDefaults = { ...defaults, ...params, maxDataSize }; | ||
| const createRollupGetCallValueParams = { ...paramsWithDefaults, account }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -81,6 +81,19 @@ const consensusReleases = [ | |||||
| }, | ||||||
| { | ||||||
| // https://github.com/OffchainLabs/nitro/releases/tag/consensus-v51.1 | ||||||
| version: 51.1, | ||||||
| wasmModuleRoot: '0xc2c02df561d4afaf9a1d6785f70098ec3874765c638e3cb6dbe8d3c83333e14c', | ||||||
| maxArbOSVersion: 51.1, | ||||||
|
||||||
| maxArbOSVersion: 51.1, | |
| maxArbOSVersion: 51, |
Outdated
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.
Maybe we can add an optional disabled?: boolean property on the ConsensusRelease type that we only add to those that we do not allow. Then the check in src/createRollupPrepareTransactionRequest.ts would look something like this:
if (wasmModuleRoot.disabled) {
throw new Error();
}
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.
Yes, that's definitely cleaner. However getConsensusReleaseByVersion needs a refactor in that case since we may have multiple wasm roots for a single consensus version, as we have with 51.1. I'll have a look at it.
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.
Ah, I see what you mean. I think the 51.1 situation is unique and we hopefully won't have a similar one in the future. We could just add a special case in getConsensusReleaseByVersion for 51.1 to return the proper one, and for the standard case just find by version
Outdated
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.
same here
| maxArbOSVersion: 51.1, | |
| maxArbOSVersion: 51, |
Uh oh!
There was an error while loading. Please reload this page.
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.
I don't think we're using this anymore