Skip to content
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.

Sign up for GitHub

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

Jump to bottom

orchestration - chain - ICQ Balance Query #9072

Closed
ivanlei opened this issue Mar 12, 2024 · 2 comments · Fixed by #9198
Closed

orchestration - chain - ICQ Balance Query #9072

ivanlei opened this issue Mar 12, 2024 · 2 comments · Fixed by #9198
Assignees
@0xpatrickdev
Labels
enhancement New feature or request

Comments

@ivanlei
Copy link
Contributor

ivanlei commented Mar 12, 2024 •
edited by 0xpatrickdev
Loading

What is the Problem Being Solved?

the chain object from #9063 should expose an API for making ICQ balance queries.

Additionally, the OrchestrationService object should allow for chains unknown to the API to make ICQ balance queries.

Description of the Design

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations
@ivanlei ivanlei added the enhancement New feature or request label Mar 12, 2024
@0xpatrickdev 0xpatrickdev mentioned this issue Apr 5, 2024
Merged
@0xpatrickdev 0xpatrickdev self-assigned this Apr 5, 2024
@0xpatrickdev
Copy link
Member

I updated the description to include (slightly paraphrased) language from #9063 :

Additionally, allow for chains unknown to the API additional details like IBC connection info may be needed.

This is in light of #9198, which adds async createQueryConnection(controllerConnectionId : IBCConnectionID) => QueryConnectionKit['connection'] to OrchestrationService. The planned behavior is for Chain objects to create and store and one of these objects and share it among users, but the facet is also available to any consumers of OrchestrationService.

If the idea is to do a provide pattern, where we only deal in Chain objects and OrchestrationService consumers are not permitted to request their own icqcontroller port / connection (only networkVat / portAllocator consumers), we can move the createQueryConnection and createAccount methods to a separate facet. We should also document the intended behavior more explicitly in #9063.

@0xpatrickdev 0xpatrickdev mentioned this issue Apr 29, 2024
Closed
0xpatrickdev added a commit that referenced this issue Apr 30, 2024
@0xpatrickdev
feat(cosmic-proto): add icq/v1 proto + codegen for @agoric/orchestration
0d3d02e 
- sourced from: https://github.com/cosmos/ibc-apps/tree/18248c35e913b3ccba99a2756612b1fcb3370a0d/modules/async-icq/proto/icq/v1
- refs: #9072
0xpatrickdev added a commit that referenced this issue Apr 30, 2024
@0xpatrickdev
feat(cosmic-proto): add JsonSafe and RequestQueryJson types
6c8690f 
- refs: #9072
0xpatrickdev added a commit that referenced this issue Apr 30, 2024
@0xpatrickdev
feat(network): add allocateICQControllerPort to PortAllocator
65f9c3c 
- refs: #9072
0xpatrickdev added a commit that referenced this issue Apr 30, 2024
@0xpatrickdev
feat(orchestration) add support for queries (icq/v1)
c8aa0e4 
- refs: #9072
0xpatrickdev added a commit to 0xpatrickdev/agoric-sdk that referenced this issue May 2, 2024
@0xpatrickdev
feat(cosmic-proto): add icq/v1 proto + codegen for @agoric/orchestration
c3abd33 
- sourced from: https://github.com/cosmos/ibc-apps/tree/18248c35e913b3ccba99a2756612b1fcb3370a0d/modules/async-icq/proto/icq/v1
- refs: Agoric#9072
0xpatrickdev added a commit to 0xpatrickdev/agoric-sdk that referenced this issue May 2, 2024
@0xpatrickdev
feat(cosmic-proto): add JsonSafe and RequestQueryJson types
f4bb5dd 
- refs: Agoric#9072
0xpatrickdev added a commit to 0xpatrickdev/agoric-sdk that referenced this issue May 2, 2024
@0xpatrickdev
feat(network): add allocateICQControllerPort to PortAllocator
cc610cb 
- refs: Agoric#9072
0xpatrickdev added a commit to 0xpatrickdev/agoric-sdk that referenced this issue May 2, 2024
@0xpatrickdev
feat(orchestration): add support for queries (icq/v1)
ce0f0fd 
- refs: Agoric#9072
0xpatrickdev added a commit that referenced this issue May 2, 2024
@0xpatrickdev
feat(cosmic-proto): add icq/v1 proto + codegen for @agoric/orchestration
a90b6a2 
- sourced from: https://github.com/cosmos/ibc-apps/tree/18248c35e913b3ccba99a2756612b1fcb3370a0d/modules/async-icq/proto/icq/v1
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 2, 2024
@0xpatrickdev
feat(network): add allocateICQControllerPort to PortAllocator
e29d40c 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 2, 2024
@0xpatrickdev
feat(cosmic-proto): add JsonSafe and RequestQueryJson types
8dd43c8 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 2, 2024
@0xpatrickdev
feat(orchestration): add support for queries (icq/v1)
bf17425 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 2, 2024
@0xpatrickdev
feat(cosmic-proto): add icq/v1 proto + codegen for @agoric/orchestration
98213dc 
- sourced from: https://github.com/cosmos/ibc-apps/tree/18248c35e913b3ccba99a2756612b1fcb3370a0d/modules/async-icq/proto/icq/v1
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 2, 2024
@0xpatrickdev
feat(network): add allocateICQControllerPort to PortAllocator
160d332 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 2, 2024
@0xpatrickdev
feat(cosmic-proto): add JsonSafe and RequestQueryJson types
8fe1060 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 2, 2024
@0xpatrickdev
feat(orchestration): add support for queries (icq/v1)
1a5d3b3 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(cosmic-proto): add icq/v1 proto + codegen for @agoric/orchestration
bc977ce 
- sourced from: https://github.com/cosmos/ibc-apps/tree/18248c35e913b3ccba99a2756612b1fcb3370a0d/modules/async-icq/proto/icq/v1
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(network): add allocateICQControllerPort to PortAllocator
ac539d0 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(cosmic-proto): add JsonSafe and RequestQueryJson types
6fc3011 
- refs: #9072
@dckc
Copy link
Member

dckc commented May 3, 2024

I see a couple design questions in the PR:

What ICQ spec are we going by?

0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(cosmic-proto): add icq/v1 proto + codegen for @agoric/orchestration
5ef8daf 
- sourced from: https://github.com/cosmos/ibc-apps/tree/18248c35e913b3ccba99a2756612b1fcb3370a0d/modules/async-icq/proto/icq/v1
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(network): add allocateICQControllerPort to PortAllocator
8450559 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(cosmic-proto): add JsonSafe and RequestQueryJson types
05a3a04 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(cosmic-proto): add icq/v1 proto + codegen for @agoric/orchestration
910e038 
- sourced from: https://github.com/cosmos/ibc-apps/tree/18248c35e913b3ccba99a2756612b1fcb3370a0d/modules/async-icq/proto/icq/v1
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(network): add allocateICQControllerPort to PortAllocator
b819aa9 
- refs: #9072
0xpatrickdev added a commit that referenced this issue May 3, 2024
@0xpatrickdev
feat(cosmic-proto): add JsonSafe and RequestQueryJson types
fe9dab4 
- refs: #9072
@mergify mergify bot closed this as completed in #9198 May 6, 2024
mergify bot added a commit that referenced this issue May 6, 2024
@mergify
feat(orchestration): icq balance query (#9198)
8e94b2a 
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

<!-- Most PRs should close a specific Issue. All PRs should at least
reference one or more Issues. Edit and/or delete the following lines as
appropriate (note: you don't need both `refs` and `closes` for the same
one): -->

closes: #9072
refs: #9042 
refs: #9162 
## Description

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review.
-->

- Binds an `icqcontroller-*` port and sends a query packet using
`CosmosQuery` (`/icq/v1/packet.js`) and `RequestQuery`
(`/tendermint/abci/types.js`).
- Adds `.queryBalance([denom])` method to `StakingAccountHolder` exo in
`StakeAtom` contract, using `QueryBalanceRequest`
(`/cosmos/bank/v1beta1/query.js`)
- adds `icq/v1` protos from
[cosmos/ibc-apps#8e64543](https://github.com/cosmos/ibc-apps/tree/18248c35e913b3ccba99a2756612b1fcb3370a0d/modules/async-icq/proto/icq/v1)
- adds `JsonSafe`, `typeUrlToGrpcPath`, and `toRequestQueryJson` helpers
to `@agoric/cosmic-proto`
- ensures we are using `ChainAddress` objects in all orchestration code
(#9162)

Able to confirm port creation and successful balances queries in e2e
testing:
<details>
<summary>relayer logs</summary>

```log
2024-04-05T01:57:06.907563Z  INFO ThreadId(29) worker.batch{chain=agoriclocal}:supervisor.handle_batch{chain=agoriclocal}:supervisor.process_batch{chain=agoriclocal}:worker.channel{channel=channel::channel-0/icqcontroller-0:agoriclocal->osmosis-test}: 🎊  osmosis-test => OpenTryChannel(OpenTry { port_id: icqhost, channel_id: channel-1, connection_id: connection-1, counterparty_port_id: icqcontroller-0, counterparty_channel_id: channel-0 }) at height 0-470
2024-04-05T01:57:06.907586Z  INFO ThreadId(29) worker.batch{chain=agoriclocal}:supervisor.handle_batch{chain=agoriclocal}:supervisor.process_batch{chain=agoriclocal}:worker.channel{channel=channel::channel-0/icqcontroller-0:agoriclocal->osmosis-test}: channel handshake step completed with events: OpenTryChannel(OpenTry { port_id: icqhost, channel_id: channel-1, connection_id: connection-1, counterparty_port_id: icqcontroller-0, counterparty_channel_id: channel-0 })
2024-04-05T01:57:11.647420Z  INFO ThreadId(48) worker.batch{chain=osmosis-test}:supervisor.handle_batch{chain=osmosis-test}:supervisor.process_batch{chain=osmosis-test}:worker.channel{channel=channel::channel-1/icqhost:osmosis-test->agoriclocal}: 🎊  agoriclocal => OpenAckChannel(OpenAck { port_id: icqcontroller-0, channel_id: channel-0, connection_id: connection-0, counterparty_port_id: icqhost, counterparty_channel_id: channel-1 }) at height 0-52
2024-04-05T01:57:11.647448Z  INFO ThreadId(48) worker.batch{chain=osmosis-test}:supervisor.handle_batch{chain=osmosis-test}:supervisor.process_batch{chain=osmosis-test}:worker.channel{channel=channel::channel-1/icqhost:osmosis-test->agoriclocal}: channel handshake step completed with events: OpenAckChannel(OpenAck { port_id: icqcontroller-0, channel_id: channel-0, connection_id: connection-0, counterparty_port_id: icqhost, counterparty_channel_id: channel-1 })
```
</details>

<img width="1557" alt="Screenshot 2024-04-19 at 4 47 17 PM"
src="https://github.com/Agoric/agoric-sdk/assets/11021913/d569bcfb-f0e0-43dc-8a0a-b87ae601a731">


### Security Considerations

<!-- Does this change introduce new assumptions or dependencies that, if
violated, could introduce security vulnerabilities? How does this PR
change the boundaries between mutually-suspicious components? What new
authorities are introduced by this change, perhaps by new API calls?
-->

### Scaling Considerations

<!-- Does this change require or encourage significant increase in
consumption of CPU cycles, RAM, on-chain storage, message exchanges, or
other scarce resources? If so, can that be prevented or mitigated? -->

### Documentation Considerations

<!-- Give our docs folks some hints about what needs to be described to
downstream users.

Backwards compatibility: what happens to existing data or deployments
when this code is shipped? Do we need to instruct users to do something
to upgrade their saved data? If there is no upgrade path possible, how
bad will that be for users?

-->

### Testing Considerations

<!-- Every PR should of course come with tests of its own functionality.
What additional tests are still needed beyond those unit tests? How does
this affect CI, other test automation, or the testnet?
-->

### Upgrade Considerations

<!-- What aspects of this PR are relevant to upgrading live production
systems, and how should they be addressed? -->
@0xpatrickdev 0xpatrickdev mentioned this issue May 6, 2024
Open
@Jorge-Lopes Jorge-Lopes mentioned this issue May 23, 2024
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@0xpatrickdev 0xpatrickdev

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(orchestration): icq balance query Agoric/agoric-sdk
3 participants
@dckc @ivanlei @0xpatrickdev