Skip to content
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: Check that makeInstance() returns an actual invite #904

Merged
merged 2 commits into from
Apr 11, 2020

Conversation

Chris-Hibbert
Copy link
Contributor

Includes a test.

closes #820

Copy link
Contributor

@katelynsills katelynsills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to let me know if the changes I suggested aren't desirable.

@@ -543,7 +543,12 @@ const makeZoe = (additionalEndowments = {}) => {
// Once the contract is made, we add the publicAPI to the
// contractRecord
instanceTable.update(instanceHandle, { publicAPI });
return invite;
return inviteIssuer.isLive(invite).then(success => {
if (success) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use an assert here and reverse the return and the error check?

Suggested change
if (success) {
assert(success, details`invites must be issued by the inviteIssuer.`);
return invite;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

const issuerKeywordRecord = harden({ Contribution: moolaR.issuer });

// 1: Alice tries to create an instance, but the contract is badly written.
zoe.makeInstance(installationHandle, issuerKeywordRecord).then(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use t.throws here. Is there a reason not to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makeInstance is async. I couldn't find a way to await the result. It doesn't throw until the test is done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t.rejects() works.

Copy link
Contributor

@katelynsills katelynsills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the test.only and then it looks good to me!

@Chris-Hibbert Chris-Hibbert force-pushed the checkInvite branch 2 times, most recently from 8eab627 to ab54fb9 Compare April 11, 2020 17:18
@Chris-Hibbert Chris-Hibbert merged commit e6f3cb7 into master Apr 11, 2020
@Chris-Hibbert Chris-Hibbert deleted the checkInvite branch April 11, 2020 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Zoe] Check that invite field of makeInstance return value is an actual invite
2 participants