chore: move acir docs to code declaration #5040
Conversation
…rom the readme are already removed and some are missing)
|
Can give this a proper look tomorrow |
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Apologies for the delay will look today |
| /// Calculates the SHA256 hash of the inputs. | ||
|
|
||
| /// Computes SHA256 of the inputs | ||
| /// - inputs are a byte array, i.e a vector of (FieldElement, 8) |
There was a problem hiding this comment.
| /// - inputs are a byte array, i.e a vector of (FieldElement, 8) | |
| /// - inputs are a byte array, i.e a vector of (witness, 8) |
Any reason we decided to say FieldElement rather than witness here? Same for other places where we say FieldElement
There was a problem hiding this comment.
I'm not sure, this was copy/pasted to match existing examples
There was a problem hiding this comment.
Could you link to where from?
There was a problem hiding this comment.
Ah from the original docs
| /// uses 0 as domain separator. | ||
| /// | ||
| /// In Barretenberg, PedersenHash is doing the same as PedersenCommitment, | ||
| /// except that it prepends the inputs with their length. |
There was a problem hiding this comment.
You could state how a commitment is expected to be additively homomorphic above in the PedersenCommitment comments. Then here you can say this is also how a pedersen hash differs
| /// scalars (FieldElement, N) a vector of low and high limbs of input | ||
| /// scalars `[s1_low, s1_high, s2_low, s2_high, ...]`. (FieldElement, N) | ||
| /// For Barretenberg, they must both be less than 128 bits. | ||
| /// - output: (FieldElement, N) a vector of x and y coordinates of output |
There was a problem hiding this comment.
I believe we just return a tuple of two witnesses here representing x and y
There was a problem hiding this comment.
An MSM multiplies the points and scalars and sums the results
| /// field of the main one, and vice-versa. The curves used by the proving | ||
| /// system are dependent on the proving system (and/or its configuration). |
There was a problem hiding this comment.
| /// field of the main one, and vice-versa. The curves used by the proving | |
| /// system are dependent on the proving system (and/or its configuration). | |
| /// field of the main one, and vice-versa. |
This feels slightly redundant no? Especially as we already state "Calls to "gadgets" which rely on backends implementing support" above
| /// ACIR arrays all have a known fixed length (given in the MemoryInit | ||
| /// opcode below) | ||
| /// | ||
| /// - predicate: an arithmetic expression that disable the opcode when it is null. |
There was a problem hiding this comment.
| /// - predicate: an arithmetic expression that disable the opcode when it is null. | |
| /// - predicate: an arithmetic expression that disables execution of the opcode |
Wdym when it is null?
There was a problem hiding this comment.
I'd say "evaluates to zero" rather than "is null"
There was a problem hiding this comment.
Also "it" is a bit ambiguous here as it can be read as the predicate or the opcode.
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Description
Problem*
Resolves a few minor inaccuracies in the ACIR docs and moves their primary location to Rust.
Summary*
ACIR docs get stale quickly when detached from code/tests. This PR moves them to the code for now; we expect to move them back once ACIR is more stable.
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.