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

Validate mempool-specific transaction consensus rules #2707

Closed
Tracked by #2309
conradoplg opened this issue Aug 30, 2021 · 4 comments
Closed
Tracked by #2309

Validate mempool-specific transaction consensus rules #2707

conradoplg opened this issue Aug 30, 2021 · 4 comments
Labels
C-enhancement Category: This is an improvement

Comments

@conradoplg
Copy link
Collaborator

conradoplg commented Aug 30, 2021

Motivation

Transactions must be validated before being added to the mempool. In #2679 we just call the existing transaction verifier. The only specific check added there is to make sure there aren't coinbased inputs if it's a mempool transaction.

However, there may be other mempool-specific rules that we need to enforce. We should review which of them are already being checked and implement any rules missing, if any.

Specifications

There isn't any official specification for mempool-specific rules, but https://en.bitcoin.it/wiki/Protocol_rules#.22tx.22_messages seems to be a good reference. It may be also helpful to check what zcashd does.

ZIP 401: Addressing Mempool Denial-of-Service might have some useful rules, but it's zcashd-specific.

Designs

Related Work

Follow up to #2679

@conradoplg conradoplg added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Aug 30, 2021
@teor2345 teor2345 mentioned this issue Aug 30, 2021
59 tasks
@teor2345 teor2345 changed the title Validate mempool transactions Validate mempool-specific transaction consensus rules Aug 30, 2021
@teor2345
Copy link
Contributor

I added ZIP-401 to the specs section, added this ticket to the tracking issue, and tweaked the name so it made sense in the tracking issue.

@mpguerra mpguerra added this to the 2021 Sprint 19 milestone Sep 14, 2021
@mpguerra
Copy link
Contributor

@mpguerra mpguerra removed this from the 2021 Sprint 20 milestone Sep 27, 2021
@mpguerra mpguerra added P-Low and removed S-needs-triage Status: A bug report needs triage labels Oct 12, 2021
@mpguerra
Copy link
Contributor

mpguerra commented Nov 3, 2021

Quoting @teor2345 from a source external to github:

We checked the ZIPs and specs for mempool-specific consensus rules. And we're validating the 4 we know about. (Transaction size limit, no coinbase transactions, expiry of transactions in the mempool, and clearing the mempool on network upgrades.)

Anything that we are not validating is therefore an "undocumented mempool transaction validation rule" and as such I will close this issue for now. If we do find any undocumented mempool transaction validation rules we should open a separate issue for each rule that we find.

@mpguerra mpguerra closed this as completed Nov 3, 2021
@teor2345
Copy link
Contributor

teor2345 commented Nov 4, 2021

We checked the ZIPs and specs for mempool-specific consensus rules. And we're validating the 4 we know about. (Transaction size limit, no coinbase transactions, expiry of transactions in the mempool, and clearing the mempool on network upgrades.)

Anything that we are not validating is therefore an "undocumented mempool transaction validation rule" and as such I will close this issue for now. If we do find any undocumented mempool transaction validation rules we should open a separate issue for each rule that we find.

Just for the record, the Bitcoin and zcashd mempools cache transactions with missing inputs (orphans), until they can be validated.

For Zebra, we decided on an alternative implementation:

  • connect to lots of nodes
  • crawl other nodes' mempools periodically
  • un-reject transactions with missing outputs when the chain tip block changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: This is an improvement
Projects
None yet
Development

No branches or pull requests

4 participants