-
Notifications
You must be signed in to change notification settings - Fork 61
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: mapped accounts #436
Conversation
Co-authored-by: Ajaz Ahmed Ansari <[email protected]>
Codecov Report
@@ Coverage Diff @@
## develop #436 +/- ##
===========================================
- Coverage 54.47% 54.38% -0.10%
===========================================
Files 163 164 +1
Lines 12045 12255 +210
===========================================
+ Hits 6562 6665 +103
- Misses 4987 5087 +100
- Partials 496 503 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Looks good so far;
xcclookup will require a new query type to return map of addresses, indexed by chain ID (I don't think we need them bech32ified, because the first thing we do in xcclookup is decode the address anyway) for a given address:
/quicksilver/interchainstaking/v1/mapped_addresses/quick1abcdef
which is why we prefix the local address mapping by address and not chain, so we prefix iterator over it for this query :)
if we do, we will need to add the account prefix to the ProtocolDataConnection structs
* add mapped account query * format * rebase * return map instead of struct * Update x/interchainstaking/client/cli/query.go Co-authored-by: Alex Johnson <[email protected]> * Update x/interchainstaking/keeper/grpc_query.go * Update x/interchainstaking/keeper/grpc_query_test.go * fix lint * use sdkConfig --------- Co-authored-by: Alex Johnson <[email protected]>
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.
a couple of nits remaining
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.
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.
lgtm
remove unused import
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.
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.
lgtm
1. Summary
Fixes QCK-452
2.Type of change
3. Implementation details
4. How to test/use
5. Checklist
6. Limitations (optional)
7. Future Work (optional)