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

Tests: Fix deserialization of reject messages #7912

Merged
merged 1 commit into from Apr 21, 2016

Conversation

Projects
None yet
3 participants
@sdaftuar
Member

sdaftuar commented Apr 19, 2016

Assume that reject messages for blocks or transactions due to reason REJECT_MALFORMED will not include the hash of the block or tx being rejected.

I found BIP 61 to be slightly ambiguous on how to deserialize in this case, but it seems logical to assume that the peer would have bailed out on parsing the message and thus wouldn't know what hash to include. This is also consistent with the behavior of bitcoin core.

@MarcoFalke Should these strings "block" and "tx" have the b prepended to them as well? I wasn't sure if leaving them out of #7853 was intentional or an oversight.

@laanwj laanwj added the Tests label Apr 20, 2016

@MarcoFalke

View changes

Show outdated Hide outdated qa/rpc-tests/test_framework/mininode.py
Tests: Fix deserialization of reject messages
Assume that reject messages for blocks or transactions due to reason
REJECT_MALFORMED will not include the hash of the block or tx being rejected.
@sdaftuar

This comment has been minimized.

Show comment
Hide comment
@sdaftuar

sdaftuar Apr 20, 2016

Member

Updated with bytes prefix.

Member

sdaftuar commented Apr 20, 2016

Updated with bytes prefix.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 20, 2016

Member

utACK 807fa47

Member

laanwj commented Apr 20, 2016

utACK 807fa47

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke
Member

MarcoFalke commented Apr 20, 2016

utACK 807fa47

@MarcoFalke MarcoFalke referenced this pull request Apr 20, 2016

Merged

[qa] Switch to py3 #7814

1 of 1 task complete

@MarcoFalke MarcoFalke merged commit 807fa47 into bitcoin:master Apr 21, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Apr 21, 2016

Merge #7912: Tests: Fix deserialization of reject messages
807fa47 Tests: Fix deserialization of reject messages (Suhas Daftuar)

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Jun 9, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 12, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 12, 2016

nomnombtc added a commit to nomnombtc/bitcoin that referenced this pull request Nov 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment