[qa] Overhaul p2p_compactblocks.py #15660
Conversation
|
This makes changes like that proposed in #15633 much easier to make (I have a relatively small diff to the test that would support that change in particular, should we want to adopt a different behavior). |
There was a problem hiding this comment.
utACK 5bc755f
I've left some comments inline. One is a bug while the rest are style nits.
Whilst reviewing this, a lot of tidy-ups suggested themselves to me, so I went ahead and made a branch here: https://github.com/jnewbery/bitcoin/tree/tidy_up_p2p_compactblocks. I think the result is quite a bit clearer and easier to follow than the existing p2p_compactblocks.py. I've split out the commits in a vaguely logical way so it should be pretty easy to review. Feel free to take any or all of those changes, or we can follow this up with another PR.
5bc755f to
bbd2d9d
Compare
|
I've taken a bunch of the feedback and cherry-picked a few commits from @jnewbery's branch, and updated this PR. Old version is here: https://github.com/sdaftuar/bitcoin/commits/15660.1 |
|
tested ACK f97b821. Please squash commits. |
Remove tests of: - compactblock behavior in a simulated pre-segwit version of bitcoind This should have been removed a long time ago, as it is not generally necessary for us to test the behavior of old nodes (except perhaps if we want to test that upgrading from an old node to a new one behaves properly) - compactblock behavior during segwit upgrade (ie verifying that network behavior before and after activation was as expected) This is unnecessary to test now that segwit activation has already happened. Includes changes by John Newbery.
f97b821 to
7813eb1
Compare
|
Squashed. |
|
utACK 7813eb1 Travis failure is unrelated. I've restarted it. |
7813eb1 [qa] Overhaul p2p_compactblocks.py (Suhas Daftuar) Pull request description: Remove tests of: - compactblock behavior in a simulated pre-segwit version of bitcoind This should have been removed a long time ago, as it is not generally necessary for us to test the behavior of old nodes (except perhaps if we want to test that upgrading from an old node to a new one behaves properly) - compactblock behavior during segwit upgrade (ie verifying that network behavior before and after activation was as expected) This is unnecessary to test now that segwit activation has already happened. ACKs for commit 7813eb: jnewbery: utACK 7813eb1 Tree-SHA512: cadf035e6f822fa8cff974ed0c2e88a1d4d7da559b341e574e785fd3d309cc2c98c63bc05479265dc00550ae7b77fc3cbe815caae7f68bcff13a04367dca9b52
Remove tests of:
compactblock behavior in a simulated pre-segwit version of bitcoind
This should have been removed a long time ago, as it is not generally
necessary for us to test the behavior of old nodes (except perhaps if we
want to test that upgrading from an old node to a new one behaves properly)
compactblock behavior during segwit upgrade (ie verifying that network
behavior before and after activation was as expected)
This is unnecessary to test now that segwit activation has already happened.