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

Respond to inbound TransactionsById with mempool content #2725

Merged
merged 5 commits into from
Sep 6, 2021

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Sep 2, 2021

Motivation

Now that we have a mempool we can reply to TransactionsById requests.

This will close #1077

Solution

Respond to the TransactionsById with mempool content.

Review

This is pretty straightforward after we made the first mempool message in #2720. I used the same test to avoid repeating a bunch of setup code but this can be changed if it is more clear to have separated ones.

@jvff already reviewed #2720 and this is following the same line so is a good candidate but i think anyone working in the mempool can review as well.

Reviewer Checklist

  • Code makes sense
  • New test pass
  • All current tests pass

Follow Up Work

In bitcoin implementation there is an additional notfound response to this message with the non found hashes. It seems this is not needed in zcash network (#1077 (comment)) however we might need more research. #2726 was opened for this.

dconnolly
dconnolly previously approved these changes Sep 2, 2021
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

Looks great! Some small style notes

Co-authored-by: Deirdre Connolly <durumcrustulum@gmail.com>
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

🎉

@dconnolly dconnolly enabled auto-merge (squash) September 6, 2021 19:38
@dconnolly dconnolly modified the milestone: 2021 Sprint 17 Sep 6, 2021
@dconnolly dconnolly merged commit 65e308d into ZcashFoundation:main Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send inbound TransactionsById requests to the mempool storage service
2 participants