-
Notifications
You must be signed in to change notification settings - Fork 104
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
Eth2 Light Client #582
Eth2 Light Client #582
Conversation
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.
Added some minor tips & advice :)
Co-authored-by: Vincent Geddes <vincent@snowfork.com>
/// Best available header to switch finalized head to if we see nothing else | ||
#[pallet::storage] | ||
pub(super) type BestValidUpdate<T: Config> = | ||
StorageValue<_, Option<LightClientUpdate>, ValueQuery>; // TODO: Maybe I should use OptionQuery here instead of Option? |
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.
Yeah, OptionQuery
would work better here to simplify later queries.
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'll change this when I call the light from the relayer, if that's OK? I'd like to see how it makes a difference, practically, by calling the light client from the relayer with ValueQuery
first and then see how OptionQuery
simplifies it.
@vgeddes @alistair-singh @musnit I think this is ready for review, even if there are still some unfinished parts. I would like to open smaller PRs from here on out. |
eth2_ssz_types = { version = "0.2.1" } | ||
tree_hash = { version = "0.4.0" } | ||
tree_hash_derive = { version = "0.4.0" } | ||
milagro_bls = { git = "https://github.com/sigp/milagro_bls", tag = "v1.4.2" } |
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 lib has an std feature, which we should not enable by default.
milagro_bls = { git = "https://github.com/sigp/milagro_bls", tag = "v1.4.2" } | |
milagro_bls = { git = "https://github.com/sigp/milagro_bls", tag = "v1.4.2", default-features = false } |
"sp-std/std", | ||
"snowbridge-core/std", | ||
"snowbridge-ethereum/std", | ||
] |
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.
Need to add the std feature of milagro_bls here. So that when we want to compile the code to native machine code (not WASM) we can enable std feature and allow linking to the core std library.
codec = { version = "2.2.0", package = "parity-scale-codec", default-features = false, features = ["derive"] } | ||
scale-info = { version = "1.0", default-features = false, features = ["derive"] } | ||
hex = { package = "rustc-hex", version = "2.1.0", default-features = false } | ||
hex-literal = { version = "0.3.1", optional = 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.
hex_literal is only being used in test modules, so you remove it from this section (It doesn't support no_std so will cause problems otherwise). I see it is already listed under dev-dependencies, so that's good.
serde = { version = "1.0.130", optional = true } | ||
codec = { version = "2.2.0", package = "parity-scale-codec", default-features = false, features = ["derive"] } | ||
scale-info = { version = "1.0", default-features = false, features = ["derive"] } | ||
hex = { package = "rustc-hex", version = "2.1.0", 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.
This dependency seems to be unused.
name = "snowbridge-ethereum-beacon-light-client" | ||
description = "Snowbridge Beacon Light Client Pallet" | ||
version = "0.0.1" | ||
edition = "2021" |
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 reason why the build is still failing after adding edition 2021 is because the github workflow doing the build is using an older version of the rust toolchain.
This commit is the version I used for edition 2021 in the v0.9.17
PR that is not yet merged in.
7167fdc
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.
Thanks Alistair! 👍 So the edition 2021 should work after the rust toolchain upgrade? I've started updating the other crate's editions too: https://github.com/Snowfork/snowbridge/pull/605/files
Dependent on #591 to update Rust toolchain for the Cargo edition |
Closing in favour of #610. |
Starts adding the Eth2 light client scaffolding in a new pallet.