-
Notifications
You must be signed in to change notification settings - Fork 0
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: tx rate limit #7
Conversation
TODO: need to add error to cosmos-sdk after merge.(like ErrMempoolIsFull) |
@dudong2 |
Yes, need to register an error in the cosmos-sdk to check that a comet-related error occurred at the broadcast tx level. |
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 except for just one trivial comment.
mempool/errors.go
Outdated
} | ||
|
||
func (e ErrMempoolRateLimitExceeded) Error() string { | ||
return fmt.Sprintf("mempool rate limit exceeded: rate %d, count %d", e.Rate, e.Count) |
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 think rate: %d
is little bit obvious.
How about rate: "%d txs per block
?
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.
agree, i changed rate to limit
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
* feat: Rate limit * chore: Change default valuetest: Fix tests * chore: Change default value to -1 * test: Add mock func for ResetRateLimitCounter * chore: Rate -> Limit
ref. a81c3c7
PR checklist
.changelog
(we use unclog to manage our changelog)docs/
orspec/
) and code comments