Fix #24049: signed integer overflow in SeenLocal #26399

1. 
   ViralTaco commented at 2:09 am on October 27, 2022: none

   This commit makes the behavior defined (unsigned integer overflow as if: it->second.nscore = static_cast<int>(static_cast<unsigned> (it->second.nscore) + 1U);) See: Issue #24049; Discussion on stackoverflow.
2. Fix #24049: signed integer overflow in `SeenLocal`

   This commit makes the code valid.

   474504fa4a
3. ViralTaco marked this as ready for review on Oct 27, 2022
4. 
   hebasto commented at 8:19 am on October 27, 2022: member

   @ViralTaco

   Did you see a discussion in #24090? Especially, #24090 (comment) and #24090 (comment)?

5. 
   DrahtBot commented at 2:09 pm on October 27, 2022: contributor

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

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #25390 (sync: introduce a thread-safe generic container and use it to remove a bunch of “GlobalMutex"es by vasild)

   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.

6. 
   luke-jr commented at 5:32 pm on October 27, 2022: member

   Doesn’t appear to actually fix the issue though?
7. Merge branch 'bitcoin:master' into patch-1 1910df5463
8. maflcko added the label Needs rebase on Oct 31, 2022
9. DrahtBot removed the label Needs rebase on Oct 31, 2022
10. Merge branch 'bitcoin:master' into patch-1 58d9a4fd58
11. Merge branch 'bitcoin:master' into patch-1 a1f1af1cde
12. 
    maflcko commented at 2:33 pm on November 18, 2022: member

    Looking at the previous comments, this doesn’t seem to be a fix. Let’s continue discussion in the issue for now.
13. maflcko closed this on Nov 18, 2022

    ---

14. 
    ViralTaco commented at 7:20 pm on November 20, 2022: none

    Looking at the previous comments, this doesn’t seem to be a fix. Let’s continue discussion in the issue for now.

    It doesn’t “seem[sic]” to fix the issue?
    What issue? The code before this patch has undefined behavior. It can do anything, but it doesn’t have to. It could do nothing.

    The first sentence in your comment only makes sense in a vacuum. The standard is a thing. I agree with the second part. Let’s.

15. 
    maflcko commented at 7:42 pm on November 20, 2022: member

    The burden to prepare, explain and motivate a change for reviewers is on the author. You created a one-line patch in 4 commits, ignored previous discussions, ignored two review comments, with no apparent progress in about a month. I closed this for now, because it can’t be merged according to the guidelines. At a minimum for this simple one line change, you’ll have to create at most one commit, with an explanation, and you’ll have to reply to review comments. All of this is explained in https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md
16. bitcoin locked this on Nov 20, 2023

Contributors
ViralTaco hebasto DrahtBot luke-jr maflcko