fix: TS bindings audit response (barretenberg#2426)#22362
Open
johnathan79717 wants to merge 5 commits intomerge-train/barretenbergfrom
Open
fix: TS bindings audit response (barretenberg#2426)#22362johnathan79717 wants to merge 5 commits intomerge-train/barretenbergfrom
johnathan79717 wants to merge 5 commits intomerge-train/barretenbergfrom
Conversation
…t checks streamG1Data/streamG2Data used `response.body!` which would crash without a useful error if the response body was null. Add explicit null checks with descriptive error messages. Addresses I2 from AztecProtocol/barretenberg#2426.
The async shared memory backend hardcoded HARDWARE_CONCURRENCY to 16 when no thread count was specified. Use os.cpus().length (capped at 16) to match the socket backend behavior. Addresses I1 from AztecProtocol/barretenberg#2426.
…roy() The sync shared memory backend sent SIGTERM but never confirmed the process actually exited. Add a 1-second timeout that escalates to SIGKILL as a defensive measure. The timeout is unref'd so it won't prevent Node from exiting. Addresses F4/F5 from AztecProtocol/barretenberg#2426.
The async shared memory backend's destroy() killed the process without rejecting pending promise callbacks, causing them to hang forever. Also, if the process died unexpectedly, pending callbacks were never notified. Add a rejectPendingCallbacks() helper (matching the socket backend pattern) and call it from both destroy() and the process exit handler. Addresses F2/F3 from AztecProtocol/barretenberg#2426.
johnathan79717
force-pushed
the
jh/ts-bindings-audit-response
branch
from
3cfbd1b to
0b0f96f
Compare
The G2 element is a fixed 128-byte constant from the trusted setup. Rather than downloading and verifying it, hardcode it directly (matching the C++ side which already uses get_bn254_g2_crs_element()). This eliminates the G2 download, disk cache, and IndexedDB cache entirely. For G1, add spot-check verification of the first two compressed elements against known values from bn254_crs_data.hpp, after download and after streaming to disk. Addresses F1 from AztecProtocol/barretenberg#2426.
johnathan79717
force-pushed
the
jh/ts-bindings-audit-response
branch
from
0b0f96f to
068dc49
Compare
ludamad
reviewed
Apr 7, 2026
| return true; | ||
| } | ||
|
|
||
| /** |
Collaborator
There was a problem hiding this comment.
Do we want to do this when we also have HTTPS? But also It seems a little weird to have this logic outside of the BB API core call for srs init where we pass this data.
Collaborator
|
Looks overall okay, mainly the one comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses all 7 findings from the TypeScript bindings audit AztecProtocol/barretenberg#2426: 1 medium, 4 low, 2 informational.
get_bn254_g2_crs_element()), eliminating the G2 download/cache entirely. For G1, add spot-check verification of the first two compressed elements against known trusted setup values.os.cpus().length(capped at 16) for HARDWARE_CONCURRENCY default instead of hardcoded 16.response.body!non-null assertions with explicit null checks and descriptive errors.Test plan
yarn build:esmcompiles with no new errorsnpx jest net_crspasses all 6 tests (G1 spot-check, hardcoded G2, integrity rejection)