Calling zoe.redeem inside a contract may be a footgun

[DavidBruant]

Issue created after the analysis here: #780 (comment)

[katelynsills]

Autoswap uses zoe.redeem to escrow liquidity tokens. I think we cannot eliminate this. If we did make a change, we would want to streamline this process rather than remove it.

[DavidBruant]

I'm having an intuition that there is an underlying design problem somewhere
Maybe it's not related to forbidding redeem, maybe it's something else

I'm reopening and i'm going to try to find what the underlying design problem may be

[DavidBruant]

Zoe provides "offer safety". My understanding of offer safety is that it protects against bugs in a smart contract code and this protection makes that it's not possible to loose an asset even if there is a bug in the contract.
The way Zoe achieves that is by separating asset handling (assets are posted to, claimed by and reallocated by Zoe) and smart contract code (which only manipulates offerHandles refering to assets, but never assets directly. And offerHandles are accessed by a user and the contract)

I think that letting smart contracts manipulate directly assets/invites/payouts is a breach of the separation of concerns that enable offer safety and this could lead to Zoe not really providing offer safety

I looked at where/how redeem is used in all existing contract code. I see 3 instances of redeem that are internal to contracts:

1. in autoswap:

agoric-sdk/packages/zoe/src/contracts/autoswap.js

Lines 148 to 155 in 0ab9ebf

	const { inviteHandle: tempLiqHandle, invite } = zoe.makeInvite();
	const zoeService = zoe.getZoeService();
	return zoeService
	.redeem(
	invite,
	proposal,
	harden({ Liquidity: liquidityPaymentP }),
	)

2. in makeEmptyOffer

agoric-sdk/packages/zoe/src/contracts/helpers/zoeHelpers.js

Lines 108 to 111 in 0ab9ebf

	makeEmptyOffer: () => {
	const { inviteHandle, invite } = zoe.makeInvite();
	return zoeService.redeem(invite).then(() => inviteHandle);
	},

3. in the current (but soon another version will replace it) version of the Opera ticket contract:

agoric-sdk/packages/zoe/src/contracts/operaConcertTicket.js

Lines 88 to 103 in adc846a

	// create an zoe invite...
	const { invite, inviteHandle: ticketInviteHandle } = zoe.makeInvite();
	// ...and redeem it right away
	return zoeService
	.redeem(
	invite,
	harden({
	want: {
	Money: moneyIssuer
	.getAmountMath()
	.make(expectedAmountPerTicket.extent * ticketNumbers.length),
	},
	give: { Ticket: ticketsAmount },
	}),
	harden({ Ticket: mint.mintPayment(ticketsAmount) }), // mint and pass to Zoe right away
	)

As i highlighted, the 3 usages have one thing in common which is creating an invite first and then redeeming it right away

At least, i think this would be better served by a dedicated method on the zoe contract facet
Its signature could be:
{offerHandle, payout} = zoeContractFacet.makeOffer(proposal, payments) (i do not feel strongly on the method name)

Interestingly, the usages i showed above are the only usages of zoe.getZoeService() in all current contracts, so maybe or maybe not getZoeService could be removed from the zoe contract facet

[katelynsills]

At least, i think this would be better served by a dedicated method on the zoe contract facet
Its signature could be:
{offerHandle, payout} = zoeContractFacet.makeOffer(proposal, payments) (i do not feel strongly on the method name) Interestingly, the usages i showed above are the only usages of zoe.getZoeService() in all current contracts, so maybe or maybe not getZoeService could be removed from the zoe contract facet

Interestingly, this was actually the design we started with! It got changed to zoe.getZoeService() on Mark's request, but maybe we should revisit after Zoe Alpha

[erights]

Happy to revisit. Until then, I do like the way it is now better. Will explain when we revisit.

[DavidBruant]

Interestingly, the usages i showed above are the only usages of zoe.getZoeService() in all current contracts, so maybe or maybe not getZoeService could be removed from the zoe contract facet

On a call today, @erights suggested that a contract should be able to do what any participant can do and i understand and agree with this idea
So i'm on the side of keeping zoefc.getZoeService()

But i still believe that zoefc.makeOffer(proposal, payments) is less footgun-ny than makeInvite + redeem it inside the contract

[erights]

I misunderstood the intent. Yes, I support doing some like this. Perhaps this itself but my understanding isn't that solid yet. And we all now support keeping getZoeService.

Good idea and good analysis. My apologies for not reading deeper earlier. Thanks!

[katelynsills]

Is #993 enough of a solution to close this issue?

[erights]

Is #993 enough of a solution to close this issue?

Yes.

[DavidBruant]

Yes, i think it's good enough for now
Thank you very much for this work!

[katelynsills]

Closed by #993