Opera ticket zoe contract

[DavidBruant]

Closes #543

To run the test independently, run:
tape -r esm 'test/unitTests/contracts/test-operaConcertTicket.js' | tap-spec
(in zoe package and with ./node_modules/.bin in your $PATH)

I'm currently, stuck with a weird error

payment not found: zoeInvite
    FAILED ASSERTION false
(node:14889) UnhandledPromiseRejectionWarning: Error: payment not found: (a string)
See console for error data.
    at Object.complain (/home/david/projects/agoric-sdk/packages/assert/src/assert.js:112:14)
    at fail (/home/david/projects/agoric-sdk/packages/assert/src/assert.js:131:20)
    at assert (/home/david/projects/agoric-sdk/packages/assert/src/assert.js:155:5)
    at /home/david/projects/agoric-sdk/packages/ERTP/src/issuer.js:169:9
    at async Test.<anonymous> (/home/david/projects/agoric-sdk/packages/zoe/test/unitTests/contracts/test-operaConcertTicket.js:50:7)

but the payment is the return value of await zoe.makeInstance, so i'm fairly confident and surprised this wouldn't be caught by other tests.
I'm doing something else and will be back to it in a couple hours

[DavidBruant]

I've moved forward

My experience so far is that writing a contract is hard because of a combinaison of:

• ERTP+Zoe APIs very inconsistently return the object or a promise for the object
• what returns a promise and what does not is not really documented yet (and that may not be important in a world with the ~. operator)
• error messages do not help with the difference
• in general, error messages lack context and as a consequence only provide a vague idea of what is going wrong
• tape reporting is noisy and so, the relevant debugging information is hard to get
• ERTP is hard, but i'm over the hill now, so it's easier, but Zoe adds another layer of concepts

[DavidBruant]

A first version of the contract is ready, so i'm marking this PR as ready for review

There are cosmetic aspects (lint/prettier/commit messages) that will need to be addressed, but the PR is ready for functional review

[katelynsills]

Thanks, David! This looks like it's nearly there. I think the major thing to change is to remove the getSalesPayment method and use the Zoe payout system. Other comments are small. I will do another pass after the linting errors are fixed - it was hard to view in my local VSCode environment with the errors, so I may have missed things.

[DavidBruant]

I will do another pass after the linting errors are fixed - it was hard to view in my local VSCode environment with the errors, so I may have missed things.

I didn't have eslint enabled on VSCode so i didn't realize how annoying it might have been to read the code. I've enabled it and fixed the errors

[DavidBruant]

The only two remaining things are the removal of getSalesPayment and removal of async/await

I'm going to try to work on getSalesPayment (and try to understand better how reallocate works first)

[DavidBruant]

I removed async/awaits and i've rewritten so that the Opera uses payouts. For this i had the Opera creating the offers and redeeming invites directly (instead of having the contract do it)
As a consequence the contract code is smaller

I think this is ready for a review

[katelynsills]

I think we're making good progress, but I think we can make this contract simpler. Let's schedule some time to discuss how we can do that.

[DavidBruant]

I rewrote the contract according to the discussion i've had with @katelynsills
I think it's in a really good state

I'm particularly satisfied with the description of wanted allocations in performExchange where i used amountMath.add/substract.This is a lot clearer than what i was doing previously (making the amounts manually). This is possible because the auditorium position is represented by a single offerHandle instead of several ones as it was previously

[katelynsills]

This is looking really good to me. Some small typos and other questions, but I think after those are fixed, we are good to go. Thanks!

[DavidBruant]

but I think after those are fixed, we are good to go

@katelynsills Have you seen #780 (comment) ?
I feel like the fact that anyone can buy tickets for free makes this contract a poor example of the safety guarantees that Zoe can provide if used correctly (unlike this contract with a want nothing proposal from the auditorium)

Before merging, i'd like to have your go on this point specifically because i've become skeptical of merging this

[katelynsills]

but I think after those are fixed, we are good to go

@katelynsills Have you seen #780 (comment) ?

Sorry, I missed the comment. I responded above. I think also, @erights should take a look at this and approve it before we merge

[DavidBruant]

I addressed comments and change a bit how the tests are structured
I added Joker who is the person who's going to be doing things he shouldn't with the contract (and he should fail every time)
I added one test where Joker tries to buy a ticket for 1 moola (instead of 22). The tests expects him to fail, but as the code is now, he succeeds, so the test fails

There are 2 ways to fix the contract for the test to succeed (Joker fails to buy a ticket for 1 moola):

• Keep the auditorium position represented by 1 want: nothing offer and add a check in the contract (in performExchange) to reject the offer if the buyer payment is below 22 moolas
• Change the auditorium position to be represented by as many offers as there are tickets. Each offer is want: 22 moolas (Zoe does the offer safety check so it's not necessary to add something for this in the contract)

[erights]

Do the first bulleted choice

• Keep the auditorium position represented by 1 want: nothing offer and add a check in the contract (in performExchange) to reject the offer if the buyer payment is below 22 moolas

not the second one. The second one better approximates the semantics of multiples, but I prefer not to do it that way until we have genuine multiples.

[erights]

I have reviewed the contract and the review thread (including resolved items) about the contract. To both of you, good analysis and refinement, thanks! I have nothing more to add beyond comments already posted.

I skipped the test code, and the review comment threads on the test code, completely. If there's something in there you'd like to call my attention to, please do.

The contract-enforced offer safety by performExchange is the only remaining issue I know of before this can be merged.

[DavidBruant]

rebase on top of current master (with zoe helper import changes) + force-push

[katelynsills]

Looks good to me! I will squash and merge