-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: contract code to mint tickets #8
Conversation
And update contract code to check and trade against current inventory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Posting my open questions with current code
* @param {number} n | ||
* @returns {Amount} | ||
*/ | ||
const multiply = (amount, n) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question
I didn't see a AmountMath.multiply
function so I implemented it myself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import {
makeRatio,
multiplyBy,
} from '@agoric/zoe/src/contractSupport/ratio.js';
import { Nat } from '@endo/nat';
/**
*
* @param {Amount<'nat'>} amount
* @param {number} n
* @returns {Amount}
*/
const multiply = (amount, n) => {
const r = makeRatio(Nat(n), amount.brand, 1n);
return multiplyBy(amount, r);
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I seem to have some trouble making multiply
work for Amount<AssetKind>
instead of Amount<'nat'>
, am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our ratio library doesn't support non-fungible amounts and says so in its type annotations... which means you may have to trace thru all the callers and update their types to Amount<'nat'>
.
In some places, a typecast is called for. For example, zcf.getTerms()
doesn't know whether the brands are for fungible assets or not. And you're not in a position to doubt the caller that started the contract - that's part of the reliance set. You could ask the brand what its AssetKind is, but it could lie, so it's not worth the bother.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for sharing in draft form. good stuff here.
* @param {number} n | ||
* @returns {Amount} | ||
*/ | ||
const multiply = (amount, n) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import {
makeRatio,
multiplyBy,
} from '@agoric/zoe/src/contractSupport/ratio.js';
import { Nat } from '@endo/nat';
/**
*
* @param {Amount<'nat'>} amount
* @param {number} n
* @returns {Amount}
*/
const multiply = (amount, n) => {
const r = makeRatio(Nat(n), amount.brand, 1n);
return multiplyBy(amount, r);
};
updating the UI code seems to fit more comfortably in a separate PR |
This reverts commit 10388d6.
* @param {number} n | ||
* @returns {Amount} | ||
*/ | ||
const multiply = (amount, n) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I seem to have some trouble making multiply
work for Amount<AssetKind>
instead of Amount<'nat'>
, am I missing something?
[newItems, buyerSeat, want], | ||
[buyerSeat, proceeds, { Price: totalPrice }], | ||
// tickets from inventory to buyer | ||
[inventorySeat, buyerSeat, want], | ||
]), | ||
); | ||
|
||
buyerSeat.exit(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question, for my learning
What to do with proceeds
and inventorySeat
?
Seems like we could get the proceeds when contract is closed by the creator via creatorFacet. Should we keep inventorySeat
open even in the case when all tickets are sold?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, a creatorFacet
method is a typical way to collect the proceeds. (Seems fine to postpone to a later PR, if we bother to do it at all)
See, for example, collectFees.js and the contracts that import it.
as to inventorySeat
... when they're all sold, we could shut the contract down (there's a shutdown method on zcf)
Or we could have a creatorFacet method to resupply it for the next event or something. But that seems like over-kill.
// M.splitRecord({ tradePrice: AmountShape, maxTickets: M.nat() }), | ||
// getting an error of | ||
// customTerms: inventory: [1]: {} - Must have missing properties ["tradePrice","maxTickets"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question
Not sure why M.splitRecord(...)
doesn't work here- inventory
has tradePrice
and maxTickets
in the values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the liberty of pushing a fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good stuff!
Marking this PR as done in the project board since this is merged. |
My first stab at updating the contract to mint tickets, notably, update contract terms to accept
inventory
then checkwant
against inventory to make sure we have enough inventory and checkgive
against total price to make sure user offers enough tokens.This is still pretty rough so any and all feedbacks are welcomed.
TODOs:
test-contract.js
pass, it's usingmockBootstrap
and I don't know quite understand how it works