Ensure rate limits are not applied to packets that aren't ics20s #7357
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PaddyMc
approved these changes
Jan 26, 2024
czarcas7ic
reviewed
Jan 29, 2024
Comment on lines
+562
to
+563
| // This will error out, but not because of rate limiting | ||
| suite.Require().NotContains(err.Error(), "rate limit") |
There was a problem hiding this comment.
Instead of notContains, could we just match the error that we get from this? Just in case we update the rate limit error in the future.
Is a nit though.
There was a problem hiding this comment.
I was thinking the same but from a different direction: if the underlying error changes in ibc, this will continue to pass. But I can add the other check when I untangle the uncommitted changes on my current branch
czarcas7ic
approved these changes
Jan 29, 2024
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What is the purpose of the change
Non-ics20 packets were being caught by rate limits. We check the packets now and skip rate limits if they don't have an amount or denom.
Testing and Verifying
added a unit test
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)