Do not use mempool for GETDATA for tx accepted after the last mempool req. #8080
Conversation
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
| pushed = true; | ||
| int64_t txtime; | ||
| if (mempool.lookup(inv.hash, tx, txtime)) { | ||
| if (txtime <= pfrom->timeLastMempool) { |
There was a problem hiding this comment.
wouldn't if (mempool.lookup(inv.hash, tx, textile) && txtime <= pfrom->timeLastMempool) { also work?
There was a problem hiding this comment.
Indeed, current layout is an artifact of my local copy having an else for logging.
|
utACK ef6c31142e52e56af93486986319f5499e72c7df |
1 similar comment
|
utACK ef6c31142e52e56af93486986319f5499e72c7df |
|
Changed the variable name and added a comment in response to Petertodd's comments, also avoided the nested if per jonasschnelli's comments. |
|
utACK 8e9890f |
| @@ -413,6 +414,8 @@ class CNode | |||
| // Used for BIP35 mempool sending, also protected by cs_inventory | |||
| bool fSendMempool; | |||
|
|
|||
| // Last time a mempool | |||
There was a problem hiding this comment.
Please extend the comment here, e.g. Last time of mempool BIP35 request?
|
Concept ACK. |
|
Can one of the admins verify this patch? |
… req. The ability to GETDATA a transaction which has not (yet) been relayed is a privacy loss vector. The use of the mempool for this was added as part of the mempool p2p message and is only needed to fetch transactions returned by it.
laanwj
added a commit
that referenced
this pull request
May 31, 2016
… last mempool req. 7e908c7 Do not use mempool for GETDATA for tx accepted after the last mempool req. (Gregory Maxwell)
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Dec 22, 2017
…ter the last mempool req. 7e908c7 Do not use mempool for GETDATA for tx accepted after the last mempool req. (Gregory Maxwell)
codablock
added a commit
to codablock/dash
that referenced
this pull request
Feb 7, 2018
This should have been part of the Bitcoin bitcoin#8080 backporting but was missed due to manual conflict resolution.
UdjinM6
pushed a commit
to dashpay/dash
that referenced
this pull request
Feb 8, 2018
This should have been part of the Bitcoin bitcoin#8080 backporting but was missed due to manual conflict resolution.
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
…ter the last mempool req. 7e908c7 Do not use mempool for GETDATA for tx accepted after the last mempool req. (Gregory Maxwell)
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
…#1904) This should have been part of the Bitcoin bitcoin#8080 backporting but was missed due to manual conflict resolution.
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Feb 28, 2019
…#1904) This should have been part of the Bitcoin bitcoin#8080 backporting but was missed due to manual conflict resolution.
CryptoCentric
pushed a commit
to absolute-community/absolute
that referenced
this pull request
Mar 2, 2019
…#1904) This should have been part of the Bitcoin bitcoin#8080 backporting but was missed due to manual conflict resolution.
This was referenced Dec 26, 2020
furszy
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Jan 23, 2021
9b9c616 Fix missing zapwallettxes mode in wallet_hd.py functional test (furszy) d6d0ad9 [logs] fix zapwallettxes startup logs (John Newbery) 006c503 [wallet] fix zapwallettxes interaction with persistent mempool (John Newbery) c6d45c6 Adapting and connecting mempool_persist.py functional test to the test runner. (furszy) 4f26a4e Control mempool persistence using a command line parameter. (John Newbery) 5d949de [Qt] Do proper shutdown (Jonas Schnelli) e60da98 Allow shutdown during LoadMempool, dump only when necessary (Jonas Schnelli) c0a0e81 Moving TxMempoolInfo tx member to CTransactionRef (furszy) f0c2255 Add mempool.dat to doc/files.md (furszy) 8e52226 Add DumpMempool and LoadMempool (Pieter Wuille) 44c635d Add AcceptToMemoryPoolWithTime function (Pieter Wuille) 6bbc6a9 Add feedelta to TxMempoolInfo (Pieter Wuille) 9979f3d [mempool] move removed and conflicts transaction ref list to vector. (furszy) 4f672c2 Make removed and conflicted arguments optional to remove (Pieter Wuille) e51c4b8 Bypass removeRecursive in removeForReorg (Pieter Wuille) 54cf7c0 Get rid of CTxMempool::lookup() entirely (furszy) 35bc2a9 Finished the switch CTransaction storage in mempool to CTransactionRef and introduced the timeLastMempoolReq coming from btc#8080. (furszy) d10583b An adapted version of btc@b5599147533103efea896a1fc4ff51f2d3ad5808 (furszy) cb4fc6c An adapted version of btc@ed7068302c7490e8061cb3a558a0f83a465beeea (furszy) 9645775 Split up and optimize transaction and block inv queues (furszy) 68bc68f Don't do mempool lookups for "mempool" command without a filter (furszy) 7624823 mapNextTx: use pointer as key, simplify value (furszy) 191c62e Return mempool queries in dependency order (Pieter Wuille) 23c9f3e Eliminate TX trickle bypass, sort TX invs for privacy and priority. (furszy) 6ebfd17 tiny test fix for mempool_tests (Alex Morcos) 8c0016e Check all ancestor state in CTxMemPool::check() (furszy) 91c6096 Add ancestor feerate index to mempool (Suhas Daftuar) 64e84e2 Add ancestor tracking to mempool This implements caching of ancestor state to each mempool entry, similar to descendant tracking, but also including caching sigops-with-ancestors (as that metric will be helpful to future code that implements better transaction selection in CreatenewBlock). (furszy) 8325bb4 Fix mempool limiting for PrioritiseTransaction Redo the feerate index to be based on mining score, rather than fee. (furszy) 1fa40ac Remove work limit in UpdateForDescendants() The work limit served to prevent the descendant walking algorithm from doing too much work by marking the parent transaction as dirty. However to implement ancestor tracking, it's not possible to similarly mark those descendant transactions as dirty without having to calculate them to begin with. This commit removes the work limit altogether. With appropriate chain limits (-limitdescendantcount) the concern about doing too much work inside this function should be mitigated. (furszy) ba32375 Rename CTxMemPool::remove -> removeRecursive remove is no longer called non-recursively, so simplify the logic and eliminate an unnecessary parameter (furszy) c30fa16 CTxMemPool::removeForBlock now uses RemoveStaged (furszy) Pull request description: Ending up 2020 with a large PR :). Included a good number of performance and privacy improvements over the mempool and inv processing/sending areas + added mempool cache dump/load. Almost finishing with #1726 work, getting closer to 9725, and getting closer to be able to decouple the message processing thread <-> wallet and the wallet <-> GUI locks dependencies (which will be another long story.. but well, step by step). The final goal is clear, a much faster syncing process using pivx-qt (a good speed up for pivxd as well), smoother visual navigation when big wallets are syncing, less thread synchronization issues, among all of the improvements that will be coming with the backports adaptations. Adapted the following PRs to our sources: bitcoin#7062 —> only eb30666. bitcoin#7174 —> complete. bitcoin#7562 —> only c5d746a. bitcoin#7594 —> complete. bitcoin#7840 —> complete. bitcoin#7997 —> complete. bitcoin#8080 —> complete bitcoin#8126 —> except e9b4780 (we don't have the mapRelay) and c2a4724 (we don't have the relay expiration vector). bitcoin#8448 —> complete bitcoin#8515 —> complete bitcoin#9408 —> complete bitcoin#9966 —> complete bitcoin#10330 —> complete ACKs for top commit: random-zebra: ACK 9b9c616 Fuzzbawls: ACK 9b9c616 Tree-SHA512: 186bd09bbb19b55ec0d46dc27291663a0df2d60d8238c661a8c9b8cbf3677b6f8a92fa56bd7346a3cb5293712eeccc5d6542ee3c441d35a4f61e7d12e2ce489a
zkbot
added a commit
to zcash/zcash
that referenced
this pull request
Aug 17, 2021
ZIP 239 preparations 3 Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#8080 - bitcoin/bitcoin#8082 - bitcoin/bitcoin#8126 - bitcoin/bitcoin#7910 - This is the unsquashed version of bitcoin/bitcoin#8149 - We take three cleanup commits to the protocol / `CInv` code. - bitcoin/bitcoin#8822 - bitcoin/bitcoin#8880 - Excluding the first commit (we don't have the comment it fixes yet). - bitcoin/bitcoin#19322
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The ability to GETDATA a transaction which has not (yet) been relayed
is a privacy loss vector.
The use of the mempool for this was added as part of the mempool p2p
message and is only needed to fetch transactions returned by it.