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

Relocate transaction verification #1507

Merged

Conversation

Qiao-Jin
Copy link
Contributor

CLose #1479

Copy link
Member

@vncoelho vncoelho 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, as soon as possible we filter better.

@shargon
Copy link
Member

shargon commented Mar 24, 2020

I think that this will produce the same behaviour than parallel verification, but hard to test it because we will need more nodes.

@Qiao-Jin
Copy link
Contributor Author

I think that this will produce the same behaviour than parallel verification, but hard to test it because we will need more nodes.

Have seperate sender fee check from transaction verication and thus avoid unsafe situation, plz have a check:)

@erikzhang
Copy link
Member

The same transactions from different nodes will be verified multiple times?

@Tommo-L
Copy link
Contributor

Tommo-L commented Aug 13, 2020

The same transactions from different nodes will be verified multiple times?

I think it's acceptable. If one arrives first, it will enter the mempool, and the next same tx will be filtered.

@erikzhang
Copy link
Member

@Qiao-Jin Can you review my changes?

Copy link
Contributor Author

@Qiao-Jin Qiao-Jin left a comment

Choose a reason for hiding this comment

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

I think this is OK.

Copy link
Contributor Author

@Qiao-Jin Qiao-Jin 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

Copy link
Contributor Author

@Qiao-Jin Qiao-Jin 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

Copy link
Contributor Author

@Qiao-Jin Qiao-Jin 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

Copy link
Contributor Author

@Qiao-Jin Qiao-Jin 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

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.

Move transaction verification to RemoteNode
6 participants