[0.18] Revert GetData randomization change (#14897) #15839

1. 
   sdaftuar commented at 5:17 PM on April 17, 2019: member

   This is for 0.18, not master -- I propose we revert the getdata change for the 0.18 release, rather than try to continue patching it up. It seems like we've turned up several additional bugs that slipped through initial review (see #15776, #15834), and given the potential severe consequences of these bugs I think it'd make more sense for us to delay releasing this code until 0.19.

   Since the bugfix PRs are getting review, I think we can leave #14897 in master, but we can separately discuss if it should be reverted in master as well if anyone thinks that would be more appropriate.

2. Revert "Change in transaction pull scheduling to prevent InvBlock-related attacks"

   This reverts commit 1cff3d6cb017aea87d16cbda0768bbab256d16da.

   8602d8b213
3. 
   naumenkogs commented at 5:31 PM on April 17, 2019: member

   I think reverting for 0.18 is reasonable.

   Regarding reverting in master: it seems like you're the only person working on that chunk of code, so keeping it there perhaps won't hurt for now? I'd propose just to spend more time looking for potential issues there.

4. 
   MarcoFalke commented at 6:56 PM on April 17, 2019: member

   sad ACK for 0.18 (:cry:)

5. DrahtBot added the label P2P on Apr 17, 2019
6. DrahtBot added the label Tests on Apr 17, 2019
7. DrahtBot added the label Utils/log/libs on Apr 17, 2019
8. 
   gmaxwell commented at 9:38 PM on April 17, 2019: contributor

   utACK. We should be more liberal with reverting stuff when issues are found before release. And if we're sad about delays in getting fixes out, more aggressive at backporting working changes...

9. fanquake added this to the milestone 0.18.0 on Apr 17, 2019
10. 
    MarcoFalke commented at 2:26 PM on April 18, 2019: member

    Unless there are objections, this will be merged before todays meeting, so that rc4 can be tagged before the weekend.

11. 
    jamesob commented at 3:26 PM on April 18, 2019: member

    Concept ACK

12. laanwj added the label Needs backport on Apr 18, 2019
13. 
    laanwj commented at 3:42 PM on April 18, 2019: member

    I tend to agree this is the best solution for now. Could always backport the randomization again for 0.18.X[X>0] if there's more confidence in it then.

14. MarcoFalke removed the label Needs backport on Apr 18, 2019
15. MarcoFalke added the label Backport on Apr 18, 2019
16. 
    Empact commented at 4:37 PM on April 18, 2019: member

    Concept ACK

17. MarcoFalke merged this on Apr 18, 2019
18. MarcoFalke closed this on Apr 18, 2019

    ---

19. MarcoFalke referenced this in commit 607b1b7498 on Apr 18, 2019
20. MarcoFalke referenced this in commit 825ecb5758 on Apr 26, 2019
21. DrahtBot locked this on Dec 16, 2021

Milestone
0.18.0