feat(l1): rpc endpoint eth_getFilterChanges #889
Conversation
crates/networking/rpc/Cargo.toml
Outdated
| @@ -14,7 +14,7 @@ bytes.workspace = true | |||
| tracing.workspace = true | |||
| tracing-subscriber.workspace = true | |||
| ethereum_rust-core.workspace = true | |||
| ethereum_rust-storage.workspace = true | |||
| ethereum_rust-storage = { path = "../../storage/store" } | |||
There was a problem hiding this comment.
you have to do this in addition to adding it as dev dependency?
crates/storage/store/storage.rs
Outdated
| @@ -698,6 +698,22 @@ fn hash_key(key: &H256) -> Vec<u8> { | |||
| .to_vec() | |||
| } | |||
|
|
|||
| // USED ONLY FOR TESTS. | |||
There was a problem hiding this comment.
I would remove this from this PR and create a separate ticket to see if we want to tackle this.
It seems every crate has a different way of setting up a store for tests, and if we decide to create a helper to abstract this boilerplace, it should be done across the board.
I would keep this PR focused on the feature instead.
There was a problem hiding this comment.
But without this specific function; I cannot test the feature properly since I did not find a way to simulate the addition of blocks between RPC calls, how can I test it then? should I skip testing it for now?
There was a problem hiding this comment.
Ah, I saw the test now. I would not add such a high level test here, it looks like a full fledged integration test.
Personally, I would just have unit tests inside the rust modules. If you have trouble testing the functionality, it may mean that you should move the business logic to a separate module (I think we discussed this at some point). For integration tests, we could leverage something like hive (we have our own fork) to write custom tests, or we could build or own framework.
Anyway, since these endpoints are not high priority right now, I would just eliminate that test, and create a ticket to work on this at some point.
There was a problem hiding this comment.
it may mean that you should move the business logic to a separate module (I think we discussed this at some point)
I can't remember if we did, but I'll keep in in mind for the future.
As per the PR:
- I've removed the test.
- I've removed the function from the storage crate.
- I've added an issue to eventually test this endpoint.
crates/storage/store/storage.rs
Outdated
| @@ -19,7 +19,7 @@ use std::fmt::Debug; | |||
| use std::sync::Arc; | |||
| use tracing::info; | |||
|
|
|||
| mod engines; | |||
| pub mod engines; | |||
crates/networking/rpc/Cargo.toml
Outdated
| @@ -29,5 +29,6 @@ reqwest.workspace = true | |||
| [dev-dependencies] | |||
| hex-literal = "0.4.1" | |||
|
|
|||
|
|
|||
**Motivation** - This endpoint is useful to poll events from contracts. **Description** - Moved filtering logic to a specific function under logs.rs Closes lambdaclass#427 .
Motivation
Description
Closes Implement
eth_getFilterChanges#427 .