-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix(net): Extend blocks-relay-only to also ignore some Dash-specific messages/invs #4888
Conversation
This pull request has conflicts, please rebase. |
LGFM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK for squash merge
This pull request has conflicts, please rebase. |
rebased after #4898 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK for squash merge
As I tested on UI tab "Peers" in Tools Window, the "Outbound block-relay" connections lasts same long as "Outbound" connections. LGTM, |
…stead (#5339) ## Issue being fixed or feature implemented This refactoring is a follow-up changes to backport bitcoin#17164 (PR #5314) These changes are reduce difference in implementation for our code and bitcoin's ## What was done? Removed a flag m_block_relay_peer. Instead I call IsAddrRelayPeer() that has same information now. It changes logic introduced in #4888 due to dash-specific code. ## How Has This Been Tested? Run unit/functional tests. ## Breaking Changes No breaking changes ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone
comment was moved to net.h in 678df63 (dashpay#4888) and removed entirely in 796353a (dashpay#5771). the comment is being restored back to where it is upstream, in CNode::RelayAddrsWithConn.
Dash uses a lot more CNode::RelayAddrsWithConn checks than Bitcoin (esp. since a483122 (dashpay#4888)), so bitcoin#21186 will not adequately cover the removal of RelayAddrsWithConn usages. As IsBlockOnlyConn and RelayAddrsWithConn are mutually exclusive and bitcoin#21186 does away with RelayAddrsWithConn, we can replace all Dash-added usages of RelayAddrsWithConn with NOT IsBlockOnlyConn checks instead.
Dash uses a lot more CNode::RelayAddrsWithConn checks than Bitcoin (esp. since a483122 (dashpay#4888)), so bitcoin#21186 will not adequately cover the removal of RelayAddrsWithConn usages. As IsBlockOnlyConn and RelayAddrsWithConn are mutually exclusive and bitcoin#21186 does away with RelayAddrsWithConn, we can replace all Dash-added usages of RelayAddrsWithConn with NOT IsBlockOnlyConn checks instead.
comment was moved to net.h in 678df63 (dashpay#4888) and removed entirely in 796353a (dashpay#5771). the comment is being restored back to where it is upstream, in CNode::RelayAddrsWithConn.
Dash uses a lot more CNode::RelayAddrsWithConn checks than Bitcoin (esp. since a483122 (dashpay#4888)), so bitcoin#21186 will not adequately cover the removal of RelayAddrsWithConn usages. As IsBlockOnlyConn and RelayAddrsWithConn are mutually exclusive and bitcoin#21186 does away with RelayAddrsWithConn, we can replace all Dash-added usages of RelayAddrsWithConn with NOT IsBlockOnlyConn checks instead.
Dash uses a lot more CNode::RelayAddrsWithConn checks than Bitcoin (esp. since a483122 (dashpay#4888)), so bitcoin#21186 will not adequately cover the removal of RelayAddrsWithConn usages. As IsBlockOnlyConn and RelayAddrsWithConn are mutually exclusive and bitcoin#21186 does away with RelayAddrsWithConn, we can replace all Dash-added usages of RelayAddrsWithConn with NOT IsBlockOnlyConn checks instead.
Dash uses a lot more CNode::RelayAddrsWithConn checks than Bitcoin (esp. since a483122 (dashpay#4888)), so bitcoin#21186 will not adequately cover the removal of RelayAddrsWithConn usages. When possible to query with RelayAddrsWithPeer, that should be used, as that value is the most reliable, else we rely on the former mutual exclusivity of IsBlockOnlyConn and RelayAddrsWithConn to fill in the blanks where a more reliable query isn't available. Note: To prevent builds from breaking, a change has been made in InstantSend code despite it breaking functionality. A commit later will repair it by creating a way to access RelayAddrsWithPeer.
comment was moved to net.h in 678df63 (dashpay#4888) and removed entirely in 796353a (dashpay#5771). the comment is being restored back to where it is upstream, in CNode::RelayAddrsWithConn.
Dash uses a lot more CNode::RelayAddrsWithConn checks than Bitcoin (esp. since a483122 (dashpay#4888)), so bitcoin#21186 will not adequately cover the removal of RelayAddrsWithConn usages. When possible to query with RelayAddrsWithPeer, that should be used, as that value is the most reliable, else we rely on the former mutual exclusivity of IsBlockOnlyConn and RelayAddrsWithConn to fill in the blanks where a more reliable query isn't available. Note: To prevent builds from breaking, a change has been made in InstantSend code despite it breaking functionality. A commit later will repair it by creating a way to access RelayAddrsWithPeer.
comment was moved to net.h in 678df63 (dashpay#4888) and removed entirely in 796353a (dashpay#5771). the comment is being restored back to where it is upstream, in CNode::RelayAddrsWithConn.
Dash uses a lot more CNode::RelayAddrsWithConn checks than Bitcoin (esp. since a483122 (dashpay#4888)), so bitcoin#21186 will not adequately cover the removal of RelayAddrsWithConn usages. When possible to query with RelayAddrsWithPeer, that should be used, as that value is the most reliable, else we rely on the former mutual exclusivity of IsBlockOnlyConn and RelayAddrsWithConn to fill in the blanks where a more reliable query isn't available. Note: To prevent builds from breaking, a change has been made in InstantSend code despite it breaking functionality. A commit later will repair it by creating a way to access RelayAddrsWithPeer.
-blocksonly
was already kind of broken prior to #4862 cause it was not taking into account the fact that we use our "inventory system" not only for transactions but for a set of other messages too. As a result a node running in block-relay-only mode would miss things like e.g.gobject
andgobjvote
and would fork off eventually. After #4862 such node would also constantly disconnect its peers for various violations.