Fix unsigned integer overflow in LoadMempool #24227

It doesn't seem ideal to have an integer sanitizer enabled, but then disable it for the whole validation.cpp file.

This removes one of the two violations.

This should be a refactor.

utACK aaaae5349d1d5985081751c3a3b7386333d48cef, I reviewed the code, and agree it makes sense to merge

I've pushed a new version, which:

• is a smaller diff and easier to review, as it doesn't change any types
• doesn't introduce a presumed signed-integer-overflow when the mempool.dat on disk is badly corrupted

utACK

Code Review ACK https://github.com/bitcoin/bitcoin/pull/24227/commits/fadcd031390dd4588bbb1c07e5020a7131312050

Tried running below code to confirm:

  uint64_t i = 4;
  while (--i)
  { 
  cout<<i;
  }
  cout<<i;
  

  uint64_t i = 4;
  while (i--)
  { 
  cout<<i;
  }
  cout<<i;
  

  uint64_t i = 4;
  while (i)
  { 
   --i;
  cout<<i;
  }
  cout<<i;
  

It is also according to coding style mentioned in docs: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c

Is the PR name really accurate, this can't overflow, maybe only underflow?

No idea, but the suppression is called unsigned-integer-overflow, which is why I picked the title.

No idea, but the suppression is called unsigned-integer-overflow, which is why I picked the title.

I presume there are more unsigned-integer-overflow in validation.cpp preventing you from removing the suppression?

Have you seen the pull request you commented on? #24196#pullrequestreview-868681878

Have you seen the pull request you commented on? #24196 (review)

Ahh, thanks, I forgot they were related / about that PR