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

feat: query smart wallet auto-provision fee #97

Merged
merged 3 commits into from
Apr 17, 2024
Merged

Conversation

samsiegart
Copy link
Contributor

@samsiegart samsiegart commented Mar 26, 2024

refs #78

This queries the smart wallet auto-provision fee in the wallet connection component, and exposes it in the react useAgoric hook as well.

Also fixed a bug where the purse notifier was updating every time the bank was polled, even if the balances didn't change. (cc @0xpatrickdev)

Testing

Tested on this branch of dapp-offer-up: https://github.com/Agoric/dapp-offer-up/compare/test-network-dropdown

Steps:

  • Build ui-kit with yarn && yarn workspaces run prepack
  • Set up link with cd packages/react-components && yarn link
  • Clone dapp-offer-up and checkout this branch: https://github.com/Agoric/dapp-offer-up/compare/test-network-dropdown
  • Start the local chain with docker (the offer-up contract doesn't necessarily have to be deployed to test this functionality, just a local chain)
  • Link the local version of this packages in dapp-offer-up: cd ui && yarn link @agoric/react-components
  • Run the local UI: cd ui && yarn && yarn dev
  • Try modifying the dapp to read the provision fee: const { smartWalletProvisionFee } = useAgoric()
  • Observe after connecting to wallet that the provision fee is accessible

For an e2e example with the provision dialog, see #99

@LuqiPan
Copy link

LuqiPan commented Mar 28, 2024

@samsiegart could you help me and other reviewers understand what tests have you done on this PR?

@samsiegart
Copy link
Contributor Author

@samsiegart could you help me and other reviewers understand what tests have you done on this PR?

Sure yea, just updated the PR description. Lmk if you'd like more clarification or want to walk through it together. We're pretty close to being able to set up an example dapp in this repo with e2e testing, which would make this type of thing easier to test.

@@ -17,6 +17,7 @@
"@agoric/assert": "^0.6.0",
"@agoric/cache": "^0.3.2",
"@agoric/casting": "^0.4.3-u13.0",
"@agoric/cosmic-proto": "0.3.0",
Copy link
Member

Choose a reason for hiding this comment

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

does it work with 0.4.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following up, looks like 0.4.0 isn't published, tried 0.4.1-dev-e2e36cc.0 but doesn't seem to have the correct interface, and best not to risk Agoric/dapp-inter#252. Will keep at 0.3.0 for now.

@samsiegart samsiegart requested a review from turadg April 16, 2024 19:27
res(fee);
} catch (e) {
console.error('Error querying smart wallet provision fee', address);
if (attempts >= MAX_ATTEMPTS_TO_WATCH_BANK) {
Copy link
Member

Choose a reason for hiding this comment

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

retry logic seems ripe for generalization. e.g. https://caolan.github.io/async/v2/retry.js.html

or this light package https://github.com/vercel/async-retry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, I do lean towards "Write Everything Twice", and there's one other place we do a fetch with retry here, but seems like something we can generalize without a package for next time.

@samsiegart samsiegart merged commit dbb5cbc into main Apr 17, 2024
1 check passed
@samsiegart samsiegart deleted the provision-fee branch April 17, 2024 00:43
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