-
Notifications
You must be signed in to change notification settings - Fork 5.7k
BIP153: SENDTEMPLATE #1937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
BIP153: SENDTEMPLATE #1937
Conversation
fixed, published |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting idea, thanks for sharing.
bip-ajtowns-sendtemplate.md
Outdated
5. A block containing the transactions making up the payload must not | ||
exceed four million weight units as defined in [BIP-141][BIP-141]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered allowing more data than just the top block? Otherwise, would you recommend that a node requests a template after each block is found for the first few new blocks after they come online? Have you considered a reconciliation-based approach akin to Erlay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See discussion in the delving thread.
I think having two blocks of txs could make sense, but in adversarial situations it doubles the potential memory usage and it's not immediately clear what would be a sensible way of addressing that.
My thought is that deploying a simple design and getting some real-world data to iterate on makes sense. You could update the specification to announce "SENDTEMPLATE 2" indicating you support v2, which indicates to your peers they can send you a "GETTEMPLATE 2" message requesting a 2MvB template rather than a 1MvB template, eg.
Likewise sending set differences through could make sense (and wouldn't need the complexity of minisketch since the sender knows their own previous templates), but there's still a lot of design space left, so getting real-world data also seems useful for resolving that. (Likewise an updated spec could allow SENDTEMPLATE 2D
to indicate deltas are supported, and GETTEMPLATE 2 $hash
could request a 2MvB template encoded as a delta against the previous template with hash $hash
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the limit from 4M weight units to 8M weight units.
55a8927
to
4352c1b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From an editorial perspective, this looks close to ready. I noticed that there is no Rationale section, yet. You could perhaps link to the Delving discussion as that seems to have been the most active one I saw.
Updated with post-history in metadata and some rationale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good improvements.
Time to request a bip number then I guess? |
Let’s call this BIP 153 then! |
|
||
### `gettemplate` | ||
|
||
1. The `gettemplate` message is defined as a message with `pchCommand == |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to not also allocate a message ID from the range specific in BIP 324?
The guidelines specify that if a message is sent more than once per connection then a number you be allocated (IIUC both this and the template
message):
We recommend reserving 1-byte type IDs for message types that are sent more than once per direction per connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I have in my backlog an idea for having these ids get determined dynamically during initial connection negotiation, which avoids the problem of BIPs attempting to assign the same id to different messages)
potentially providing better privacy than if only the nodes directly | ||
involved in the transaction would rebroadcast it. | ||
|
||
## Specification |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice there's no p2p protocol version bump, why is that?. IIUC, this message is only useful if you're talking with a peer that's directly peered, or is the Bitcoin node backing a mining pool. A p2p version bump would permit nodes to seek out other nodes that meet this requirement, instead of blindly connecting out to find such nodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to seek out peers that support a feature you need a service flag, not a protocol version. Using a pre-verack negotiation message gives you all the benefits of a protocol version bump, with less potential for contention where two bips developed at the same time both bump the protocol version to the same value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah I meant service flag 😅.
The service flag would also allow for peers to find other supporting peers via DNS queries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hopeful that it will eventually be widely supported enough not to need people to seek out supporting clients explicitly -- it should be pretty low overhead to just provide templates. Also, I think you'd want something more subtle than just "does the node support this protocol" when working out who to peer with -- you want templates that are about equally as diverse as miners: getting the same template from multiple peers isn't very helpful, and getting templates that don't match any miners also isn't helpful. So I think it makes more sense to get some experience with the behaviour of things before speccing out best practices or new features for peer discovery.
6fbdd71
to
86d20db
Compare
@ajtowns: What’s your status on this one? Are you still planning changes or waiting for some people to finish their review of this PR? |
I think this is fine to merge as draft, further review/changes can happen in that state as I understand it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may have overlooked it, but at first glance it looks like the draft is missing important specification on how to implement the goals set forth in the Motivation section. For instance, which peers to request templates from, when/how often, and how the template data is to be used. (Perhaps also limits on memory use for storing templates and/or how this interacts with the -maxmempool limits.) Do you intend to add these to the Specification or Recommendations section, or for a separate BIP?
A few non-blocking comments follow.
* adding a payload to the `sendtemplate` message to indicate that the | ||
`gettemplate` payload will be accepted | ||
|
||
## Recommendations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this could use some recommendations re privacy implications?
IIRC, there was some push back augments devs re BIP 35 (mempool
message), as it can be used to implement relay correlation and tagging attacks against peers. As the message enables a peer to obtain the entire mempool of another peer, it can feasibly be used by an adversary to figure out: who relayed what, which transactions may have originated directly from the node, etc.
Some related PRs from the bitcoind repo:
- Remove BIP35 mempool p2p message bitcoin#27426 -- an attempt to remove the command all together, but was later abandoned.
- Several performance and privacy improvements to inv/mempool handling bitcoin#7840 -- restrcited the type of peers that bitcoind will respond to w/ responses to the
mempool
message (white list applied), the start of some of the inv trickle/batch re address privacy concerns
Also pool operators may want to filter out transactions that they received via out of band transaction submission.
86d20db
to
26a1f71
Compare
I see those as implementation quality issues, which would be added in the "recommendations" section, once it's more clear what those recommendations should be. |
I don't see how this doesn't have major privacy leaks, similar to the |
Adds a BIP for SENDTEMPLATE, GETTEMPLATE, TEMPLATE p2p messages. Discussion links:
https://delvingbitcoin.org/t/sharing-block-templates/1906
https://gnusha.org/pi/bitcoindev/[email protected]/T/#u