Remove pointless and confusing shift in RelayAddress #23970

1. 
   MarcoFalke commented at 10:26 am on January 4, 2022: member

   The second argument written to the siphash is already quantized to 24 hours, so it seems confusing to quantize the first argument to 32 bits (out of 64 bits).

   The shifting is pointless, we should get rid of it. It seems to be a silly evolution of this 2010 Satoshi code: 5cbf753 (where it made sense because everything was XORed together, and the address used the high bits, while the time used the low ones).

   (Copied from #18642 (comment))

   (The original code was uint256 hashRand = hashSalt ^ (((int64)addr.ip)<<32) ^ ((GetTime()+addr.ip)/(24*60*60));)

   This also allows to remove a integer sanitizer suppression for the whole file.

2. MarcoFalke added the label Refactoring on Jan 4, 2022
3. MarcoFalke renamed this:
   refactor: Remove pointless and confusing shift in RelayAddress
   Remove pointless and confusing shift in RelayAddress
   on Jan 4, 2022
4. MarcoFalke added the label P2P on Jan 4, 2022
5. refactor: Remove pointless and confusing shift in RelayAddress fa9f4554ca
6. MarcoFalke force-pushed on Jan 4, 2022
7. 
   DrahtBot commented at 6:37 am on January 5, 2022: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #23542 (net: open p2p connections to nodes that listen on non-default ports by vasild)
   • #18642 (Use std::chrono for the time to rotate destination of addr messages + tests by naumenkogs)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

8. 
   promag commented at 3:30 pm on January 5, 2022: member

   Code review ACK fa9f4554ca5e64e7960b74dd0ad8fb1cd7c2f091.
9. 
   laanwj commented at 4:32 pm on January 5, 2022: member

   Code review ACK fa9f4554ca5e64e7960b74dd0ad8fb1cd7c2f091 Agree with the reasoning that shifting (essentially moving the zeroes) is pointless statistically when using a “real” hash algorithm like siphash.
10. 
    sipa commented at 5:41 pm on January 5, 2022: member

    utACK fa9f4554ca5e64e7960b74dd0ad8fb1cd7c2f091
11. fanquake merged this on Jan 5, 2022
12. fanquake closed this on Jan 5, 2022

    ---

13. sidhujag referenced this in commit 8bb46dbb3c on Jan 6, 2022
14. MarcoFalke deleted the branch on Jan 6, 2022
15. DrahtBot locked this on Jan 6, 2023

Contributors
MarcoFalke DrahtBot promag laanwj sipa

Labels
Refactoring P2P