Wallet UI working with new Smart Wallet contract #6134

turadg · 2022-09-04T21:35:19Z

closes (almost): #6127 (leave open until Product verifies)

Stacked on,

new smart wallet #6084

Description

Wallet UI has a WalletBackendAdapter for how the UI queries RPC nodes for data. That messaging system has changed with the new smart wallet and at least the adapter needs to be revised to match it.

Further changes may be warranted or necessary.

Security Considerations

Documentation Considerations

Testing Considerations

packages/wallet/ui/src/util/WalletBackendAdapter.js

packages/casting/src/iterable.js

packages/wallet/api/src/marshal-contexts.js

packages/wallet/ui/src/components/Proposal.jsx

packages/wallet/ui/src/util/WalletBackendAdapter.js

samsiegart · 2022-09-13T06:24:59Z

packages/wallet/ui/src/util/WalletBackendAdapter.js

+    /** @type {number} */
+    let firstHeight;
+    for await (const { blockHeight } of iterateReverse(follower)) {
+      // TODO: Only set firstHeight and break if the value contains all our state.


How do we tell when we get to a snapshot block?

They don't exist yet, which is why we only have this comment to describe what the code should eventually do.

packages/wallet/ui/src/util/WalletBackendAdapter.js

packages/casting/src/iterable.js

dckc

I looked at it by commit; the code looks fine, but I haven't tested that it works.

samsiegart

LGTM!

turadg · 2022-09-14T14:49:07Z

packages/cosmic-swingset/Makefile

@@ -122,6 +122,7 @@ scenario2-setup-nobuild:
 	$(AGCH) --home=t1/bootstrap keys add bootstrap --keyring-backend=test
 	$(AGCH) --home=t1/bootstrap keys show -a bootstrap --keyring-backend=test > t1/bootstrap-address
 	$(AGCH) --home=t1/n0 add-genesis-account `cat t1/bootstrap-address` $(BOOT_COINS)
+	# Create


turadg · 2022-09-14T14:49:29Z

packages/cosmic-swingset/Makefile

-
+
+# Send some USDC to the vbank/provision module account where it can be traded for IST.
+fund-provision-pool: wait-for-cosmos


👍 ty

idea: make fund-acct depend on this having run. E.g. make this have a side-effect of noting its completion and have fund-acct depend on that, so that it runs if necesssary..

copied to 8fa8326

turadg · 2022-09-14T14:52:58Z

packages/smart-wallet/src/utils.js

@@ -9,7 +9,7 @@ import { observeIteration, subscribeEach } from '@agoric/notifier';
 * If this proves to be a problem we can add an option to this or a related
 * utility to reset state from RPC.
 *
- * @param {ERef<StoredSubscriber<import('./smartWallet').UpdateRecord>>} updates
+ * @param {ERef<Subscriber<import('./smartWallet').UpdateRecord>>} updates


👍 this doesn't care about storage

turadg · 2022-09-14T14:53:42Z

packages/smart-wallet/test/devices.js

@@ -1,6 +1,6 @@
 import bundleCentralSupply from '@agoric/vats/bundles/bundle-centralSupply.js';
 import bundleMintHolder from '@agoric/vats/bundles/bundle-mintHolder.js';
-import bundleWalletFactory from '@agoric/vats/bundles/bundle-legacy-walletFactory.js';
+import bundleWalletFactory from '@agoric/vats/bundles/bundle-walletFactory.js';


We have a yarn build for this package now that builds ../bundles/bundle-walletFactory.js.

Consider using that instead, so while making changes to this package you don't have to build all vats.

I lean toward: you can assume yarn build is up-to-date for other packages, but changes within a package should get bundled at test-time within a package. So this one is a little awkward: it depends on yarn build in vats whenever a change to the walletFactory contract in this package is changed.

hm. not sure I have a concrete suggestion.

by that principle shouldn't this be as suggested?

Suggested change

import bundleWalletFactory from '@agoric/vats/bundles/bundle-walletFactory.js';

import bundleWalletFactory from '../bundles/bundle-walletFactory.js';

yes, that's good, provided the test.before() auto-updates that bundle.

packages/wallet/api/src/marshal-contexts.js

turadg · 2022-09-14T14:57:12Z

packages/wallet/ui/src/components/ConnectionSettingsDialog.jsx

@@ -62,7 +62,7 @@ const ConnectionSettingsDialog = ({
  const classes = useStyles();
  const smartConnectionHrefs = allConnectionConfigs.map(({ href }) => href);

-  const [config, setConfig] = useState(connectionConfig);
+  const [config, setConfig] = useState(connectionConfig || {});


better to express defaults in the params

The connectionConfig may be falsy but not undefined (such as when it's null). I found it was necessary to handle all falsy values.

packages/wallet/ui/src/components/Proposal.jsx

turadg · 2022-09-14T15:07:41Z

packages/wallet/ui/src/util/WalletBackendAdapter.js

@@ -130,15 +130,80 @@ export const makeWalletBridgeFromFollower = (
    ]),
  );

+  // We assume just one cosmos purse per brand.


could ref #6126 (though I now see it is below)

turadg · 2022-09-14T15:09:50Z

packages/wallet/ui/src/util/WalletBackendAdapter.js

+            brand: descriptor.brand,
+            brandPetname: descriptor.petname,
+            pursePetname: descriptor.petname,
+            displayInfo: descriptor.displayInfo,


the intent might be more clear with destructring,

Suggested change

brand: descriptor.brand,

brandPetname: descriptor.petname,

pursePetname: descriptor.petname,

displayInfo: descriptor.displayInfo,

...descriptor

It's a remapping of properties, so I found following the current style was clearest for me.

turadg · 2022-09-14T15:10:26Z

packages/wallet/ui/src/util/WalletBackendAdapter.js

+            currentAmount,
+            value: currentAmount.value,


odd to have value right next to an amount that has the same value. please remove the redundant one.

I suspect it's an external design constraint; that is: removing the redundant one would involve something like upgrading all the dapps (or: the half of the dapps that use the one or the other).

I found both were necessary.

turadg force-pushed the ta/split-smartwallet branch 6 times, most recently from 9b85ce3 to 708972f Compare September 7, 2022 20:00

turadg force-pushed the 6127-psm-new-wallet branch from ab040e8 to 494c540 Compare September 7, 2022 20:05

Base automatically changed from ta/split-smartwallet to master September 7, 2022 21:12

turadg mentioned this pull request Sep 8, 2022

feat(vats): upgrade to psm-first smart-wallet #6163

Merged

michaelfig force-pushed the 6127-psm-new-wallet branch from 1e9e464 to 1c69784 Compare September 8, 2022 21:59

turadg commented Sep 9, 2022

View reviewed changes

packages/wallet/ui/src/util/WalletBackendAdapter.js Outdated Show resolved Hide resolved

michaelfig force-pushed the 6127-psm-new-wallet branch from 1c69784 to 0afcac4 Compare September 10, 2022 03:16

turadg mentioned this pull request Sep 12, 2022

BREAKING CHANGE: remove legacy walletFactory #6186

Closed

michaelfig force-pushed the 6127-psm-new-wallet branch 2 times, most recently from 70ab19a to 6d73875 Compare September 12, 2022 23:17

michaelfig requested a review from samsiegart September 12, 2022 23:20

michaelfig self-assigned this Sep 12, 2022

michaelfig added the wallet label Sep 12, 2022

turadg commented Sep 12, 2022

View reviewed changes

packages/casting/src/iterable.js Outdated Show resolved Hide resolved

samsiegart reviewed Sep 13, 2022

View reviewed changes

michaelfig force-pushed the 6127-psm-new-wallet branch from 6d73875 to 195058f Compare September 14, 2022 01:33

michaelfig marked this pull request as ready for review September 14, 2022 02:04

michaelfig added the automerge:no-update (expert!) Automatically merge without updates label Sep 14, 2022

dckc reviewed Sep 14, 2022

View reviewed changes

packages/casting/src/iterable.js Show resolved Hide resolved

dckc reviewed Sep 14, 2022

View reviewed changes

samsiegart self-requested a review September 14, 2022 03:16

samsiegart approved these changes Sep 14, 2022

View reviewed changes

turadg commented Sep 14, 2022

View reviewed changes

turadg and others added 2 commits September 14, 2022 20:14

refactor(vat): remove legacy walletFactory

694a12a

fix(smart-wallet): invitation brand is remote

6613136

michaelfig added 14 commits September 14, 2022 20:14

fix(wallet-api): tolerate null slots

5cf9803

feat(wallet-ui): start displaying balances

0f36da9

feat(wallet-ui): translate proposalTemplate to proposal

bd91fed

fix(wallet-ui): increase gas allowance

53eff35

fix(wallet-ui): don't crash if purse not found

f0b591b

feat(wallet-ui): adapt smart wallet proposals

cef7f34

feat(casting): provide reverse iteration

65906f5

fix(wallet-ui): reverse iterate to process all update deltas

65e681d

feat(cosmic-swingset): new fund-provision-pool target

a8890db

chore: clean up docs and stray logging

34faf26

fix(casting): implement getReverseIterable

7f5a791

test(vats): increase jsDepth for lint:types

795cbaa

ci(integration): temporarily disable NPM deploys

c877e0d

fix(vats): point to @agoric/smart-wallet/src/walletFactory.js

50ff847

michaelfig force-pushed the 6127-psm-new-wallet branch from 99b9e1f to 50ff847 Compare September 15, 2022 03:49

Merge branch 'master' into 6127-psm-new-wallet

d0d5d00

mergify bot merged commit a390655 into master Sep 15, 2022

mergify bot deleted the 6127-psm-new-wallet branch September 15, 2022 05:41

turadg mentioned this pull request Sep 15, 2022

clean up smart wallet #6208

Merged

Tartuffo mentioned this pull request Sep 15, 2022

support new smart wallet in Wallet UI #6127

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wallet UI working with new Smart Wallet contract #6134

Wallet UI working with new Smart Wallet contract #6134

turadg commented Sep 4, 2022 •

edited by michaelfig

Loading

samsiegart Sep 13, 2022

michaelfig Sep 14, 2022

dckc left a comment

samsiegart left a comment

turadg Sep 14, 2022

turadg Sep 14, 2022

turadg Sep 14, 2022

turadg Sep 14, 2022

turadg Sep 14, 2022

dckc Sep 14, 2022

turadg Sep 14, 2022

dckc Sep 14, 2022

turadg Sep 14, 2022

michaelfig Sep 15, 2022

turadg Sep 14, 2022

turadg Sep 14, 2022

michaelfig Sep 15, 2022 •

edited

Loading

turadg Sep 14, 2022

dckc Sep 15, 2022 •

edited

Loading

michaelfig Sep 15, 2022



		# Send some USDC to the vbank/provision module account where it can be traded for IST.
		fund-provision-pool: wait-for-cosmos

	import bundleWalletFactory from '@agoric/vats/bundles/bundle-walletFactory.js';
	import bundleWalletFactory from '../bundles/bundle-walletFactory.js';

Wallet UI working with new Smart Wallet contract #6134

Wallet UI working with new Smart Wallet contract #6134

Conversation

turadg commented Sep 4, 2022 • edited by michaelfig Loading

Description

Security Considerations

Documentation Considerations

Testing Considerations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

samsiegart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelfig Sep 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc Sep 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg commented Sep 4, 2022 •

edited by michaelfig

Loading

michaelfig Sep 15, 2022 •

edited

Loading

dckc Sep 15, 2022 •

edited

Loading