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

Un-reject mempool transactions if the rejection depends on the current tip #2844

Merged
merged 3 commits into from
Oct 7, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Oct 7, 2021

Motivation

Some mempool rejections only apply to the current tip, so we want to un-reject them whenever the tip changes.

We also want to make sure the rejection lists don't get too big.

Closes #2694.
Implements part of #2759 (some size limits).

Specifications

The mempool rejection rules aren't really specified anywhere.

We're re-using the size limit from ZIP-401 for all our reject lists:

The size of RecentlyEvicted SHOULD never exceed eviction_memory_entries entries,
which is the constant 40000.

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

Solution

  • Split mempool storage errors into tip-based and chain-based
  • Expire tip rejections every time we get a new block
  • Enforce MAX_EVICTION_MEMORY_ENTRIES for mempool reject lists

We might want to wait to add more tests until after PR #2821 or ZIP-401, because the APIs are still changing a lot.

Review

Anyone can review this PR, but I think @conradoplg wants it so he can complete PR #2821.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

@teor2345 teor2345 added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium labels Oct 7, 2021
@teor2345 teor2345 requested a review from conradoplg October 7, 2021 04:29
@teor2345 teor2345 self-assigned this Oct 7, 2021
@mpguerra mpguerra linked an issue Oct 7, 2021 that may be closed by this pull request
@conradoplg conradoplg force-pushed the mempool-iter-name-cleanup branch from 0c83f4b to 0d52fec Compare October 7, 2021 15:21
Base automatically changed from mempool-iter-name-cleanup to main October 7, 2021 17:43
FailedVerification and SpendConflict rejections only apply to the current tip.
The next tip can provide missing inputs, or evict conflicting transactions.
@conradoplg conradoplg force-pushed the un-reject-at-next-block branch from c97c515 to a329f5b Compare October 7, 2021 18:58
@conradoplg
Copy link
Collaborator

Rebased and force-pushed after mempool-iter-name-cleanup was merged

Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@conradoplg conradoplg merged commit 664d438 into main Oct 7, 2021
@conradoplg conradoplg deleted the un-reject-at-next-block branch October 7, 2021 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Un-reject transactions which depend on newly created outputs
2 participants