feat(l1): implement TrieIterator + GetAccountRange snap request handling logic
#960
Conversation
This PR re-writes this branch's commits on top of the main branch: - Implement TrieIterator - Consume Trie when building TrieIterator - Implement `Store::iter_accounts` using TrieIterator
fmoletta
changed the title
TrieIterator + GetAccountRange snap request handling [WIP]TrieIterator + GetAccountRange snap request handling logic
| @@ -38,6 +46,14 @@ impl Message { | |||
| Message::Ping(msg) => msg.encode(buf), | |||
| Message::Pong(msg) => msg.encode(buf), | |||
| Message::Status(msg) => msg.encode(buf), | |||
| Message::GetAccountRange(msg) => { | |||
| 0x21_u8.encode(buf); | |||
There was a problem hiding this comment.
this is quite manual. Would be nice to refactor code so that a message is associated to a message id (probably out of scope of this PR).
There was a problem hiding this comment.
We could add a method code to the Message enum and use it for encoding/decoding so that we onlty write the code number once in the codebase
There was a problem hiding this comment.
Sounds like a good solution.
There was a problem hiding this comment.
Ill create an issue for it
There was a problem hiding this comment.
I noticed that the msg_id is encoded within the RLPxMessage::encode(..) for the other Message types like PingMessage, HelloMessage. Would it not work if we move 0x21_u8.encode(buf) into the RLPxMessage::encode(..) method of snap::GetAccountRange?
There was a problem hiding this comment.
We are discussing moving the encoding of the code number to the encoding of the Message enum for the other messages too. The code number is already being decoded on the enum's decode Message so encoding it on the enum's decode method would keep encoding/decoding simmetrical for all involved structs
| @@ -50,6 +66,8 @@ impl Display for Message { | |||
| Message::Ping(_) => "p2p:Ping".fmt(f), | |||
| Message::Pong(_) => "p2p:Pong".fmt(f), | |||
| Message::Status(_) => "eth:Status".fmt(f), | |||
| Message::GetAccountRange(_) => "snap::GetAccountRange".fmt(f), | |||
There was a problem hiding this comment.
single colons (:) to be homogeneous with the rest of the representations.
Maybe we should use / for all instead 🤔
…andling logic (lambdaclass#960) **Motivation** Handling snap capability message `GetAccountRange` <!-- Why does this pull request exist? What are its goals? --> **Description** * Add functionality to iterate the Trie (TrieIterator) * Add functionality to iterate over all accounts in the state (Store::iter_accounts) * Add logic to handle `GetAccountRange` snap request * Fix slim encoding of `AccountState` * Remove unneeded trait `RLPEncodeSlim` **Notes** * ~We don't have the listen loop implemented so this PR only adds the standalone logic for handling the request and creating a response, we still need to plug it in to the main loop.~ * ~We are not able to run the hive test suite due to missing listen loop + old blocks being used by the test suite. I instead copied the state from a Geth execution (loading genesis + importing chain) and used that state to replicate hive tests as unit tests. These tests could be removed once we fix those two problems~ <!-- A clear and concise general description of the changes this PR introduces --> <!-- Link to issues: Resolves lambdaclass#111, Resolves lambdaclass#222 --> Partially addresses lambdaclass#184 --------- Co-authored-by: Esteban Dimitroff Hodi <[email protected]>
Motivation
Handling snap capability message
GetAccountRangeDescription
GetAccountRangesnap requestAccountStateRLPEncodeSlimNotes
We don't have the listen loop implemented so this PR only adds the standalone logic for handling the request and creating a response, we still need to plug it in to the main loop.We are not able to run the hive test suite due to missing listen loop + old blocks being used by the test suite. I instead copied the state from a Geth execution (loading genesis + importing chain) and used that state to replicate hive tests as unit tests. These tests could be removed once we fix those two problemsPartially addresses #184