Skip to content

Add test for -blocksonly and -whitelistforcerelay param interaction #18530

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.

Sign up for GitHub

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
Merged
merged 2 commits into from
May 21, 2020
Merged

Add test for -blocksonly and -whitelistforcerelay param interaction #18530

merged 2 commits into from
May 21, 2020

Conversation

glowang
Copy link
Contributor

@glowang glowang commented Apr 5, 2020 •
edited
Loading

Related to: #18428

When -blocksonly is turned on, a node would still relay transactions from whitelisted peers. This funcitonality has not been tested.

@fanquake fanquake added the Tests label Apr 5, 2020
glowang
glowang commented Apr 5, 2020
View reviewed changes
glowang
glowang commented Apr 5, 2020
View reviewed changes
glowang
glowang commented Apr 5, 2020
View reviewed changes
glowang
glowang commented Apr 5, 2020
View reviewed changes
@glowang glowang changed the title WIP: add test for -blocksonly and -whitelistforcerelay Help needed: add test for -blocksonly and -whitelistforcerelay Apr 5, 2020
@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 5, 2020

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.

@DrahtBot DrahtBot mentioned this pull request Apr 5, 2020
Closed
maflcko
maflcko reviewed Apr 5, 2020
View reviewed changes
@maflcko
Copy link
Member

maflcko commented Apr 5, 2020

Concept ACK

maflcko
maflcko reviewed Apr 8, 2020
View reviewed changes
maflcko
maflcko reviewed Apr 8, 2020
View reviewed changes
@glowang glowang force-pushed the add_blocksonly_tests branch from 037f2d3 to bc432ee Compare April 12, 2020 00:25
maflcko
maflcko reviewed Apr 12, 2020
View reviewed changes
glowang
glowang commented Apr 12, 2020
View reviewed changes
@glowang glowang force-pushed the add_blocksonly_tests branch 2 times, most recently from c273658 to 63924b8 Compare April 12, 2020 03:38
@glowang glowang changed the title Help needed: add test for -blocksonly and -whitelistforcerelay Add test for -blocksonly and -whitelistforcerelay Apr 12, 2020
@glowang glowang marked this pull request as ready for review April 12, 2020 03:38
@glowang glowang force-pushed the add_blocksonly_tests branch from 63924b8 to 209da1b Compare April 12, 2020 03:41
@glowang glowang changed the title Add test for -blocksonly and -whitelistforcerelay Add test for -blocksonly and -whitelistforcerelay param interaction Apr 12, 2020
@glowang glowang force-pushed the add_blocksonly_tests branch 2 times, most recently from ef96f10 to f348b9b Compare April 18, 2020 21:56
glowang
glowang commented Apr 18, 2020
View reviewed changes
glowang
glowang commented Apr 18, 2020
View reviewed changes
@glowang glowang requested a review from maflcko April 18, 2020 22:00
maflcko
maflcko reviewed Apr 18, 2020
View reviewed changes
@maflcko
Copy link
Member

maflcko commented Apr 18, 2020

From travis:

test/functional/p2p_blocksonly.py:68:83: W291 trailing whitespace

test/functional/p2p_blocksonly.py:74:1: W293 blank line contains whitespace

^---- failure generated from test/lint/lint-python.sh


This diff appears to have added new lines with trailing whitespace.

The following changes were suspected:

diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py

@@ -59,0 +59,20 @@ class P2PBlocksOnly(BitcoinTestFramework):

+        assert_equal(self.nodes[0].testmempoolaccept([sigtx])[0]['allowed'], True)

+

^---- failure generated from test/lint/lint-whitespace.sh

@glowang glowang force-pushed the add_blocksonly_tests branch 2 times, most recently from 0a828f4 to 13cff3a Compare April 25, 2020 23:15
glowang
glowang commented Apr 25, 2020
View reviewed changes
@glowang glowang force-pushed the add_blocksonly_tests branch from 13cff3a to 18e5f49 Compare April 28, 2020 15:31
maflcko
maflcko reviewed Apr 28, 2020
View reviewed changes
@glowang glowang requested a review from maflcko April 29, 2020 05:47
@glowang glowang force-pushed the add_blocksonly_tests branch from 18e5f49 to c51c8a2 Compare May 4, 2020 05:41
@glowang glowang force-pushed the add_blocksonly_tests branch from c51c8a2 to be01449 Compare May 5, 2020 04:57
@glowang
Copy link
Contributor Author

glowang commented May 10, 2020 •
edited
Loading

All comments have been addressed. This PR is ready for review.

maflcko
maflcko approved these changes May 10, 2020
View reviewed changes
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK. Thanks for working on the documentation and adding tests.

@glowang glowang requested a review from maflcko May 15, 2020 03:43
maflcko
maflcko reviewed May 15, 2020
View reviewed changes
@glowang glowang force-pushed the add_blocksonly_tests branch from 248d2b8 to 0ea5d70 Compare May 17, 2020 16:01
@glowang glowang requested a review from maflcko May 17, 2020 16:02
@glowang
Copy link
Contributor Author

glowang commented May 17, 2020

net_processing.cpp file comment has been updated. ready for re-review.

@maflcko
Copy link
Member

maflcko commented May 17, 2020

ACK 0ea5d70

@maflcko
Copy link
Member

maflcko commented May 21, 2020

Thanks again for improving documentation and adding tests. This should be ready for merge now.

@maflcko maflcko merged commit cfe22a5 into bitcoin:master May 21, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 21, 2020
Merge bitcoin#18530: Add test for -blocksonly and -whitelistforcerela…
8c3da42 
…y param interaction

0ea5d70 Updated comment for the condition where a transaction relay is denied (glowang)
be01449 Add test for param interaction b/w -blocksonly and -whitelistforcerelay (glowang)

Pull request description:

  Related to: bitcoin#18428

  When -blocksonly is turned on, a node would still relay transactions from whitelisted peers. This funcitonality has not been tested.

ACKs for top commit:
  MarcoFalke:
    ACK 0ea5d70

Tree-SHA512: 4e99c88281cb518cc67f5f3be7171a7b413933047b5d24a04bb3ff2210a82e914d69079f64cd5bac9206ec435e21a622c8e69cedbc2ccb39d2328ac5c01668e5
@t-bast t-bast mentioned this pull request Sep 11, 2020
Closed
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 8, 2021
@glowang @PiRK
Add test for param interaction b/w -blocksonly and -whitelistforcerelay
5abc699 
Summary:
Updated comment for the condition where a transaction relay is denied

This is a backport of Core [[bitcoin/bitcoin#18530 | PR18530]]

Test Plan: test/functional/test_runner.py p2p_blocksonly

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: majcosta

Differential Revision: https://reviews.bitcoinabc.org/D9145
@maflcko maflcko mentioned this pull request Mar 18, 2021
Closed
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@maflcko maflcko Awaiting requested review from maflcko

1 more reviewer

@RANDALL-hub RANDALL-hub RANDALL-hub left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@glowang @DrahtBot @maflcko @RANDALL-hub @fanquake