Tidy consensus #2156
Tidy consensus #2156
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2156 +/- ##
==========================================
- Coverage 69.01% 69.01% -0.01%
==========================================
Files 684 684
Lines 50468 50475 +7
==========================================
Hits 34833 34833
- Misses 15635 15642 +7
Continue to review full report at Codecov.
|
| // try to acquire the correct one | ||
| if(auto newLedger = impl().acquireLedger(prevLedgerID)) | ||
| { | ||
| prevLedger = *newLedger; |
There was a problem hiding this comment.
Ledger_t looks like it's usually a thin wrapper around a shared_ptr (i.e. RCLCxLedger). There's a minor benefit if we move here instead of copy.
|
👍 |
src/ripple/app/misc/Validations.cpp
Outdated
| @@ -93,6 +93,10 @@ class ValidationsImp : public Validations | |||
|
|
|||
| auto pubKey = app_.validators ().getTrustedKey (signer); | |||
|
|
|||
| // Do not process or forward partial validations. | |||
There was a problem hiding this comment.
Just for my own education, what are the consequences of not forwarding partial validations? My understanding is that part of the role of a partial validation is that it lets other folks know that validator is still on the job, although it is not contributing during this round. Will not forwarding the partial cause some observers to remove this validator?
There was a problem hiding this comment.
Good question. I fixed to properly forward partial validations. At this time, we aren't actually using them to do anything special aside from including them in the partial validation stream, but you are correct that they should be forwarded.
| { | ||
| // Only forward if current | ||
| return isCurrent; | ||
| } |
There was a problem hiding this comment.
Nit: The {} really aren't necessary.
|
Merged as a89be5b |
Simplify the use of and bow-out logic in
handleWrongLedger