Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
303 stars 191 forks source link

Smart wallet cannot handle invitations in offer proposals #9378

Open samsiegart opened 1 month ago

samsiegart commented 1 month ago

Describe the bug

In the invitation purse published to vstorage, invitations have a null slot value for their "handle" property. When trying to make an offer through the smart wallet with an invitation in the proposal, this null slot causes an error:

2024-05-17 12:14:54 dapp-agoric-basics-agd-1  | 2024-05-17T19:14:54.000Z SwingSet: vat: v43: walletFactory.fromBridge: { blockHeight: 17129, blockTime: 1715973292, owner: 'agoric1rwwley550k9mmk6uq6mm6z4udrg8kyuyvfszjk', spendAction: '{"body":"#{\\"method\\":\\"executeOffer\\",\\"offer\\":{\\"id\\":1715973270172,\\"invitationSpec\\":{\\"callPipe\\":[[\\"makeFirstInvitation\\",[[\\"$0.Alleged: Zoe Invitation issuer#board0371\\"]]]],\\"instancePath\\":[\\"swaparoo\\"],\\"source\\":\\"agoricContract\\"},\\"offerArgs\\":{\\"addr\\":\\"agoric1rwwley550k9mmk6uq6mm6z4udrg8kyuyvfszjk\\"},\\"proposal\\":{\\"give\\":{\\"Fee\\":{\\"brand\\":\\"$1.Alleged: IST brand#board0257\\",\\"value\\":\\"+1\\"},\\"Invitation\\":{\\"brand\\":\\"$2.Alleged: Zoe Invitation brand#board0074\\",\\"value\\":[{\\"customDetails\\":{\\"give\\":{\\"Fee\\":{\\"brand\\":\\"$1\\",\\"value\\":\\"+1\\"},\\"Ticket\\":{\\"brand\\":\\"$3.Alleged: Ticket brand#board06299\\",\\"value\\":{\\"#tag\\":\\"copyBag\\",\\"payload\\":[[\\"frontRow\\",\\"+1\\"]]}}},\\"want\\":{\\"IST\\":{\\"brand\\":\\"$1\\",\\"value\\":\\"+50000000\\"}}},\\"description\\":\\"matchOffer\\",\\"handle\\":\\"$4.Alleged: InvitationHandle#null\\",\\"installation\\":\\"$5.Alleged: BundleIDInstallation#board003101\\",\\"instance\\":\\"$6.Alleged: InstanceHandle#board016105\\"}]}},\\"want\\":{}}}}","slots":["board0371","board0257","board0074","board06299",null,"board003101","board016105"]}', type: 'WALLET_SPEND_ACTION' }
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  | 2024-05-17T19:14:54.006Z SwingSet: ls: v43: Logging sent error stack (Error#4)
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  | 2024-05-17T19:14:54.006Z SwingSet: ls: v43: Error#4: slots: [4]: null (an object) - Must be a string
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  | 2024-05-17T19:14:54.007Z SwingSet: ls: v43: Error: slots: [4]: null (an object) - Must be a string
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at construct ()
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at Error (/bundled-source/.../node_modules/ses/src/error/tame-error-constructor.js:56)
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at makeError (/bundled-source/.../node_modules/ses/src/error/assert.js:243)
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at throwLabeled (.../patterns/src/utils.js:132)
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at applyLabelingError (.../patterns/src/utils.js:154)
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at every ()
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at mustMatch (.../patterns/src/patterns/patternMatchers.js:528)
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at fromBridge (.../smart-wallet/src/walletFactory.js:193)
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at fromBridge (.../smart-wallet/src/walletFactory.js:183)
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at apply ()
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at apply ()
2024-05-17 12:14:54 dapp-agoric-basics-agd-1  |  at In "fromBridge" method of (walletActionHandler) (.../exo/src/exo-tools.js:42)

To Reproduce

Send a WalletSpendAction with an invitation amount in the proposal, observe the error in the smart wallet.

Expected behavior

A client can execute offers with invitation amounts in proposals.

Additional context

Discovered when trying to swap an invitation in https://github.com/Agoric/dapp-agoric-basics/pull/42

dckc commented 1 month ago

The smart wallet uses a read-only marshaller when publishing wallet contents; if something isn't already on the board, the smartWallet doesn't add it.

A work-around is for the contract that creates the invitation to put the invitation handle on the board.

https://github.com/Agoric/agoric-sdk/blob/734e8635002e01b3e27477e93998dda942c7fae8/packages/smart-wallet/src/walletFactory.js#L224

https://github.com/Agoric/agoric-sdk/blob/734e8635002e01b3e27477e93998dda942c7fae8/packages/smart-wallet/src/smartWallet.js#L263

https://github.com/Agoric/agoric-sdk/blob/734e8635002e01b3e27477e93998dda942c7fae8/packages/smart-wallet/src/smartWallet.js#L381

dckc commented 1 month ago

@erights points out that Amounts are designed to be not sensitive - parties should be able to want: them without having them.

It looks like none of the other things in the current and update records that the smartWallet publishes are sensitive either.

So changing getReadonlyMarshaller to getPublishingMarshaller would be one approach.

Downside: the board has no mechanism for collecting entries. So the invitation handles would consume space indefinitely.

dckc commented 2 weeks ago

In today's office hours, I talked with @Jovonni and co about...

~/projects/agoric-sdk/packages/smart-wallet
11:48 connolly@bldbox$ git diff .
diff --git a/packages/smart-wallet/src/offerWatcher.js b/packages/smart-wallet/src/offerWatcher.js
index 3b2bc004f..6994d6dbd 100644
--- a/packages/smart-wallet/src/offerWatcher.js
+++ b/packages/smart-wallet/src/offerWatcher.js
@@ -191,6 +191,11 @@ export const prepareOfferWatcher = baggage => {
               }
               facets.helper.updateStatus({ result: UNPUBLISHED_RESULT });
               break;
+              case 'remotable':{
+                const amt = await E(invitationIssuer).getAmountOf(result); // isLivePayment?
+                await E(invitationPurse).deposit(result)
+              }
+
             default:
               // drop the result
               facets.helper.updateStatus({ result: UNPUBLISHED_RESULT });
samsiegart commented 2 weeks ago

In office hourse, @Jovonni and co talked about...

Trying to grok... this snippet is about returning an invitation from an offer result, rather than from a payout. This allows Alice to give an invitation to Bob without Bob exiting his seat, but I'm unclear on how this addresses the null slot issue here.

dckc commented 1 week ago

oops; I think that comment should have gone on #8550