-
Notifications
You must be signed in to change notification settings - Fork 142
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
chore: remove ssz_rs dependency #1671
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.
Thanks, LGTM 👍 !
crates/light-client/Cargo.toml
Outdated
trin-validation.workspace = true | ||
ethereum_ssz.workspace = 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.
can you sort these alphabetically
.expect("Unable to convert state root to bytes"), | ||
)?; | ||
let leaf_hash = leaf_object.tree_hash_root(); | ||
let state_root = bytes32_to_node(&Bytes32::from(attested_header.state_root.0.to_vec()))?; |
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.
let state_root = bytes32_to_node(&Bytes32::from(attested_header.state_root.0.to_vec()))?; | |
let state_root = B256::from_slice(&attested_header.state_root.0.to_vec()))?; |
genesis_root: Bytes32, | ||
) -> Result<Bytes32> { |
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.
genesis_root: Bytes32, | |
) -> Result<Bytes32> { | |
genesis_root: Bytes32, | |
) -> Result<B256> { |
let d = [start, end].concat(); | ||
Ok(d.to_vec().try_into()?) | ||
Ok(d.to_vec().into()) |
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.
Ok(d.to_vec().into()) | |
Ok(B256::from_slice(d)) |
) -> Result<Node> { | ||
let genesis_root = genesis_root.to_vec().try_into()?; | ||
) -> Result<B256> { | ||
let genesis_root = genesis_root.to_vec().into(); |
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.
let genesis_root = genesis_root.to_vec().into(); | |
let genesis_root = B256::from_slice(genesis_root); |
} | ||
|
||
pub fn compute_domain( | ||
domain_type: &[u8], | ||
fork_version: Vector<u8, 4>, | ||
fork_version: FixedVector<u8, U4>, | ||
genesis_root: Bytes32, |
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.
genesis_root: Bytes32, | |
genesis_root: B256, |
object_root: Bytes32, | ||
domain: Bytes32, |
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.
object_root: Bytes32, | |
domain: Bytes32, | |
object_root: B256, | |
domain: B256, |
genesis_validator_root: Bytes32, | ||
} | ||
|
||
pub fn compute_signing_root(object_root: Bytes32, domain: Bytes32) -> Result<Node> { | ||
let mut data = SigningData { | ||
pub fn compute_signing_root(object_root: Bytes32, domain: Bytes32) -> B256 { |
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.
pub fn compute_signing_root(object_root: Bytes32, domain: Bytes32) -> B256 { | |
pub fn compute_signing_root(object_root: B256, domain: B256) -> B256 { |
crates/light-client/src/types.rs
Outdated
|
||
pub type Bytes32 = Vector<u8, 32>; | ||
pub type Bytes32 = FixedVector<u8, U32>; |
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.
Might as well remove this type Bytes32
and replace all with B256
@KolbyML, thanks for the feedback, I made the changes you suggested! |
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.
looks good after the 2 last problems are fixed I will merge the PR.
This PR looks clean 🚀
@@ -17,6 +17,7 @@ anyhow.workspace = true | |||
async-trait.workspace = true | |||
chrono.workspace = true | |||
ethportal-api.workspace = true | |||
ethereum_ssz.workspace = 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.
this isn't worked alphabetically
crates/light-client/src/utils.rs
Outdated
// pub fn bytes_to_bytes32(bytes: &[u8]) -> Bytes32 { | ||
// FixedVector::from(bytes.to_vec()) | ||
// } | ||
|
||
use crate::types::Bytes32; | ||
|
||
pub fn bytes_to_bytes32(bytes: &[u8]) -> Bytes32 { | ||
Vector::from_iter(bytes.to_vec()) | ||
} | ||
|
||
pub fn bytes32_to_node(bytes: &Bytes32) -> Result<Node> { | ||
Ok(Node::from_bytes(bytes.as_slice().try_into()?)) | ||
} | ||
// pub fn bytes32_to_node(bytes: &Bytes32) -> Result<B256> { | ||
// Ok(B256::from_slice(bytes.iter().as_slice())) | ||
// } |
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.
please delete this code as it is no longer used
@oac1771 could you dm me on discord and I will help you fix your git history |
Thank you for your first contribution 🚀 |
What was wrong?
FIxes #1417
light-client crate used ssz-rs crate for SSZ handling instead of ethereum-ssz crate
How was it fixed?
This PR removes the ssz-rs dependency and uses ethereum-ssz instead
To-Do