Improve private chains tracking #348
Conversation
src/zen/delay.cpp
Outdated
| { | ||
| // blocks on tip get penalty reduced by 1; | ||
| // others get penalty increased proportionally to distance from tip | ||
| return std::max<int>((activeChainHeight - newBlock.nHeight),-1); |
There was a problem hiding this comment.
This is more compact than before but we have not the trace anymore; should we keep it?
src/zen/delay.cpp
Outdated
| { | ||
| // blocks on tip get penalty reduced by 1; | ||
| // other blocks get penalty increased proportionally to distance from tip | ||
| int64_t blockDelay = std::max<int64_t>((activeChainHeight - newBlock.nHeight),int64_t(-1)); |
There was a problem hiding this comment.
Is this code behaving the same way if for example activeChainHeight is MIN_INT64 and newBlock.height is MIN_INT64 +1?
Why this PR needed this change?
There was a problem hiding this comment.
Change is purely to improve readability, I can roll it back should there not be agreement on such improvement.
Moreover this function is called uniquely by AddToBlockIndex, with activeChainHeight set to chainActive.Height(). chainActive.Height() is either positive, or -1 (at start, when no block has been added yet). In both cases, the value far away for int64_t boundaries, which would cause the refactored code to diverge from the original
There was a problem hiding this comment.
Reverted change as agree offline
|
To be rebased on top of current development to make a new CI run pass, ready to be merged once CI passes. |
cronicc
force-pushed
the
improve_private_chains_tracking
branch
from
6f1b906
to
cabf249
Compare
|
Not merging until I had a chance to look at #366 |
In cases where 2 branches have the same length, the 'active' branch is not guaranteed to be at list index 0 in the getchaintips response. Fixed by sorting the list on 'status' k:v.
Along @cronicc indications we are going to extend getchaintips with a --with-penalties=true/false paramenter to include, for each branch: