feat: add the ability for a contract to get a synchronous seat #1389

Chris-Hibbert · 2020-08-07T18:37:58Z

zcf.addEmptySeat() would return a zcfSeat that the contract could use
to hold allocations.

untested

katelynsills

I think we don't want to burn the associated assets, and will need a way to get the payouts. I think the seats made this way should be indistinguishable from other seats after being made - there should be an associated UserSeat that gets made.

katelynsills · 2020-08-07T22:36:34Z

packages/zoe/src/zoeService/zoe.js

+              const instanceAdmin = instanceToInstanceAdmin.get(instance);
+              instanceAdmin.removeZoeSeatAdmin(seatAdmin);
+
+              // burn the holdings to keep Zoe's book straight


I don't think we ever want to burn the holdings - the contract or someone else probably wants them

okay. Made available to contract. This needs a test.

katelynsills · 2020-08-07T22:37:00Z

packages/zoe/src/zoeService/zoe.js

+
+          const { notifier, updater } = makeNotifierKit();
+
+          const seatAdmin = {


I think we should use the same seatAdmin code and not have duplication here

katelynsills · 2020-08-07T22:37:19Z

packages/zoe/src/zoeService/zoe.js

@@ -209,6 +209,34 @@ function makeZoe(vatAdminSvc) {
                brandToPurse.init(brand, E(issuer).makeEmptyPurse());
              }
            }),
+        makeEmptySeat: (keyword, initialAllocation) => {


If it is an empty seat, why does it have an initial allocation?

katelynsills · 2020-08-07T22:43:11Z

packages/zoe/src/contractFacet/contractFacet.js

+
+        E(zoeInstanceAdmin)
+          .makeEmptySeat(keyword, initialAllocation, proposal)
+          .then(({ seatAdmin: zoeSeatAdmin, seatData }) => {


Let's call this the ZoeSeatAdmin in the returned object too. It's called that in Zoe.

katelynsills · 2020-08-07T22:45:33Z

packages/zoe/src/contractFacet/contractFacet.js

+        });
+        const { zcfSeat, zcfSeatAdmin } = makeSeatAdmin(
+          allSeatStagings,
+          zoeSeatAdminPromiseKit.promise,


Nice, since this is remote anyways, and we need to call E(zoeSeatAdmin), we can use a promise. Cool!

katelynsills · 2020-08-07T22:46:50Z

packages/zoe/src/contractFacet/contractFacet.js

@@ -225,6 +241,33 @@ export function buildRootObject() {
        issuerTable.getIssuerRecordByIssuer(issuer).brand,
      getAmountMath,
      makeZCFMint,
+      addEmptySeat: keyword => {


why is there a keyword?

It's gone now.

zcf.addEmptySeat() would return a zcfSeat that the contract could use to hold allocations. untested

drop keyword parameter in addEmptySeat() extract makeZoeSeatAdminKit, use it for emptySeat, too. rename zcf seat construction to use 'Kit'

erights · 2020-08-09T19:33:43Z

packages/zoe/src/contractFacet/contractFacet.js

@@ -164,6 +166,20 @@ export function buildRootObject() {
      return zcfMint;
    };

+    function updateNotiferFrom(updater, notifier, nextCount = undefined) {


Can you just use updateFromNotifier in packages/notifier?

yes, of course.

erights · 2020-08-09T19:44:48Z

packages/zoe/src/contractFacet/contractFacet.js

@@ -164,6 +166,20 @@ export function buildRootObject() {
      return zcfMint;
    };

+    function updateNotiferFrom(updater, notifier, nextCount = undefined) {
+      notifier.getUpdateSince(nextCount).then(


Isn't this notifier actually a reference to a remote notifier, the one in the zoe vat? If so, this would need to be E(notifier).

erights · 2020-08-09T20:11:08Z

packages/zoe/src/internal-types.js

@@ -64,7 +64,7 @@

 /**
 * Make the ZCF seat and seat admin
- * @callback MakeSeatAdmin
+ * @callback MakeZcfSeatAdmin


MakeZcfSeatAdminKit

erights · 2020-08-09T20:13:39Z

packages/zoe/src/zoeService/zoe.js

@@ -207,35 +230,24 @@ function makeZoe(vatAdminSvc) {
                brandToPurse.init(brand, E(issuer).makeEmptyPurse());
              }
            }),
+        makeEmptySeat: (initialAllocation, proposal) => {


I like the generality of leaving it up to zcf to determine the initialAllocation and proposal. But in that case, this method needs a new name.

I see it returns a kit so its name should reflect that too.

makeOfferlessSeat is my starting point.

erights · 2020-08-09T21:30:19Z

packages/zoe/src/zoeService/zoe.js

          const offerResultPromiseKit = makePromiseKit();
          const exitObjPromiseKit = makePromiseKit();
-          const { notifier, updater } = makeNotifierKit();
-          let currentAllocation = initialAllocation;
-
          const instanceAdmin = instanceToInstanceAdmin.get(instance);

          /** @type {ZoeSeatAdmin} */


ZoeSeatAdminKit

erights · 2020-08-09T21:39:28Z

packages/zoe/src/zoeService/zoe.js

@@ -207,35 +230,24 @@ function makeZoe(vatAdminSvc) {
                brandToPurse.init(brand, E(issuer).makeEmptyPurse());
              }
            }),
+        makeEmptySeat: (initialAllocation, proposal) => {


Add a declaration of makeEmptySeat (or whatever it's called) to the ZoeInstanceAdmin type.

erights · 2020-08-09T21:54:04Z

packages/zoe/src/zoeService/zoeSeat.js

+  brandToPurse,
+  promises = {},
+) => {
+  const payoutPromiseKit = promises.payout || makePromiseKit();


This payout option is unused. Is it needed?

erights · 2020-08-09T21:59:27Z

packages/zoe/src/zoeService/zoeSeat.js

+import { assert } from '@agoric/assert';
+import { E } from '@agoric/eventual-send';
+
+export const makeZoeSeatAdminKit = (


Good. Pulling this out into a separately reusable function made everything better.

Not quite everything, but it did help a lot.

erights · 2020-08-09T23:00:15Z

packages/zoe/src/contractFacet/contractFacet.js

@@ -256,15 +258,44 @@ export function buildRootObject() {
        issuerTable.getIssuerRecordByIssuer(issuer).brand,
      getAmountMath,
      makeZCFMint,
+      addEmptySeat: () => {


Does addEmptySeat sound like it's adding one that already exists?

Should it be makeEmptySeatKit?

I don't much like the name here for a user-facing method, but it fits our model for a makeFooKit, so done.

improved type annotations rename makeEmptySeat to makeOfferlessSeat don't return the zcfSeatAdmin to the contract

This is good enough to merge to the branch.

Chris-Hibbert force-pushed the syncEmptySeat branch 2 times, most recently from c5379cb to 9db00c7 Compare August 7, 2020 22:31

katelynsills previously requested changes Aug 7, 2020

View reviewed changes

Chris-Hibbert force-pushed the syncEmptySeat branch from 9db00c7 to 7ab7d0e Compare August 7, 2020 23:35

michaelfig force-pushed the new-zoe-spike-2 branch from 3554262 to 4585c45 Compare August 9, 2020 00:30

Chris-Hibbert force-pushed the syncEmptySeat branch from 7ab7d0e to 3f4b02d Compare August 9, 2020 18:43

Chris-Hibbert added 2 commits August 9, 2020 11:48

feat: add the ability for a contract to get a synchronous seat

5cb4e50

zcf.addEmptySeat() would return a zcfSeat that the contract could use to hold allocations. untested

refactor: cleanup synchronous ZCF seat creation

5235f5b

drop keyword parameter in addEmptySeat() extract makeZoeSeatAdminKit, use it for emptySeat, too. rename zcf seat construction to use 'Kit'

Chris-Hibbert force-pushed the syncEmptySeat branch from 3f4b02d to 5235f5b Compare August 9, 2020 18:49

erights reviewed Aug 9, 2020

View reviewed changes

refactor: use updateFromNotifier from library

6e897bd

erights previously requested changes Aug 9, 2020

View reviewed changes

erights reviewed Aug 9, 2020

View reviewed changes

refactor: post review cleanups; mostly type info

0b761fe

improved type annotations rename makeEmptySeat to makeOfferlessSeat don't return the zcfSeatAdmin to the contract

Chris-Hibbert force-pushed the syncEmptySeat branch from 397de87 to 0b761fe Compare August 9, 2020 23:52

Chris-Hibbert self-assigned this Aug 9, 2020

Chris-Hibbert merged commit 32883ab into new-zoe-spike-2 Aug 9, 2020

Chris-Hibbert deleted the syncEmptySeat branch August 9, 2020 23:55

katelynsills mentioned this pull request Aug 21, 2020

Allow Empty Seat to Have a Proposal with Want, Exit #1582

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add the ability for a contract to get a synchronous seat #1389

feat: add the ability for a contract to get a synchronous seat #1389

Chris-Hibbert commented Aug 7, 2020

katelynsills left a comment

katelynsills Aug 7, 2020

Chris-Hibbert Aug 9, 2020

katelynsills Aug 7, 2020

Chris-Hibbert Aug 9, 2020

katelynsills Aug 7, 2020

Chris-Hibbert Aug 9, 2020

katelynsills Aug 7, 2020

katelynsills Aug 7, 2020

katelynsills Aug 7, 2020

Chris-Hibbert Aug 8, 2020

erights Aug 9, 2020

Chris-Hibbert Aug 9, 2020

erights Aug 9, 2020

Chris-Hibbert Aug 9, 2020

erights Aug 9, 2020

Chris-Hibbert Aug 9, 2020

erights Aug 9, 2020

erights Aug 9, 2020

Chris-Hibbert Aug 9, 2020

erights Aug 9, 2020

Chris-Hibbert Aug 9, 2020

erights Aug 9, 2020

Chris-Hibbert Aug 9, 2020

erights Aug 9, 2020

erights Aug 9, 2020

Chris-Hibbert Aug 9, 2020

erights Aug 9, 2020

Chris-Hibbert Aug 9, 2020


		const { notifier, updater } = makeNotifierKit();

		const seatAdmin = {

feat: add the ability for a contract to get a synchronous seat #1389

feat: add the ability for a contract to get a synchronous seat #1389

Conversation

Chris-Hibbert commented Aug 7, 2020

katelynsills left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment