Skip to content

test: Wait for 'cmpctblock' in p2p_compactblocks when it is expected#19631

Merged
laanwj merged 1 commit intobitcoin:masterfrom
Empact:2020-07-receive_block_announcment
Aug 9, 2020
Merged

test: Wait for 'cmpctblock' in p2p_compactblocks when it is expected#19631
laanwj merged 1 commit intobitcoin:masterfrom
Empact:2020-07-receive_block_announcment

Conversation

@Empact
Copy link
Contributor

@Empact Empact commented Jul 30, 2020 •
edited
Loading

This is a more narrowly-construed wait which eliminates the possibility of the
wait being triggered by other messages.

Note received_block_announcement reflect three possible messages:

bitcoin/test/functional/p2p_compactblocks.py

Lines 34 to 53 in edec7f7

def on_cmpctblock(self, message):
self.block_announced = True
self.last_message["cmpctblock"].header_and_shortids.header.calc_sha256()
self.announced_blockhashes.add(self.last_message["cmpctblock"].header_and_shortids.header.sha256)
def on_headers(self, message):
self.block_announced = True
for x in self.last_message["headers"].headers:
x.calc_sha256()
self.announced_blockhashes.add(x.sha256)
def on_inv(self, message):
for x in self.last_message["inv"].inv:
if x.type == MSG_BLOCK:
self.block_announced = True
self.announced_blockhashes.add(x.hash)
# Requires caller to hold mininode_lock
def received_block_announcement(self):
return self.block_announced

Prompted by looking into: #19449

@Empact @billygarrison
test: Wait for 'cmpctblock' in p2p_compactblocks when it is expected
9e165d0 
This is a more narrowly-construed wait which eliminates the possibility of the
wait being triggered by other messages.

Co-authored-by: Billy Garrison <[email protected]>
@fanquake fanquake added the Tests label Jul 30, 2020
@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 31, 2020 •
edited
Loading

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

Conflicts

No conflicts as of last run.

@DrahtBot DrahtBot mentioned this pull request Jul 31, 2020
Closed
theStack
theStack approved these changes Aug 6, 2020
View reviewed changes
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

ACK 9e165d0
Waiting for message type x is better than waiting for message types x, y, z if only x is expected :)

@laanwj
Copy link
Member

laanwj commented Aug 9, 2020 •
edited
Loading

Code review ACK 9e165d0

@laanwj laanwj merged commit be11f94 into bitcoin:master Aug 9, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 9, 2020
@laanwj
Merge bitcoin#19631: test: Wait for 'cmpctblock' in p2p_compactblocks…
eaa8d08 
… when it is expected

9e165d0 test: Wait for 'cmpctblock' in p2p_compactblocks when it is expected (Ben Woosley)

Pull request description:

  This is a more narrowly-construed wait which eliminates the possibility of the
  wait being triggered by other messages.

  Note `received_block_announcement` reflect three possible messages:
  https://github.com/bitcoin/bitcoin/blob/edec7f7c254294cd5c46ae5cf304353d458bb852/test/functional/p2p_compactblocks.py#L34-L53

  Prompted by looking into: bitcoin#19449

ACKs for top commit:
  laanwj:
    Code review ACK 9e165d0
  theStack:
    ACK bitcoin@9e165d0

Tree-SHA512: bc4a9c8bf031c8a7efb40d9625feaa3fd1f56f3b75da7034944af71ccea44328a6c708ab0c13fea85fb7cf4fd9043fe90eb94a25e95b2d42be44c2962b4904ce
@Empact Empact deleted the 2020-07-receive_block_announcment branch August 14, 2020 04:38
@adamjonas adamjonas mentioned this pull request Jan 14, 2021
Closed
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 7, 2021
@Empact @PiRK
test: Wait for 'cmpctblock' in p2p_compactblocks when it is expected
0a4beac 
Summary:
This is a more narrowly-construed wait which eliminates the possibility of the
wait being triggered by other messages.

Co-authored-by: Billy Garrison <[email protected]>

This is a backport of [[bitcoin/bitcoin#19631 | core#19631]]

Test Plan: `test/functional/test_runner.py p2p_compa*`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D10065
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 17, 2021
@laanwj @PastaPastaPasta
Merge bitcoin#19631: test: Wait for 'cmpctblock' in p2p_compactblocks…
29132a1 
… when it is expected

9e165d0 test: Wait for 'cmpctblock' in p2p_compactblocks when it is expected (Ben Woosley)

Pull request description:

  This is a more narrowly-construed wait which eliminates the possibility of the
  wait being triggered by other messages.

  Note `received_block_announcement` reflect three possible messages:
  https://github.com/bitcoin/bitcoin/blob/edec7f7c254294cd5c46ae5cf304353d458bb852/test/functional/p2p_compactblocks.py#L34-L53

  Prompted by looking into: bitcoin#19449

ACKs for top commit:
  laanwj:
    Code review ACK 9e165d0
  theStack:
    ACK bitcoin@9e165d0

Tree-SHA512: bc4a9c8bf031c8a7efb40d9625feaa3fd1f56f3b75da7034944af71ccea44328a6c708ab0c13fea85fb7cf4fd9043fe90eb94a25e95b2d42be44c2962b4904ce
@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

1 more reviewer

@theStack theStack theStack approved these changes

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

@Empact @DrahtBot @laanwj @theStack @fanquake