Skip to content

Conversation

dergoegge
Copy link
Member

This line was accidentally removed in #22778.

Receiving a filterload message implies that we should relay txs to the sender (CNode::m_relays_txs = true). CNode::m_relays_txs is only used for the inbound eviction logic, so removing the line might have slightly changed the eviction behaviour but nothing else.

@fanquake fanquake added the P2P label Jun 22, 2022
@dergoegge dergoegge changed the title [net processing] Set CNode::m_relays_txs=true when receiving BIP37 filters p2p: Set CNode::m_relays_txs=true when receiving BIP37 filters Jun 22, 2022
@laanwj
Copy link
Member

laanwj commented Jun 22, 2022

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK e7a9133

So it actually happened that the redundant fields CNode::m_relays_txs and Peer::TxRelay::m_relay_txs went out of sync (one is true while the other is false) :-(

@DrahtBot
Copy link
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25268 (refactor: Introduce EvictionManager by dergoegge)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake fanquake merged commit e05564d into bitcoin:master Jun 23, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 27, 2022
… BIP37 filters

e7a9133 [net processing] Set CNode::m_relays_txs=true when receiving BIP37 filters (dergoegge)

Pull request description:

  This line was accidentally removed in bitcoin#22778.

  Receiving a `filterload` message implies that we should relay txs to the sender (`CNode::m_relays_txs = true`). `CNode::m_relays_txs` is only used for the inbound eviction logic, so removing the line might have slightly changed the eviction behaviour but nothing else.

ACKs for top commit:
  laanwj:
    Code review ACK e7a9133
  vasild:
    ACK e7a9133

Tree-SHA512: 19c5df0f579f707c6c7900d12a6b71ac69e802be64f7d2fdcc40ac714c918dc4c17def164592f8836cc105a03daefefca3ca5e10423145eca8db4348c27c9cfc
@jnewbery
Copy link
Contributor

post-merge ACK e7a9133

Thanks @dergoegge!

@bitcoin bitcoin locked and limited conversation to collaborators Jun 29, 2023
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