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.

Sign up for GitHub

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

Jump to bottom

fix(rpc): Return verification errors from sendrawtransaction RPC method #8788

Merged
merged 8 commits into from
Aug 30, 2024
Merged

fix(rpc): Return verification errors from sendrawtransaction RPC method #8788

merged 8 commits into from
Aug 30, 2024

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Aug 20, 2024 •
edited
Loading

Motivation

We want to return an error from the sendrawtransaction RPC method if a transaction cannot be verified and added to the mempool.

Closes #8778.

Solution

Adds a QueueRpc mempool request to wait for a transaction verification result and return an error message to the client if the transaction was not verified and added to Zebra's mempool.

Tests

This PR still needs tests.

Follow-up Work

  • @hhanh00 reported that Zebra was failing to add transactions to its mempool or return an error describing why transactions weren't added to the mempool, we want to return a specific error so we can fix the issue.
  • We likely need an update to lightwalletd to handle these errors correctly.
  • Handle verification errors in inbound service by incrementing peer connection "bad score" when tracking misbehaving nodes

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@arya2 arya2 added C-bug Category: This is a bug C-enhancement Category: This is an improvement I-usability Zebra is hard to understand or use A-rpc Area: Remote Procedure Call interfaces lightwalletd any work associated with lightwalletd A-mempool Area: Memory pool transactions P-High 🔥 labels Aug 20, 2024
@arya2 arya2 self-assigned this Aug 20, 2024
@arya2 arya2 added do-not-merge Tells Mergify not to merge this PR and removed do-not-merge Tells Mergify not to merge this PR labels Aug 27, 2024
@arya2 arya2 marked this pull request as ready for review August 30, 2024 02:42
@arya2 arya2 requested review from a team as code owners August 30, 2024 02:42
@arya2 arya2 requested review from upbqdn and removed request for a team August 30, 2024 02:42
@arya2
checks for closed channel errors in sendrawtransaction and updates a …
33b7767 
…prop test to check that verification errors are propagated correctly
@arya2 arya2 requested a review from oxarbitrage August 30, 2024 14:40
@mergify mergify bot merged commit 6b95d27 into main Aug 30, 2024
135 checks passed
@mergify mergify bot deleted the send-tx-errors branch August 30, 2024 20:09
@arya2 arya2 mentioned this pull request Oct 9, 2024
Merged
7 tasks
dmidem pushed a commit to QED-it/zebra that referenced this pull request Oct 29, 2024
@arya2 @dmidem
fix(rpc): Return verification errors from sendrawtransaction RPC me…
6879118 
…thod (ZcashFoundation#8788)

* Adds a mempool request to wait for a transaction verification result and uses it in `sendrawtransaction` RPC method

* removes unnecessary clone

* fix clippy warnings

* returns verification errors for all `mempool::Queue` requests, removes `QueueRpc` request variant

* returns oneshot channel in mempool::Response::Queue

* updates a test vector to check for download or verification error in mempool::response::Queued result receiver

* Always require tokio as a dependency in zebra-node-services

* checks for closed channel errors in sendrawtransaction and updates a prop test to check that verification errors are propagated correctly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@upbqdn upbqdn upbqdn approved these changes

@oxarbitrage oxarbitrage Awaiting requested review from oxarbitrage

Assignees

@arya2 arya2

Labels
A-mempool Area: Memory pool transactions A-rpc Area: Remote Procedure Call interfaces C-bug Category: This is a bug C-enhancement Category: This is an improvement I-usability Zebra is hard to understand or use lightwalletd any work associated with lightwalletd P-High 🔥
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Return verification errors from sendrawtransaction RPC method
2 participants
@arya2 @upbqdn