-
Notifications
You must be signed in to change notification settings - Fork 541
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
Helios sp1 integration #689
base: main
Are you sure you want to change the base?
Conversation
f421178
to
9058edb
Compare
pallets/vector/Cargo.toml
Outdated
@@ -20,6 +20,9 @@ sp-runtime = { workspace = true, default-features = false } | |||
sp-std = { workspace = true, default-features = false } | |||
sp-core = { workspace = true, default-features = false } | |||
frame-benchmarking = { workspace = true, default-features = false, optional = true } | |||
# TODO is there a release version? |
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 4.0.0-rc1
tag should be out in the next few days, which you can use. For now, I'd recommend pinning to a specific commit.
pallets/vector/Cargo.toml
Outdated
@@ -38,6 +41,7 @@ ethabi.workspace = true | |||
[dev-dependencies] | |||
pallet-balances = { workspace = true, default-features = false, features = ["std"] } | |||
pallet-timestamp = { workspace = true, default-features = false } | |||
sp1-sdk = { git = "https://github.com/succinctlabs/sp1", branch = "dev", default-features = false} |
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.
Same here.
@@ -164,13 +164,16 @@ pub const ROTATE_VK: &str = r#"{"vk_json":{ | |||
] | |||
}}"#; | |||
|
|||
const SP1_VERIFICATION_KEY: [u8; 32] = |
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.
Add a description for how this is generated.
); | ||
ensure!(is_valid.is_ok(), Error::<T>::VerificationFailed); | ||
|
||
Head::<T>::set(new_head); |
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.
This seems correct.
pallets/vector/src/mock.rs
Outdated
@@ -232,9 +232,9 @@ pub fn new_test_ext() -> sp_io::TestExternalities { | |||
.unwrap(); | |||
|
|||
vector_bridge::GenesisConfig::<Test> { | |||
finality_threshold: 461, | |||
// finality_threshold: 461, |
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.
nit: These should be deleted, rather than commented.
pallets/vector/src/tests.rs
Outdated
}); | ||
} | ||
|
||
// #[test] |
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.
delete empty test
c23909b
to
22421bb
Compare
22421bb
to
e5e1210
Compare
fd13707
to
c898edb
Compare
2be1820
to
caa4878
Compare
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.
LGTM with overall looking since i don't have all the context.
2 libs still need real version instead of revisions.
Are benchmarks done in the reference machine ?
Correct, it needs running benchmarks on the referent machine and re-running build API once the review changes are done on the pallet. |
19704bb
to
c177bc5
Compare
Pull Request type
Helios sp1 integration.
https://github.com/succinctlabs/sp1-helios/blob/main/contracts/src/SP1LightClient.sol
Please add the labels corresponding to the type of changes your PR introduces:
Description
Related Issues
Testing Performed
Checklist
cargo test
.cargo fmt
.cargo build --release
andcargo build --release --features runtime-benchmarks
.cargo clippy
.