Repository navigation
backport: v0.26 bitcoin#27379, bitcoin#27625 - #109
Open
DCG-Claude wants to merge 2 commits into
Open
DCG-Claude wants to merge 2 commits into
DCG-Claude wants to merge 2 commits into
Conversation
3fa4c54 [net processing] Pass TxRelay to FindTxForGetData instead of Peer (dergoegge) c85ee76 [net processin] Don't take cs_main in FindTxForGetData (dergoegge) Pull request description: Addresses left over feedback from bitcoin#26140. * bitcoin#26140 (comment) * bitcoin#26140 (comment) `mapRelay` is only accessed from the message processing thread and does not need to be kept in sync with anything validation specific, it is therfore perfectly fine to have it guarded by `g_msgproc_mutex`. ACKs for top commit: jnewbery: utACK 3fa4c54 hebasto: ACK 3fa4c54, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 3ef84bfe4abfa8d991a7e65d9184221294d80e0df0bbb47f0270ab6ca1593266c98abf83c610f9f86b4d16c7a4b62bcf83f8856c68d3c2e10894bff6ed3e88cd Dash adaptations: - src/net_processing.cpp: FindTxForGetData takes (const Peer::TxRelay&, const uint256& txid) not GenTxid, since Dash has no wtxid relay; Dash's asset-unlock lookup in it is kept. - src/net_processing.cpp: the new FindTxForGetData(*tx_relay, inv.hash, ...) call goes in Dash's IsGenTxMsg() branch, keeping the DSTX/spork/LLMQ/ChainLock/ISLock getdata paths. - src/net_processing.cpp: keeps Peer's const GetTxRelay() overload that upstream deletes, because Dash's own IsInvInFilter(const Peer&) still calls it. - src/net_processing.cpp: the FindTxForGetData declaration and mapRelay/g_relay_expiration GUARDED_BY(g_msgproc_mutex) changes are applied to Dash's existing member layout, not upstream's reordered block.
faa2976 Remove mapRelay (MarcoFalke) fccecd7 net_processing: relay txs from m_most_recent_block (Anthony Towns) Pull request description: `mapRelay` (used to relay announced transactions that are no longer in the mempool) has issues: * It doesn't have an absolute memory limit, only an implicit one based on the rate of transaction announcements * <strike>It doesn't have a use-case</strike> EDIT: see below Fix all issues by removing `mapRelay`. For more context, on why a transaction may have been removed from the mempool, see https://github.com/bitcoin/bitcoin/blob/c2f2abd0a4f4bd18bfca41b632d21d803479f3f4/src/txmempool.h#L228-L238 For my rationale on why it is fine to not relay them: Reason | | Rationale -- | -- | -- `EXPIRY` | Expired from mempool | Mempool expiry is by default 2 weeks and can not be less than 1 hour, so a transaction can not be in `mapRelay` while expiring, unless a re-broadcast happened. This should be fine, because the transaction will be re-added to the mempool and potentially announced/relayed on the next re-broadcast. `SIZELIMIT` | Removed in size limiting | A low fee transaction, which will be relayed by a different peer after `GETDATA_TX_INTERVAL` or after we sent a `notfound` message. Assuming it ever made it to another peer, otherwise it will happen on re-broadcast (same as with `EXPIRY` above). `REORG` | Removed for reorganization | Block races are rare, so reorgs should be rarer. Also, the transaction is likely to be re-accepted via the `disconnectpool` later on. If not, it seems fine to let the originating wallet deal with rebroadcast in this case. `BLOCK` | Removed for block | EDIT: Needed for compact block relay, see bitcoin#27625 (comment) `CONFLICT` | Removed for conflict with in-block transaction | The peer won't be able to add the tx to the mempool anyway, unless it is on a different block, in which case it seems fine to let the originating wallet take care of the rebroadcast (if needed). `REPLACED` | Removed for replacement | EDIT: Also needed for compact block relay, see bitcoin#27625 (comment) ? ACKs for top commit: sdaftuar: ACK faa2976 ajtowns: ACK faa2976 glozow: code review ACK faa2976 Tree-SHA512: 64ae3e387b001bf6bd5b6c938e7317f4361f9bc0b8cc5d8f63a16cda2408d2f634a22f8157dfcd8957502ef358208292ec91e7d70c9c2d8a8c47cc0114ecfebd Dash adaptations: - src/net_processing.cpp: m_most_recent_block_txs and the FindTxForGetData lock annotation go at Dash's own PeerManagerImpl member layout; git had misplaced them in a duplicated upstream block, which was dropped - src/net_processing.cpp: NewPoWValidBlock keys block txs by txid plus GetInstanceHash() for stable-txid asset unlocks, in place of upstream's GetWitnessHash() (Dash has no segwit) - src/net_processing.cpp: FindTxForGetData uses Dash's plain uint256 txid argument, not upstream's GenTxid gtxid.GetHash() - src/net_processing.cpp: SendMessages removes the mapRelay/g_relay_expiration block from Dash's GetTxAnnouncement/DSTX announce loop, not from upstream's wtxid-relay loop - test/functional/p2p_leak_tx.py: test_notfound_on_replaced_tx replaces tx_a by mining a conflicting tx_b with generateblock, because Dash has no RBF - test/functional/p2p_leak_tx.py: drops MSG_WTX/wtxid (no segwit), waits on txid, and bumps mocktime 60s before wait_for_broadcast as test_tx_in_block already does
Collaborator
Author
mempool_limit.py fails 3/3 (not a flake): bitcoin#27675's entry_sequence field grows CTxMemPoolEntry's estimated per-entry memory from 368 to 384 bytes (measured locally on arm64), and with Dash's -maxmempool=5 / 73 large-tx setup that seems to be enough that submitpackage's first parent triggers a trim, evicting a ~65KB filler tx and leaving maxmempool-bytes=136750 > 100000. The backport matches upstream; the fix is a Dash-specific mempool_limit.py adjustment, but I couldn't build dashd here (tree not configured) to check any change, so I didn't make one. bitcoin#27675 has had its repair lane and is being dropped from this branch. 🤖 backportsys, on behalf of the Dash backport pipeline. |
DCG-Claude
changed the title
DCG-Claude
force-pushed
the
backport-0.26-b199-src-net
branch
from
October 19, 2026 04:55
cbd5721 to
8ee8acb
Compare
Collaborator
Author
|
Branch rewritten at
🤖 backportsys, on behalf of the Dash backport pipeline. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated Bitcoin Core v0.26 backports, batch
backport-0.26-b199-src-net.cd99b2b6608ee8acb444Provenance
Each commit passed: cherry-pick (adapted by an Opus lane only where conflicts existed), build, touched tests, a mechanical diff-of-diffs check (every upstream hunk landed; no added line without an upstream counterpart), and an independent Opus verification lane where anything was adapted. Gate rows and lane artifacts are in the backportsys DB.