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

Precalculate the fee at on_term_open #1900

Merged
merged 3 commits into from Dec 3, 2019
Merged

Conversation

@Remagpie
Copy link
Contributor

Remagpie commented Nov 28, 2019

Depends on #1895

This PR introduces on_term_open, and calculates the final fee distribution there.
The detailed design is as follows:

1. State structure

State leaf at makeKey("IntermediateRewards") contains [previous, current] value, and they mean the previous and current intermediate reward that the validators collected.

In era=1, it now contains [current, calculated], and they mean the current intermediate reward, and the calculated result of (reward - penalty).

2. on_term_open

on_term_open is called at the first block of the term, and before any of the transactions in the block is processed.
The term start is detected by the following condition:
block_number == metadata.last_term_finished_block_num() + 1

When the era is zero, it does nothing.
When the era is 1, it calculates the penalty that each validator should take, and substracts it from the current reward in the state, and saves the result in calculated part.
After that, it clears the current reward.

3. on_term_close

When the era is 1, the calculated fee is read from the state and CCC is distributed according to that value.
Note that although it is not strictly required to do this, the calculated part is cleared to empty map here.

@Remagpie Remagpie force-pushed the Remagpie:on-term-open branch 2 times, most recently from f873804 to 66e8b2f Nov 29, 2019
@Remagpie Remagpie added do-not-merge and removed do-not-merge labels Nov 29, 2019
@Remagpie

This comment has been minimized.

Copy link
Contributor Author

Remagpie commented Nov 29, 2019

I tested it in the local network and found out that there is some problem with fee distribution.
The distributed amount should be the same with before, but fee-monitor says there are some difference.
I'll try to figure out what's going on.

};

let banned = stake::Banned::load_from_state(block.state())?;
let start_of_the_current_term_header =

This comment has been minimized.

Copy link
@majecty

majecty Dec 2, 2019

Contributor

Does the header contains seal here?

This comment has been minimized.

Copy link
@Remagpie

Remagpie Dec 2, 2019

Author Contributor

I checked the related parts, and I'm pretty sure that the seal exists at this point.

`on_open_block` is called when the block is created, before
processing any transactions included in the block.
@Remagpie Remagpie force-pushed the Remagpie:on-term-open branch from 66e8b2f to f960800 Dec 2, 2019
@Remagpie Remagpie changed the title Precalculate the fee at on_term_open [WIP] Precalculate the fee at on_term_open Dec 3, 2019
Remagpie added 2 commits Nov 27, 2019
At `on_term_close`, the term_common_params is not updated to the
new parameters yet. But the parameters in the current block's
state is updated, so we should get the `era` from there.
@Remagpie Remagpie force-pushed the Remagpie:on-term-open branch from f960800 to 807ad63 Dec 3, 2019
@Remagpie Remagpie changed the title [WIP] Precalculate the fee at on_term_open Precalculate the fee at on_term_open Dec 3, 2019
@Remagpie Remagpie removed the do-not-merge label Dec 3, 2019
@Remagpie Remagpie requested review from sgkim126, foriequal0, HoOngEe and majecty Dec 3, 2019
@Remagpie

This comment has been minimized.

Copy link
Contributor Author

Remagpie commented Dec 3, 2019

Finished testing it on the local network, and the fee distribution is correctly working as I expected now.
Please review the changed parts again.

@foriequal0 foriequal0 mentioned this pull request Dec 3, 2019
3 of 4 tasks complete
@majecty
majecty approved these changes Dec 3, 2019
@HoOngEe
HoOngEe approved these changes Dec 3, 2019
@Remagpie Remagpie merged commit b172e19 into CodeChain-io:era Dec 3, 2019
10 checks passed
10 checks passed
Actions - build (macOS-10.14)
Details
Actions - clippy
Details
Actions - lint
Details
Actions - build (ubuntu-18.04)
Details
Actions - rustfmt
Details
Actions - unit test (macOS-latest)
Details
Actions - unit test (ubuntu-latest)
Details
Summary no rules match, no planned actions
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@foriequal0 foriequal0 mentioned this pull request Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.