Smart contrat Recurance proof system impl #677
Conversation
…crypto into paul/feature/fri
.gitignore
Outdated
| # Soldity artifacts | ||
| crypto/stark-verifier-ethereum/artifacts | ||
| crypto/stark-verifier-ethereum/cache | ||
| crypto/stark-verifier-ethereum/typechain |
There was a problem hiding this comment.
It's better to keep this in the crypto/stark-verifier-ethereum/.gitignore. If we ever move the folders around or rename them, this would break and you wouldn't think of looking here. If you keep it in a local .gitignore everything will just work.
There was a problem hiding this comment.
Please remove these, now that they are in the local file again.
| for (uint256 i = coefficents.length - 2; i > 0; i--) { | ||
| b = fmul(b, x); | ||
| b = fadd(b, coefficents[i]); | ||
| require(coefficents.length > 0, 'Evaluation undefined'); |
There was a problem hiding this comment.
It would make sense to define this as zero. OTOH it's easy to accidentally have zero arrays, so an explicit [0] avoids accidents. (No action)
|
|
||
| // This trivial Fibonacci system returns constant values which are true only for one proof | ||
| // It should only be used for testing purposes | ||
| contract Recurance is ConstraintSystem { |
There was a problem hiding this comment.
- Recurrence
(and in a couple more places)
| struct Options { | ||
| // The number of occurrences of the `v/verbose` flag | ||
| /// Verbose mode (-v, -vv, -vvv, etc.) | ||
| #[allow(clippy::cast_possible_truncation)] |
There was a problem hiding this comment.
Where does this cast happen? In the argument parsing?
There was a problem hiding this comment.
This is actually something, that I don't know. I haven't touched this function at all and this error popped up. I can a todo which says to investigate
crypto/stark/src/proof.rs
Outdated
|
|
||
| #[derive(Clone, Debug, PartialEq)] | ||
| pub struct Proof(Vec<u8>); | ||
| pub struct Proof(pub Vec<u8>); |
There was a problem hiding this comment.
Member should not be pub, use as_bytes() instead. This needs to change.
|
|
||
| impl SoliditySeralize for Commitment { | ||
| fn sol_encode(&self) -> String { | ||
| format!("\"0x{}\"", encode(self.hash().as_bytes())) |
There was a problem hiding this comment.
Delegate to self.hash().sol_encode() ?
| } | ||
| } | ||
|
|
||
| // TODO - Make this function smaller |
There was a problem hiding this comment.
Yeah, the problem is that Proof has no structure. This needs to change firsts. Out of scope for this PR though.
| value: FieldElement, | ||
| exponent: usize, | ||
| index: usize, | ||
| pub(crate) value: FieldElement, |
There was a problem hiding this comment.
Hmm.. we should really use this struct outside of this module. But this probably requires a refactor of the test constraints. Out of scope for this PR.
recmo
left a comment
recmo
left a comment
There was a problem hiding this comment.
Looks great! I think you forgot to remove the patsh form /.gitignore. Otherwise good to go!
.gitignore
Outdated
| # Soldity artifacts | ||
| crypto/stark-verifier-ethereum/artifacts | ||
| crypto/stark-verifier-ethereum/cache | ||
| crypto/stark-verifier-ethereum/typechain |
There was a problem hiding this comment.
Please remove these, now that they are in the local file again.
| struct Options { | ||
| // The number of occurrences of the `v/verbose` flag | ||
| /// Verbose mode (-v, -vv, -vvv, etc.) | ||
| #[allow(clippy::cast_possible_truncation)] |
wipwhile in development.bug,enhancement.Changelog.md.Semver: https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md
https://semver.org/