/bitcoin

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix headers announcements edge case #7919

Merged
merged 1 commit into from Apr 22, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
P2P
Projects
None yet
Milestone
No milestone
8 participants
@sdaftuar @morcos @jonasschnelli @sipa @paveljanik @TheBlueMatt @MarcoFalke @laanwj
@sdaftuar
Member

sdaftuar commented Apr 20, 2016

Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect. However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.

FYI I encountered this bug once while running mempool_packages.py, and was able to reproduce reliably by repeatedly invoking invalidateblock/reconsiderblock on the tip.

Perhaps we should backport this to 0.12 as well?
@sdaftuar
Fix headers announcements edge case 
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect.  However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.
3a99fb2
@morcos
Member

morcos commented Apr 20, 2016

utACK

@jonasschnelli jonasschnelli added the P2P label Apr 21, 2016

@jonasschnelli
Member

jonasschnelli commented Apr 21, 2016

utACK 3a99fb2
@sipa
Owner

sipa commented Apr 21, 2016

utACK 3a99fb2
@paveljanik
Contributor

paveljanik commented Apr 21, 2016

utACK 3a99fb2
@TheBlueMatt
Contributor

TheBlueMatt commented Apr 22, 2016

utACK 3a99fb2

On April 20, 2016 11:26:28 AM PDT, Suhas Daftuar notifications@github.com wrote:

Previously we would assert that if every block in
vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect. However,
there are
edge cases where this assumption could be violated (eg using
invalidateblock /
reconsiderblock), so just check for this case and revert to
inv-announcement
instead.

FYI I encountered this bug once while running mempool_packages.py,
and was able to reproduce reliably by repeatedly invoking
invalidateblock/reconsiderblock on the tip.

Perhaps we should backport this to 0.12 as well?
You can view, comment on, or merge this pull request online at:

#7919

-- Commit Summary --

  • Fix headers announcements edge case

-- File Changes --

M src/main.cpp (16)

-- Patch Links --

https://github.com/bitcoin/bitcoin/pull/7919.patch
https://github.com/bitcoin/bitcoin/pull/7919.diff

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#7919

@laanwj laanwj added the Needs backport label Apr 22, 2016

@laanwj laanwj merged commit 3a99fb2 into bitcoin:master Apr 22, 2016
1 check passed

1 check passed

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

laanwj added a commit that referenced this pull request Apr 22, 2016

@laanwj
Merge #7919: Fix headers announcements edge case 
3a99fb2 Fix headers announcements edge case (Suhas Daftuar)
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 74810B012346C9A6 Learn about signing commits
7617682

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Apr 27, 2016

@sdaftuar @MarcoFalke
Fix headers announcements edge case 
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect.  However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.

Github-Pull: #7919
Rebased-From: 3a99fb2
43c14ac
@MarcoFalke
Member

MarcoFalke commented Jun 9, 2016

Backported as part of #7938. Removing label 'Needs backport'.

@MarcoFalke MarcoFalke removed the Needs backport label Jun 9, 2016

zander added a commit to zander/bitcoinclassic that referenced this pull request Jun 16, 2016

@sdaftuar @zander
Fix headers announcements edge case 
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect.  However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.

Github-Pull: #7919
Rebased-From: 3a99fb2
8611bea

thokon00 added a commit to faircoin/faircoin that referenced this pull request Jun 28, 2016

@sdaftuar @thokon00
Fix headers announcements edge case 
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect.  However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.

Github-Pull: #7919
Rebased-From: 3a99fb2
f4cb7eb

@dgenr8 dgenr8 referenced this pull request in bitcoinxt/bitcoinxt Jul 20, 2016

Closed

[WIP] Direct headers announcement #137

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

@sdaftuar @nomnombtc
Fix headers announcements edge case 
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect.  However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.

Github-Pull: #7919
Rebased-From: 3a99fb2
e9edf81

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

@sdaftuar @nomnombtc
Fix headers announcements edge case 
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect.  However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.

Github-Pull: #7919
Rebased-From: 3a99fb2
1e3b7ed

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

@sdaftuar @nomnombtc
Fix headers announcements edge case 
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect.  However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.

Github-Pull: #7919
Rebased-From: 3a99fb2
8eb18b8

sickpig added a commit to sickpig/BitcoinUnlimited that referenced this pull request Nov 14, 2016

@sdaftuar @sickpig
Fix headers announcements edge case 
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect.  However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.

Github-Pull: #7919
Rebased-From: 3a99fb2
This commit was signed with a verified signature.
@sickpig sickpig Andrea Suisani
GPG key ID: A9CA1D0EE3789741 Learn about signing commits
4622fcd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment