feat(rpc): rpc handler for eth_getUncleByBlock{Hash/Number}AndIndex #1595

fkrause98 · 2023-03-01T16:49:35Z

Changes:

Added handler for eth_getUncleByBlockHashAndIndex and eth_getUncleByBlockNumberAndIndex.
Related: Tracking issue RPC handler impl #1225.

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

mattsse · 2023-03-01T17:10:57Z

crates/rpc/rpc/src/eth/api/block.rs

+        let block_id = block_id.into();
+        let index = usize::from(index);
+        let uncles = self.client().ommers(block_id)?.unwrap_or_default();
+        let Some(uncle_header) = uncles.get(index) else {


Suggested change

let Some(uncle_header) = uncles.get(index) else {

let Some(uncle_header) = uncles.skip(index).next() else {

I'm guessing you meant? 😄 : let Some(uncle_header) = uncles.into_iter().skip(index).next() else {

Clippy suggests nth, so I've used that instead 🙂 .

mattsse · 2023-03-01T17:13:18Z

crates/rpc/rpc/src/eth/api/block.rs

+        let Some(uncle_header) = uncles.get(index) else {
+            return Ok(None)
+        };
+        self.block(BlockId::from(uncle_header.number), false).await


this is not correct, we need to convert he Header into a block response instead.

An uncle doesn't contain individual transactions.

https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_getunclebyblockhashandindex

So need to construct an RPC block from header only.

Does that mean that I should add an RPC type like Block, that lacks a transactions field and use it as the return type then? 😄

Correct it should be the same as get block with transaction hashes only

I've added this struct here for the response and a few more changes. Please, let me know what you think and if there's something else to improve 🙂.

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

mattsse · 2023-03-03T08:00:22Z

crates/rpc/rpc-types/src/eth/block.rs

+/// An uncle block RPC response representation.
+#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct OmmerBlock {


I think there's been some confusion about what to return for uncle.

we need to return the actual Block type:

e.g.

cast rpc eth_getUncleByBlockNumberAndIndex "0x27102" "0x0" --rpc-url "https://eth-mainnet.alchemyapi.io/v2/Lc7oIGYeL_QvInzI0Wiu_pOZZDEKBrdf" | jq

{ "difficulty": "0x6222410d3e2", "extraData": "0xd783010002844765746887676f312e342e32856c696e7578", "gasLimit": "0x2fefd8", "gasUsed": "0x0", "hash": "0x7ddcbe9b51cbead49b0edbb49b1a889a9cbc687cebd2cf05af43124d92472b5b", "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", "miner": "0x52bc44d5378309ee2abf1539bf71de1b7d7be3b5", "mixHash": "0x98786cd3fc481f59562137daea90e3b7663eacbcee64f758d15e38aca31089c5", "nonce": "0xdb601d60e4b0146a", "number": "0x270ff", "parentHash": "0x00e1e32b60c8a8b24ad81cd9cecab49574424add31933c861bea90df2dce628b", "receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", "sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347", "size": "0x21d", "stateRoot": "0xda97dd606da5ed4642c2fbb0c80c0cbf6951d5fd175bb258e538c9edaf6fcfab", "timestamp": "0x55e19b96", "transactionsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", "uncles": [] }

so it's the same as a regular but but some fields are default values, like bloom, gasUsed

Oh, ok! Pushed some changes to use the original Block type, and this is what I get:

curl --location --request POST 'http://127.0.0.1:8545/' \ --header 'Content-Type: application/json' \ --data-raw '{ "jsonrpc": "2.0", "method": "eth_getUncleByBlockNumberAndIndex", "params": [ "0x21", "0x0" ], "id": "0" } ' | jq -S ".result"

{ "author": "0xdb312d1d6a2ccc64dd94a3892928bac82b4e8c15", "difficulty": "0x40888145a", "extraData": "0x476574682f76312e302e302d30636463373634372f6c696e75782f676f312e34", "gasLimit": "0x1388", "gasUsed": "0x0", "hash": "0x962b18a4fcdd215e11db01308f6818735ea1b33377a82af39cb89fa6d4178204", "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", "miner": "0xdb312d1d6a2ccc64dd94a3892928bac82b4e8c15", "mixHash": "0x5870078c7ab1637370a603260f3b25d5f006077df2fa0f868d0970ea32496075", "nonce": "0x3673ed3d817b95bf", "number": "0x1f", "parentHash": "0xd1eb5ef56d30f0d773cccd3075c7fcca14ec9013db6b76344c69dbbaffaa9b8c", "receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", "sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347", "size": "0x220", "stateRoot": "0x1624d23c380d999ac180022f08f008849ed5a404e50a0a8c704d5a536742e471", "timestamp": "0x55ba4322", "transactionsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", "uncles": [] }

It matches the response that I get from Alchemy, except from one small thing, the "author" key.

mattsse · 2023-03-03T08:00:54Z

crates/rpc/rpc/src/eth/api/block.rs

-            return Ok(None)
-        };
-        self.block(BlockId::from(uncle_header.number), false).await
+        let uncle = uncles.into_iter().skip(index).next().map(|uncle| uncle.into());


oh this should be equivalent to nth()

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

mattsse

only a few nits, otherwise lgtm

fyi: geth marshals uncles like:

https://github.com/ethereum/go-ethereum/blob/ee530c0d5aa70d2c00ab5691a89ab431b73f8165/internal/ethapi/api.go#L814-L814

which we do now mirror.

I wonder if these changes would make the block type less usable for third party crates. But for simplicity this is ok.

mattsse · 2023-03-04T10:06:43Z

crates/rpc/rpc-types/src/eth/block.rs

+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub transactions: Option<BlockTransactions>,


the Option seems redundant if we have BlockTransactions::is_uncle

You're right, changed it here 🙂

mattsse · 2023-03-04T10:07:46Z

crates/rpc/rpc-types/src/eth/block.rs

+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub total_difficulty: Option<U256>,


right, this is excluded if uncle:

https://github.com/ethereum/go-ethereum/blob/ee530c0d5aa70d2c00ab5691a89ab431b73f8165/internal/ethapi/api.go#L1277-L1279

mattsse · 2023-03-04T10:11:56Z

crates/rpc/rpc-types/src/eth/block.rs

 #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(untagged)]
 pub enum BlockTransactions {
    /// Only hashes
    Hashes(Vec<H256>),
    /// Full transactions
    Full(Vec<Transaction>),
+    /// Special case for uncle response.
+    NoTx,


can rename this to Uncle

crates/rpc/rpc-types/src/eth/block.rs

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

mattsse

lgtm
tysm, nice job!

fkrause98 and others added 4 commits February 28, 2023 19:07

Add uncle by hash and index

b3049f3

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Add unknown ommer error

561b18e

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Do not use UnknownOmmer

7c6ffdf

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Fix clippy

d37401c

fkrause98 requested a review from gakonst as a code owner March 1, 2023 16:49

mattsse requested changes Mar 1, 2023

View reviewed changes

fkrause98 and others added 6 commits March 1, 2023 18:50

Fix http test

342db87

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Use OmmerBlock type

4ca1920

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Add from impl for OmmerBlock, skip root serialize in header

184eff7

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Use map instead of let else

61295f5

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Add comment in from impl

6ac7582

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Use nth instead of skip+next

406fbbb

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

mattsse requested changes Mar 3, 2023

View reviewed changes

fkrause98 and others added 2 commits March 3, 2023 14:13

Go back to use Block

e8aa1cc

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Fix test

b8391d6

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

mattsse requested changes Mar 4, 2023

View reviewed changes

mattsse added the A-rpc Related to the RPC implementation label Mar 4, 2023

fkrause98 and others added 2 commits March 6, 2023 10:46

PR Comments

bd65589

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

Remove extra space

b4df9b5

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

fkrause98 requested review from mattsse and removed request for gakonst March 6, 2023 13:52

Fix tests

a5b8c53

Co-authored-by: lambdaclass-user <github@lambdaclass.com>

mattsse approved these changes Mar 6, 2023

View reviewed changes

mattsse merged commit ce2b83e into paradigmxyz:main Mar 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rpc): rpc handler for eth_getUncleByBlock{Hash/Number}AndIndex #1595

feat(rpc): rpc handler for eth_getUncleByBlock{Hash/Number}AndIndex #1595

fkrause98 commented Mar 1, 2023

mattsse Mar 1, 2023

fkrause98 Mar 1, 2023

fkrause98 Mar 2, 2023

mattsse Mar 1, 2023

fkrause98 Mar 1, 2023 •

edited

Loading

gakonst Mar 1, 2023

fkrause98 Mar 2, 2023 •

edited

Loading

mattsse Mar 3, 2023

fkrause98 Mar 3, 2023

mattsse Mar 3, 2023

mattsse left a comment

mattsse Mar 4, 2023

fkrause98 Mar 6, 2023

mattsse Mar 4, 2023

mattsse Mar 4, 2023

fkrause98 Mar 6, 2023

mattsse left a comment

	let Some(uncle_header) = uncles.get(index) else {
	let Some(uncle_header) = uncles.skip(index).next() else {

		#[serde(skip_serializing_if = "Option::is_none")]
		pub transactions: Option<BlockTransactions>,

		#[serde(skip_serializing_if = "Option::is_none")]
		pub total_difficulty: Option<U256>,

feat(rpc): rpc handler for eth_getUncleByBlock{Hash/Number}AndIndex #1595

feat(rpc): rpc handler for eth_getUncleByBlock{Hash/Number}AndIndex #1595

Conversation

fkrause98 commented Mar 1, 2023

Changes:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fkrause98 Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fkrause98 Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

fkrause98 Mar 1, 2023 •

edited

Loading

fkrause98 Mar 2, 2023 •

edited

Loading