Clarify the safety of NetClock::time_point arithmetic:

[HowardHinnant]

• NetClock::rep is uint32_t and can be error-prone when
  used with subtraction.
• Fixes Fix time arithmetic in Validations and Consensus #3656

High Level Overview of Change

To address #3656 a comprehensive survey of all uses of NetClock::time_point in rippled was made. Only two minor were found.

1. src/ripple/consensus/Validations.h has this expression:

signTime > (now - p.validationCURRENT_EARLY)

This is the expression #3656 refers to as being vulnerable to unsigned overflow. However it turns out this expression is fine because p.validationCURRENT_EARLY is a 64 bit signed integral type. Thus the 32 but unsigned now is losslessly converted to signed 64 bit prior to the conversion. Additionally the 32 but unsigned signTime is converted to signed 64 bit prior to the comparison.

The expression is correct and bullet-proof as is. A comment to this effect has been added.

2. src/ripple/app/tx/impl/CreateOffer.cpp has this expression:

NetClock::time_point const when{ctx_.view().parentCloseTime()};

While correct, using explicit conversion syntax for implicit construction is error-prone because one might accidentally choose an unwanted explicit constructor. This code is safer using implicit syntax to ensure that only an implicit constructor is chosen:

NetClock::time_point const when = ctx_.view().parentCloseTime();

[nbougalis]

While you're here can you please also take a look at https://github.com/ripple/rippled/blob/develop/src/ripple/overlay/impl/Handshake.cpp#L268-L282 as well? Thanks! 🖖

[HowardHinnant]

I actually did look at this, and found the logic sound. However I should have removed the comment. I'll do that.

The reason it is ok is that the a > b check is done and if needed, b-a is computed, converted to signed, and then negated.

[gregtatcam]

👍 LGTM