Proposer finalization rewards #1081
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
Nizametdinov
requested review from
scravy,
frolosofsky,
Gnappuraz and
kostyantyn
and removed request for
scravy
scravy
reviewed
May 7, 2019
cmihai
reviewed
May 8, 2019
scravy
reviewed
May 8, 2019
Merged
|
Needs rebase. |
Nizametdinov
force-pushed
the
proposer-finalization-rewards
branch
from
32ba35c
to
5be8a8f
Compare
Nizametdinov
requested review from
cmihai and
scravy
and removed request for
cmihai
frolosofsky
reviewed
May 16, 2019
| const Dependency<BlockDB> m_block_db; | ||
|
|
||
| CScript GetRewardScript(const CBlockIndex &index) const { | ||
| // TODO UNIT-E: implement more efficient reward script retrieval |
There was a problem hiding this comment.
If I understood correct, this function will be called on every block in epoch, on every checkpoint. Then, reading blocks from disk is super inefficient. I faced with this when worked on commits and full-sync, on my machine every block read could pause the process for up to 500ms. I suggest avoid disk reading.
There was a problem hiding this comment.
@Nizametdinov Is this comment resolved/acted upon?
There was a problem hiding this comment.
@scravy I will create an issue when this branch is merged.
Gnappuraz
reviewed
May 16, 2019
| if (num_reward_outputs > 1 && !(prev_block.pprev->nStatus & BLOCK_HAVE_DATA)) { | ||
| // prev_block is a parent block of the snapshot which was used for ISD. | ||
| // We do not have data for the ancestor blocks of prev_block. | ||
| // TODO UNIT-E: implement proper validation of finalization rewards after ISD |
There was a problem hiding this comment.
I'd like to have an issue for this. It seems that we cannot check if the rewards are being payed to the correct scripts here, correct?
There was a problem hiding this comment.
Did we create an issue for this?
| @@ -60,7 +60,9 @@ def test_empty_blocks_balance(self, created_money, nodes): | |||
| coinstake_tx_id = block_info['tx'][0] | |||
| coinstake_tx_info = nodes[node_idx].gettransaction(coinstake_tx_id) | |||
|
|
|||
| created_money += coinstake_tx_info['details'][0]['amount'] | |||
| # TODO UNIT-E: gettransaction should not return fees for coinbase transactions | |||
There was a problem hiding this comment.
I would like to have an issue here as well. We don't want to forget this stuff.
There was a problem hiding this comment.
Did we create an issue for this?
There was a problem hiding this comment.
Not sure, can't find anything.
Nizametdinov
force-pushed
the
proposer-finalization-rewards
branch
3 times, most recently
from
17ed35d
to
c27f7f3
Compare
Nizametdinov
commented
May 17, 2019
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
clang showed the following error:
wallet/wallet.cpp:2577:17: error: calling function 'IsLockedCoin'
requires holding mutex 'cs_wallet' exclusively
[-Werror,-Wthread-safety-analysis]
if (IsLockedCoin(entry.first, i)) {
^
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Nizametdinov
force-pushed
the
proposer-finalization-rewards
branch
from
cde2613
to
a288d8c
Compare
Ruteri
reviewed
Jun 4, 2019
Signed-off-by: Azat Nizametdinov <[email protected]>
Signed-off-by: Azat Nizametdinov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request implements proposer finalization rewards.
The first block of every epoch must contain finalization rewards for the block proposers of the previous epoch. The number of finalization reward outputs equals the epoch length. The scripts to which finalization rewards are sent are taken from the reward (0th) outputs of the previous epoch coinbase transactions. The reward amount must depend on the percentage of votes included in the previous epoch, but this will be implemented in following PRs.
See some additional details in #418.