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.

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

Add Lisk Mainnet to the Superchain Registry #502

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

matjazv
Copy link

@matjazv matjazv commented Aug 22, 2024

Adding a New Chain

This PR adds Lisk Mainnet to the registry.

Checklist

  • I have declared the chain at the appropriate Superchain Level.
  • I have run just validate 1135 locally to ensure all local validation checks pass.
  • I have run just codegen to ensure that the chainlist and other generated files are up-to-date and include my chain.

Additional Information

To enable the just add-chain and just codegen commands to function correctly, I had to make a few changes:

  1. op-geth (config.go)
  • Adding Lisk Mainnet chain ID into the constants:
    LiskMainnetChainID = 1135
  • The same will eventually apply to Lisk Sepolia.
  1. op-geth (superchain.go):
  • The Lisk Mainnet network has custom values for the following variables, necessitating a new case statement:
case LiskMainnetChainID:
    out.Optimism.EIP1559Elasticity = 20
    out.Optimism.EIP1559Denominator = 1000
    out.Optimism.EIP1559DenominatorCanyon = newUint64(1000)
  • The same will eventually apply to Lisk Sepolia.
  1. check-genesis.go:
  • Currently, EIP1559DenominatorCanyon is hardcoded to 250, which does not align with Lisk Mainnet (and Sepolia).
  • For now, I have commented out these two lines.

Additionally, the just validate command also fails for some of the unit tests.

@matjazv matjazv requested review from a team as code owners August 22, 2024 07:45
@sbvegan sbvegan added the M-new-chain-request Meta: New Chain Request label Aug 22, 2024
@sbvegan sbvegan self-assigned this Aug 22, 2024
@sbvegan sbvegan requested a review from geoknee August 23, 2024 14:12
@geoknee geoknee added the F-do-not-merge Flag: Do Not Merge label Aug 28, 2024
@matjazv
Copy link
Author

matjazv commented Sep 10, 2024

With all recent updates, changes inside Additional Information are not needed anymore to successfully execute just add-chain command.

@geoknee
Copy link
Collaborator

geoknee commented Sep 10, 2024

With all recent updates, changes inside Additional Information are not needed anymore to successfully execute just add-chain command.

Excellent!

Comment on lines 19 to 28
"baseFeeVaultRecipient": "0xdA6e5640aFB2ED212Ba3a6fd83076e2ad3daD185",
"l1FeeVaultRecipient": "0xdA6e5640aFB2ED212Ba3a6fd83076e2ad3daD185",
"sequencerFeeVaultRecipient": "0xdA6e5640aFB2ED212Ba3a6fd83076e2ad3daD185",
"baseFeeVaultMinimumWithdrawalAmount": "0x8ac7230489e80000",
"l1FeeVaultMinimumWithdrawalAmount": "0x8ac7230489e80000",
"sequencerFeeVaultMinimumWithdrawalAmount": "0x8ac7230489e80000",
"baseFeeVaultWithdrawalNetwork": 0,
"l1FeeVaultWithdrawalNetwork": 0,
"sequencerFeeVaultWithdrawalNetwork": 0,
"proxyAdminOwner": "0xdA6e5640aFB2ED212Ba3a6fd83076e2ad3daD185",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please double check these changes? The automatic genesis validation check validate-genesis-allocs uses this data, and the existing values were enough to make it pass, the new values cause it to fail.

Copy link
Author

Choose a reason for hiding this comment

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

I have confirmed the changes with our RaaS provider, which were made when ownership of the chain was transferred from the deployer EOA to a multisig. The values were also adjusted (decreased) for fee withdrawals. As a result, the new values are correct.

This was not the case for Lisk Sepolia so genesis validation check passes inside its PR.

Copy link
Author

Choose a reason for hiding this comment

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

All those changes were made around 22nd July 2024.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it. For the genesis validation routine, we need the deploy-config as it was at the time the original genesis was generated, we don't actually care about changes that happened after that in this context. So best to revert these changes (the deploy-config.json won't be used for anything else).

Copy link
Author

Choose a reason for hiding this comment

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

I've reverted the changes inside deploy-config.json file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-do-not-merge Flag: Do Not Merge M-new-chain-request Meta: New Chain Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants