ZIP 239 preparations 3 #5274
Merged
ZIP 239 preparations 3 #5274
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
This brings the code more closely in line with upstream, and prepares for subsequent performance improvements.
… 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. (cherry picked from commit bitcoin/bitcoin@7e908c7)
This reduces the rate of not founds by better matching the far end expectations, it also improves privacy by removing the ability to use getdata to probe for a node having a txn before it has been relayed. (cherry picked from commit bitcoin/bitcoin@4d8993b)
(cherry picked from commit bitcoin/bitcoin@1b9e6d3)
str4d
added
C-upstream-port
Contributor
Author
|
@zkbot try |
Contributor
|
⌛ Trying commit 57cf4c271cc68d6c5e8a64e73cf42470519589f7 with merge 1d7ed0617499d3c6377e763eeeeeeea7fb0279b5... |
Contributor
|
💔 Test failed - pr-try |
Contributor
Author
|
RPC test failures are reproducible (though |
(cherry picked from commit bitcoin/bitcoin@8d39d7a)
* Switch mapRelay to use shared_ptr<CTransaction> * Switch the relay code to copy mempool shared_ptr's, rather than copying the transaction itself. * Change vRelayExpiration to store mapRelay iterators rather than hashes (smaller and faster). (cherry picked from commit bitcoin/bitcoin@dbfb426)
(cherry picked from commit bitcoin/bitcoin@e9b4780)
(cherry picked from commit bitcoin/bitcoin@c2a4724)
(cherry picked from commit bitcoin/bitcoin@288d85d)
(cherry picked from commit sipa/bitcoin@e35d020)
(cherry picked from commit sipa/bitcoin@adb1c09)
(cherry picked from commit sipa/bitcoin@97d7402)
In principle, the checksums of P2P packets are simply 4-byte blobs which are the first four bytes of SHA256(SHA256(payload)). Currently they are handled as little-endian 32-bit integers half of the time, as blobs the other half, sometimes copying the one to the other, resulting in somewhat confused code. This PR changes the handling to be consistent both at packet creation and receiving, making it (I think) easier to understand. (cherry picked from commit bitcoin/bitcoin@41e58fa)
The P2P network uses a fixed protocol, these sizes shouldn't change based on what happens to be the architecture. (cherry picked from commit bitcoin/bitcoin@305087b)
Also move the enum to the top, and remove a deceptive TODO comment. (cherry picked from commit bitcoin/bitcoin@2c09a52)
This concretizes the numbers and adds a comment to make it clear that these numbers are fixed by the protocol, and may avoid people forgetting to claim numbers in the future (e.g. issue #8500). Also gets rid of a weird unused `MSG_TYPE_MAX` in the middle of the enumeration (thanks @paveljanik for noticing). (cherry picked from commit bitcoin/bitcoin@1df3111)
PushInventory() is currently called with a CInv object, which can be a MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine whether to add the hash to setInventoryTxToSend or vInventoryBlockToSend. Since the caller always knows what type of inventory they're pushing, the CInv is wastefully constructed and thrown away, and tx/block relay is being split out, we split the function into PushTxInventory() and PushBlockInventory(). (cherry picked from commit bitcoin/bitcoin@f52d403) Zcash: Adjusted to account for bitcoin/bitcoin#15759 not having been backported.
Contributor
Author
|
RPC test failures were due to me originally trying to backport bitcoin/bitcoin#8126 before a needed commit (adding score-based sorting) from bitcoin/bitcoin#6898, and then once the latter was backported in #5269 I mis-rebased this branch so that the score-based sorting was inconsistent across two APIs, which resulted in the node segfaulting somewhere in the depths of diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index bdf14e7eb7..25b0d37cbc 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -699,7 +699,7 @@ public:
// As of https://github.com/bitcoin/bitcoin/pull/8126 this comparator
// duplicates that function (as it doesn't need to hold a dependency
// on the mempool).
- return CompareTxMemPoolEntryByFee()(*a, *b);
+ return CompareTxMemPoolEntryByScore()(*a, *b);
}
};
} |
nuttycom
approved these changes
Aug 17, 2021
Contributor
nuttycom
left a comment
nuttycom
left a comment
There was a problem hiding this comment.
utACK; verified against the upstream changes.
Contributor
Author
|
@zkbot r+ |
Contributor
|
📌 Commit 8469683 has been approved by |
Contributor
Contributor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
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.
Cherry-picked from the following upstream PRs:
CInvcode.