-
Notifications
You must be signed in to change notification settings - Fork 29
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
feat(l1): initiate rlpx connections from discv4 #936
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.
Left some comments, is there a proper way I can test this yet?
**Motivation** We want to use [risc zero](https://risczero.com/) as proving system, we need to build ethereum_rust's crates to accomplish it. **Description** Replace `size_of` with `core::mem::size_of` in order to build the crate for the desired target.
**Motivation** Provide proper attribution to the Geth code for networking. **Description** While implementing the discovery protocol, we've been inspired by `geth` code. This pr adds clearer and more explicit references to the relevant sections of the Geth code. Closes None
**Motivation** After walking through the README as someone new to the project everything worked as expected except for the Hive test runs. **Description** This PR adds golang to the `.tool-versions` (commented, given that it's not needed for the rest of the node execution) and a small prereq section to the hive readme about installing the plugin and uncommenting it in case of wanting to run the hive tests locally. Also I added a couple of services to the kurtosis `network-params.yml`, `el_forkmon` works as a monitor of the canonical chain and checks all clients are synced between themselves and the amount of bad blocks they've processed. On the other hand `tx|blob_spammer` adds more sustance to the run, it mainly affects other nodes given that we don't have p2p already implemented but will be picked up then.
**Motivation** <!-- Why does this pull request exist? What are its goals? --> To separate the localnet run from the `ethereum_rust_l2` CLI. The CLI should be used as a developer tool to improve the developer experience and for making more serious deployments (like deploying the stack on testnet). **Description** <!-- A clear and concise general description of the changes this PR introduces --> - Now, to run a localnet you just need to change your current dir to the `l2` crate and run `make`. - Adds a `help` command to the Makefile and a description for every command. --------- Co-authored-by: Federico Borello <[email protected]>
**Motivation** This PR goal is to add validations in transact that are not done in upper layers **Description** The validations are based in [yellow paper's](https://ethereum.github.io/yellowpaper/paper.pdf) transaction execution section (the 6th one). At the moment there is a lack of tests of the Cancun fork. Also, removes some constants about transactions intrinsic gas costs, because we don't do those calculations in this layer. <!-- Link to issues: Resolves #111, Resolves #222 --> Closes #900
**Motivation** <!-- Why does this pull request exist? What are its goals? --> Some tools like Metamask send big `u64` numbers as `id` in RPC requests that breaks the parser. **Description** <!-- A clear and concise general description of the changes this PR introduces --> Change the `RpcRequestId::Number` type from `i32` to `u64`. <!-- Link to issues: Resolves #111, Resolves #222 -->
**Motivation** Revival of #818 Adds the GetReceipts request and Receipts response for the eth protocol **Description** - Adds GetReceipts and Receipts with encode/decode capabilities - rustic test with UDP sockets sending the data - Implements get_all_receipts_by_hash for storage Closes #386 Closes #387
**Description** - Moved blocks P2P encoding/decoding into it's own module like with receipts and transactions - Removed useless tests
**Motivation** Implement the issue "Move opcodes to their corresponding folder" #863 **Description** Moved the opcodes from block.rs in environment.rs accordings to the list commented at the beginning of each file. The modification was verified by running the tests via 'make test'. Resolves #863 Closes #863
**Motivation** <!-- Why does this pull request exist? What are its goals? --> `send`, `deploy` and `call` commands are very useful commands that are always needed. `calldata` command allows to use this commands in an easier way. **Description** <!-- A clear and concise general description of the changes this PR introduces --> Create some commands in the CLI that implement this methods, with different flags to customize the requests. <!-- Link to issues: Resolves #111, Resolves #222 -->
**Motivation** Clippy was throwing some warnings, applying it suggestions. **Description** Command: ```sh cargo clippy --all-targets --all-features --workspace -- -D warnings ``` Output to Fix: ``` warning: it looks like you're manually copying between slices --> cmd/ethereum_rust_l2/src/commands/test.rs:77:5 | 77 | / for i in 0..64 { 78 | | buffer[i] = public_key[i + 1]; 79 | | } | |_____^ help: try replacing the loop by: `buffer.copy_from_slice(&public_key[1..(64 + 1)]);` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy = note: `#[warn(clippy::manual_memcpy)]` on by default warning: unnecessary `if let` since only the `Ok` variant of the iterator element is used --> cmd/ethereum_rust_l2/src/commands/test.rs:133:21 | 133 | for line in lines { | ^ ----- help: try: `lines.flatten()` | _____________________| | | 134 | | if let Ok(pk) = line { 135 | | let thread = tokio::spawn(transfer_from( 136 | | pk, ... | 144 | | } 145 | | } | |_____________________^ | help: ...and remove the `if let` statement in the for loop --> cmd/ethereum_rust_l2/src/commands/test.rs:134:25 | 134 | / if let Ok(pk) = line { 135 | | let thread = tokio::spawn(transfer_from( 136 | | pk, 137 | | to_address.clone(), ... | 143 | | threads.push(thread); 144 | | } | |_________________________^ = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten = note: `#[warn(clippy::manual_flatten)]` on by default warning: using `clone` on type `H160` which implements the `Copy` trait --> cmd/ethereum_rust_l2/src/commands/test.rs:137:33 | 137 | ... to_address.clone(), | ^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `to_address` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy = note: `#[warn(clippy::clone_on_copy)]` on by default ```
I'm testing it locally setting up a geth node and connecting it with ours. Eventually this will be tested on hive tests that test the |
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.
LGTM,
My biggest concert is proper error handling, but we'll address it in future issues.
Motivation
DiscV4 protocol discovers new peers, and after deciding it is a valid peer, a TCP connection is established and RLPx protocol starts
Description
Now, when receiving a DiscV4 Pong message and after evaluating the peer, a RLPx connection is created and established. If handshake fails it removes the peer from the Kademlia table.
Closes #837.
Also removed some hard-coded testing code.
Once the listen loop is built (#840) there may be other conditions where a peer has to be discarded. (eg. when the other peer sends a Disconnect message). After #840 is completed we may create some more issues on this regard.