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

import emission reward ranges #2499

Merged
merged 22 commits into from
Aug 15, 2019
Merged

import emission reward ranges #2499

merged 22 commits into from
Aug 15, 2019

Conversation

ayrat555
Copy link
Contributor

@ayrat555 ayrat555 commented Aug 3, 2019

a part of #2493
fixes #2489
closes #2080

Changelog

  • import emission reward ranges

Upgrading

All you need to do is set CHAIN_SPEC_PATH env var with parity chain specification or create a similar specification with emission reward ranges and genesis account balances.

I'll abstract this logic for POA Network in the next PR

@ayrat555 ayrat555 changed the title parse emission reward ranges import emission reward ranges Aug 3, 2019
@coveralls
Copy link

coveralls commented Aug 3, 2019

Pull Request Test Coverage Report for Build c32976dc-11e5-4a02-9327-e351a3e81b9d

  • 31 of 52 (59.62%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 79.032%

Changes Missing Coverage Covered Lines Changed/Added Lines %
apps/explorer/lib/explorer/chain/block/emission_reward.ex 0 1 0.0%
apps/explorer/lib/explorer/chain_spec/genesis_data.ex 3 23 13.04%
Totals Coverage Status
Change from base Build 40ef733c-0c96-4391-964b-9a12e1e93cb8: -0.2%
Covered Lines: 5190
Relevant Lines: 6567

💛 - Coveralls

Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayrat555 PR checklist👮 is here. You introduced a new ENV VAR and didn't add to the list of PRs in corresponding MD file. I would suggest not to clear checklist but look over it and add necessary marks.

Also, it is hard to tell how the solution works without description. Please, clarify how the solution works. It will help to find possible bugs in the logic before we published it into production.

@ayrat555
Copy link
Contributor Author

ayrat555 commented Aug 8, 2019

@vbaranov done

@vbaranov vbaranov self-requested a review August 8, 2019 10:59
Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayrat555

Please set master for Version column. Because we cannot know for sure, that it will be a part of 2.0.3 release. I will update the version before release.

All you need to do is set CHAIN_SPEC_PATH env var with parity chain specification or create a similar specification with emission reward ranges and genesis account balances.

  • Should it be a local file?
  • absolute/relative path?
  • Could it be a link to a chain spec on Github, for instance

I think we should support both options (local file/link). For public chains, it is preferable to use the latter option since spec is published in Github.

@ayrat555
Copy link
Contributor Author

ayrat555 commented Aug 8, 2019

@vbaranov currently it only supports absolute path. I'll implement fetching from url now

@ayrat555
Copy link
Contributor Author

ayrat555 commented Aug 8, 2019

@vbaranov I added the ability to fetch from remote url

Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayrat555

  • which DB tables do you fill with data from chain spec?
  • Where do you write genesis balances?

@vbaranov
Copy link
Member

vbaranov commented Aug 9, 2019

@ayrat555 getminedblocks is still unusable because it returns 502 Error, I suppose, because of a huge amount of blocks. We should add pagination for this query.

@ayrat555
Copy link
Contributor Author

ayrat555 commented Aug 9, 2019

@vbaranov emission_rewards table is populated for emission ranges

@ayrat555
Copy link
Contributor Author

ayrat555 commented Aug 9, 2019

@vbaranov address_coin_balances is populated for genesis balances

@vbaranov
Copy link
Member

vbaranov commented Aug 9, 2019

@ayrat555

@vbaranov address_coin_balances is populated for genesis balances

stg: /address/0xebd37b256563e30c6f9289a8e2702f0852880833/coin_balances

Screenshot 2019-08-09 at 19 27 20

The same is for every address from the genesis block. How is the date for the genesis block chosen?

@ayrat555
Copy link
Contributor Author

ayrat555 commented Aug 9, 2019

@ayrat555
Copy link
Contributor Author

I created an issue #2546

Copy link
Member

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ayrat555 I removed "fixing" keyword for #2493 because this PR doesn't fix that issue completely due to

@ayrat555 getminedblocks is still unusable because it returns 502 Error, I suppose, because of a huge amount of blocks. We should add pagination for this query.

@ayrat555
Copy link
Contributor Author

@vbaranov will you create a separate issue?

@vbaranov
Copy link
Member

@vbaranov will you create a separate issue?

No. I would just suggest not to close #2493 with this PR. Because that issue is definitely clear: API endpoint doesn't work. And this PR is the 1st step to resolve that issue.

@ayrat555
Copy link
Contributor Author

sounds good to me

@vbaranov vbaranov merged commit 27dff9b into master Aug 15, 2019
@vbaranov vbaranov deleted the ab-import-chain-spec-data branch August 15, 2019 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants