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

Hope to support the config which is generated from version: op-contracts/v2.0.0-beta.2 #420

Open
atenjin opened this issue Jul 25, 2024 · 3 comments

Comments

@atenjin
Copy link

atenjin commented Jul 25, 2024

Is your feature request related to a problem? Please describe.
We have try the newest main branch (0fb0dcbefc50882f1bb02fafcb27f47b463875c9) to test whether it can support the chain which is generated by op-contracts/v2.0.0-beta.2
Our chain is using:

  1. custom gas token
  2. op-plasma with KeccakCommitment
  3. isFraudProof is false

Describe the solution you'd like
can let the deploy-config in op-contracts/v2.0.0-beta.2 work well

Describe alternatives you've considered
For now we can only modify local code, but can not pass the ci. And a related issue is:

ethereum-optimism/optimism#11185 (comment)

Additional context
In our test, the issue at least contains:

  1. in getting-started/.deploy address json file, we should add FaultDisputeGame PermissionedDisputeGame in manual, which should contain in the rollup.json when execute the script in op-contracts/v2.0.0-beta.2, so it's a bug in this value
  2. also in this json file, we need to delete AnchorStateRegistry and AnchorStateRegistryProxy which is a bed condition in this repo to judge whether it uses FraudProof
  3. op side advice us to config Eip1559Elasticity to 10, but in check-genesis step, it requires us to set 6:
    image
  4. for custome gas token, when this contract is version op-contracts/v2.0.0-beta.2. :
    result, err := sc.GasPayingToken(&opts)
    if strings.Contains(err.Error(), "execution reverted") {
    // This happens when the SystemConfig contract
    // does not yet have the CGT functionality.
    return nil, nil
    }

    L206 err will be nil, but in L208, the err.Error() will panic due to null pointer
  5. when using op-plasma the rollup check will meet
    image, related to issue When use GenericCommitment in alt-da, some logic is useless or incorrect in op-node optimism#11185 (comment), meanwhile, it's better if the legency field can use old value, rather then must set them to default/0 value
@geoknee
Copy link
Collaborator

geoknee commented Aug 23, 2024

Thanks for raising this issue -- there is a lot to unpack. I will look into this and get back to you!

@geoknee
Copy link
Collaborator

geoknee commented Aug 30, 2024

For point 3:

  • The advice to set to set eip1559Elasticity to 10 may not have been correct. OP Mainnet uses a value of 6.
  • Since Store Optimism Config in config toml files #510, you can freely set this parameter and should not encounter an error when adding the chain

@geoknee
Copy link
Collaborator

geoknee commented Aug 30, 2024

For point 4:

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

No branches or pull requests

3 participants
@geoknee @atenjin and others