Add logging and improve counting of amendment votes from UNL
High Level Overview of Change
Adds debug-level logging of the amendment vote counting and decision making process. Also, tweaks the way the vote threshold is computed to only account for validators that the node has votes for, whether fresh or cached.
Context of Change
Some UNL validators were not reflecting an accurate count of the total votes for given amendments. (The problem seemed to resolve itself when the node restarted.) This made it look like the amendment did not have 80% majority, when it actually did. The situation did not resolve itself until the vote count increased again.
Because this processing was completely transparent, there was no way to tell why the votes were inaccurate, and because restarting seemed to fix the issue, it couldn't be reproduced with modified code.
Despite appearances, this issue is not security sensitive because
- It can only delay acceptance of an amendment
- It can not force an amendment to be accepted prematurely
- There is a workaround
While adding logging, I noticed and fixed a possible bug in the way that the total validations are counted, which affects the way the threshold is computed. Even with this fixed, though, it doesn't explain why the counts are off.
The primary purpose of this change / PR is to have logging available in case something like this ever happens again.
Type of Change
- [X] Bug fix (non-breaking change which fixes an issue)
- [X] Refactor (non-breaking change that only restructures code)
Test Plan
- Set
log_levelforAmendmentsandLedgerConsensusto debug or trace. - After a flag ledger, search the debug log for the name and/or hash of the amendment of interest. Or just search for
Amendment:. - There will be a lot more logs than before.
Codecov Report
Attention: Patch coverage is 90.41096% with 7 lines in your changes missing coverage. Please review.
Project coverage is 78.2%. Comparing base (
844646d) to head (ba93f58). Report is 2 commits behind head on develop.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/xrpld/app/misc/detail/AmendmentTable.cpp | 90.0% | 7 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #5173 +/- ##
=========================================
+ Coverage 77.9% 78.2% +0.3%
=========================================
Files 791 790 -1
Lines 68006 67738 -268
Branches 8346 8177 -169
=========================================
- Hits 52967 52955 -12
+ Misses 15039 14783 -256
| Files with missing lines | Coverage Δ | |
|---|---|---|
| src/xrpld/app/consensus/RCLConsensus.cpp | 65.4% <ø> (ø) |
|
| src/xrpld/app/misc/AmendmentTable.h | 100.0% <100.0%> (ø) |
|
| src/xrpld/app/misc/detail/AmendmentTable.cpp | 95.5% <90.0%> (-1.3%) |
:arrow_down: |
@vvysokikh1 I took your two suggestions. Could you confirm your approval?