From b7b59fd3704ba96174a764c90e664527635a56a7 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Thu, 18 Jul 2024 15:15:23 -0700 Subject: [PATCH 1/4] fix(async-flow): fix endowment equate bug --- packages/async-flow/src/equate.js | 24 ++++++++++++++++-------- packages/async-flow/test/equate.test.js | 10 +++++----- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/async-flow/src/equate.js b/packages/async-flow/src/equate.js index f11df50e911..9bfa3bba4a3 100644 --- a/packages/async-flow/src/equate.js +++ b/packages/async-flow/src/equate.js @@ -31,7 +31,14 @@ export const makeEquate = bijection => { Fail`unequal ${g} vs ${h}`; return; } - if (bijection.has(g, h)) { + if (bijection.hasGuest(g) && bijection.guestToHost(g) === h) { + // Note that this can be true even when + // `bijection.hostToGuest(h) !== g` + // but only when the two guests represent the same host, as + // happens with unwrapping. That why we do this one-way test + // rather than the two way `bijection.has` test. + // Even in this one-way case, we have still satisfied + // the equate, so return. return; } const gPassStyle = passStyleOf(g); @@ -41,7 +48,8 @@ export const makeEquate = bijection => { // TODO when we do, delete the `throw Fail` line and uncomment // the two lines below it. // We *do* support passing a guest wrapper of a hostVow back - // to the host, but that would be cause by `bijection.has` above. + // to the host, but that would be caught by `bijection.guestToHost` + // test above. throw Fail`guest promises not yet passable`; // `init` does not yet do enough checking anyway. For this case, // we should ensure that h is a host wrapper of a guest promise, @@ -92,10 +100,10 @@ export const makeEquate = bijection => { case 'remotable': { // Note that we can send a guest wrapping of a host remotable // back to the host, - // but that should have already been taken care of by the - // `bijection.has` above. + // but that should have already been caught by the + // `bijection.guestToHost` above. throw Fail`cannot yet send guest remotables to host ${g} vs ${h}`; - // `init` does not yet do enough checking anyway. For this case, + // `unwrapInit` does not yet do enough checking anyway. For this case, // we should ensure that h is a host wrapper of a guest remotable, // which is a wrapping we don't yet support. // bijection.unwrapInit(g, h); @@ -104,10 +112,10 @@ export const makeEquate = bijection => { case 'promise': { // Note that we can send a guest wrapping of a host promise // (or vow) back to the host, - // but that should have already been taken care of by the - // `bijection.has` above. + // but that should have already been caught by the + // `bijection.guestToHost` above. throw Fail`cannot yet send guest promises to host ${g} vs ${h}`; - // `init` does not yet do enough checking anyway. For this case, + // `unwrapInit` does not yet do enough checking anyway. For this case, // we should ensure that h is a host wrapper of a guest promise, // which is a wrapping we don't yet support. // bijection.unwrapInit(g, h); diff --git a/packages/async-flow/test/equate.test.js b/packages/async-flow/test/equate.test.js index 5be4100794e..1ae04e08fe6 100644 --- a/packages/async-flow/test/equate.test.js +++ b/packages/async-flow/test/equate.test.js @@ -57,24 +57,24 @@ const testEquate = (t, zone, showOnConsole = false) => { bij.unwrapInit(g1, h1); t.notThrows(() => equate(g1, h1)); t.throws(() => equate(g1, h2), { - message: 'internal: g->h "[Alleged: g1]" -> "[Vow]" vs "[Alleged: h1]"', + message: 'unequal passStyles "remotable" vs "tagged"', }); t.throws(() => equate(g2, h1), { - message: 'internal: unexpected h->g "[Alleged: h1]" -> "[Alleged: g1]"', + message: 'unequal passStyles "promise" vs "remotable"', }); bij.unwrapInit(g2, h2); equate(g2, h2); t.throws(() => equate(g1, h2), { - message: 'internal: g->h "[Alleged: g1]" -> "[Vow]" vs "[Alleged: h1]"', + message: 'unequal passStyles "remotable" vs "tagged"', }); t.throws(() => equate(g2, h1), { - message: 'internal: g->h "[Promise]" -> "[Alleged: h1]" vs "[Vow]"', + message: 'unequal passStyles "promise" vs "remotable"', }); equate(harden([g1, g2]), harden([h1, h2])); t.throws(() => equate(harden([g1, g2]), harden([h1, h1])), { - message: '[1]: internal: g->h "[Promise]" -> "[Alleged: h1]" vs "[Vow]"', + message: '[1]: unequal passStyles "promise" vs "remotable"', }); const gErr1 = harden(makeError(X`error ${'redacted message'}`, URIError)); From 1edbcee21ab3a51f166dc30b4a56b8e7eef08dac Mon Sep 17 00:00:00 2001 From: Mathieu Hofman Date: Wed, 2 Oct 2024 05:12:01 +0000 Subject: [PATCH 2/4] fix(async-flow): unwrapped function is a remotable --- packages/async-flow/src/endowments.js | 10 +++++++--- packages/async-flow/test/endowments.test.js | 10 +++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/async-flow/src/endowments.js b/packages/async-flow/src/endowments.js index 909fb06389f..8be19ff71ed 100644 --- a/packages/async-flow/src/endowments.js +++ b/packages/async-flow/src/endowments.js @@ -1,7 +1,12 @@ import { Fail } from '@endo/errors'; import { E } from '@endo/eventual-send'; import { isPromise } from '@endo/promise-kit'; -import { isRemotable, isPassable, GET_METHOD_NAMES } from '@endo/pass-style'; +import { + isRemotable, + isPassable, + GET_METHOD_NAMES, + Far, +} from '@endo/pass-style'; import { M, objectMap } from '@endo/patterns'; import { prepareVowTools, toPassableCap } from '@agoric/vow'; import { isVow } from '@agoric/vow/src/vow-utils.js'; @@ -63,8 +68,7 @@ export const prepareEndowmentTools = (outerZone, outerOptions = {}) => { const functionUnwrapper = outerZone.exo('FunctionUnwrapper', UnwrapperI, { unwrap(guestWrapped) { - const unwrapped = (...args) => guestWrapped.apply(args); - return harden(unwrapped); + return Far('UnwrappedFunction', (...args) => guestWrapped.apply(args)); }, }); diff --git a/packages/async-flow/test/endowments.test.js b/packages/async-flow/test/endowments.test.js index 45aa83f7caf..f9df904fb60 100644 --- a/packages/async-flow/test/endowments.test.js +++ b/packages/async-flow/test/endowments.test.js @@ -16,6 +16,7 @@ import { makeDurableZone } from '@agoric/zone/durable.js'; import { forwardingMethods, prepareEndowmentTools } from '../src/endowments.js'; import { makeConvertKit } from '../src/convert.js'; import { prepareBijection } from '../src/bijection.js'; +import { makeEquate } from '../src/equate.js'; const { ownKeys } = Reflect; @@ -114,13 +115,20 @@ const testEndowmentPlay = async (t, zone, gen, isDurable) => { t.is(unwrapped.storable.exo.name(), `${gen} exo`); t.is(passStyleOf(unwrapped.far), 'remotable'); t.is(unwrapped.far.name(), `${gen} far`); - t.false(isPassable(unwrapped.function)); + t.is(passStyleOf(unwrapped.function), 'remotable'); t.is(typeof unwrapped.function, 'function'); t.is(unwrapped.function(), `${gen} function`); t.is(unwrapped.array[0](), `${gen} f1`); t.false(isPassable(unwrapped.state)); t.is(typeof unwrapped.state, 'object'); t.is(unwrapped.state[`${gen}_foo`], `${gen} foo`); + + const equate = makeEquate(bij); + + const { state: _1, ...passableUnwrapped } = unwrapped; + const { state: _2, ...passableWrapped } = wrapped; + + t.notThrows(() => equate(harden(passableUnwrapped), harden(passableWrapped))); }; const testEndowmentBadReplay = async (_t, _zone, _gen, _isDurable) => { From e34bed7cab015a29d3eb277beecf81b22d33981c Mon Sep 17 00:00:00 2001 From: Mathieu Hofman Date: Wed, 2 Oct 2024 05:12:41 +0000 Subject: [PATCH 3/4] test(async-flow): add a test for equate bug --- packages/async-flow/test/endowments.test.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/async-flow/test/endowments.test.js b/packages/async-flow/test/endowments.test.js index f9df904fb60..d047b534aa0 100644 --- a/packages/async-flow/test/endowments.test.js +++ b/packages/async-flow/test/endowments.test.js @@ -85,7 +85,16 @@ const testEndowmentPlay = async (t, zone, gen, isDurable) => { t.is(endowment.state[`${gen}_foo`], `${gen} foo`); t.is(wrapped.state.get(`${gen}_foo`), `${gen} foo`); - const makeBijection = prepareBijection(zone, unwrap); + const guestWrappers = new Map(); + const unwrapSpy = (hostWrapped, guestWrapped) => { + const unwrapped = unwrap(hostWrapped, guestWrapped); + if (unwrapped !== guestWrapped) { + guestWrappers.set(hostWrapped, guestWrapped); + } + return unwrapped; + }; + + const makeBijection = prepareBijection(zone, unwrapSpy); const bij = zone.makeOnce('bij', makeBijection); const makeGuestForHostRemotable = hRem => { @@ -129,6 +138,9 @@ const testEndowmentPlay = async (t, zone, gen, isDurable) => { const { state: _2, ...passableWrapped } = wrapped; t.notThrows(() => equate(harden(passableUnwrapped), harden(passableWrapped))); + for (const [hostWrapped, guestWrapped] of guestWrappers) { + t.notThrows(() => equate(guestWrapped, hostWrapped)); + } }; const testEndowmentBadReplay = async (_t, _zone, _gen, _isDurable) => { From 0b3803526819cbe7a9ddf82075fd4a7fe2b0c38d Mon Sep 17 00:00:00 2001 From: Mathieu Hofman Date: Wed, 2 Oct 2024 05:13:19 +0000 Subject: [PATCH 4/4] test(orchestration): contract upgrade no longer failing --- packages/boot/test/orchestration/contract-upgrade.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/boot/test/orchestration/contract-upgrade.test.ts b/packages/boot/test/orchestration/contract-upgrade.test.ts index 7f453ed990d..7f34dc1341f 100644 --- a/packages/boot/test/orchestration/contract-upgrade.test.ts +++ b/packages/boot/test/orchestration/contract-upgrade.test.ts @@ -29,7 +29,7 @@ test.after.always(t => t.context.shutdown?.()); * upgrades, and the ability to resume an async-flow for which a host vow * settles after an upgrade.) */ -test.failing('resume', async t => { +test('resume', async t => { const { walletFactoryDriver, buildProposal, evalProposal, storage } = t.context; @@ -73,8 +73,6 @@ test.failing('resume', async t => { buildProposal('@agoric/builders/scripts/testing/fix-buggy-sendAnywhere.js'), ); - // FIXME https://github.com/Agoric/agoric-sdk/issues/9303 - // This doesn't yet get past 'sending' t.deepEqual(getLogged(), [ 'sending {0} from cosmoshub to cosmos1whatever', 'got info for denoms: ibc/toyatom, ibc/toyusdc, ubld, uist',