Skip to content

Conversation

rebroad
Copy link
Contributor

@rebroad rebroad commented May 3, 2021

This is a simpler replacement for #21805. The only change now to the current functionality is that it requires that the actual (i.e. not rounded up or down) minrelayfee has changed since the last sent feefilter message.

This helps increase privacy (by not sending repeated messages above and below the actual, thereby revealing the actual), and reduces network traffic (due to fewer messages).

This pull will make almost no functional changes without #21618 given that it's extremely rare that the actual minrelayfee will remain constant, so it's safe to merge, and will be there for privacy safety in the event that a pull is later merged which encourages the minrelayfee to remain constant.

TODO - need to fix the test that is failing as soon as I work out how to run the test - can someone comment with this info please?

@fanquake fanquake added the P2P label May 3, 2021
@rebroad rebroad closed this May 3, 2021
@rebroad rebroad reopened this May 3, 2021
@rebroad rebroad marked this pull request as draft May 3, 2021 16:26
@0xB10C
Copy link
Contributor

0xB10C commented May 5, 2021

TODO - need to fix the test that is failing as soon as I work out how to run the test - can someone comment with this info please?

e.g. with test/functional/test_runner.py p2p_feefilter.py. See test#running-the-tests for details.

@rebroad rebroad force-pushed the SteadierFeefilter branch from 121b280 to 9c0fec5 Compare May 16, 2021 07:34
@rebroad
Copy link
Contributor Author

rebroad commented May 16, 2021

I've changed the algorithm - simpler - it only sends a new feefilter if the current rounded filter (so still giving privacy) is closer to the current filter than the last sent one was.

@DrahtBot
Copy link
Contributor

DrahtBot commented May 19, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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.

@maflcko
Copy link
Member

maflcko commented Oct 22, 2021

Closing for now. The tests failed, this needs rebase and there hasn't been any activity for months.

You can leave a comment if you want this to be reopened. (Or create a new pull, since there haven't been any comments here). Though, please make sure the code is passing tests and is ready for review.

@maflcko maflcko closed this Oct 22, 2021
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants