-
Notifications
You must be signed in to change notification settings - Fork 8
[DO NOT MERGE] L3 Validium #164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
SecretKey::try_from_bytes(input.secret_key) | ||
.expect("validium key") | ||
.public_key() | ||
.to_bytes(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd leave a comment saying that the validium protocol spec requires this to be a 33-byte compressed public key.
daccae0
to
66d1f4e
Compare
replace the latest incorrect commits by force-pushing |
* fix issues in e2e test with coordinator * add debug fields * update commits * Soundness issues in Validium (#196) * fix: soundness issues in validium mode * chore: rename method validium pi_hash * chore: rename another pi_hash validium * update commit --------- Co-authored-by: Zhuo Zhang <[email protected]> Co-authored-by: Rohit Narurkar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just note: no actual change here except for the formatting
}; | ||
|
||
// println!("chunk_info = {}", chunk_info); | ||
println!("chunk_info = {}", chunk_info); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we disable this debugging log?
#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] | ||
pub struct BundleWitness { | ||
/// The version byte as per [version][types_base::version]. | ||
pub version: u8, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A note to scroll monorepo side: version
is added and still not reflected in libzkp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean?
test-execute-chunk-multi: | ||
@cargo test $(CARGO_CONFIG_FLAG) --release -p scroll-zkvm-integration --test chunk_circuit test_execute_multi -- --exact --nocapture | ||
|
||
test-execute-validium-chunk: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new test has not passed yet, may be caused by the out-dated testing samples.
Also, there is no tips for how to obtain VALIDIUM_KEY
. Should we resolve them before merging?
<batch::PayloadV8 as Payload>::from_envelope(&enveloped).validate(h, infos); | ||
} | ||
ReferenceHeader::Validium(_h) => { | ||
todo!() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since validium never handling payload data, should here be unreachable
?
No description provided.