Skip to content
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

ZIP-239: v5 mempool transaction protocol versions #2451

Closed
Tracked by #2309
teor2345 opened this issue Jul 7, 2021 · 4 comments
Closed
Tracked by #2309

ZIP-239: v5 mempool transaction protocol versions #2451

teor2345 opened this issue Jul 7, 2021 · 4 comments
Labels
A-network Area: Network protocol updates or fixes A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU-5 Network Upgrade: NU5 specific tasks

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jul 7, 2021

Motivation

When the mempool is implemented, Zebra must conform to ZIP-239 in order to properly relay transactions to the network.

This ticket focuses on activating the new inv message variant at the specified protocol version, and rejecting it beforehand.

Scheduling and Alternatives

If necessary, this ticket can be delayed until later in the mempool work.

After NU5 activates on testnet and mainnet, we can set INITIAL_MIN_NETWORK_PROTOCOL_VERSION to Nu5. If we make this change, we do not need to implement the consensus rules in this ticket.

Specifications

ZIP-239 describes the consensus rules for a new MSG_WTX inventory type, that MUST be used when relaying V5 transactions.

An inv or getdata message MUST NOT use the MSG_WTX inv type ... on peer connections that have not negotiated at least the peer protocol version specified in Deployment (170014).

it is possible for a node to receive an inv or getdata message with a MSG_WTX inv type, on a peer connection that has negotiated protocol version 170014 or later, before NU5 has activated. The node MUST handle this case in the same way that it would after NU5 activation when it has no v5 transactions to relay

Receiving a MSG_WTX inv type on a peer connection that has negotiated a protocol version before 170014, on the other hand, SHOULD be treated as a protocol error.

Related Work

#2233 and #2446 added an initial Message::Wtx variant for ZIP-239, and implemented serialization and deserialization of it.

The transaction ID for V5 transactions (with the auth_digest) is defined in ZIP-244, and implemented in #2129.

#2449 implements the remaining data structures for WTXs, and sends them for v5 transactions.

@teor2345 teor2345 added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage NU-5 Network Upgrade: NU5 specific tasks P-Medium A-network Area: Network protocol updates or fixes labels Jul 7, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Jul 19, 2021
@mpguerra mpguerra mentioned this issue Aug 11, 2021
59 tasks
@teor2345
Copy link
Contributor Author

We might be able to close this ticket, because it's covered by other tickets.

Here's how we handle each network protocol rule:

An inv or getdata message MUST NOT use the MSG_WTX inv type ... on peer connections that have not negotiated at least the peer protocol version specified in Deployment (170014).

We won't send the MSG_WTX inv type until we have v5 transactions in our mempool.
We don't expect to get v5 transactions until we're connected to zcashd peers which have activated NU5.

After NU5 activation, we'll reject connections from outdated peers.
We should also eventually disconnect existing outdated peers.

Tickets:

it is possible for a node to receive an inv or getdata message with a MSG_WTX inv type, on a peer connection that has negotiated protocol version 170014 or later, before NU5 has activated. The node MUST handle this case in the same way that it would after NU5 activation when it has no v5 transactions to relay

We don't activate the mempool until we've reached the chain tip, which means we'll act like it's empty.
We don't expect to get v5 transactions until after NU5 activation (see above).

Tickets:

Receiving a MSG_WTX inv type on a peer connection that has negotiated a protocol version before 170014, on the other hand, SHOULD be treated as a protocol error.

Zebra will ignore these messages before NU5 activation (see above).
After NU5 activation, it will respond as if the connection had a valid version.

This is acceptable behaviour, because the protocol rule is only a SHOULD, not a MUST.

@mpguerra mpguerra added this to the 2021 Sprint 20 milestone Aug 26, 2021
@dconnolly
Copy link
Contributor

Closing in agreement with #2451 (comment)

@teor2345
Copy link
Contributor Author

teor2345 commented Sep 20, 2021

We don't actually need to implement this ticket, because in the worst case, Zebra:

  • parses but drops v5 transactions before NU5 activation
  • accepts and sends v5 transactions after NU5 activation

@dconnolly dconnolly removed this from the 2021 Sprint 19 milestone Sep 20, 2021
@teor2345
Copy link
Contributor Author

Here's some more analysis of interactions with legacy Bitcoin protocol implementations:

Currently, if Zebra receives a 36-byte WTX message, parsing will always fail. The InventoryHash parser will expect body_len + 32 bytes for WTX, but the message parser only provides body_len bytes.

The body_len limit is applied even if the message has trailing bytes.

(This assumes the peer is honest-but-buggy. A malicious peer could modify body_len as well. But if they can do that, they can send arbitrary data, so there's no additional vulnerability here.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-network Area: Network protocol updates or fixes A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

No branches or pull requests

3 participants