Skip to content

Pay the block propose rewards at the end of the term #1559

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

Merged
merged 6 commits into from
May 30, 2019
Merged

Pay the block propose rewards at the end of the term #1559

merged 6 commits into from
May 30, 2019

Conversation

sgkim126
Copy link
Contributor

No description provided.

@sgkim126 sgkim126 added consensus do-not-merge Do not merge (for mergify.io) labels May 24, 2019
@sgkim126
Copy link
Contributor Author

It depens on #1535

@sgkim126 sgkim126 changed the title [WIP] Pay the block propose rewards at the end of the term Pay the block propose rewards at the end of the term May 28, 2019
@sgkim126 sgkim126 requested review from foriequal0 and majecty May 28, 2019 05:20
@sgkim126 sgkim126 added do-not-merge Do not merge (for mergify.io) and removed do-not-merge Do not merge (for mergify.io) labels May 28, 2019
@sgkim126
Copy link
Contributor Author

The last commit based on #1562.
I'll change the commit if the spec changes.

@majecty
Copy link
Contributor

majecty commented May 28, 2019

on_new_block is only called in the proposer node, not in other validator nodes.
IMO other validators also need to call on_new_block when they received a proposal block or received a block from the sync extension to create the same state with the proposer.

Sorry. I found that on_new_block is called when importing a block.

@sgkim126
Copy link
Contributor Author

@majecty @foriequal0
I removed on_new_block and made the tendermint engine pay the reward after one term.

@sgkim126 sgkim126 removed the do-not-merge Do not merge (for mergify.io) label May 29, 2019
.ok_or(EngineError::CannotOpenBlock)?;

let (start_of_the_next_term, start_of_the_current_term) = {
let end_of_the_previous_term = block.state().metadata()?.unwrap().last_term_finished_block_num();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confusing what is the {current, next, previous} terms.
term id is calculated by the header.timestamp. The block in this function has different term which is calculated from the header, and the block's term is term id - 1. Am I right?

What is the current term? Is it from the block's timestamp or from the block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It's the most confusing part because the current block is the end of the term but the term Id in the metadata is not updated.
The current term in this context is the term that this block closes. The reason which uses the current block to calculate end_of_the_previous_term is that because the last block of the previous term is stored in states.

@sgkim126
Copy link
Contributor Author

@majecty I updated the PR.

Seulgi Kim and others added 4 commits May 30, 2019 10:44
Currently, the block reward is paid per block; this patch makes the
reward is paid at the end of the term if the term seconds is determined.

Solo and Tendermint are different. Solo pays the fee at the end of the
term, but Tendermint pays the fee at the end of the next term.
foriequal0
foriequal0 previously approved these changes May 30, 2019
Copy link
Contributor

@majecty majecty left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot merged commit ab9519e into CodeChain-io:master May 30, 2019
@sgkim126 sgkim126 deleted the fee branch May 30, 2019 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants