feat: update wasm module root for consensus v51.1#644
Conversation
| } | ||
| } | ||
|
|
||
| if (!chainIsArbitrum(chainId) && isBlacklistedWasmModuleRoot(wasmModuleRoot)) { |
There was a problem hiding this comment.
We don't need the chainIsArbitrum check, since this is only for deployment of brand new chains. It will never evaluate to true
There was a problem hiding this comment.
Indeed. Should we however prevent upgrades to one of these wasm roots ? Prod arbitrum chains won't ever upgrade to these older wasm roots but what about internal tests ?
There was a problem hiding this comment.
We could cover the upgrade path in another PR once we start working on upgrades. I wouldn't worry about it too much as only RaaS-es might do so. But they also wait on a heads-up from us before upgrading so I think we are ok
src/wasmModuleRoot.ts
Outdated
| }, | ||
| ] as const satisfies readonly ConsensusRelease[]; | ||
|
|
||
| const blacklistedConsensusReleases = [ |
There was a problem hiding this comment.
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.
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.
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
alxdca
force-pushed
the
chore/bump-wasmroot
branch
from
b9bb5b5 to
21f427c
Compare
| ); | ||
| } | ||
|
|
||
| const chainId = params.config.chainId; |
There was a problem hiding this comment.
I don't think we're using this anymore
src/wasmModuleRoot.ts
Outdated
| // https://github.com/OffchainLabs/nitro/releases/tag/consensus-v51.1 | ||
| version: 51.1, | ||
| wasmModuleRoot: '0x28b6ad83ed87b21a87c73f7a0296a135ebc7074e449efb289ececccad771ccd6', | ||
| maxArbOSVersion: 51.1, |
There was a problem hiding this comment.
this is actually an error from before but we might as well fix it here
| maxArbOSVersion: 51.1, | |
| maxArbOSVersion: 51, |
src/wasmModuleRoot.ts
Outdated
| // https://github.com/OffchainLabs/nitro/releases/tag/consensus-v51.1 | ||
| version: 51.1, | ||
| wasmModuleRoot: '0xc2c02df561d4afaf9a1d6785f70098ec3874765c638e3cb6dbe8d3c83333e14c', | ||
| maxArbOSVersion: 51.1, |
There was a problem hiding this comment.
same here
| maxArbOSVersion: 51.1, | |
| maxArbOSVersion: 51, |
spsjvc
changed the title
2 participants
This PR bumps the wasm root to 0xc2c02df561d4afaf9a1d6785f70098ec3874765c638e3cb6dbe8d3c83333e14c (consensus v51.1).
It also adds guards to prevent users from using the following wasm roots:
The guards are skipped if the chain is any arbitrum chain.