Avoid the use of abs64 in timedata #20141

pull sipa wants to merge 1 commits into bitcoin:master from sipa:202010_no_abs64 changing 1 files +3 −7

sipa commented at 12:37 AM on October 13, 2020: member

Fixes #20135.
sipa force-pushed on Oct 13, 2020
fanquake added the label Refactoring on Oct 13, 2020
sipa force-pushed on Oct 13, 2020

in src/timedata.cpp:78 in fc66451d5c outdated

  73 | @@ -79,7 +74,8 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
  74 |          int64_t nMedian = vTimeOffsets.median();
  75 |          std::vector<int64_t> vSorted = vTimeOffsets.sorted();
  76 |          // Only let other nodes change our time by so much
  77 | -        if (abs64(nMedian) <= std::max<int64_t>(0, gArgs.GetArg("-maxtimeadjustment", DEFAULT_MAX_TIME_ADJUSTMENT))) {
  78 | +        int64_t max_adjustment = std::max<int64_t>(0, gArgs.GetArg("-maxtimeadjustment", DEFAULT_MAX_TIME_ADJUSTMENT));
  79 | +        if (nMedian < -max_adjustment || nMedian > max_adjustment) {

kallewoof commented at 2:46 AM on October 13, 2020:

I think this is flipped. Now if nMedian is beyond the max nTimeOffset is set to nMedian, whereas it was the opposite before:

Example: max = 4, nMedian = -3 Before: abs64(-3) <= 4 → 3 <= 4 → true Now: -3 < -4 || -3 > 4 → false

sipa commented at 3:12 AM on October 13, 2020:

Uh, fixed. Thanks!

Avoid the use of abs64 in timedata d1292f25f2
sipa force-pushed on Oct 13, 2020

MarcoFalke commented at 7:34 AM on October 13, 2020: member

ACK d1292f25f272401da0c58580521c74b1fa03a9ad 🎹

<details><summary>Show signature and timestamp</summary>

Signature:

Timestamp of file with hash 9182aa2ebb693c9de1307af30edc9e597d470f51bd1cdb4b1d6b11923c3ef0db -

</details>

kallewoof approved
kallewoof commented at 7:55 AM on October 13, 2020: member

ACK d1292f25f272401da0c58580521c74b1fa03a9ad

Verified no behavioral change.
practicalswift commented at 9:07 AM on October 13, 2020: contributor

ACK d1292f25f272401da0c58580521c74b1fa03a9ad
jonatack commented at 10:05 AM on October 13, 2020: member

ACK d1292f25f272401da0c58580521c74b1fa03a9ad code/logic review, verified there are no remaining callers of abs64(), verified no warnings in a debug build
fanquake merged this on Oct 13, 2020
fanquake closed this on Oct 13, 2020
fanquake referenced this in commit 0b64310fb6 on Oct 16, 2020
fanquake referenced this in commit ee0082b886 on Oct 16, 2020
MarcoFalke referenced this in commit a2fa11f9de on Nov 18, 2020
MarkLTZ referenced this in commit 5dee1e1f8f on Nov 21, 2020
MarkLTZ referenced this in commit c61e643b75 on Nov 21, 2020
MarcoFalke referenced this in commit 5baaea4410 on Dec 2, 2020
Fabcien referenced this in commit f61fd42904 on Nov 12, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#20135 UBSan warning when fuzzing abs64(...)#20150 [0.19] Backports #20166 [0.20] Backports