Skip to content

Conversation

@0xOmarA
Copy link
Contributor

@0xOmarA 0xOmarA commented Jul 11, 2025

Summary

This PR fixes an error encountered in the Kitchensink transaction submission flow where the alloy would error out with the following error:

2025-07-11T06:25:15.322799Z ERROR alloy_provider::blocks: failed to fetch block, number: 0, err: deserialization error: invalid value: string "0x2d79dd80ff729c000", expected a 8 byte hex string at line 1 column 1369
    at alloy-provider-1.0.9/src/blocks.rs:164 on tokio-runtime-worker ThreadId(81)
    in revive_dt_node::geth::Submitting transaction with transaction: TransactionRequest { from: Some(0x90f8bf6a479f320ead074411a4b0e7944ea8c9c1), to: Some(Create), gas_price: Some(5000000), max_fee_per_gas: None, max_priority_fee_per_gas: None, max_fee_per_blob_gas: None, gas: Some(5000000), value: None, input: TransactionInput { input: Some(0x36303830383036303430353233343630313435373562363065323930383136303139383233396633356235663830666466653630383036303034333631303135363030653537356235663830666435623566333536306530316336333039613331656134313436303231353735623566383066643562333436306138353735623630323036303033313933363031313236306134353735623336363032333132313536306130353735623630323038313031383138313130363766666666666666666666666666666666383231313137363038633537356236303430353238303930333636303234313136303838353735623630323039313630303439303562363032343832313036303739353735623530353035313630343035313930383135326633356238333830393138333335383135323031393130313930363036363536356235663830666435623633346534383762373136306530316235663532363034313630303435323630323435666664356235663830666435623566383066643562356638306664666561323634363937303636373335383232313232303736636533313630616337646137326631373331633936613464343965636136653263363663326464633866393436646338393239623430656666303265303636343733366636633633343330303038316430303333), data: None }, nonce: Some(0), chain_id: Some(420420420), access_list: None, transaction_type: None, blob_versioned_hashes: None, sidecar: None, authorization_list: None }

Description

I have a breakdown of this issue and its causes in #33. To summarize:

  • alloy is expecting the gasLimit to be a u64 (despite none of the Ethereum specs saying that it should be a u64)
  • They attempt to decode our gas limit in the block header into a u64.
  • This decode fails since our gas limit is 9 bytes long, which is one byte above the 8 bytes that can be represented with a u64.
  • Therefore the decoding fails.
  • alloy allows clients to define custom networks with custom types for everything such as the block header, the block structure, the transaction structure, and so much more.
  • We can fix this error with allow by defining a custom KitchenSink alloy network which uses a u128 for the gas limit. We can then use this network anytime we create an alloy provider.

0xOmarA added 4 commits July 11, 2025 14:49
This commit adds the `--dev` argument to the `substrate-node` to allow
the chain to keep advancing as time goes own. We have found that if this
option is not added then the chain won't advance forward.
@0xOmarA
Copy link
Contributor Author

0xOmarA commented Jul 13, 2025

It looks like this solution will not be needed in the long-term based on this discussion on element. So, once the new gas implementation is in place we can go ahead and remove the custom alloy network implementation here (I think)

This was referenced Jul 14, 2025
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
struct KitchenSinkNetwork;

impl Network for KitchenSinkNetwork {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything that doesn't need to be changed is used as is from the Network implementation of Ethereum.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense

Copy link
Member

@xermicus xermicus left a comment

Choose a reason for hiding this comment

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

I am wondering is this something that could be fixed on our RPC to gain more compatibility with Ethereum?

@xermicus
Copy link
Member

Interesting find btw. 😀

@0xOmarA
Copy link
Contributor Author

0xOmarA commented Jul 16, 2025

@xermicus Yup! I think the team is planning on doing that according to this discussion in element, so it seems like this is in the plans. I thought I'd add this until we get that in and then we can remove the custom network :)

.clone()
.open(self.kitchensink_stderr_log_file_path())?;
self.process_substrate = Command::new(&self.substrate_binary)
.arg("--dev")
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why did we add this ? The plan was to start the substrate node using the chainspec file generated by the geth genesis file, so we start from the same state on both of the nodes (same addresses, etc). Wouldn't the --dev pollute this state ?

Copy link
Member

Choose a reason for hiding this comment

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

--chain has precedence if explicitly set. --dev does not override any explicitly set flags.

@xermicus
Copy link
Member

See also: paritytech/polkadot-sdk#9177

@0xOmarA
Copy link
Contributor Author

0xOmarA commented Jul 17, 2025

@xermicus Oh, nice! So it seems like this will soon be fixed in new versions of the polkadot SDK?

@0xOmarA
Copy link
Contributor Author

0xOmarA commented Jul 17, 2025

@xermicus Would you be okay with us merging in this change until the PR you linked is merged into upstream polkadot-sdk and we update the version we use in this repo?

(It doesn't relate to geth vs geth but it would be nice to get it merged so we can merge some of the other PRs that depend on this one)

@xermicus
Copy link
Member

Yes if it helps you to move on please feel free to merge it.

@0xOmarA
Copy link
Contributor Author

0xOmarA commented Jul 18, 2025

@xermicus Sounds good, can I please get a review on this PR? :)

@0xOmarA 0xOmarA added this pull request to the merge queue Jul 18, 2025
Merged via the queue into main with commit 854e8d9 Jul 18, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants