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

base upgrade-next on top of latest (use-upgrade-14) #9204

Merged
merged 8 commits into from
Apr 18, 2024
Merged

Conversation

turadg
Copy link
Member

@turadg turadg commented Apr 6, 2024

closes: #9206

Description

With Agoric/agoric-3-proposals#141 a3p:latest now includes dev-upgrade-14 branch's upgrade handler. This removes the redundant parts from the UNRELEASED upgrade handler and its tests in a3p-integration.

It also introduces a convention initial.test.js to make clear what tests are of the post-upgrade state, to be run before tests that can modify the state. Another option is to number the tests so they run in order. I figure this can happen when needed; the test.sh allows any logic.

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

no

Testing Considerations

Tests

Upgrade Considerations

Upgrade tests

@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Apr 6, 2024
turadg added a commit that referenced this pull request Apr 8, 2024
:latest was bumped to include u14 by Agoric/agoric-3-proposals#141

but master can't yet apply on top of u14 #9204
@turadg turadg marked this pull request as draft April 8, 2024 16:45
@turadg turadg changed the title upgrade upgrade-next to be on top of use-upgrade-14 base upgrade-next on top of latest (use-upgrade-14) Apr 8, 2024
@turadg turadg added the force:integration Force integration tests to run on PR label Apr 8, 2024
@turadg
Copy link
Member Author

turadg commented Apr 9, 2024

failing in ./run_prepare.sh a:upgrade-next:

#23 22.49 logs: []
#23 22.49 raw_log: 'out of gas in location: WritePerByte; gasWanted: 200000, gasUsed: 204684:
#23 22.49   out of gas'

@Chris-Hibbert
Copy link
Contributor

I'm happy to see this. I'll wait for tests to turn green before rebasing my upgrade-next work to it.

Copy link

cloudflare-workers-and-pages bot commented Apr 15, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: fb74bbd
Status: ✅  Deploy successful!
Preview URL: https://0687fd77.agoric-sdk.pages.dev
Branch Preview URL: https://ta-use-upgrade-14.agoric-sdk.pages.dev

View logs

@turadg
Copy link
Member Author

turadg commented Apr 15, 2024

Gas is solved by using syntheti-chain 0.0.9.

Now it's failing on this in probeZcfBundleCap.test.js.

   23:     detailsAfter.incarnation,                               
  wf incarnation must increase by 2
  Difference (- actual, + expected):
  - 2
  + 4

@Chris-Hibbert PTAL

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.

Why remove the core eval submission test?

@turadg
Copy link
Member Author

turadg commented Apr 15, 2024

Why remove the core eval submission test?

bc I thought it was part of u14. (see test: remove upgrade-14 content from upgrade-next )

if that's not correct, please request the change or push a fixup

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.

Let's keep the core-eval test, it's currently the only integration test we have for #8884

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep this test around, it's not specific to any upgrade.

Copy link
Member

@mhofman mhofman Apr 15, 2024

Choose a reason for hiding this comment

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

Oh good point about moving it to a z:acceptance layer! I think we can have "test only" layers now?

Edit: not officially, but we can have a /agoric.swingset.CoreEvalProposal which has a eval.sh script that does nothing.

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.

More changes needed in the plumbing of upgrade info due to sdk 0.46 changes

a3p-integration/proposals/a:upgrade-next/package.json Outdated Show resolved Hide resolved
@Chris-Hibbert
Copy link
Contributor

Now it's failing on this in probeZcfBundleCap.test.js.

Yep. The test that the walletFactory upgrade was successful fails because that upgrade failed.

Stack trace
2024-04-16T16:33:40.956Z block-manager: block 1140 begin
2024-04-16T16:33:40.990Z SwingSet: vat: v1: evaluateBundleCap { manifestBundleRef: { bundleID: 'b1-2e1dc81899761c1aeefe17ab07e8475242760d9b9b0bbf35d3ac66f85cb1b06a38fbee963ba19716eb8f143c89dd8267e1d9805e577715d90a80a00ea430f0e6' }, manifestGetterName: 'getManifestForProbeZcfBundleCap', vatAdminSvc: Promise [Promise] {} }
2024-04-16T16:33:41.446Z SwingSet: vat: v1: execute { manifestGetterName: 'getManifestForProbeZcfBundleCap', bundleExports: [ 'getManifestForProbeZcfBundleCap', 'probeZcfBundleCap' ] }
2024-04-16T16:33:41.447Z SwingSet: vat: v1: coreProposal: probeZcfBundleCap
block produced
Waiting for proposal 15 to pass (status=PROPOSAL_STATUS_VOTING_PERIOD)
1
2024-04-16T16:33:43.464Z SwingSet: xsnap: v43: error during vat dispatch() of startVat,[object Object] buildRootObject unresolved
2024-04-16T16:33:43.481Z SwingSet: kernel: WARNING: vat v43 failed to upgrade from incarnation 2 (startVat)
2024-04-16T16:33:43.485Z block-manager: block 1140 commit
2024-04-16T16:33:43.546Z block-manager: block 1141 begin
2024-04-16T16:33:43.549Z SwingSet: ls: v2: Logging sent error stack (RemoteError#1)
2024-04-16T16:33:43.549Z SwingSet: ls: v2: RemoteError#1: vat-upgrade failure
2024-04-16T16:33:43.549Z SwingSet: ls: v2: Error: vat-upgrade failure
 at construct ()
 at Error (/bundled-source/.../node_modules/ses/src/error/tame-error-constructor.js:56)
 at makeError (/bundled-source/.../node_modules/ses/src/error/assert.js:243)
 at decodeErrorCommon (/bundled-source/.../node_modules/@endo/marshal/src/marshal.js:281)
 at decodeFromSmallcaps (/bundled-source/.../node_modules/@endo/marshal/src/encodeToSmallcaps.js:434)
 at map ()
 at map ()
 at fromCapData (/bundled-source/.../node_modules/@endo/marshal/src/marshal.js:356)
 at deliver (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1084)
 at dispatchToUserspace (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1509)
 at runWithoutMetering (/bundled-source/.../packages/swingset-xsnap-supervisor/lib/supervisor-subprocess-xsnap.js:60)
 at ()

2024-04-16T16:33:43.550Z SwingSet: ls: v2: RemoteError#1 ERROR_NOTE: Sent as error:liveSlots:v2#70001
2024-04-16T16:33:43.551Z SwingSet: ls: v9: Logging sent error stack (RemoteError(error:liveSlots:v2#70001)#1)
2024-04-16T16:33:43.551Z SwingSet: ls: v9: RemoteError(error:liveSlots:v2#70001)#1: vat-upgrade failure
2024-04-16T16:33:43.552Z SwingSet: ls: v9: Error: vat-upgrade failure
 at construct ()
 at Error (/bundled-source/.../node_modules/ses/src/error/tame-error-constructor.js:56)
 at makeError (/bundled-source/.../node_modules/ses/src/error/assert.js:243)
 at decodeErrorCommon (/bundled-source/.../node_modules/@endo/marshal/src/marshal.js:281)
 at decodeFromSmallcaps (/bundled-source/.../node_modules/@endo/marshal/src/encodeToSmallcaps.js:434)
 at fromCapData (/bundled-source/.../node_modules/@endo/marshal/src/marshal.js:356)
 at notifyOnePromise (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1225)
 at notify (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1249)
 at dispatchToUserspace (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1514)
 at runWithoutMetering (/bundled-source/.../packages/swingset-xsnap-supervisor/lib/supervisor-subprocess-xsnap.js:60)
 at ()

2024-04-16T16:33:43.552Z SwingSet: ls: v9: RemoteError(error:liveSlots:v2#70001)#1 ERROR_NOTE: Sent as error:liveSlots:v9#70001
2024-04-16T16:33:43.554Z SwingSet: vat: v1: CORE_EVAL failed: (RemoteError(error:liveSlots:v9#70001)#3)
2024-04-16T16:33:43.554Z SwingSet: vat: v1: RemoteError(error:liveSlots:v9#70001)#3: vat-upgrade failure
2024-04-16T16:33:43.554Z SwingSet: vat: v1: Error: vat-upgrade failure
 at construct ()
 at Error (/bundled-source/.../node_modules/ses/src/error/tame-error-constructor.js:56)
 at makeError (/bundled-source/.../node_modules/ses/src/error/assert.js:243)
 at decodeErrorCommon (/bundled-source/.../node_modules/@endo/marshal/src/marshal.js:281)
 at decodeFromSmallcaps (/bundled-source/.../node_modules/@endo/marshal/src/encodeToSmallcaps.js:434)
 at fromCapData (/bundled-source/.../node_modules/@endo/marshal/src/marshal.js:356)
 at notifyOnePromise (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1225)
 at notify (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1249)
 at dispatchToUserspace (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1514)
 at runWithoutMetering (/bundled-source/.../packages/swingset-xsnap-supervisor/lib/supervisor-subprocess-xsnap.js:60)
 at ()

2024-04-16T16:33:43.555Z SwingSet: xsnap: v1: RemoteError(error:liveSlots:v9#70001)#3 ERROR_NOTE: Sent as error:liveSlots:v1#70001
2024-04-16T16:33:43.555Z SwingSet: ls: v1: Logging sent error stack (RemoteError(error:liveSlots:v9#70001)#3)
2024-04-16T16:33:43.556Z SwingSet: xsnap: v10: UnhandledPromiseRejectionWarning: (RemoteError(error:liveSlots:v1#70001)#4)
2024-04-16T16:33:43.556Z SwingSet: xsnap: v10: RemoteError(error:liveSlots:v1#70001)#4: vat-upgrade failure
2024-04-16T16:33:43.557Z SwingSet: xsnap: v10: Error: vat-upgrade failure
 at construct ()
 at Error (/bundled-source/.../node_modules/ses/src/error/tame-error-constructor.js:56)
 at makeError (/bundled-source/.../node_modules/ses/src/error/assert.js:243)
 at decodeErrorCommon (/bundled-source/.../node_modules/@endo/marshal/src/marshal.js:281)
 at decodeFromSmallcaps (/bundled-source/.../node_modules/@endo/marshal/src/encodeToSmallcaps.js:434)
 at fromCapData (/bundled-source/.../node_modules/@endo/marshal/src/marshal.js:356)
 at notifyOnePromise (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1225)
 at notify (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1249)
 at dispatchToUserspace (/bundled-source/.../packages/swingset-liveslots/src/liveslots.js:1514)
 at runWithoutMetering (/bundled-source/.../packages/swingset-xsnap-supervisor/lib/supervisor-subprocess-xsnap.js:60)
 at ()

I'm investigating...

@mhofman mhofman marked this pull request as ready for review April 18, 2024 18:04
@mhofman mhofman requested a review from gibson042 April 18, 2024 18:04
@mhofman mhofman dismissed their stale review April 18, 2024 18:05

authored changes addressing request

Copy link
Member Author

@turadg turadg left a comment

Choose a reason for hiding this comment

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

I reviewed and Approve. I opened the PR so GH won't let that count. @mhofman if you approve you can merge.

@@ -1,3 +1,5 @@
# syntax=docker/dockerfile:1.4
Copy link
Member Author

Choose a reason for hiding this comment

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

what happens without this?

Copy link
Member

Choose a reason for hiding this comment

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

My docker engine won't grok --link directives without it.

Copy link
Member

Choose a reason for hiding this comment

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

@@ -48,7 +48,9 @@ export const probeZcfBundleCap = async (
await E(adminNode).upgrade(zoeBundleCap, {});

// STEP 4: restart WF ////////////////////////
await E(walletAdminFacet).restartContract(privateArgs);
// Need to use `upgradeContract` instead of `restartContract`
// See https://github.com/Agoric/agoric-sdk/issues/9249
Copy link
Member Author

Choose a reason for hiding this comment

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

ahh

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.

Carrying @turadg approval of the final changes

@mhofman mhofman added the automerge:rebase Automatically rebase updates, then merge label Apr 18, 2024
gibson042 added a commit that referenced this pull request Apr 18, 2024
@mergify mergify bot merged commit d95c80d into master Apr 18, 2024
79 of 86 checks passed
@mergify mergify bot deleted the ta/use-upgrade-14 branch April 18, 2024 18:47
gibson042 added a commit that referenced this pull request Apr 18, 2024
initial.test.js copied from #9204, minus checks for "network" and
"localchain" vats (which on master are installed by core proposals).
gibson042 added a commit that referenced this pull request Apr 18, 2024
initial.test.js copied from #9204, minus vats that are missing in the
release-mainnet1B image.
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 force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

base a3p-integration on upgrade-14
4 participants