search

dtr-org / unit-e

A digital currency for a new era of decentralized trust
https://unit-e.io
MIT License
45 stars 15 forks source link

Finalize the latest epoch #1082

Closed kostyantyn closed 5 years ago

kostyantyn commented 5 years ago

When we have two consecutive justifications, finalize the latest one instead of the previous one.

Rationale Let's say we have these two consecutive justified links.

                J
source epoch ------- target hash                                          
|                              |
e2[101, .., 150] e3[151, ... 200] e4[201, ... 250]
                 |                              |
                 source epoch ------- target hash                                          
                                 J

In Casper the finalized epoch would be e4 and in our case it's e3. However, we proved in feature_fork_choice_forked_finalize_epoch.py that reorging e4, in this case, is not possible anymore as it will revert finalization for e3. Because of that we introduced FinalizationState::GetCheckpointHeightAfterFinalizedEpoch as a point of "not going back". However, if we count e4 as finalized, we can drop this extra entity. Moreover, we used to have such a convention and we changed but as it turned out wasn't a good idea.

Aditional fix CalculateWithdrawAmount was broken as it didn't correctly take reward into account which led to a shrunk amount over time. We didn't have tests to catch it. Now we have finalizationstate_calculate_withdraw_amount_tests.cpp test that tests this part and shows that the amount is properly increasing.

However the finalization_withdraw.py still shows that after withdraw the balance is not larger than the initial deposit (it's actually equal to initial deposit - fee). The reason is that the reward for finalizers is not implemented and I'll continue working on it in the following PR.

Resolves #1092

Signed-off-by: Kostiantyn Stepaniuk kostia@thirdhash.com

pep8speaks commented 5 years ago

Hello @kostyantyn! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2019-05-14 13:18:40 UTC
scravy commented 5 years ago

Concept ACK 5f330a7e5849a3fb2ef0fd78487b8d5df6ce34df

scravy commented 5 years ago

Needs rebase.

kostyantyn commented 5 years ago

rebased with master

scravy commented 5 years ago

Travis Lint Stage:

* Checking consistency between dispatch tables and vRPCConvertParams
WARNING: conversion mismatch for argument named amount ([('deposit', False), ('sendtoaddress', True), ('settxfee', True), ('sendfrom', True), ('move', True)])
WARNING: conversion mismatch for argument named test_fee ([('sendtypeto', True), ('stakeat', False)])
* Running remaining lint scripts
./test/functional/finalization_slash_itself.py:13:1: F401 'test_framework.util.sync_mempools' imported but unused
^---- failure generated from lint-python.sh
kostyantyn commented 5 years ago

@scravy thanks! fixed the python lint

frolosofsky commented 5 years ago

ConceptACK 47edbaa37836d51203dff61b4365db9a8632d669

Gnappuraz commented 5 years ago

Did a quick review, it looks ok, utACK 149fb96