-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat: add mempool interfaces #13249
feat: add mempool interfaces #13249
Conversation
709d9f5
to
b5c4da1
Compare
b5c4da1
to
d610042
Compare
Codecov Report
@@ Coverage Diff @@
## main #13249 +/- ##
==========================================
+ Coverage 54.78% 55.02% +0.23%
==========================================
Files 660 653 -7
Lines 56914 55521 -1393
==========================================
- Hits 31182 30549 -633
+ Misses 23232 22519 -713
+ Partials 2500 2453 -47
|
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
// minimal as possible, only requiring applications to define the size of the | ||
// transaction to be used when reaping and getting the transaction itself. | ||
// Interface type casting can be used in the actual app-side mempool implementation. | ||
type MempoolTx interface { |
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.
Wish I had a chance to review this prior to merge. I really think this should not exist here.
IMO all mempool logic should live in a mempool
subpackage in baseapp/mempool
.
|
||
if err == nil { | ||
|
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.
@kocubinski please. undo these changes in #13262 :)
* working out interfaces * integration to checkTx * use struct fields directly in sz calculation * fix typo * nil guard on mempool * Remove tx builder method * impl with panic
Description
Ref: #13150
Followed by: #13262
I am proposing
Select(...)
instead ofReapMaxBytes(...)
as in ADR-60, since theProcessProposal
steps can be simplified fromto
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change