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

chore: Sync Endo versions #5576

Merged
merged 16 commits into from
Jun 29, 2022
Merged

chore: Sync Endo versions #5576

merged 16 commits into from
Jun 29, 2022

Conversation

kriskowal
Copy link
Member

  • chore: Sync Endo versions
  • chore: Remove patches for prior Endo versions
  • chore: Update yarn.lock

@kriskowal kriskowal requested a review from turadg as a code owner June 11, 2022 00:53
@kriskowal kriskowal marked this pull request as draft June 14, 2022 16:39
@turadg turadg removed their request for review June 16, 2022 23:26
@kriskowal
Copy link
Member Author

@gibson042 I’m investigating a failure after Endo sync in the new publish-kit code. The new ses version has better unhandled rejection tracking and eventual-send no longer hides them in the cushions. https://github.com/Agoric/agoric-sdk/runs/6945401712?check_suite_focus=true

@kriskowal
Copy link
Member Author

@warner Another batch of unhandled rejections emanate because some vats tests lack a fake ibc vat. https://github.com/Agoric/agoric-sdk/runs/6945401863?check_suite_focus=true

@kriskowal kriskowal force-pushed the kris-sync-endo branch 2 times, most recently from 717ff53 to 06c876a Compare June 18, 2022 04:57
@warner
Copy link
Member

warner commented Jun 19, 2022

I think #5626 should fix the complaints from these two swingset tests:

  • yarn test -vs test/test-vpid-liveslots.js -m "liveslots vpid handling case7 reject"
  • yarn test -vs test/test-vpid-liveslots.js -m "liveslots vpid handling case7 promise-reject"

But the packages/vats one is probably something @dckc or @michaelfig needs to look at. It looks like test-boot.js is not providing all the fake vats that the code wants to load, and the fake createVat() function defined at

const createVat = bundleCap => {
const name = fakeBundleCaps.get(bundleCap);
assert(name);
switch (name) {
case 'zcf':
return fakeVatAdmin.createVat(zcfBundleCap);
default: {
const buildRoot = vatRoots[name];
if (!buildRoot) {
throw Error(`TODO: load vat ${name}`);
}
is not happy about it.

Something in bootstrap is tolerating the failure to create vat-ibc but isn't putting a .catch on the Promise it gets back from E(vatAdminSvc).createVatByName().

@dckc
Copy link
Member

dckc commented Jun 20, 2022

... It looks like test-boot.js is not providing all the fake vats that the code wants to load

That part is straightforward to address (see 2 line patch below).

Something in bootstrap is tolerating the failure to create vat-ibc but isn't putting a .catch on the Promise it gets back from E(vatAdminSvc).createVatByName().

I'm looking into this part.

diff --git a/packages/vats/test/test-boot.js b/packages/vats/test/test-boot.js
index c631f63ac..f12e67aee 100644
--- a/packages/vats/test/test-boot.js
+++ b/packages/vats/test/test-boot.js
@@ -15,6 +15,7 @@ import { bridgeCoreEval } from '../src/core/chain-behaviors.js';
 import { makePromiseSpace } from '../src/core/utils.js';
 import { buildRootObject as bankRoot } from '../src/vat-bank.js';
 import { buildRootObject as boardRoot } from '../src/vat-board.js';
+import { buildRootObject as ibcRoot } from '../src/vat-ibc.js';
 import { buildRootObject as mintsRoot } from '../src/vat-mints.js';
 import { buildRootObject as networkRoot } from '../src/vat-network.js';
 import { buildRootObject as priceAuthorityRoot } from '../src/vat-priceAuthority.js';
@@ -26,6 +27,7 @@ import { devices } from './devices.js';
 const vatRoots = {
   bank: bankRoot,
   board: boardRoot,
+  ibc: ibcRoot,
   mints: mintsRoot,
   network: networkRoot,
   priceAuthority: priceAuthorityRoot,

@kriskowal
Copy link
Member Author

@michaelfig We can knock out another couple failing tests due to another unhandled rejection in packages/deploy-script-support/test/unitTests/test-coreProposalBehavior.js because in packages/deploy-script-support/src/coreProposalBehavior.js, agoricNamesAdmin does not implement lookupAdmin. This is because the test does not provide an agoricNamesAdmin in the consume powers. You would not know happen to build or buy a fake agoric names admin?

SES_UNHANDLED_REJECTION: (TypeError#1)
TypeError#1: Cannot deliver lookupAdmin to target; typeof target is undefined
TypeError: Cannot deliver "lookupAdmin" to target; typeof target is "undefined"
TypeError#1 ERROR_NOTE: Thrown from: (Error#2) : 2 . 0
  Error#2: Event: 1.1
Nested error under TypeError#1
    at behavior (packages/deploy-script-support/src/coreProposalBehavior.js:90:46)
    at async packages/deploy-script-support/test/unitTests/test-coreProposalBehavior.js:17:18

@dckc
Copy link
Member

dckc commented Jun 22, 2022

... You would not know happen to build or buy a fake agoric names admin?

lightly tested:

index 97f073f04..8e6574a5b 100644
--- a/packages/deploy-script-support/test/unitTests/test-coreProposalBehavior.js
+++ b/packages/deploy-script-support/test/unitTests/test-coreProposalBehavior.js
@@ -3,6 +3,7 @@ import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';
 import { E } from '@endo/far';
 import '@agoric/vats/src/core/types.js';
 import { runModuleBehaviors } from '@agoric/vats/src/core/utils.js';
+import { makeNameHubKit } from '@agoric/vats/src/nameHub.js';
 import { makeCoreProposalBehavior } from '../../src/coreProposalBehavior.js';
 
 test('coreProposalBehavior', async t => {
@@ -14,8 +15,12 @@ test('coreProposalBehavior', async t => {
     getManifestCall,
     log: t.log,
   });
+  const { nameAdmin: agoricNamesAdmin } = makeNameHubKit();
   const result = await behavior({
-    consume: { board: { getValue: id => `boardValue:${id}` } },
+    consume: {
+      board: { getValue: id => `boardValue:${id}` },
+      agoricNamesAdmin,
+    },
     aParams: 'aparms',

@kriskowal
Copy link
Member Author

@gibson042 I’m investigating a failure after Endo sync in the new publish-kit code. The new ses version has better unhandled rejection tracking and eventual-send no longer hides them in the cushions. https://github.com/Agoric/agoric-sdk/runs/6945401712?check_suite_focus=true

May need a hand from @michaelfig for this one too. I’m not sure how it’s possible that this is producing an unhandled rejection:

      tailP = HandledPromise.reject(
        new Error('Cannot read past end of iteration.'),
      );
      // Suppress unhandled rejection error.
      tailP.catch(() => {});

It looks like we’re doing the right think by sinking the tailP, but for some reason, it’s retching when it’s collected.

@kriskowal
Copy link
Member Author

agoricNamesAdmin

Thank you! The next obstacle is finding a suitable installation and putting it in the name hub. I see something called installation farther down. I also see the nameAdmin implements set(key, value, admin), so I’m guessing key is "installation", value is the installation, and the admin is something else.

@dckc
Copy link
Member

dckc commented Jun 22, 2022

... I also see the nameAdmin implements set(key, value, admin), so I’m guessing key is "installation", value is the installation, and the admin is something else.

I think the 3rd arg, admin, is optional, and you can skip it in this case.

@dckc
Copy link
Member

dckc commented Jun 22, 2022

...
I think the 3rd arg, admin, is optional, and you can skip it in this case.

Hm... no, I think there's more to it in this case.

@michaelfig
Copy link
Member

It looks like we’re doing the right think by sinking the tailP, but for some reason, it’s retching when it’s collected.

It's not tailP that's retching. It's a promise chained off of it. Look for calls to .subscribeAfter to be the culprits, such as subscribeEach and makeLatestIterator.

It would be nice if our unhandled rejection code could also report the latest call site that created an unhandled rejection by chaining. I proposed something like that for XS, but the Moddable folks didn't choose to implement it (as of yet).

@kriskowal
Copy link
Member Author

It looks like we’re doing the right think by sinking the tailP, but for some reason, it’s retching when it’s collected.

It's not tailP that's retching. It's a promise chained off of it. Look for calls to .subscribeAfter to be the culprits, such as subscribeEach and makeLatestIterator.

It would be nice if our unhandled rejection code could also report the latest call site that created an unhandled rejection by chaining. I proposed something like that for XS, but the Moddable folks didn't choose to implement it (as of yet).

The test in question is the test for subscribeEach in @agoric/notifiers. I’m still investigating. The usage looks legitimate to me; there’s an async for loop that exhausts the subscription, but it’s a feedback loop, which might complicate things.

@kriskowal kriskowal force-pushed the kris-sync-endo branch 2 times, most recently from 4dbae29 to 0b124e7 Compare June 24, 2022 17:49
@kriskowal kriskowal requested a review from mhofman June 28, 2022 20:54
Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

LGTM (assuming tests pass)

@kriskowal kriskowal marked this pull request as ready for review June 28, 2022 22:59
@kriskowal kriskowal added the automerge:rebase Automatically rebase updates, then merge label Jun 28, 2022
@mergify mergify bot merged commit a3c06c1 into master Jun 29, 2022
@mergify mergify bot deleted the kris-sync-endo branch June 29, 2022 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants