Feature/shard validation

[undertome]

This was rebased on pull requests 3152 and 3252, which have yet to me merged to develop. To see only new changes you can compare against base pull/3252/head. Verfication testing pending new shard archives.

[codecov-io]

Codecov Report

Merging #3297 into develop will increase coverage by 0.00%.
The diff coverage is 52.06%.

@@           Coverage Diff            @@
##           develop    #3297   +/-   ##
========================================
  Coverage    70.44%   70.44%           
========================================
  Files          682      682           
  Lines        54392    54466   +74     
========================================
+ Hits         38315    38369   +54     
- Misses       16077    16097   +20     

Impacted Files	Coverage Δ
src/ripple/app/main/Application.h	100.00% <ø> (ø)
src/ripple/net/SSLHTTPDownloader.h	100.00% <ø> (ø)
src/ripple/net/impl/DatabaseDownloader.cpp	91.30% <0.00%> (ø)
src/ripple/nodestore/impl/DatabaseShardImp.cpp	13.74% <0.00%> (+0.25%)	⬆️
src/ripple/nodestore/impl/DatabaseShardImp.h	52.00% <ø> (+20.00%)	⬆️
src/ripple/nodestore/impl/Shard.cpp	0.00% <0.00%> (ø)
src/ripple/nodestore/impl/Shard.h	0.00% <ø> (ø)
src/ripple/rpc/handlers/DownloadShard.cpp	0.00% <0.00%> (ø)
src/ripple/net/impl/DatabaseBody.ipp	41.37% <14.28%> (-0.73%)	⬇️
src/ripple/rpc/impl/ShardArchiveHandler.cpp	61.45% <58.40%> (-1.69%)	⬇️
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9771210...de7abfd. Read the comment docs.

[scottschurr]

Nice job with the two commit messages, but I'd like to request a couple small changes.

1. For the heading if there are no additional following lines omit the period. I'm not sure why that's a guideline, but there you are. However both of your commit messages are nice, long, useful messages with multiple lines. So, in your case, both of the heading lines should end with a colon (:). That way folks can tell there's additional information if they only see the first line.

   Also, when the commit message includes multiple lines please include one line of white space between the heading and the following lines. Some tools treat lines that have no blank lines between them as though they are paragraphs and then concatenate the lines. Including a blank line between the heading and the following paragraphs will prevent that concatenation. That, in turn, will improve readability.

2. Please limit line lengths to 60 characters for the heading and 72 characters for the body. Some tools add indentations above and beyond what you put in. So in those cases limiting your line lengths will make the commit message easier to read for everyone. The heading lines in your commit messages are great, but a few of the lines in the bodies of the messages are a bit long.

So the commit message for your top-most commit might look like this:

Validate LastLedgerHash for downloaded shards:

* Add documentation for shard validation
* Retrieve last ledger hash for imported shards
* Compare the reference hash to the lastLedgerHash in
  Shard::finalize
* Limit retry attempts for imported shards with unconfirmed last
  ledger hashes
* Use a common function for removing failed shards
* Add new ShardInfo::State for imported shards

Here's a useful guide to good commit messages: https://chris.beams.io/posts/git-commit/.

He says to limit the heading to 50 characters. I can seldom achieve that, so I allow myself to go up to 60 characters for the heading. But the rest of the guidance I try to follow.

I hope that helps.

[miguelportilla]

@undertome the lastLedgerHash members in ShardInfo may be better encapsulated in a new class. I wrote an alternate version of your code in this branch: https://github.com/miguelportilla/rippled/commits/shard-validation-alternate

Disclaimer: I haven't tested this code.
If you end up using it, the class should probably be broken out into its own files.

[scottschurr]

👍 Looks good to me. I left some comments for you to consider, however none of them are worth blocking the pull request over.

[scottschurr]

👍 LGTM.

[miguelportilla]

👍

[scottschurr]

Still looks good to me.

[miguelportilla]

👍