Skip to content

chore(alpha-testnet)!: apply version -> rollup_version#13180

Merged
Maddiaa0 merged 3 commits intoalpha-testnetfrom
md/alpha-testnet-version
Apr 1, 2025
Merged

chore(alpha-testnet)!: apply version -> rollup_version#13180
Maddiaa0 merged 3 commits intoalpha-testnetfrom
md/alpha-testnet-version

Conversation

@Maddiaa0
Copy link
Copy Markdown
Member

apply #13145 to alpha testnet

@Maddiaa0 Maddiaa0 changed the title Md/alpha testnet version chore!(alpha-testnet): apply version -> rollup_version Mar 31, 2025
@Maddiaa0 Maddiaa0 changed the title chore!(alpha-testnet): apply version -> rollup_version chore(alpha-testnet)!: apply version -> rollup_version Mar 31, 2025
curl -s -X POST -H 'content-type: application/json' \
-d '{"jsonrpc":"2.0","method":"pxe_getNodeInfo","params":[],"id":67}' \
127.0.0.1:{{ .Values.pxe.service.nodePort }} | grep -q '"protocolVersion":1'
127.0.0.1:{{ .Values.pxe.service.nodePort }} | grep -q '"rollupVersion":1'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've just realised: isn't this readiness probe going to cause the pxe to always be flagged as fail if we ever increase it from 1? Shouldn't we unhardcode that value?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, this is addressed in lasse's pr -- but will apply here too

Comment on lines +68 to +69
const [versionVersion, l1ChainId, l1RollupAddress, rollupVersion, l2ProtocolContractsTreeRoot, l2CircuitsVkTreeRoot] =
compressed.split('-');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't the rollup address imply the rollup version? If we're including the rollup address in here, couldn't we remove the rollup version?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the change made in #13171, only the rollupVersion should be needed since the rollup address, l1 chain id and genesis state is hashed together to get the compute it 👀

@Maddiaa0 Maddiaa0 force-pushed the md/alpha-testnet-version branch from 2ab2e16 to b31a57b Compare April 1, 2025 09:58
@Maddiaa0 Maddiaa0 force-pushed the md/alpha-testnet-version branch from 3330459 to a48e2e1 Compare April 1, 2025 10:52
@Maddiaa0 Maddiaa0 merged commit 43eff55 into alpha-testnet Apr 1, 2025
5 checks passed
@Maddiaa0 Maddiaa0 deleted the md/alpha-testnet-version branch April 1, 2025 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants