8000 Add block number parameter to governance.chainConfigAt() API by ian0371 · Pull Request #1704 · 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.

Add block number parameter to governance.chainConfigAt() API #1704

Merged
merged 8 commits into from
Nov 21, 2022
Merged

Add block number parameter to governance.chainConfigAt() API #1704

merged 8 commits into from
Nov 21, 2022

Conversation

ian0371
Copy link
Contributor
@ian0371 ian0371 commented Nov 16, 2022 •
edited by blukat29
Loading

Proposed changes

This PR

  • introduces governance.chainConfigAt(num)
  • changes the semantic of governance.chainConfig to return the latest chainConfig
  • At num=0, ReadGovernance uses DefaultEpoch because istanbul.epoch might not exist at program startup
  • Remove unused methods
  • closes Ambiguous chainConfig API name #1615

Before:

> governance.chainConfig
{
  chainId: 1001,
  deriveShaImpl: 2,
  governance: {
    governanceMode: "single",
...
}

After:

> governance.chainConfig // returns the latest
{
  chainId: 1001,
  deriveShaImpl: 0,
  governance: {
    governanceMode: "single",
...
}

> governance.chainConfig() // latest block
{
  chainId: 1001,
  deriveShaImpl: 0,
  governance: {
    governanceMode: "single",
...
}

> governance.chainConfig(0) // specified block
{
  chainId: 1001,
  deriveShaImpl: 2,
  governance: {
    governanceMode: "single",
...
}

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

Related issues

#1615

Further comments

@kjhman21 kjhman21 added this to the v1.10 (Kore) milestone Nov 16, 2022
blukat29
blukat29 reviewed Nov 17, 2022
View reviewed changes
@blukat29 blukat29 changed the title Change governance.chainConfig to klay.chainConfig() Add block number parameter to governance.chainConfig() API Nov 17, 2022
@ian0371 ian0371 force-pushed the chainconfig-api branch 2 times, most recently from f88722e to d2398e4 Compare November 17, 2022 06:41
@ian0371 ian0371 marked this pull request as ready for review November 17, 2022 07:40
blukat29
blukat29 reviewed Nov 18, 2022
View reviewed changes
blukat29
blukat29 previously approved these changes Nov 20, 2022
View reviewed changes
yoomee1313
yoomee1313 reviewed Nov 20, 2022
View reviewed changes
@ian0371 ian0371 changed the title Add block number parameter to governance.chainConfig() API Add block number parameter to governance.chainConfigAt() API Nov 21, 2022
@ian0371 ian0371 merged commit 6aefe1c into klaytn:dev Nov 21, 2022
@ian0371 ian0371 mentioned this pull request Nov 25, 2022
Merged
7 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@yoomee1313 yoomee1313 yoomee1313 left review comments

@blukat29 blukat29 blukat29 approved these changes

@JayChoi1736 JayChoi1736 JayChoi1736 approved these changes

@aidan-kwon aidan-kwon Awaiting requested review from aidan-kwon

@mckim19 mckim19 Awaiting requested review from mckim19

@jimni1222 jimni1222 Awaiting requested review from jimni1222

Assignees

@ian0371 ian0371

Labels
None yet
Projects
None yet
Milestone
v1.10.0 (Kore)
Development

Successfully merging this pull request may close these issues.

Ambiguous chainConfig API name
5 participants
@ian0371 @blukat29 @yoomee1313 @JayChoi1736 @kjhman21
0