Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: add reth-rpc-types to serialize block to json-rpc #942

Merged
merged 2 commits into from
Oct 3, 2023

Conversation

carver
Copy link
Collaborator

@carver carver commented Sep 29, 2023

What was wrong?

We didn't have a clean data structure for representing the combined header/block thing that's returned by eth_getBlockByHash and friends.

How was it fixed?

Import the reth-rpc-types for Block and add a conversion utility from the trin Header into the reth Header which is a field in Block.

To understand more about how this is used, see #908

To-Do

  • Clean up commit history

@carver carver requested a review from njgheorghita September 29, 2023 22:05
@carver
Copy link
Collaborator Author

carver commented Sep 29, 2023

Don't let the line count spook you. It's mostly in the Cargo.lock.

@carver
Copy link
Collaborator Author

carver commented Sep 29, 2023

@KolbyML any idea what's going on here with Windows support?

  C:\Users\circleci\.cargo\git\checkouts\c-kzg-4844-87b9b7bbf5cb76f8\f5f6f86\src\c_kzg_4844.h:27:10: fatal error: 'stdio.h' file not found
  thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ClangDiagnostic("C:\\Users\\circleci\\.cargo\\git\\checkouts\\c-kzg-4844-87b9b7bbf5cb76f8\\f5f6f86\\src\\c_kzg_4844.h:27:10: fatal error: 'stdio.h' file not found\n")', C:\Users\circleci\.cargo\git\checkouts\c-kzg-4844-87b9b7bbf5cb76f8\f5f6f86\bindings\rust\build.rs:157:10

@KolbyML
Copy link
Member

KolbyML commented Sep 30, 2023

We are currently using Windows server 2019 I will quickly see if I can get 2022 running and test if it works

@KolbyML
Copy link
Member

KolbyML commented Sep 30, 2023

It seems like updating the CI to a newer windows server resolved the issue here is a PR with the CI fix #945

@carver carver force-pushed the add-reth-rpc-types branch 2 times, most recently from 084dd6e to 7406365 Compare September 30, 2023 17:55
Copy link
Collaborator

@njgheorghita njgheorghita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

}
}

fn u256_to_uint256(u256: U256) -> Uint<256, 4> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we could create our own custom wrapped type around Uint<256, 4> & then implement From rather than these conversion methods. Seems slightly cleaner, but also wrapped types can be annoying to manage, so I'm perfectly fine leaving this as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll take a look. My intuition is that wrapping it just to unwrap it immediately is probably not worth the coding overhead, but I'll explore it a bit.

Copy link
Collaborator Author

@carver carver Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an obvious win to me. I think I'll leave it as-is unless you're seeing something I'm not about why it's better:

diff --git a/ethportal-api/src/types/execution/header.rs b/ethportal-api/src/types/execution/header.rs
index e9407b8..91598ed 100644
--- a/ethportal-api/src/types/execution/header.rs
+++ b/ethportal-api/src/types/execution/header.rs
@@ -1,3 +1,5 @@
+use std::ops::Deref;
+
 use ethereum_types::{Bloom, H160, H256, H64, U256};
 use reth_rpc_types::Header as RpcHeader;
 use rlp::{Decodable, DecoderError, Encodable, Rlp, RlpStream};
@@ -232,7 +234,7 @@ impl From<Header> for RpcHeader {
             transactions_root: transactions_root.to_fixed_bytes().into(),
             receipts_root: receipts_root.to_fixed_bytes().into(),
             logs_bloom: logs_bloom.to_fixed_bytes().into(),
-            difficulty: u256_to_uint256(difficulty),
+            difficulty: *RethUint256::from(difficulty),
             number: Some(u64_to_uint256(number)),
             gas_limit: u256_to_uint256(gas_limit),
             gas_used: u256_to_uint256(gas_used),
@@ -252,6 +254,24 @@ impl From<Header> for RpcHeader {
     }
 }
 
+struct RethUint256(Uint<256, 4>);
+
+impl From<U256> for RethUint256 {
+    fn from(u256: U256) -> Self {
+        let mut bytes = [0u8; 32];
+        u256.to_big_endian(&mut bytes);
+        Self(Uint::from_be_bytes(bytes))
+    }
+}
+
+impl Deref for RethUint256 {
+    type Target = Uint<256, 4>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
 fn u256_to_uint256(u256: U256) -> Uint<256, 4> {
     let mut bytes = [0u8; 32];
     u256.to_big_endian(&mut bytes);

I guess this is another option, but still doesn't win me over:

diff --git a/ethportal-api/src/types/execution/header.rs b/ethportal-api/src/types/execution/header.rs
index e9407b8..91598ed 100644
--- a/ethportal-api/src/types/execution/header.rs
+++ b/ethportal-api/src/types/execution/header.rs
@@ -232,7 +234,7 @@ impl From<Header> for RpcHeader {
             transactions_root: transactions_root.to_fixed_bytes().into(),
             receipts_root: receipts_root.to_fixed_bytes().into(),
             logs_bloom: logs_bloom.to_fixed_bytes().into(),
-            difficulty: u256_to_uint256(difficulty),
+            difficulty: RethUint256::from(difficulty).0,
             number: Some(u64_to_uint256(number)),
             gas_limit: u256_to_uint256(gas_limit),
             gas_used: u256_to_uint256(gas_used),
@@ -252,6 +254,24 @@ impl From<Header> for RpcHeader {
     }
 }
 
+struct RethUint256(Uint<256, 4>);
+
+impl From<U256> for RethUint256 {
+    fn from(u256: U256) -> Self {
+        let mut bytes = [0u8; 32];
+        u256.to_big_endian(&mut bytes);
+        Self(Uint::from_be_bytes(bytes))
+    }
+}
+
 fn u256_to_uint256(u256: U256) -> Uint<256, 4> {
     let mut bytes = [0u8; 32];
     u256.to_big_endian(&mut bytes);

@@ -195,6 +197,73 @@ impl PartialEq for Header {
}
}

impl From<Header> for RpcHeader {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a docstring comment here explaining why we need this RpcHeader type.

@carver carver force-pushed the add-reth-rpc-types branch from 7406365 to 601c754 Compare October 3, 2023 00:56
@carver carver merged commit 2bea420 into ethereum:master Oct 3, 2023
@carver carver deleted the add-reth-rpc-types branch October 3, 2023 01:04
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants