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

Use unmined types for transaction verifier mempool requests and responses #2666

Merged
merged 6 commits into from
Aug 25, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Aug 25, 2021

Motivation

Now that zebra-network uses unmined types for transactions, we also need to use them in the transaction verifier's Mempool request.

Solution

  • Use UnminedTx in transaction verifier Mempool requests
  • Use UnminedTxId as the transaction verifier response type

We'll want the full witnessed ID even for Block requests, because it will help us diagnose mined transaction verification failures, and auth data root verification failures.

Minor cleanup:

  • Refactor is_mempool into a transaction verifier request method
  • Improve debug and trace logs

Review

@conradoplg might want this PR before starting work on #2637, so the types match up.

This PR is based on PR #2665, feel free to rebase it on main if needed.

Reviewer Checklist

  • Code implements Specs and Designs
  • Existing tests pass

We'll add more tests for this code as part of the mempool work.

@teor2345 teor2345 added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium labels Aug 25, 2021
@teor2345 teor2345 requested a review from conradoplg August 25, 2021 00:43
@teor2345 teor2345 self-assigned this Aug 25, 2021
dconnolly
dconnolly previously approved these changes Aug 25, 2021
Base automatically changed from init-transaction-verifier to main August 25, 2021 15:07
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! I've rebased and force-pushed after #2665 was merged, so I'll leave it for @teor2345 or @dconnolly to check if I did that correctly and merge this (and address @daira's comments if desired)

@teor2345 teor2345 enabled auto-merge (squash) August 25, 2021 20:27
@teor2345 teor2345 merged commit 2ed6679 into main Aug 25, 2021
@teor2345 teor2345 deleted the mempool-verifier-types branch August 25, 2021 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants