Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: contract code to mint tickets #8
Changes from 5 commits
d553e19
11754dd
791417c
81fcca5
cc658e7
c101065
10388d6
f706103
0332a6e
bd8bef9
d5d5482
2bc29bb
5d9e6f8
0cdb78c
fec0cad
8774bd6
216b293
02bde11
f828cad
c5c4b31
4438977
9a56f16
727b350
1b8a012
5be8bbb
70a347c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check failure on line 33 in contract/src/agoric-basics.contract.js
GitHub Actions / all
Check failure on line 39 in contract/src/agoric-basics.contract.js
GitHub Actions / all
Check warning on line 47 in contract/src/agoric-basics.contract.js
GitHub Actions / all
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 myselfThere 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.
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 forAmount<AssetKind>
instead ofAmount<'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.Check failure on line 75 in contract/src/agoric-basics.contract.js
GitHub Actions / all
Check warning on line 84 in contract/src/agoric-basics.contract.js
GitHub Actions / all
Check failure on line 87 in contract/src/agoric-basics.contract.js
GitHub Actions / all
Check warning on line 95 in contract/src/agoric-basics.contract.js
GitHub Actions / all
Check failure on line 102 in contract/src/agoric-basics.contract.js
GitHub Actions / all
Check warning on line 114 in contract/src/agoric-basics.contract.js
GitHub Actions / all
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
andinventorySeat
?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.