fix(bb.js): (breaking change) bundles bb.js properly so that it works in the browser and in node #1855

charlielye · 2023-08-29T09:57:29Z

Reverts prior work and builds upon original webpack.
Bundles workers and wasms into the output bundle. ~12MB but compresses with e.g. brotli to about 1.5MB (consumers responsbility). This required a fair bit of refactoring of the bb_wasm ts code into base, main and thread to avoid what webpack saw as circular dependency.
Manually testing and working with consumers, zero config webpack, vite, next.js, hardhat.
Automatically tests:
- bb: all acir test vectors through prove_and_verify
- bb and bb.js: all command line interface commands.
- bb.js: a single test through single and multithreaded on chrome and webkit.
bootstrap_docker.sh can be used to build and run these tests as per CCI.

…ol/barretenberg#544)" This reverts commit 9cf96aa.

circuits/cpp/barretenberg/acir_tests/README.md

kevaundray · 2023-08-29T10:24:03Z

circuits/cpp/barretenberg/ts-example-dapp/src/index.ts

+  }
+  return bytes;
+}
+


can you add a comment to say where this example came from?

circuits/cpp/barretenberg/ts-example-dapp/webpack.config.js

kevaundray · 2023-08-29T10:26:10Z

circuits/cpp/barretenberg/ts-headless-acir-test/src/index.ts

+  const colors = parts[parts.length - 1].split(" color: ");
+  parts[parts.length - 1] = colors.shift()!;
+
+  // console.log({ message, parts, colors });


Suggested change

// console.log({ message, parts, colors });

circuits/cpp/barretenberg/ts-headless-acir-test/src/index.ts

circuits/cpp/barretenberg/ts/package.json

kevaundray · 2023-08-29T11:08:08Z

circuits/cpp/barretenberg/ts/src/barretenberg_wasm/barretenberg_wasm_base/index.ts

+          // this.logger(`get_data hit ${key} size: ${data.length} dest: ${outBufAddr}`);
+          // this.logger(Buffer.from(data.slice(0, 64)).toString('hex'));


Suggested change

// this.logger(`get_data hit ${key} size: ${data.length} dest: ${outBufAddr}`);

// this.logger(Buffer.from(data.slice(0, 64)).toString('hex'));

circuits/cpp/barretenberg/ts/src/barretenberg_wasm/barretenberg_wasm_base/index.ts

kevaundray · 2023-08-29T11:09:34Z

circuits/cpp/barretenberg/ts/src/barretenberg_wasm/index.ts

+   * Single threaded (doesn't spawn any child thread workers).
+   * Best used for gaining access to short lived functions like crypto hashes etc.
+   */
+  // public static async new() {


Is this needed?

kevaundray · 2023-08-29T11:10:04Z

circuits/cpp/barretenberg/ts/webpack.config.js

+export default {
+  target: 'web',
+  mode: 'production',
+  // devtool: false,


Suggested change

// devtool: false,

kevaundray · 2023-08-29T11:12:23Z

I have not used ! to specify a breaking change as I think the release-please on aztec-packages cannot handle that right now.

Can you specify what the breaking changes are?

kobyhallx · 2023-08-30T13:56:25Z

circuits/cpp/barretenberg/ts/cjs-entry/index.cjs

+  return loadedModule;
+}
+
+module.exports = { loadModule };


This approach will cause inconsistent API surface.
Once would expect to require from package in the same way between cjs and esm.

import { Barretenberg, RawBuffer, Crs } from "@aztec/bb.js"

vs

const { loadModule } = await import("@aztec/bb.js"); const { Barretenberg, RawBuffer, Crs } = await loadModule();

That's right.
It maybe possible to "degrade" the esm experience to the requirement of having to call loadModule, but we shouldn't do this to support an old system for the sake of api consistency here.
If there is a quick and clean (e.g. 1 day of effort) solution then please do as separate PR. Otherwise the effort and ongoing build complexity our engineers are exposed to isn't justified.
Just a reminder, browsers and node are ESM compatible. The requirement for CJS support is to support a handful of frameworks that have not yet upgraded to ESM. This need for 'loadModule' is a temporary stop-gap for users that are still using these lagging frameworks.
I think this demonstrates our values of "good enough" and not investing further time in something for the sake of perfection.
The README makes the esm vs cjs import approach clear at least.

…tiy. We have acir_tests still separate.

…ling

This reverts commit 17357db.

🤖 I have created a new Aztec Packages release --- ## [0.1.0-alpha50](v0.1.0-alpha49...v0.1.0-alpha50) (2023-09-05) ### ⚠ BREAKING CHANGES * update to acvm 0.24.0 ([#1925](#1925)) ### Features * **892:** add hints for matching transient read requests with correspondi… ([#1995](#1995)) ([0955bb7](0955bb7)) * Add support for assert messages & runtime call stacks ([#1997](#1997)) ([ac68837](ac68837)) * **Aztec.nr:** Kernel return types abstraction ([#1924](#1924)) ([3a8e702](3a8e702)) * **ci:** use content hash in build system, restrict docs build to *.ts or *.cpp ([#1953](#1953)) ([0036e07](0036e07)) * do not allow slot 0 in `noir-libs` ([#1884](#1884)) ([54094b4](54094b4)), closes [#1692](#1692) * throwing when submitting a duplicate tx of a settled one ([#1880](#1880)) ([9ad768f](9ad768f)), closes [#1810](#1810) * typos, using Tx.clone functionality, better naming ([#1976](#1976)) ([00bca67](00bca67)) ### Bug Fixes * add retry_10 around ensure_repo ([#1963](#1963)) ([0afde39](0afde39)) * Adds Mac cross compile flags into barretenberg ([#1954](#1954)) ([3aaf91e](3aaf91e)) * bb meta-data ([#1960](#1960)) ([712e0a0](712e0a0)) * **bb.js:** (breaking change) bundles bb.js properly so that it works in the browser and in node ([#1855](#1855)) ([1aa6f59](1aa6f59)) * Benchmark preset uses clang16 ([#1902](#1902)) ([4f7eeea](4f7eeea)) * build ([#1906](#1906)) ([8223be1](8223be1)) * **ci:** Incorrect content hash in some build targets ([#1973](#1973)) ([0a2a515](0a2a515)) * circuits should not link openmp with -DMULTITHREADING ([#1929](#1929)) ([cd1a685](cd1a685)) * compilation on homebrew clang 16.06 ([#1937](#1937)) ([c611582](c611582)) * docs preprocessor line numbers and errors ([#1883](#1883)) ([4e7e290](4e7e290)) * ensure CLI command doesn't fail due to missing client version ([#1895](#1895)) ([88086e4](88086e4)) * error handling in acir simulator ([#1907](#1907)) ([165008e](165008e)) * Fix off by one in circuits.js when fetching points from transcript ([#1993](#1993)) ([cec901f](cec901f)) * format.sh issues ([#1946](#1946)) ([f24814b](f24814b)) * master ([#1981](#1981)) ([6bfb053](6bfb053)) * More accurate c++ build pattern ([#1962](#1962)) ([21c2f8e](21c2f8e)) * polyfill by bundling fileURLToPath ([#1949](#1949)) ([1b2de01](1b2de01)) * Set correct version of RPC & Sandbox when deploying tagged commit ([#1914](#1914)) ([898c50d](898c50d)) * typescript lookup of aztec.js types ([#1948](#1948)) ([22901ae](22901ae)) * unify base64 interface between mac and linux (cherry-picked) ([#1968](#1968)) ([ee24b52](ee24b52)) * Update docs search config ([#1920](#1920)) ([c8764e6](c8764e6)) * update docs search keys ([#1931](#1931)) ([03b200c](03b200c)) ### Miscellaneous * **1407:** remove forwarding witnesses ([#1930](#1930)) ([cc8bc8f](cc8bc8f)), closes [#1407](#1407) * **1879:** add use of PrivateKernelPublicInputs in TS whenever relevant ([#1911](#1911)) ([8d5f548](8d5f548)) * acir tests are no longer base64 encoded ([#1854](#1854)) ([7fffd16](7fffd16)) * Add back double verify proof to test suite ([#1986](#1986)) ([f8688d7](f8688d7)) * add CLI test to canary flow ([#1918](#1918)) ([cc68958](cc68958)), closes [#1903](#1903) * Add safemath noir testing ([#1967](#1967)) ([cb1f1ec](cb1f1ec)) * **Aztec.nr:** remove implicit imports ([#1901](#1901)) ([c7d5190](c7d5190)) * **Aztec.nr:** Remove the open keyword from public functions ([#1917](#1917)) ([4db8603](4db8603)) * **ci:** build docs on every pr ([#1955](#1955)) ([c200bc5](c200bc5)) * Enable project-specific releases for dockerhub too ([#1721](#1721)) ([5d2c082](5d2c082)) * reduce max circuit size in bb binary ([#1942](#1942)) ([c61439b](c61439b)) * Reference noir master for acir tests ([#1969](#1969)) ([86b72e1](86b72e1)) * remove debug output from `run_acir_tests` script ([#1970](#1970)) ([74c83c5](74c83c5)) * storing `&mut context` in state vars ([#1926](#1926)) ([89a7a3f](89a7a3f)), closes [#1805](#1805) * sync bb master ([#1947](#1947)) ([eed58e1](eed58e1)) * update to acvm 0.24.0 ([#1925](#1925)) ([e728304](e728304)) * Update to acvm 0.24.1 ([#1978](#1978)) ([31c0a02](31c0a02)) * updating docs to clang16 ([#1875](#1875)) ([a248dae](a248dae)) ### Documentation * **keys:** Complete addresses are now broadcast ([#1975](#1975)) ([92068ad](92068ad)), closes [#1936](#1936) * limitations, privacy, roadmap ([#1759](#1759)) ([0cdb27a](0cdb27a)) * put dev docs before spec ([#1944](#1944)) ([f1b29cd](f1b29cd)) * storage and state variables ([#1725](#1725)) ([fc72f84](fc72f84)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

charlielye added 7 commits August 25, 2023 18:02

Revert "feat: Modify bb.js to be compatible with next.js (AztecProtoc…

dcca716

…ol/barretenberg#544)" This reverts commit 9cf96aa.

The work. I accidently did it on barretenberg repo originally 🤦

e7d3696

Simplify bb.js api (remove sync api). Avoids double embedding the wasms.

fcef7c1

Move bin-tests to be part of acir_tests just as another flow.

13417bf

Refactor common code out of barretenberg_wasm.

ecf872e

Headless chrome runner.

7b77197

Vite example, to be sure. Playwright to test all 3 browsers.

50d97c9