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

refactor: remove obsolete rewards config override #9151

Conversation

mercepluka
Copy link

@mercepluka mercepluka commented Jun 6, 2023

Removing obsolete rewards config overrides used for enabling inflation in older versions. Now both testnet and mainnet versions are beyond the ENABLE_INFLATION_PROTOCOL_VERSION version (36)

@mercepluka mercepluka changed the title Remove obsolete rewards config override refactor: remove obsolete rewards config override Jun 6, 2023
@mercepluka mercepluka marked this pull request as ready for review June 6, 2023 14:57
@mercepluka mercepluka requested a review from a team as a code owner June 6, 2023 14:57
Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

looks like this change would remove the ability to replay history within the same binary. cc @Ekleog

@mercepluka
Copy link
Author

mercepluka commented Jun 7, 2023

Can you please explain this @bowenwang1996

@Longarithm
Copy link
Member

@mercepluka One property of blockchain is that it gives an option to replay all chain history since genesis, which means that we must be able to re-process any single existing block and verify that it was recorded correctly. This feature is actually in use.

Here you can see that our mainnet genesis protocol version is 29 https://explorer.near.org/stats. This PR changes behaviour for protocol version 36, which means that with that change we won't be able to replay mainnet blocks for this version correctly, so we can't proceed with that.

However, you can take a look at #8197 - we have a big task to require nearcore binary to support only latest protocol versions, to avoid maintenance of very old versions in newest binary - as your PR proposes.

@mercepluka
Copy link
Author

@Longarithm thanks for the detailed answer. Is it ok to leave this PR for now and merge it once the limited replayability feature is landed?

@Longarithm
Copy link
Member

Depends on if you are fine with waiting until it is finished, because I can imagine it taking a long time. cc @Ekleog

@mercepluka
Copy link
Author

mercepluka commented Jun 14, 2023

@Longarithm, I discussed this with @mhalambek today and this is not extremely urgent to Calimero. We would prefer if this PR remains open and just merge once limited replayability is landed

@Longarithm
Copy link
Member

@mercepluka do you plan to continue working on it in the near future? If not, could you move it to Draft to indicate that it is not under active development for now?

@Longarithm
Copy link
Member

Closing for now. Feel free to reopen when you continue working on it!

@Longarithm Longarithm closed this Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants