Send fewer feefilter messages (and avoid the wobbling issue) #21805

1. 
   rebroad commented at 3:55 pm on April 29, 2021: contributor

   Fixes #21635
2. Send fewer feefilter messages (avoid the wobbling number issue) e4a0e0d024
3. DrahtBot added the label P2P on Apr 29, 2021
4. 
   DrahtBot commented at 8:10 pm on April 29, 2021: 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:

   • #21160 (net/net processing: Move tx inventory into net_processing by jnewbery)

   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.

5. 
   MarcoFalke commented at 5:45 am on April 30, 2021: member

   -0. There are at most 6 feefilter expected to be sent per hour. In practice it should be less due to rounding. No need to micro-optimize the bandwidth here.
6. 
   rebroad commented at 10:50 am on April 30, 2021: contributor

   -0. There are at most 6 feefilter expected to be sent per hour. In practice it should be less due to rounding. No need to micro-optimize the bandwidth here.

   Hi, no, actually the rounding causes it to send them more often as the number “wobbles” above and below the actual. Please see the attached log. The wobbling happens most often when the actual isn’t moving much:-

   2021-04-30T05:59:05.941 send feefilter WOBBLE 2297 (actual: 2346) peer=110 2021-04-30T06:02:07.715 send feefilter WOBBLE 2297 (actual: 2346) peer=56 2021-04-30T06:03:33.621 send feefilter WOBBLE 2527 (actual: 2346) peer=110 2021-04-30T06:08:49.501 send feefilter WOBBLE 2297 (actual: 2346) peer=98 2021-04-30T06:09:21.618 send feefilter WOBBLE 2527 (actual: 2346) peer=76 2021-04-30T06:10:31.663 send feefilter WOBBLE 2527 (actual: 2346) peer=88 2021-04-30T06:11:43.064 send feefilter WOBBLE 2297 (actual: 2346) peer=88 2021-04-30T06:14:01.180 send feefilter WOBBLE 2297 (actual: 2346) peer=81 2021-04-30T06:15:41.912 send feefilter WOBBLE 2297 (actual: 2346) peer=110 2021-04-30T06:16:39.465 send feefilter WOBBLE 2297 (actual: 2346) peer=76 2021-04-30T06:18:14.436 send feefilter WOBBLE 2527 (actual: 2346) peer=87 2021-04-30T06:21:56.656 send feefilter WOBBLE 2527 (actual: 2346) peer=81 2021-04-30T06:22:31.126 send feefilter 2297 (actual: 2346) peer=114 2021-04-30T06:25:48.578 send feefilter WOBBLE 2297 (actual: 2346) peer=87 2021-04-30T06:26:08.049 send feefilter WOBBLE 2527 (actual: 2346) peer=88 2021-04-30T06:30:05.929 send feefilter WOBBLE 2527 (actual: 2346) peer=78 2021-04-30T06:31:18.270 send feefilter WOBBLE 2527 (actual: 2346) peer=98 2021-04-30T06:32:56.377 send feefilter WOBBLE 2297 (actual: 2346) peer=78

   I’ll edit this comment in a moment to refer to the code I’m using, but the WOBBLE lines are essentially the existing logic, and the non-WOBBLE lines are the new logic. Hopefully you can see that the messages go up and down around the actual, thereby actually revealing the actual more than if this didn’t happen.

7. 
   rebroad commented at 11:01 am on April 30, 2021: contributor

   @MarcoFalke another fix would be to change the rounding function so that it only ever goes in one direction (rather than in two directions as it currently does).
8. 
   MarcoFalke commented at 11:04 am on April 30, 2021: member

   The excerpt shows 30 minutes and each connection is sent about 2 feefilters. (3 are expected)
9. 
   rebroad commented at 4:27 pm on April 30, 2021: contributor

   The excerpt shows 30 minutes and each connection is sent about 2 feefilters. (3 are expected)

   I’m not sure what point you’re trying to make. My point is that the round function is supposed to provide some privacy, and it’s failing to do that, and it’s also causing unnecessary traffic to be generated. Surely this is worth fixing for both these reasons.

10. 
    MarcoFalke commented at 6:45 pm on April 30, 2021: member

    causing unnecessary traffic

    You will need to provide benchmarks to support this claim. A single inv message with just one tx entry is larger than the feefilter message.

11. 
    MarcoFalke commented at 9:11 am on May 1, 2021: member

    Closing for now because for the same reason: #21635 (comment)

    Also all the tests are failing.

    In the future it might be good if you ran the tests locally before proposing a change for review.

12. MarcoFalke closed this on May 1, 2021

    ---

13. 
    rebroad commented at 10:32 am on May 3, 2021: contributor

    causing unnecessary traffic

    You will need to provide benchmarks to support this claim. A single inv message with just one tx entry is larger than the feefilter message.

    this can be derived by logical deduction simply by looking at the debug.log

14. 
    MarcoFalke commented at 10:58 am on May 3, 2021: member

    I understand that logically this reduces the traffic. However, in practice the savings should be less than the rounding errors you encounter while trying to measure.
15. DrahtBot locked this on Aug 18, 2022

Contributors
rebroad DrahtBot MarcoFalke

Labels
P2P