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

Verify inbound PushTransactions #2727

Merged
merged 5 commits into from
Sep 9, 2021
Merged

Verify inbound PushTransactions #2727

merged 5 commits into from
Sep 9, 2021

Conversation

conradoplg
Copy link
Collaborator

@conradoplg conradoplg commented Sep 2, 2021

Motivation

Transactions can be directly pushed to nodes (instead of their IDs being advertised) and we need to verify them before adding to the mempool.

Specifications

Designs

Solution

This adds a new function to the transaction dowloader/verifier to just verify a pushed transaction.

Closes #2692

Review

Must be merged after #2718.

Probably anyone working on the mempool can review this.

Reviewer Checklist

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

Follow Up Work

@conradoplg conradoplg linked an issue Sep 2, 2021 that may be closed by this pull request
@mpguerra mpguerra requested a review from dconnolly September 6, 2021 09:55
Base automatically changed from check-existing-txs to main September 8, 2021 18:51
@conradoplg conradoplg force-pushed the verify-push-transactions branch from e86d62b to 0195a7a Compare September 8, 2021 18:55
@oxarbitrage
Copy link
Contributor

As we discussed yesterday i removed the grafana changes in 9972d9e so they can be added in a stand alone PR

oxarbitrage
oxarbitrage previously approved these changes Sep 8, 2021
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

I added a new ticket to track the tests we have missing #2745

@oxarbitrage oxarbitrage enabled auto-merge (squash) September 8, 2021 22:30
@oxarbitrage oxarbitrage disabled auto-merge September 8, 2021 22:46
@oxarbitrage oxarbitrage merged commit f3ee76f into main Sep 9, 2021
@oxarbitrage oxarbitrage deleted the verify-push-transactions branch September 9, 2021 13:04
@mpguerra mpguerra mentioned this pull request Sep 9, 2021
59 tasks
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.

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.

Verify inbound PushTransactions
5 participants