feat: Parsing non-string assertion payloads in noir js#6079
Conversation
Benchmark resultsNo metrics with a significant change found. Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 16 txs.
Circuits statsStats on running time and I/O sizes collected for every circuit run across all benchmarks.
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method
Transaction processing duration by data writes.
|
| } | ||
| } | ||
|
|
||
| impl Serialize for ErrorSelector { |
There was a problem hiding this comment.
It's serialized as string since the Serde<>JsValue bridge that we use serializes u64s as JS numbers, which don't have enough precision to store u64s.
There was a problem hiding this comment.
We could also just move to u32 right? I don't foresee error selectors hitting that bound. But this is non-blocking
| } catch (error) { | ||
| const knownError = error as Error; | ||
| expect(knownError.message).to.equal('Circuit execution failed: Error: Cannot satisfy constraint'); | ||
| expect(knownError.message).to.equal('Circuit execution failed: Expected x < y but got 10 < 5'); |
There was a problem hiding this comment.
It succesfully decodes the format string now
vezenovm
left a comment
There was a problem hiding this comment.
I'm good with these changes but want to let @TomAFrench also take a look
| } | ||
| } | ||
|
|
||
| impl Serialize for ErrorSelector { |
There was a problem hiding this comment.
We could also just move to u32 right? I don't foresee error selectors hitting that bound. But this is non-blocking
|
I can take a look at this tomorrow. |
TomAFrench
left a comment
There was a problem hiding this comment.
Don't really have time to look at this properly so just going to defer to Maxim's review on this.
|
Build is failing however. |
Not sure, the FxHasher does weird things. If we switch the computation of the selector to some random-oracle hasher like sha256 I'd be more confident with moving to u32. Maybe we can do that, hash the types with sha256 and move to u32! |
This PR adds a noirc_abi_wasm entrypoint for decoding ABI error types. This entrypoint is used by noir_js when an execution error is detected to try to decode the error using the ABI.
If the result from decoding the error is a string (such as a fmt string assertion, that previously weren't decoded at all) it'll patch the error message to include the formatted string.
If the result from decoding the error is a non-string payload (structs, tuples, arrays, etc) it'll add the decodedPayload property to the error so users can get the data back from the circuit.
Also, this PR refactors ErrorSelector so it's a shared struct, instead of being casted to u64 outside of the compiler backend.