Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 206 forks source link

Support Node 18 #6489

Closed mhofman closed 1 year ago

mhofman commented 2 years ago

What is the Problem Being Solved?

Node 18 is reaching LTS imminently. We should make sure everything works when using node 18.

Description of the Design

Setup a dev env with node 18 and see what breaks Update CI to also run against node 18.

Security Considerations

None

Test Plan

Yes

sigv commented 1 year ago

When trying to run the pismoA tag with Node.js 18.x we are observing an initialization error:

launch-chain: Launching SwingSet kernel
portHandler threw (Error#1)
Error#1: You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ 'lib/mappings.wasm': ... }) before using SourceMapConsumer
  at async Promise.all (index 0)
  at async Promise.all (index 6)
  at async allValues (packages/SwingSet/src/controller/initializeSwingset.js:29:30)
  at async buildVatAndDeviceBundles (packages/SwingSet/src/controller/initializeSwingset.js:54:19)
  at async initializeSwingset (packages/SwingSet/src/controller/initializeSwingset.js:336:25)
  at async ensureSwingsetInitialized (packages/cosmic-swingset/src/launch-chain.js:113:5)
  at async buildSwingset (packages/cosmic-swingset/src/launch-chain.js:115:3)
  at async launch (packages/cosmic-swingset/src/launch-chain.js:264:52)
  at async launchAndInitializeSwingSet (packages/cosmic-swingset/src/chain-main.js:412:15)
  at async toSwingSet (packages/cosmic-swingset/src/chain-main.js:461:22)
  at async portHandlers.<computed> (packages/cosmic-swingset/src/chain-main.js:159:16)
Cannot initialize Controller Error: You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ 'lib/mappings.wasm': ... }) before using SourceMapConsumer
Error: exit status 1

This seems to be caused by mozilla/source-map#454, fixed with source-map@0.7.4. source-map@0.7.3 is in place in repository:

$ npm ls source-map
@agoric/sdk@ /home/agoric/source/agoric-upgrade-8
└─┬ @agoric/cosmic-swingset@0.39.2 -> ./packages/cosmic-swingset
  ├─┬ @agoric/deploy-script-support@0.9.4 -> ./packages/deploy-script-support
  │ └─┬ @endo/bundle-source@2.3.1
  │   ├─┬ @agoric/babel-generator@7.17.6
  │   │ └── source-map@0.5.7
  │   └── source-map@0.7.3
  ├─┬ @agoric/xsnap@0.13.2 -> ./packages/xsnap
  │ └─┬ rollup-plugin-terser@5.3.1
  │   └─┬ terser@4.8.0
  │     ├─┬ source-map-support@0.5.21
  │     │ └── source-map@0.6.1 deduped
  │     └── source-map@0.6.1
  ├─┬ agoric@0.18.2 -> ./packages/agoric-cli
  │ └─┬ dd-trace@3.3.0
  │   └─┬ @datadog/pprof@1.0.2
  │     └── source-map@0.7.3
  └─┬ c8@7.11.0
    └─┬ v8-to-istanbul@8.1.1
      └── source-map@0.7.3
mhofman commented 1 year ago

Thanks for the report. We had an internal report of a similar error. For now we recommend you use Node 16 LTS with pismoA. We may consider an intermediary patch release if we can guarantee the update is compatible with the current release.

mhofman commented 1 year ago

@sigv according to the upgrade instructions, please use Node 16. We will not be supporting Node 18 for pismoA.

sigv commented 1 year ago

@sigv according to the upgrade instructions, please use Node 16. We will not be supporting Node 18 for pismoA.

Understandable. Looking forward to having Current LTS supported on the next version then 🤞🏻

dckc commented 1 year ago

@erights reports some symptoms when trying v18.12.1:

yarn test test/swingsetTests/makeKind/test-makeKind.js
yarn run v1.22.19
$ ava --verbose test/swingsetTests/makeKind/test-makeKind.js

  ✘ [fail]: before hook Rejected promise returned by test
  ✘ 1 test remaining in test/swingsetTests/makeKind/test-makeKind.js
  ─

  before hook

  Rejected promise returned by test. Reason:

  Error {
    message: 'You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ \'lib/mappings.wasm\': ... }) before using SourceMapConsumer',
  }

  › async allValues (packages/SwingSet/src/controller/initializeSwingset.js:28:30)
  › async buildVatAndDeviceBundles (packages/SwingSet/src/controller/initializeSwingset.js:53:19)
  › async buildKernelBundles (packages/SwingSet/src/controller/initializeSwingset.js:83:37)
  › async packages/zoe/test/swingsetTests/makeKind/test-makeKind.js:21:25

  ─

  1 hook failed
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
erights commented 1 year ago

Yeah, looks like the same problem @sigv reported above at https://github.com/Agoric/agoric-sdk/issues/6489#issuecomment-1295922938

erights commented 1 year ago

Curiously, in the experiment at https://github.com/Agoric/agoric-sdk/pull/6549 , when I locally run the same

yarn test test/swingsetTests/makeKind/test-makeKind.js

under Node v18, it works.

sigv commented 1 year ago

With #6560 (source-map@0.7.4) the mentioned test case from packages/zoe succeeds as well.

yarn run v1.22.19
$ ava --verbose test/swingsetTests/makeKind/test-makeKind.js

bundling: 6.556s kernel, 0.171s contracts, 1.731s vats, 8.458s total
  ✔ defineKind swingset (4.3s)
  ─

  1 test passed
Done in 14.00s.
erights commented 1 year ago

@mhofman Now that https://github.com/Agoric/agoric-sdk/pull/6560 has been merged, can we close this issue?

mhofman commented 1 year ago

We need to add CI tests to ensure continuous Node 18 compatibility (and maybe a round of manual end to end testing on node 18)

sigv commented 1 year ago

I understand this is of fairly low priority to the team, so to keep the ball moving #6599 is available with the test suite enabling Node.js 18.x. Further manual testing can be conducted at a later time as well.

sigv commented 1 year ago

@mhofman, do you prefer to update packages/deployment/Dockerfile.sdk to also build with node:18-bullseye instead of current node:16-buster?

That should be all the Node.js framework changes for pipelines/CI (until Node.js 14 is dropped off in roughly 5 months). Manual testing can be carried out to ensure compatibility, as mentioned in previous commit.

mhofman commented 1 year ago

I think we should keep the recommended / default to 16 right now. This IPv6 default in 18 has me worried about subtle breakage, especially with Docker.

sigv commented 1 year ago

Makes sense. Then it's just a question of running manual testing before deciding to officially claim Node.js 18 as compatible. Again - thank you! 🤞🏻

dckc commented 1 year ago

fixed in #7073 / 711059c9e8a152e545a4b12b45821e803fc6623a