Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: Explain why -whitelist is used in feature_fee_estimation #16535

Merged
merged 2 commits into from Aug 6, 2019
Merged

test: Explain why -whitelist is used in feature_fee_estimation #16535

merged 2 commits into from Aug 6, 2019

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Aug 2, 2019

No description provided.

@fanquake fanquake added the Tests label Aug 2, 2019
test: Explain why -whitelist is used in feature_fee_estimation
fa76285 
Also, Remove seemingly unused and undocumented -maxorphantx=1000
@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 2, 2019 •
edited

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #16549 ([WIP] UI external signer support (e.g. hardware wallet) by Sjors)
  • #16546 ([WIP] External signer support - Wallet Box edition by Sjors)
  • #16528 ([WIP] Native Descriptor Wallets (take 2) by achow101)

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.

@practicalswift
Copy link
Contributor

ACK fa76285 -- diff looks correct

@Sjors
Copy link
Member

Sjors commented Aug 6, 2019 •
edited

ACK fa76285, every bit of clarification helps. It's clear that without -whitelist the test becomes extremely slow (it does pass).

That said, the title is overselling this PR a bit :-)

I still don't really understand what whitelist is doing. It seems to have something to do with BIP 35. Apparently whitelisting a node means we spam it more? I also understand it only applies to inbound nodes (see #9923).

To make it more clear / mysterious which whitelist feature this test relies on, you could set -whitelistforcerelay=0 and -whitelistrelay=0 (it will still run quick).

@maflcko
Copy link
Member Author

maflcko commented Aug 6, 2019

That said, the title is overselling this PR a bit :-)

Heh, yeah I think it was an oversight to only set whitelist for the first node. So I added it to the others as well.

To make it more clear / mysterious which whitelist feature this test relies on, you could set -whitelistforcerelay=0 and -whitelistrelay=0 (it will still run quick).

I hope I can make it more specific after #16248.

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Aug 6, 2019
Merge bitcoin#16535: test: Explain why -whitelist is used in feature_…
b725979 
…fee_estimation

fa76285 test: Explain why -whitelist is used in feature_fee_estimation (MarcoFalke)
faff85a test: Format feature_fee_estimation with pep8 (MarcoFalke)

Pull request description:

ACKs for top commit:
  practicalswift:
    ACK fa76285 -- diff looks correct
  Sjors:
    ACK fa76285, every bit of clarification helps. It's clear that without `-whitelist` the test becomes extremely slow (it does pass).

Tree-SHA512: 13ec7e4cd0409e7bb76cbcd344e31c0f612c8ce4a1f1ec6ceaedf345f634bc09786ed38d38920c3469b2862c856ee3e5e42534ef90f531bd8dc83c3db3c06417
@maflcko maflcko merged commit fa76285 into bitcoin:master Aug 6, 2019
@maflcko maflcko deleted the 1908-testDocFeeEst branch August 6, 2019 12:37
@maflcko maflcko mentioned this pull request Oct 12, 2019
Closed
codablock added a commit to codablock/dash that referenced this pull request Jan 4, 2020
@codablock
Add -whitelist to all nodes in smartfees.py
64fc8a4 
This speeds up mempool synchronization a lot due to tricking being forced.
This will later conflict with bitcoin#16493 and bitcoin#16535, but this can
easily be resolved (it does the same).
codablock added a commit to codablock/dash that referenced this pull request Jan 4, 2020
@codablock
Add -whitelist to all nodes in smartfees.py
9fbf327 
This speeds up mempool synchronization a lot due to trickling being forced.
This will later conflict with bitcoin#16493 and bitcoin#16535, but this can
easily be resolved (it does the same).
@codablock codablock mentioned this pull request Jan 4, 2020
Merged
UdjinM6 pushed a commit to dashpay/dash that referenced this pull request Jan 4, 2020
@codablock @UdjinM6
Add -whitelist to all nodes in smartfees.py (#3273)
88da298 
This speeds up mempool synchronization a lot due to trickling being forced.
This will later conflict with bitcoin#16493 and bitcoin#16535, but this can
easily be resolved (it does the same).
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 26, 2021
@codablock
Add -whitelist to all nodes in smartfees.py (dashpay#3273)
bc78818 
This speeds up mempool synchronization a lot due to trickling being forced.
This will later conflict with bitcoin#16493 and bitcoin#16535, but this can
easily be resolved (it does the same).
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Nov 25, 2021
@Munkybooty
Merge bitcoin#16535: test: Explain why -whitelist is used in feature_…
f4a7273 
…fee_estimation

fa76285 test: Explain why -whitelist is used in feature_fee_estimation (MarcoFalke)
faff85a test: Format feature_fee_estimation with pep8 (MarcoFalke)

Pull request description:

ACKs for top commit:
  practicalswift:
    ACK fa76285 -- diff looks correct
  Sjors:
    ACK fa76285, every bit of clarification helps. It's clear that without `-whitelist` the test becomes extremely slow (it does pass).

Tree-SHA512: 13ec7e4cd0409e7bb76cbcd344e31c0f612c8ce4a1f1ec6ceaedf345f634bc09786ed38d38920c3469b2862c856ee3e5e42534ef90f531bd8dc83c3db3c06417
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@maflcko @DrahtBot @practicalswift @Sjors @fanquake