feat: contract for single smart wallet #5701

turadg · 2022-06-30T22:42:01Z

closes: #5356

Description

Smart contract for smart wallet, forked from #5594

Names this version of the contract singleWallet so it can co-exist with the multitenant walletFactory coming in #5721 for #4484

Security Considerations

Wallet now works on-chain.

Documentation Considerations

Adds a basic README to document the difference between "solo" wallet, smart/chain wallet with a singleton contract and the multitenant factory.

Testing Considerations

CI doesn't cover everything. Ideally we test the on-chain home.wallet in CI but that's a big chunk of work and for now we can do it manually.

# tab 1 (chain)
cd packages/cosmic-swingset/
make scenario2-setup scenario2-run-chain
# starts bare chain, don’t need AMM

# tab 2 (client server)
cd packages/cosmic-swingset/
make scenario2-run-client
# CONFIRMED no errors in logs

# tab 3 (interactive)
agoric open --repl
# CONFIRMED in browser that `home.wallet` and `home.smartWallet` exist
agd query vstorage keys 'published.wallet'
# CONFIRMED it has a key like `published.wallet.agoric1nqxg4pye30n3trct0hf7dclcwfxz8au84hr3ht`
agoric follow :published.wallet.agoric1nqxg4pye30n3trct0hf7dclcwfxz8au84hr3ht
# CONFIRMED it has JSON data

dckc

heading to a meeting; got thru lib-chainStorage.js

not sure about the CORS stuff. hm.

dckc · 2022-07-05T20:55:42Z

packages/agoric-cli/src/start.js

    });
    const newAppToml = finishCosmosApp({
      appToml,
      portNum,
+      enableCors: true,


I suspect this CORS stuff is left-over from some tangentially related work. Unless you're clear on why it belongs, perhaps take it out?

dckc · 2022-07-05T20:58:06Z

packages/vats/decentral-demo-config.json

-    "walletManager": {
-      "sourceSpec": "@agoric/vats/src/vat-walletManager.js"
-    },


this config probably needs the same smart-wallet.js contract bundle as decntral-core-config.json.

dckc

good stuff, but I think changing provide from async to sync has to be rolled back or otherwise fixed.

dckc · 2022-07-06T17:39:47Z

packages/run-protocol/test/test-gov-collateral.js

@@ -15,7 +15,7 @@ import {
 import centralSupplyBundle from '@agoric/vats/bundles/bundle-centralSupply.js';
 import {
  bridgeCoreEval,
-  makeClientManager,
+  setupClientManager,


why rename it? the function does make a (new) client manager.

ah. it doesn't return the thing that it creates.

dckc · 2022-07-06T17:42:50Z

packages/run-protocol/src/proposals/utils.js

@@ -67,17 +68,6 @@ export const reserveThenDeposit = async (
  console.info('confirmed deposit for', debugName);
 };

-/** @type {<T>(store: any, key: string, make: () => T) => Promise<T>} */
-const provide = async (store, key, make) => {


the provide from @agoric/store is not async. I don't see how it can work here.

store: any was lazy (on my part)

thanks. coding too fast. thankfully the typechecker caught it too.

dckc · 2022-07-06T17:47:14Z

packages/vats/src/core/chain-behaviors.js

-  /** @type {SubscriptionRecord<PropertyMakers>} */
+  /** @type {SubscriptionRecord<PropertyMaker[]>} */


odd... I wonder why that wasn't caught before.

PropertyMakers was PropertyMaker[]. I changed it around to have PropertyMaker be named.

dckc · 2022-07-06T17:51:02Z

packages/wallet/contract/README.md

+agd query vstorage keys 'published.wallet'
+# confirm it has a key like `published.wallet.agoric1nqxg4pye30n3trct0hf7dclcwfxz8au84hr3ht`
+agoric follow :published.wallet.agoric1nqxg4pye30n3trct0hf7dclcwfxz8au84hr3ht
+# confirm it has JSON data


I think the format here is actually Justin. minor detail.

dckc

good stuff!

dckc · 2022-07-08T19:11:29Z

packages/vats/src/core/basic-behaviors.js

+  const STORAGE_PATH = 'wallet';
+
+  const storageNode = await getChildNode(chainStorage, STORAGE_PATH);
+  const marshaller = E(board).getPublishingMarshaller();


post-script: wallets have state that shouldn't be published, so using the publishing marshaller was probably not a good idea. even using the read-only marshaller would be iffy: purses shouldn't be exposed to the board vat at all.

cc @warner

The marshaller privileges and what is published are independent, no?

The latter is defined here https://github.com/Agoric/agoric-sdk/pull/5701/files#diff-b1c2b2b7ded05977f142adcd00946c70016bae3a5178a46af30ef444422e3332R60-R70

Which of those shouldn't be published? purses?

Are you saying there should be separate streams for what goes to off-chain storage and what goes to the creator?

The marshaller privileges and what is published are independent, no?

Yes.

And indeed, purses must not be published. Nor payments. Probably not offers either. Publishing issuers and dapps is probably harmless. contacts is iffy...

But not only should we not publish purses, we shouldn't use the board's readOnlyMarshaller, since that involves exposing the purses to the board and trusting the board not to publish them.

dckc · 2022-07-08T19:42:56Z

For reference, https://gist.github.com/dckc/3192a5264524635bd7261277b1fb707e has the output of agoric follow ....

turadg requested a review from michaelfig June 30, 2022 22:42

turadg force-pushed the 5356-smart-wallet-contract branch 4 times, most recently from fd1b8c0 to 9a0dd4a Compare July 5, 2022 14:24

turadg marked this pull request as ready for review July 5, 2022 14:24

turadg force-pushed the 5356-smart-wallet-contract branch from 9a0dd4a to 1ea0719 Compare July 5, 2022 20:45

turadg requested a review from dckc July 5, 2022 20:49

dckc reviewed Jul 5, 2022

View reviewed changes

turadg force-pushed the 5356-smart-wallet-contract branch from 1ea0719 to 74945ea Compare July 5, 2022 21:19

turadg requested a review from dckc July 5, 2022 21:31

turadg changed the title ~~feat: smart wallet contract~~ feat: contract for single smart wallet Jul 5, 2022

turadg added the automerge:rebase Automatically rebase updates, then merge label Jul 6, 2022

dckc requested changes Jul 6, 2022

View reviewed changes

turadg added 2 commits July 6, 2022 11:08

refactor(chainStorage): getChildNode helper

4f20bbf

feat: contract for single on-chain wallet

0184a89

turadg force-pushed the 5356-smart-wallet-contract branch from 569520a to 3e005d7 Compare July 6, 2022 18:28

turadg requested a review from dckc July 6, 2022 18:33

turadg added 3 commits July 6, 2022 14:25

refactor: rename setupClientManager

11e90d0

test(smartWallet): describe manual test

8396fe1

chore: types

80f642b

turadg force-pushed the 5356-smart-wallet-contract branch from d217901 to 80f642b Compare July 6, 2022 21:26

dckc approved these changes Jul 6, 2022

View reviewed changes

mergify bot merged commit d07a502 into master Jul 6, 2022

mergify bot deleted the 5356-smart-wallet-contract branch July 6, 2022 23:28

dckc mentioned this pull request Jul 7, 2022

Implement smart-wallet and read-only UI #5594

Closed

dckc reviewed Jul 8, 2022

View reviewed changes

dckc mentioned this pull request Jul 8, 2022

Smart wallet middleware #4398

Closed

7 tasks

dckc mentioned this pull request Jul 19, 2022

Invitations are missing on instagoric using the default propsals #5783

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: contract for single smart wallet #5701

feat: contract for single smart wallet #5701

turadg commented Jun 30, 2022 •

edited

Loading

dckc left a comment

dckc Jul 5, 2022

dckc Jul 5, 2022

dckc left a comment

dckc Jul 6, 2022

dckc Jul 6, 2022

dckc Jul 6, 2022

dckc Jul 6, 2022 •

edited

Loading

turadg Jul 6, 2022

dckc Jul 6, 2022

turadg Jul 6, 2022

dckc Jul 6, 2022

dckc left a comment

dckc Jul 8, 2022

turadg Jul 8, 2022 •

edited

Loading

dckc Jul 8, 2022

dckc commented Jul 8, 2022

		/** @type {SubscriptionRecord<PropertyMakers>} */
		/** @type {SubscriptionRecord<PropertyMaker[]>} */

feat: contract for single smart wallet #5701

feat: contract for single smart wallet #5701

Conversation

turadg commented Jun 30, 2022 • edited Loading

Description

Security Considerations

Documentation Considerations

Testing Considerations

dckc left a comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc Jul 6, 2022 • edited Loading

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

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg Jul 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc commented Jul 8, 2022

turadg commented Jun 30, 2022 •

edited

Loading

dckc Jul 6, 2022 •

edited

Loading

turadg Jul 8, 2022 •

edited

Loading