8000 Install Registry at Randao hardfork by ian0371 · Pull Request #2059 · klaytn/klaytn · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Aug 19, 2024. It is now read-only.

Install Registry at Randao hardfork #2059

Merged
merged 4 commits into from
Dec 7, 2023

Conversation

ian0371
Copy link
Contributor
@ian0371 ian0371 commented Dec 6, 2023

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, at randao 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,

GetBlsPubkey(num, ...):
	fetch kip113Addr from registry at state{num}

VerifyRandao(header, ...):
	GetBlsPubkey(header.Number - 1)

In this PR,

GetBlsPubkey(num, ...):
	if num == hf: fetch kip113Addr from ChainConfig
	if num > hf: fetch kip113Addr from registry at state{num-1}
 
VerifyRandao(header, ...):
	GetBlsPubkey(header.Number)

Note that the actual change is in getAllCached, not GetBlsPubkey.

Types of changes

Please put an x in the boxes related to your change.

  • Bugfix
  • New feature or enhancement
  • Others

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.

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

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 the randaoCompatibleBlock to postpone the hardfork.

@ian0371 ian0371 marked this pull request as draft December 6, 2023 07:49
@ian0371 ian0371 marked this pull request as ready for review December 6, 2023 09:03
@ian0371 ian0371 requested a review from blukat29 December 6, 2023 09:13
@hyunsooda
Copy link
Contributor

To make sure that the chain config has proper record values, its setting must be invoked locally(implicitly) before Randao number. Is it correct?

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")
Copy link
Member

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
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
}

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Member
@aidan-kwon aidan-kwon left a 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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0