test: Check that a block rejected due to too-new time is later accepted in p2p_invalid_block #17872

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:qa_timetoonew_retry changing 1 files +16 −0

luke-jr commented at 2:47 AM on January 5, 2020: member

I was unsure if the code sufficiently avoided caching a time-too-new rejection, so wrote this test to check it.

It looks like despite only exempting BLOCK_MUTATED, it is still okay because header failures never cache block invalidity.

This test will help ensure that if this ever changes, BLOCK_TIME_FUTURE gets excluded at the same time.

Note: Branch is a clean merge to 0.19 and master

QA: p2p_invalid_block: Check that a block rejected due to too-new time is later accepted

I was unsure if the code sufficiently avoided caching a time-too-new rejection, so wrote this test to check it.
It looks like despite only exempting BLOCK_MUTATED, it is still okay because header failures never cache block invalidity.
This test will help ensure that if this ever changes, BLOCK_TIME_FUTURE gets excluded at the same time.

ffbfba7be1

fanquake added the label Tests on Jan 5, 2020

in test/functional/p2p_invalid_block.py:147 in ffbfba7be1

 142 | +        block = create_block(tip, create_coinbase(height), t + 7201)
 143 | +        block.hashMerkleRoot = block.calc_merkle_root()
 144 | +        block.solve()
 145 | +        # NOTE: Need force_send because the block will get rejected without a getdata otherwise
 146 | +        node.p2p.send_blocks_and_test([block], node, force_send=True, success=False, reject_reason='time-too-new')#, expect_disconnect=True)
 147 | +        #self.reconnect_p2p()

jimmysong commented at 3:19 AM on January 6, 2020:

take out comment?

jimmysong commented at 3:20 AM on January 6, 2020: contributor

LGTM
in test/functional/p2p_invalid_block.py:11 in ffbfba7be1
```
  10 | @@ -11,6 +11,7 @@
  11 |  re-requested.
```
glozow commented at 10:28 PM on May 21, 2020:

Should update description to include the added test?

in test/functional/p2p_invalid_block.py:146 in ffbfba7be1

 141 | +        node.setmocktime(t)
 142 | +        block = create_block(tip, create_coinbase(height), t + 7201)
 143 | +        block.hashMerkleRoot = block.calc_merkle_root()
 144 | +        block.solve()
 145 | +        # NOTE: Need force_send because the block will get rejected without a getdata otherwise
 146 | +        node.p2p.send_blocks_and_test([block], node, force_send=True, success=False, reject_reason='time-too-new')#, expect_disconnect=True)

glozow commented at 10:33 PM on May 21, 2020:

Stray in-line comment, and I'm pretty sure we don't expect disconnect?

glozow commented at 10:34 PM on May 21, 2020: member

ACK with nits, mostly comment-related. It seems like feature_block.py and mining_basic.py both test too-new blocks but don't check that they're accepted later. I'm not sure what the difference is between invalid_block.py and feature_block.py; what's the reasoning for putting it here?
fanquake renamed this:
~~QA: p2p_invalid_block: Check that a block rejected due to too-new time is later accepted~~
test: Check that a block rejected due to too-new time is later accepted in p2p_invalid_block
on Apr 8, 2021
fanquake added the label Waiting for author on Apr 8, 2021
fanquake commented at 6:02 AM on April 8, 2021: member

Two concept ACKs, but this needs review comments addressing, added "waiting for author".
fanquake commented at 6:29 AM on May 31, 2021: member

Basically two months since the last ping and "waiting for author" addition. Going to close this as "up for grabs" if anyone wants to pick it up.
fanquake closed this on May 31, 2021
fanquake added the label Up for grabs on May 31, 2021
fanquake removed the label Waiting for author on May 31, 2021
MarcoFalke removed the label Up for grabs on Jun 2, 2021
MarcoFalke commented at 8:06 AM on June 2, 2021: member

picked up in #22120
MarcoFalke referenced this in commit dd24567a24 on Jun 17, 2021
DrahtBot locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#22120 test: p2p_invalid_block: Check that a block rejected due to too-new tim…