Relay ancestors of transactions #14318
Conversation
Prior to this commit, we'd respond with tx data for anything in mapRelay, regardless of whether the requesting peer was one that we'd sent an INV to for the transaction in question. Close this privacy leak by maintaining a set of peers to which we've relayed each transaction in mapRelay.
If we announce a transaction T to a peer, then we should also be willing to provide T's parents to that peer (in case our peer is missing those parents). This should improve propagation of transaction chains.
Reviewers, this pull request conflicts with the following ones:
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. |
| def on_getheaders(self, message): pass | ||
| def on_headers(self, message): pass | ||
| def on_mempool(self, message): pass | ||
| def on_notfound(self, message): pass |
There was a problem hiding this comment.
jamesob
left a comment
There was a problem hiding this comment.
utACK 6b66180
For future reviewers: the INV for missing parents (which we currently expect may not be answered if relay interval on the remote node has lapsed) happens here:
bitcoin/src/net_processing.cpp
Lines 2303 to 2309 in d799efe
This PR is dependent on #14220 and probably shouldn't be merged before that.
| else: | ||
| self.vec = vec | ||
|
|
||
| def deserialize(self, f): |
There was a problem hiding this comment.
Could use a better variable name than f? :-)
There was a problem hiding this comment.
This maintains consistency with existing methods in similar classes (and it's a one-line usage: not considerably different from, say, a list comprehension) so I think this is fine.
There was a problem hiding this comment.
Now I see that f is used consistently. Makes sense to follow that.
| Needs rebase |
|
@sdaftuar do you have any specific reason for closing? |
|
@laanwj This builds on a PR that I closed because it needed rework, so I just didn't want to leave this open when it wasn't ready for review. |
|
@sdaftuar thanks for the explanation, makes sense |
Respond to getdata requests for transactions that are ancestors of newly-announced transactions. Currently, our software will request the parents of an orphan transaction from the peer who announced the orphan, but if our peer is running a recent Bitcoin Core version, the request would typically be dropped. So this should improve propagation for transaction chains, particularly for nodes that are just starting up.
(continued from #14220, which is now just a bugfix PR)
Depends on: