-
Notifications
You must be signed in to change notification settings - Fork 301
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
impl rate limit for xtokens #854
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #854 +/- ##
==========================================
+ Coverage 85.25% 85.78% +0.52%
==========================================
Files 87 92 +5
Lines 10350 11055 +705
==========================================
+ Hits 8824 9483 +659
- Misses 1526 1572 +46 ☔ View full report in Codecov by Sentry. |
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
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.
there are some missing coverages reported by code coverage, but 100% sure if the report is correct but need to double check it
/// If the encoded key starts with the vec, the key is in whitelist. | ||
StartsWith(BoundedVec<u8, ConstU32<MAX_FILTER_KEY_LENGTH>>), | ||
/// If the encoded key ends with the vec, the key is in whitelist. | ||
EndsWith(BoundedVec<u8, ConstU32<MAX_FILTER_KEY_LENGTH>>), |
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 don't think we need this
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.
If the key is AccountId, Match
pattern may usually be used, but StartWith
pattern can still be used to match all the pallet accounts. Can consider retaining it, and should use StartWith
and EndWith
with caution in actual configuration.
No description provided.