-
Notifications
You must be signed in to change notification settings - Fork 47
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
Mempool should discard any transaction with repeated nullifier #1389
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Some minor nits that we should probably handle separately
pkg/core/mempool.md
Outdated
## Implementation details | ||
|
||
### Exposed methods | ||
Mempool exposes `ProcessTx` method that is concurrent-safe, implements trasaction acceptance criteria and adds valid transaction to the mempool state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/trasaction/transaction/
s/valid transaction/valid transactions/
pkg/core/mempool.md
Outdated
- Block Accepted event triggered by Chain component. | ||
|
||
`PropagateLoop` goroutine handles: | ||
- any request for transaction repropagation sent by `ProcessTx` and repropagates at proper rate. It also should prioritize propagation by transaction fee (not implemented). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you can maybe add github issue for transaction prioritization (#1240)
if err != nil { | ||
continue | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we really fail silently here? What to do if data is corrupted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m.data
represents transactions that pass all checks and are accepted in mempool. If for some odd case m.data
has accepted a corrupted tx, this (ContainAnyNullifiers) should not fail here, disallowing acceptance of other valid transactions.
fixes #1388