Skip to content

Conversation

dergoegge
Copy link
Member

CNode::m_relays_txs is meant to only be used for the eviction logic in net. TxRelay::m_relay_txs will hold the same value and is meant to be used on the application layer to determine if we will/should relay transactions to a peer.

(Shameless plug: we should really better specify the interface for updating eviction data to avoid refactors like this in the future -> #25572)

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 16, 2023

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK MarcoFalke
Concept ACK sdaftuar

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25572 (refactor: Introduce EvictionManager and use it for the inbound eviction logic by dergoegge)
  • #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.

@sdaftuar
Copy link
Member

utACK

CNode::m_relays_txs is meant to only be used for the eviction logic in net. TxRelay::m_relay_txs will hold the same value and is meant to be used on the application layer to determine if we will/should relay transactions to a peer.

@dergoegge Thanks for mentioning this understanding -- I hadn't looked at this code in a while, but I agree with this and I think this is helpful for everyone to understand as we do more work in this module.

@maflcko maflcko added the P2P label Apr 11, 2023
@maflcko
Copy link
Member

maflcko commented Apr 11, 2023

lgtm ACK 55c4795

@fanquake fanquake merged commit 53eb4b7 into bitcoin:master Apr 11, 2023
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 11, 2023
…txs usage

55c4795 [net processing] Use TxRelay::m_relay_txs over CNode::m_relays_txs (dergoegge)

Pull request description:

  `CNode::m_relays_txs` is meant to only be used for the eviction logic in `net`. `TxRelay::m_relay_txs` will hold the same value and is meant to be used on the application layer to determine if we will/should relay transactions to a peer.

  (Shameless plug: we should really better specify the interface for updating eviction data to avoid refactors like this in the future -> bitcoin#25572)

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 55c4795

Tree-SHA512: 59cfd23e32568fd96cda5570790e518242a6c76d4edf5b7d1a2a7f9724d590d2a38395504e05be0af4e98dd5c0056fc0be6568eab2818934692483a186e5181d
@bitcoin bitcoin locked and limited conversation to collaborators Apr 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants