Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
fix: mempool lane size check on
CheckTx
#561fix: mempool lane size check on
CheckTx
#561Changes from 12 commits
beb7aa4
e9adb18
fa30deb
69d83b3
1ccb70b
517d71e
357bc89
57aaa3b
4bf0a4a
e78e34e
d1dec20
4d1eedf
53649a7
4810c2c
59b2e4f
e324323
50cd6dc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 119 in abci/checktx/mempool_parity_check_tx.go
Codecov / codecov/patch
abci/checktx/mempool_parity_check_tx.go#L112-L119
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.
Does it make sense to do this check before the entire ante-handler sequence is run?
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.
I'm not sure - I think the case with a huge Tx is much less likely than the case where youre Tx just fails the antehandler. So i would rather run that and have the matching logic done later only when we need to
Check warning on line 162 in abci/checktx/mempool_parity_check_tx.go
Codecov / codecov/patch
abci/checktx/mempool_parity_check_tx.go#L159-L162
Check warning on line 164 in abci/checktx/mempool_parity_check_tx.go
Codecov / codecov/patch
abci/checktx/mempool_parity_check_tx.go#L164
Check warning on line 194 in abci/checktx/mempool_parity_check_tx.go
Codecov / codecov/patch
abci/checktx/mempool_parity_check_tx.go#L193-L194
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.
can you benchmarket how long this takes just so we have a reference on the slowdown we expected? I imagine some of this is not necessary.
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.
How would you imagine benchmarking this?
Start a small in mem test app and call this on it?
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.
benchmarking with a real simapp would be better, but probably not important to get this shipped. fine as is
Check warning on line 12 in block/base/match.go
Codecov / codecov/patch
block/base/match.go#L12