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

Payments are virtual objects. #2526

Merged
merged 1 commit into from
Mar 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/ERTP/globals.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
declare var makeKind: function;
declare var makeWeakStore: function;
2 changes: 1 addition & 1 deletion packages/ERTP/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@
"strictNullChecks": true,
"moduleResolution": "node",
},
"include": ["src/**/*.js", "exported.js"],
"include": ["src/**/*.js", "exported.js", "globals.d.ts"],
}
3 changes: 1 addition & 2 deletions packages/ERTP/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@
"test/**/test-*.js"
],
"require": [
"esm",
"@agoric/install-ses"
"esm"
]
},
"eslintConfig": {
Expand Down
22 changes: 8 additions & 14 deletions packages/ERTP/src/issuer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright (C) 2019 Agoric, under Apache License 2.0

// @ts-check
/* global makeWeakStore */

import { assert, details as X } from '@agoric/assert';
import { makeExternalStore } from '@agoric/store';
Expand All @@ -13,6 +12,7 @@ import { amountMath, MathKind } from './amountMath';
import { makeAmountMath } from './deprecatedAmountMath';
import { makeFarName, ERTPKind } from './interfaces';
import { coerceDisplayInfo } from './displayInfo';
import { makePaymentMaker } from './payment';

import './types';

Expand Down Expand Up @@ -52,17 +52,10 @@ function makeIssuerKit(
/** @type {Amount} */
const emptyAmount = amountMath.makeEmpty(amountMathKind, brand);

const {
makeInstance: makePayment,
makeWeakStore: makePaymentWeakStore,
} = makeExternalStore('payment', () =>
Far(makeFarName(allegedName, ERTPKind.PAYMENT), {
getAllegedBrand: () => brand,
}),
);
const makePayment = makePaymentMaker(allegedName, brand);

/** @type {WeakStore<Payment, Amount>} */
const paymentLedger = makePaymentWeakStore();
const paymentLedger = makeWeakStore('payment');

function assertKnownPayment(payment) {
assert(paymentLedger.has(payment), X`payment not found for ${allegedName}`);
Expand Down Expand Up @@ -167,12 +160,13 @@ function makeIssuerKit(
// other uses.

if (payments.length > 1) {
const paymentSet = new Set();
// TODO: replace with a Set that understands virtual objects
const antiAliasingStore = makeWeakStore('payment');
payments.forEach(payment => {
if (paymentSet.has(payment)) {
if (antiAliasingStore.has(payment)) {
throw new Error('same payment seen twice');
}
paymentSet.add(payment);
antiAliasingStore.init(payment, undefined);
});
}

Expand Down
22 changes: 22 additions & 0 deletions packages/ERTP/src/payment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// @ts-check
/* global makeKind */

import { Far } from '@agoric/marshal';
import { makeFarName, ERTPKind } from './interfaces';

export const makePaymentMaker = (allegedName, brand) => {
const paymentVOFactory = state => {
return {
init: b => (state.brand = b),
self: Far(makeFarName(allegedName, ERTPKind.PAYMENT), {
getAllegedBrand: () => state.brand,
}),
Comment on lines +11 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how well the Far(makeFarName(... stuff is going to get along with the virtual object machinery. The construct is certainly ugly and very hard to parse visually. I would have written this as just:

  self: {
    getAllegedBrand: () => state.brand,
  }

But if this object is going to be used as a presence we need to get that Far() wrapper on there somehow.

The return value from paymentVOMaker is going to get hardened. I don't know how harden deals with objects with properties that are already hardened. I think that's OK, but this is one of those mysterious edge cases that I always have to either ask @erights about or just try and see what happens.

Copy link
Member

Choose a reason for hiding this comment

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

harden is idempotent. That part is fine.

Copy link
Member

@dtribble dtribble Feb 24, 2021

Choose a reason for hiding this comment

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

I was expecting the Far... to be in the makeKind invocationL

const paymentMaker = makeKind(Far(makeFarName(allegedName, ERTPKind.PAYMENT), paymentVOMaker);

I don't like needing the self: section. There was a slightly different pattern that didn't require that, but I couldn't lay my hands on that issue yet.

Copy link
Member

Choose a reason for hiding this comment

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

As @dtribble said, harden() is idempotent. However Far is not: it only accepts unhardened objects (it mutates them before hardening them).

I have a branch in which makeKind asserts that the maker it was given is applying Far to the self that it returns, under the theory that the maker should be doing harden to the { init, self } pair that it returns ("don't let objects out of your sight without hardening them first"), and Far() cannot be applied to objects that are already hardened, therefore the maker must apply the Far before it hardens { init, self }.

(the assertion on that branch isn't very helpful, however, because if you make the most common mistake and don't harden the return value, it emits a particularly weird error message instead of something that would guide you towards the light)

It would certainly be more ergonomic to tell makeKind the interface name, and then have makeKind apply the Far() itself, but that would require that the maker not harden its return value, which feels counter to the habit we're trying to teach developers.

Copy link
Member

Choose a reason for hiding this comment

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

I think that integration is the right thing. It should happen within the same module. The init/self is the squishy underbelly of constructing useful remote and persistent objects. They don't need to be hardened, given that they should never escape directly.

Copy link
Member

Choose a reason for hiding this comment

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

(Retracted previous comment. I took "escape" out of context)

Copy link
Member

Choose a reason for hiding this comment

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

Oh good. To clarify for the future, I just meant that the internal description of what needs to be built -- paymentVOMaker -- on line 9 gets used on line 18 and the result of that is what gets locked down by the process. The paymentVOMaker should never escape.

};
};

const paymentMaker = makeKind(paymentVOFactory);

const makePayment = () => paymentMaker(brand);

return makePayment;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { E } from '@agoric/eventual-send';
import { assert, details as X } from '@agoric/assert';
import { makeIssuerKit } from '../../../src';

export function buildRootObject(vatPowers, vatParameters) {
const arg0 = vatParameters.argv[0];

function testBasicFunctionality(aliceMaker) {
vatPowers.testLog('start test basic functionality');
const { mint: moolaMint, issuer, amountMath } = makeIssuerKit('moola');
const moolaPayment = moolaMint.mintPayment(amountMath.make(1000));

const aliceP = E(aliceMaker).make(issuer, amountMath, moolaPayment);
return E(aliceP).testBasicFunctionality();
}

const obj0 = {
async bootstrap(vats) {
switch (arg0) {
case 'basicFunctionality': {
const aliceMaker = await E(vats.alice).makeAliceMaker();
return testBasicFunctionality(aliceMaker);
}
default: {
assert.fail(X`unrecognized argument value ${arg0}`);
}
}
},
};
return harden(obj0);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// @ts-check
/* global __dirname */

// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/swingset-vat/tools/prepare-test-env';

// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';
// eslint-disable-next-line import/no-extraneous-dependencies
import { loadBasedir, buildVatController } from '@agoric/swingset-vat';
import path from 'path';

async function main(basedir, argv) {
const dir = path.resolve(`${__dirname}/..`, basedir);
const config = await loadBasedir(dir);
const controller = await buildVatController(config, argv);
await controller.run();
return controller.dump();
}

const expected = [
'start test basic functionality',
'isLive: true',
'getAmountOf: {"brand":{},"value":1000}',
'newPayment amount: {"brand":{},"value":1000}',
'burned amount: {"brand":{},"value":200}',
'claimedPayment amount: {"brand":{},"value":200}',
'combinedPayment amount: {"brand":{},"value":600}',
];

test('test splitPayments', async t => {
const dump = await main('basicFunctionality', ['basicFunctionality']);
t.deepEqual(dump.log, expected);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { E } from '@agoric/eventual-send';

function makeAliceMaker(log) {
return harden({
make(issuer, amountMath, oldPaymentP) {
const alice = harden({
async testBasicFunctionality() {
// isLive
const alive = await E(issuer).isLive(oldPaymentP);
log('isLive: ', alive);

// getAmountOf
const amount = await E(issuer).getAmountOf(oldPaymentP);
log('getAmountOf: ', amount);

// Make Purse

const purse = E(issuer).makeEmptyPurse();

// Deposit Payment

const payment = await oldPaymentP;
await E(purse).deposit(payment);

// Withdraw Payment
const newPayment = E(purse).withdraw(amount);
const newAmount = await E(issuer).getAmountOf(newPayment);
log('newPayment amount: ', newAmount);

// splitMany
const moola200 = await E(amountMath).make(200);
const [paymentToBurn, paymentToClaim, ...payments] = await E(
issuer,
).splitMany(
newPayment,
harden([moola200, moola200, moola200, moola200, moola200]),
);

// burn
const burnedAmount = await E(issuer).burn(paymentToBurn);
log('burned amount: ', burnedAmount);

// claim
const claimedPayment = await E(issuer).claim(paymentToClaim);
const claimedPaymentAmount = await E(issuer).getAmountOf(
claimedPayment,
);
log('claimedPayment amount: ', claimedPaymentAmount);

// combine
const combinedPayment = E(issuer).combine(payments);
const combinedPaymentAmount = await E(issuer).getAmountOf(
combinedPayment,
);
log('combinedPayment amount: ', combinedPaymentAmount);
},
});
return alice;
},
});
}

export function buildRootObject(vatPowers) {
return harden({
makeAliceMaker() {
return harden(makeAliceMaker(vatPowers.testLog));
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

can these two makeAliceMaker functions have different names? Maybe the inner one is makeAliceMakerActual?

},
});
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
/* global __dirname */
// @ts-check

// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/swingset-vat/tools/prepare-test-env';

// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// @ts-check

// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/swingset-vat/tools/prepare-test-env';

// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';
import { Far } from '@agoric/marshal';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// @ts-check

// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/swingset-vat/tools/prepare-test-env';

// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// @ts-check

// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/swingset-vat/tools/prepare-test-env';

// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';
import { amountMath as m, MathKind } from '../../../src';
Expand Down
3 changes: 3 additions & 0 deletions packages/ERTP/test/unitTests/test-interfaces.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// @ts-check

// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/swingset-vat/tools/prepare-test-env';

// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';
import { getInterfaceOf } from '@agoric/marshal';
Expand Down
3 changes: 3 additions & 0 deletions packages/ERTP/test/unitTests/test-issuerObj.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// @ts-check

// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/swingset-vat/tools/prepare-test-env';

// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';
import { E } from '@agoric/eventual-send';
Expand Down
2 changes: 2 additions & 0 deletions packages/ERTP/test/unitTests/test-mintObj.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// @ts-check
// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/swingset-vat/tools/prepare-test-env';

import { Far } from '@agoric/marshal';
// eslint-disable-next-line import/no-extraneous-dependencies
Expand Down
3 changes: 2 additions & 1 deletion packages/SwingSet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@
"files": [
"bin/vat",
"src/**/*.js",
"exported.js"
"exported.js",
"tools"
],
"repository": {
"type": "git",
Expand Down
2 changes: 2 additions & 0 deletions packages/cosmic-swingset/globals.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
declare var makeKind: function;
declare var makeWeakStore: function;
2 changes: 1 addition & 1 deletion packages/cosmic-swingset/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@
"strictNullChecks": true,
"moduleResolution": "node",
},
"include": ["lib/**/*.js", "exported.js"],
"include": ["lib/**/*.js", "exported.js", "globals.d.ts"],
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @ts-check
// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/install-ses'; // calls lockdown()
import '@agoric/zoe/tools/prepare-test-env'; // calls lockdown()
// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import '@agoric/install-ses'; // calls lockdown()
import '@agoric/zoe/tools/prepare-test-env'; // calls lockdown()
// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';
import { Far } from '@agoric/marshal';
Expand Down
2 changes: 1 addition & 1 deletion packages/dapp-svelte-wallet/api/test/test-lib-wallet.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* global require */
// @ts-check
// eslint-disable-next-line import/no-extraneous-dependencies
import '@agoric/install-ses'; // calls lockdown()
import '@agoric/zoe/tools/prepare-test-env'; // calls lockdown()
// eslint-disable-next-line import/no-extraneous-dependencies
import test from 'ava';
// eslint-disable-next-line import/no-extraneous-dependencies
Expand Down
2 changes: 2 additions & 0 deletions packages/deploy-script-support/globals.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
declare var makeKind: function;
declare var makeWeakStore: function;
2 changes: 1 addition & 1 deletion packages/deploy-script-support/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@
"strictNullChecks": true,
"moduleResolution": "node",
},
"include": ["src/**/*.js", "exported.js"],
"include": ["src/**/*.js", "exported.js", "globals.d.ts"],
}
4 changes: 3 additions & 1 deletion packages/zoe/src/contractFacet/contractFacet.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// @ts-check

/* global makeWeakStore */

// This is the Zoe contract facet. Each time we make a new instance of a
// contract we will start by creating a new vat and running this code in it. In
// order to install this code in a vat, Zoe needs to import a bundle containing
Expand All @@ -9,7 +11,7 @@

import { assert, details as X, q, makeAssert } from '@agoric/assert';
import { E } from '@agoric/eventual-send';
import { makeStore, makeWeakStore } from '@agoric/store';
import { makeStore } from '@agoric/store';
import { Far, Data } from '@agoric/marshal';

import { makeAmountMath, MathKind } from '@agoric/ertp';
Expand Down
4 changes: 4 additions & 0 deletions packages/zoe/src/contractFacet/evalContractCode.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// @ts-check

/* global makeKind makeWeakStore */

import { importBundle } from '@agoric/import-bundle';
import { assert } from '@agoric/assert';

Expand All @@ -13,6 +15,8 @@ const evalContractBundle = (bundle, additionalEndowments = {}) => {
const defaultEndowments = {
console: louderConsole,
assert,
makeKind,
makeWeakStore,
};

const fullEndowments = Object.create(null, {
Expand Down
Loading