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.

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

use Telescope-based cosmic-proto #240

Merged
merged 6 commits into from
Mar 9, 2024
Merged

use Telescope-based cosmic-proto #240

merged 6 commits into from
Mar 9, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Feb 16, 2024

0.4.0 builds with Telescope, which provides some client support we were hand rolling

Copy link

github-actions bot commented Mar 7, 2024

Network:
Commit: b2f624b
Ref: refs/heads/main
IPFS v1 hash: bafybeicj2owk2ae6qhwnicclk7jjcbwianrf6m32h6tolkmnv7jqh3zcle
CF - DWeb - 4EVERLAND

@turadg turadg marked this pull request as ready for review March 8, 2024 15:29
@@ -1173,6 +1216,18 @@
cosmjs-types "^0.8.0"
long "^4.0.0"

"@cosmjs/proto-signing@^0.32.2":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it seems like we now have 0.30.x and 0.32.x versions for many of the @cosmjs/* deps

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to raise. It looks to be mostly due to leapwallet pinning versions. We could use a resolution to override but that can be brittle.

I'll wait for @samsiegart 's input before merging. Also WRT test plan

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the deployed preview and looks like it worked. I don't know much about the version thing, it's good to be aware of though, maybe upgrading leap elements would fix it.

Copy link
Collaborator

@samsiegart samsiegart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, code is much cleaner and thanks for the descriptive commit messages

@@ -45,8 +45,3 @@ declare module '@agoric/inter-protocol/src/vaultFactory/math' {
declare module 'react-view-slider' {
export const ViewSlider;
}

declare module '@agoric/cosmic-proto/swingset/query.js' {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo!

@@ -12,14 +12,19 @@ const useSmartWalletFeeQuery = (rpc: string | null) => {
const fetchParams = async () => {
assert(rpc);
try {
const params = await querySwingsetParams(rpc);
const client = await agoric.ClientFactory.createRPCQueryClient({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: the wallet connection also makes its own query client, ideally at some point we could share between these two places somehow. I think the only main downside currently is an extra "status" query when you create an extra client.

@turadg turadg merged commit b2f624b into main Mar 9, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants