Promote frontend and harden browser, onion, FFI, and lint checks#661
Conversation
RyanKung
changed the title
RyanKung
changed the title
dcecc39 to
d0b6ffb
Compare
![rings-auto-reviewer[bot]](https://avatars.githubusercontent.com/u/95871563?s=60&v=4){kind=small}
| } | ||
| let url = format!("https://{}{}", authority, normalize_path(&request.path)?); | ||
| let response = browser_fetch(&url, request, body_limit).await?; | ||
| let response = execute_https_request(&url, request, body_limit).await?; |
There was a problem hiding this comment.
[P1] Bound the exit-side HTTPS fetch lifetime
execute_exit_fetch awaits execute_https_request without an exit-side deadline or abort. The caller's 30-second timeout only removes the client-side pending request; it cannot cancel this task after the frame reaches an exit. In browser mode, fetch can remain pending indefinitely (and the native client has no explicit request timeout), so the _lease at line 564 is retained indefinitely. Repeated stalled targets can therefore exhaust the advertised max_circuits/max_streams limits, or grow without bound when those limits are 0. Please add a bounded timeout with cancellation/AbortController at the exit and cover the timeout path with a regression test.
| } | ||
|
|
||
| /// Create a new instance after validating the R1CS and witness shape. | ||
| pub fn try_new(r1cs: Arc<R1CS<F>>, witness: Vec<F>) -> Result<Self> { |
There was a problem hiding this comment.
[P2] Do not leave an unchecked public Circuit constructor
try_new now validates the R1CS and witness shape, but the public Circuit::new still constructs the same type without validation. That lets callers create an invalid state and reach StepCircuit::arity, which performs self.r1cs.num_inputs - 1 without a fallible check; malformed public R1CS data can therefore panic in debug builds or produce a bogus arity in release builds. Make the unchecked constructor private/remove it, or make every public construction path return the validation error, and add a regression test for malformed R1CS input.
| } | ||
| let url = format!("https://{}{}", authority, normalize_path(&request.path)?); | ||
| let response = browser_fetch(&url, request, body_limit).await?; | ||
| let response = execute_https_request(&url, request, body_limit).await?; |
There was a problem hiding this comment.
[P2] Reserve the response budget before reading the target body
The remaining byte budget is sampled at line 566, but the response is charged only after execute_https_request has read the complete body at line 572. With a chunked response, every concurrent request can observe the same remaining budget and download that many bytes into its own Vec; only after the target-side work is complete does record_exit_bytes reject all but the first successful charge. This makes max_bytes_per_minute ineffective as a bound on exit-side work and differs from the TCP path, which charges each chunk before forwarding it. Reserve the response budget atomically or charge chunks through the shared accounting gate, and add a concurrent streaming-response regression test.
Fixed in a2fcd44. Native and browser HTTPS exits now have bounded cancellation, response bytes are accounted per chunk, and the strict Rustacean validation suite passes.
There was a problem hiding this comment.
Re-review of a2fcd44: no new actionable findings. The previous blockers are closed: native HTTPS exits have a bounded reqwest request timeout, browser exits pass an AbortController signal and abort on timeout, response bytes are charged per chunk through the shared accounting gate, and Circuit construction/deserialization validates the R1CS and witness shape before StepCircuit access. The current QACI and CodeQL checks are green.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
1 participant
Summary
examples/frontendinto the top-levelfrontendapp, adding Home/Node surfaces, extension-aware navigation, landing assets, and modular Rust-generated theme/style code.Why
The browser frontend is no longer only an example: it now carries the project landing page, node workbench, and extension UX. That makes the browser/native proxy abstraction, extension packaging path, and repo lint policy part of the main product surface. This PR brings those paths under stricter checks while keeping the native node, browser node, and FFI wrappers aligned around the same runtime behavior.
Validation
git diff --checknpm run build:frontend-extension-scriptsuv run --with web3 --with cffi --with pytest pytest examples/ffi/tests -qcargo check -p rings-core --all-targetscargo check -p rings-core --target wasm32-unknown-unknown --no-default-features --features wasmcargo check -p rings-core --no-default-features --features dummycargo check -p rings-transport --all-targetscargo check -p rings-transport --target wasm32-unknown-unknown --no-default-features --features web-sys-webrtccargo check -p rings-transport --no-default-features --features dummycargo check -p rings-derive -p rings-rpc -p rings-snark -p rings-nodecargo check -p rings-node --all-targets