Make Mempool eviction compliant with ZIP-401

[dconnolly]

Motivation

ZIP-401 has some recommendations on how to mitigate DoS attacks on mempool implementations, including how many rejected tx's to track and for how long, and how to select a victim tx to evict from the mempool when out of space (we currently evict the oldest, they recommend a somewhat weighted random choice).

Specifications

https://zips.z.cash/zip-0401

Designs

Rejected set is recommended capped at 40K, we should check if everything we currently put in the rejected set maps to what ZIP-401 considers part of that 40K.

Sub-Tickets

• Limit the size and age of the ZIP-401 rejected transaction ID list #2759
  • Add a mempool config with ZIP-401 options #2840
  • Randomly evict mempool transactions based on their eviction weights #2780
    • Match rejected transactions correctly by TXID or WTXID #2819
    • Add the serialized transaction size to UnminedTx #2778
    • Return the transaction fee in the mempool response #2779
      • Look up the UTXOs in the transaction verifier, then pass them to verify_transparent_inputs_and_outputs #2440
    • Refactor spend conflict checks in the mempool storage to increase performance #2784

[mpguerra]

Hey team! Please add your planning poker estimate with ZenHub @conradoplg @dconnolly @jvff @oxarbitrage @upbqdn

[upbqdn]

ZIP-401 says:

There MUST be a configuration option mempoolevictionmemoryminutes, which SHOULD default to 60.

and

Nodes SHOULD remove transactions from RecentlyEvicted that were evicted more than mempoolevictionmemoryminutes minutes ago.

We were discussing that it might be a bit of a pain to deal with time in Zebra. Today I noticed Zebra actually deals with time already in order to implement the following from the protocol:

In addition, a full validator MUST NOT accept blocks with nTime more than two hours in the future according to its clock.

The implementation of that is here

zebra/zebra-consensus/src/block/check.rs

Line 159 in 1ccb2de

pub fn time_is_valid_at(

and here

zebra/zebra-chain/src/block/header.rs

Line 102 in 2f0f379

pub fn time_is_valid_at(

[teor2345]

Rejected set is recommended capped at 40K, we should check if everything we currently put in the rejected set maps to what ZIP-401 considers part of that 40K.

I wouldn't worry about the exact contents of the rejected set - ZIP-401 is not consensus-critical, so there's no requirement for exact conformance.

But having a single rejected set will make the code much simpler, easier to test, and less likely to have security issues.

[teor2345]

We were discussing that it might be a bit of a pain to deal with time in Zebra. Today I noticed Zebra actually deals with time already

There are actually 3 kinds of time here:

• consensus time: the time in the block header, which is the same for all nodes
• wall clock time: the local node's clock, which can be changed using NTP or by a sysadmin (used to check the block time)
• monotonic time: the time elapsed on the local node, which can never go backwards (ZIP-401)

Picking the right kind of time is a tricky edge-case, which is why we try to avoid dealing with time as much as possible in Zebra.

[teor2345]

This ticket's dependencies conflict with the NU5 block validation code freeze, so it needs to be done after NU5 testnet activation in sprint 19.

[mpguerra]

Changed to epic and removed from Sprint 20 since all relevant child issues are already in that sprint

[mpguerra]

We are done here 🎉