Skip to content

Conversation

@amitiuttarwar
Copy link
Contributor

Adds a release note & addresses this review comment to make expectations more explicit.

@amitiuttarwar amitiuttarwar changed the title [p2p] Small followups to #21528 [p2p] Small follow-ups to 21528 Aug 3, 2021
@maflcko
Copy link
Member

maflcko commented Aug 3, 2021

Missing #21528 (comment) ? 😅

@amitiuttarwar amitiuttarwar force-pushed the 2021-08-blackhole-followup branch from 1396662 to 7ac9c1f Compare August 3, 2021 17:19
@amitiuttarwar
Copy link
Contributor Author

@MarcoFalke fixed 😛

@DrahtBot DrahtBot added the P2P label Aug 3, 2021
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK 7ac9c1f22b15509ad1775ecd707ea8615c05d306

Feel free to ignore the optional ideas below.

@Zero-1729
Copy link
Contributor

ACK 7ac9c1f, esp. with @jonatack's suggestions above (e.g. spelling and release notes).

@maflcko
Copy link
Member

maflcko commented Aug 4, 2021

cr ACK 7ac9c1f22b15509ad1775ecd707ea8615c05d306

Copy link
Contributor

@jnewbery jnewbery left a comment

Choose a reason for hiding this comment

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

ACK 7ac9c1f22b15509ad1775ecd707ea8615c05d306

And fix a typo in the test.
Currently, this call to SetupAddressRelay will never return false because of
the previous guard that returns early if the peer is not an inbound connection.
Rather than implicitly relying on this guarantee, throw an error in the debug
build if it ever changes.
@amitiuttarwar amitiuttarwar force-pushed the 2021-08-blackhole-followup branch from 7ac9c1f to 9778b0f Compare August 4, 2021 19:36
@amitiuttarwar
Copy link
Contributor Author

thanks for reviews! updated to incorporate feedback

@Zero-1729
Copy link
Contributor

re-ACK 9778b0f

@jonatack
Copy link
Member

jonatack commented Aug 5, 2021

ACK 9778b0f

@maflcko maflcko merged commit dd981b5 into bitcoin:master Aug 5, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 5, 2021
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 1, 2022
Summary:
Backport of [[bitcoin/bitcoin#22618 | core#22618]].

Depends on D10933.

Ref T1693.

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, tyler-smith, PiRK

Reviewed By: #bitcoin_abc, tyler-smith, PiRK

Subscribers: tyler-smith

Maniphest Tasks: T1693

Differential Revision: https://reviews.bitcoinabc.org/D10934
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants