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

feat(mempool): add ZIP-317 rules to mempool #6556

Merged
merged 18 commits into from
May 2, 2023
Merged

feat(mempool): add ZIP-317 rules to mempool #6556

merged 18 commits into from
May 2, 2023

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Apr 22, 2023

Motivation

We want to add ZIP-317 relay rules to the mempool verifier, so Zebra can drop transactions before they get into the mempool storage.

Close #5336

Specifications

Solution

This pull request is checking 2 rules currently:

  • Check if transaction unpaid actions is under a threshold.
  • Check if the miner fee is greater than the transaction conventional fee.

If there are more rules they need to be added to zebra_chain::transactions::unmined::mempool_checks function.

Review

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

Follow Up Work

@github-actions github-actions bot added the C-feature Category: New features label Apr 22, 2023
@oxarbitrage oxarbitrage marked this pull request as ready for review April 25, 2023 19:30
@oxarbitrage oxarbitrage requested review from a team as code owners April 25, 2023 19:30
@oxarbitrage oxarbitrage requested review from upbqdn and removed request for a team April 25, 2023 19:30
@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Merging #6556 (00b03ad) into main (1461c91) will increase coverage by 0.32%.
The diff coverage is 97.47%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6556      +/-   ##
==========================================
+ Coverage   77.78%   78.10%   +0.32%     
==========================================
  Files         307      309       +2     
  Lines       40273    40621     +348     
==========================================
+ Hits        31325    31727     +402     
+ Misses       8948     8894      -54     

Copy link
Contributor

@teor2345 teor2345 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, thanks for doing this!
I hope it wasn't too complicated, it seems like updating the tests was a bit of work.

I just want to check where these rules are coming from, to make sure we got them all.

zebra-chain/src/transaction/unmined.rs Outdated Show resolved Hide resolved
zebra-chain/src/transaction/unmined/zip317.rs Outdated Show resolved Hide resolved
zebra-chain/src/transaction/unmined/zip317.rs Outdated Show resolved Hide resolved
zebra-chain/src/transaction/unmined/zip317.rs Outdated Show resolved Hide resolved
zebra-consensus/src/transaction/tests.rs Show resolved Hide resolved
zebra-chain/src/transaction/unmined.rs Show resolved Hide resolved
@oxarbitrage oxarbitrage self-assigned this Apr 27, 2023
@oxarbitrage oxarbitrage added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code P-High 🔥 labels Apr 27, 2023
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Thanks for these changes, I am going to go and see what I can do with the fee rate code using the Rust compiler.

zebra-chain/src/transaction/unmined/zip317.rs Outdated Show resolved Hide resolved
zebra-chain/src/transaction/unmined/zip317.rs Outdated Show resolved Hide resolved
zebra-chain/src/transaction/unmined/zip317.rs Outdated Show resolved Hide resolved
teor2345 and others added 2 commits April 28, 2023 11:07
… use usize (#6585)

* Refactor the minimum fee rate calculation to use usize

* Check for overflow if constants change
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Thanks for this, looks great!

mergify bot added a commit that referenced this pull request May 1, 2023
mergify bot added a commit that referenced this pull request May 1, 2023
@mergify mergify bot merged commit 8075d61 into main May 2, 2023
246 checks passed
@mergify mergify bot deleted the issue5336 branch May 2, 2023 00:13
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-feature Category: New features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Relay and accept mempool transactions based on the ZIP-317 fee rules
2 participants