This repository was archived by the owner on Aug 19, 2024. It is now read-only.
Install Registry at Randao hardfork #2059
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
blukat29
reviewed
Dec 6, 2023
blukat29
approved these changes
Dec 6, 2023
hyeonLewis
approved these changes
Dec 6, 2023
|
To make sure that the chain config has proper record values, its setting must be invoked locally(implicitly) before Randao number. Is it correct? |
hyunsooda
approved these changes
Dec 7, 2023
yoomee1313
approved these changes
Dec 7, 2023
aidan-kwon
reviewed
Dec 7, 2023
Comment on lines
+79
to
+86
| } else if chain.Config().IsRandaoForkEnabled(num) { | ||
| var err error | ||
| kip113Addr, err = system.ReadRegistryActiveAddr(backend, system.Kip113Name, parentNum) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } else { | ||
| return nil, errors.New("Cannot read KIP113 address from registry before Randao fork") |
There was a problem hiding this comment.
ChainBlsPubkeyProvider should be used only after Randao HF, Randao-disabled condition is not needed
Prerequisites of Randao Hardfork
- BLS contract (KIP113) should be deployed
- Validators information should be registered on the BLS contract
- Randao registry should be specified with the KIP113 contract address
Suggested change
| } else if chain.Config().IsRandaoForkEnabled(num) { | |
| var err error | |
| kip113Addr, err = system.ReadRegistryActiveAddr(backend, system.Kip113Name, parentNum) | |
| if err != nil { | |
| return nil, err | |
| } | |
| } else { | |
| return nil, errors.New("Cannot read KIP113 address from registry before Randao fork") | |
| } else { | |
| var err error | |
| kip113Addr, err = system.ReadRegistryActiveAddr(backend, system.Kip113Name, parentNum) | |
| if err != nil { | |
| return nil, err | |
| } |
There was a problem hiding this comment.
You're right. I wasn't sure what would happen when this function is invoked before Randao.
There was a problem hiding this comment.
If the function is called before Randao HF, it will fail at system.ReadRegistryActiveAddr() because the registry is registered at Randao HF. It is not an important issue, though.
aidan-kwon
approved these changes
Dec 7, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
To facilitate the activation of Randao hardfork and Registry installation at the same block, this PR installs the Registry contact at
randao hardfork(previously, atrandao hardfork - 1).It necessitates reading KIP113 address from ChainConfig for the verification of the Randao hardfork block because during the verification, the Registry does not exist at the latest block.
In short, previously,
In this PR,
Note that the actual change is in
getAllCached, notGetBlsPubkey.Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
$ make test)Further comments
Tested that hardfork block passes in the expected scenario and unexpected scenario.
In expected scenario,
randaoRegistryfield in ChainConfig is set before the hardfork pass.In unexpected scenario,
randaoRegistryfield in ChainConfig is not set at the hardfork, so the operator updates therandaoCompatibleBlockto postpone the hardfork.