Problem: url encoded characters are not properly handled in contract_by_denom #1560
Conversation
mmsqe
requested review from
yihuang and
thomas-nguy
and removed request for
a team
WalkthroughThe recent changes include updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Keeper
Client->>API: Request contract by denom
API->>Keeper: Call ContractByDenom(req)
Keeper->>Keeper: Decode req.Denom
alt Decoding Successful
Keeper->>Keeper: Query contracts with decoded denom
Keeper-->>API: Return contract information
else Decoding Failed
Keeper-->>API: Return error
end
API-->>Client: Return response
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1560 +/- ##
===========================================
+ Coverage 15.24% 36.12% +20.88%
===========================================
Files 67 97 +30
Lines 4874 7725 +2851
===========================================
+ Hits 743 2791 +2048
- Misses 4037 4585 +548
- Partials 94 349 +255 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- CHANGELOG.md (1 hunks)
- integration_tests/configs/genesis_token_mapping.jsonnet (1 hunks)
- integration_tests/test_exported_genesis.py (2 hunks)
- x/cronos/keeper/grpc_query.go (2 hunks)
Files skipped from review due to trivial changes (1)
- CHANGELOG.md
Additional context used
GitHub Check: codecov/patch
x/cronos/keeper/grpc_query.go
[warning] 25-27: x/cronos/keeper/grpc_query.go#L25-L27
Added lines #L25 - L27 were not covered by tests
[warning] 31-31: x/cronos/keeper/grpc_query.go#L31
Added line #L31 was not covered by tests
[warning] 35-35: x/cronos/keeper/grpc_query.go#L35
Added line #L35 was not covered by tests
[warning] 40-40: x/cronos/keeper/grpc_query.go#L40
Added line #L40 was not covered by tests
Additional comments not posted (14)
integration_tests/test_exported_genesis.py (8)
2-2: LGTM!The import statement for
urllib.parseis necessary for URL encoding/decoding.
6-6: LGTM!The import statement for
requestsis necessary for making HTTP requests.
40-46: LGTM!The test case for querying a contract by its denomination is comprehensive and ensures the correctness of the API response.
47-47: LGTM!The code correctly retrieves the API port using the
ports.api_portfunction.
48-49: LGTM!The code correctly URL encodes the denomination using
urllib.parse.quoteto ensure it is safe for use in a URL.
50-50: LGTM!The code correctly constructs the URL for the API request using the encoded denomination and the API port.
51-51: LGTM!The code correctly makes the HTTP GET request to the constructed URL and retrieves the response.
52-52: LGTM!The code correctly validates the API response against the expected output to ensure it matches.
x/cronos/keeper/grpc_query.go (5)
7-7: LGTM!The import statement for
net/urlis necessary for URL decoding.
25-28: LGTM!The code correctly URL decodes the denomination using
url.QueryUnescapeto ensure it is properly interpreted.Tools
GitHub Check: codecov/patch
[warning] 25-27: x/cronos/keeper/grpc_query.go#L25-L27
Added lines #L25 - L27 were not covered by tests
31-31: LGTM!The code correctly queries the external contract using the decoded denomination.
Tools
GitHub Check: codecov/patch
[warning] 31-31: x/cronos/keeper/grpc_query.go#L31
Added line #L31 was not covered by tests
35-35: LGTM!The code correctly queries the auto contract using the decoded denomination.
Tools
GitHub Check: codecov/patch
[warning] 35-35: x/cronos/keeper/grpc_query.go#L35
Added line #L35 was not covered by tests
40-40: LGTM!The code correctly handles the case when no contracts are found and returns an appropriate error message.
Tools
GitHub Check: codecov/patch
[warning] 40-40: x/cronos/keeper/grpc_query.go#L40
Added line #L40 was not covered by testsintegration_tests/configs/genesis_token_mapping.jsonnet (1)
59-62: LGTM!The new entry expands the configuration to accommodate an additional token mapping.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/cronos/types/query.pb.gois excluded by!**/*.pb.gox/cronos/types/query.pb.gw.gois excluded by!**/*.pb.gw.go
Files selected for processing (4)
- CHANGELOG.md (1 hunks)
- client/docs/swagger-ui/swagger.yaml (2 hunks)
- integration_tests/test_exported_genesis.py (2 hunks)
- proto/cronos/query.proto (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- integration_tests/test_exported_genesis.py
Additional comments not posted (2)
client/docs/swagger-ui/swagger.yaml (1)
Line range hint
1-1: File is empty.There are no code changes to review in this file.
proto/cronos/query.proto (1)
15-17: LGTM!The changes improve the semantic understanding of the method's functionality and adjust the endpoint's URL structure.
decode fix is in grpc-gateway v2, to avoid encode twice we can also pass denom as query string
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests