-
Notifications
You must be signed in to change notification settings - Fork 182
Conversation
To make sure that the chain config has proper record values, its setting must be invoked locally(implicitly) before Randao number. Is it correct? |
} 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
} 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I wasn't sure what would happen when this function is invoked before Randao.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an minor comment, but it is good to go
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,
randaoRegistry
field in ChainConfig is set before the hardfork pass.In unexpected scenario,
randaoRegistry
field in ChainConfig is not set at the hardfork, so the operator updates therandaoCompatibleBlock
to postpone the hardfork.