-
Notifications
You must be signed in to change notification settings - Fork 52
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(rpc): implement starknet_subscribeEvents WebSocket endpoint #466
Conversation
008a6c2
to
0bd7a8f
Compare
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
crates/madara/client/rpc/src/versions/user/v0_7_1/methods/read/get_events.rs
Outdated
Show resolved
Hide resolved
@@ -54,8 +55,16 @@ pub struct GetStorageProofResult { | |||
|
|||
#[versioned_rpc("V0_8_0", "starknet")] | |||
pub trait StarknetWsRpcApi { | |||
#[subscription(name = "subscribeNewHeads", unsubscribe = "unsubscribe", item = NewHead, param_kind = map)] | |||
async fn subscribe_new_heads(&self, block_id: BlockId) -> jsonrpsee::core::SubscriptionResult; | |||
#[subscription(name = "subscribeNewHeads", unsubscribe = "unsubscribeNewHeads", item = NewHead, param_kind = map)] |
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.
#[subscription(name = "subscribeNewHeads", unsubscribe = "unsubscribeNewHeads", item = NewHead, param_kind = map)] | |
#[subscription(name = "subscribeNewHeads", unsubscribe = "unsubscribe", item = NewHead, param_kind = map)] |
We want the call to unsubscribe to be starknet_unsunscribe
as per the specs.
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.
I cannot have the same name for unsubscribe for both methods. This seems strange because during an unsubscribe, the ID is provided, so there shouldn't be any conflict.
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.
Actually, the problem lies with our versioned_rpc
macro
// Each block contains two receipts: | ||
// 1. First receipt with 1 event and 1 key | ||
// 2. Second receipt with 2 events and 2 keys | ||
fn block_generator(backend: &mc_db::MadaraBackend) -> impl Iterator<Item = Vec<EmittedEvent<Felt>>> + '_ { |
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.
Consider moving this to a parametrised #[rstest::fixture]
. Tbf this might not be worth it though as it would require refactoring rpc_test_setup.
crates/madara/client/rpc/src/versions/user/v0_8_0/methods/ws/subscribe_events.rs
Outdated
Show resolved
Hide resolved
]; | ||
|
||
for _ in 0..10 { | ||
let _ = generator.next().expect("Retrieving block"); |
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.
It might be more maintainable to generate expected_events
procedurally by collecting generator
and filtering to the desired keys. I quite like how readable the declarative approach is though so idk.
crates/madara/client/rpc/src/versions/user/v0_8_0/methods/ws/subscribe_events.rs
Outdated
Show resolved
Hide resolved
@@ -1,4 +1,4 @@ | |||
[toolchain] | |||
channel = "1.81" | |||
components = ["rustfmt", "clippy", "rust-analyzer"] | |||
components = ["rust-src", "rustfmt", "clippy", "rust-analyzer"] |
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.
Why do we need rust-src
?
/// EventChannels manages a highly efficient and scalable pub/sub system for events with 16 specific channels | ||
/// plus one "all" channel. This architecture provides several key benefits: | ||
/// | ||
/// Benefits: | ||
/// - Selective Subscription: Subscribers can choose between receiving all events or filtering for specific | ||
/// senders, optimizing network and processing resources | ||
/// - Memory Efficiency: The fixed number of channels (16) provides a good balance between granularity | ||
/// and memory overhead | ||
/// - Predictable Routing: The XOR-based hash function ensures consistent and fast mapping of sender | ||
/// addresses to channels | ||
/// |
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.
Blazingly Fast™ 🦀
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.
The idea for this implementation originally came from @cchudant
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.
Its a good idea!
05d9b9c
to
241e9ee
Compare
6658df0
to
c808572
Compare
c808572
to
27cc861
Compare
Pull Request type
Please add the labels corresponding to the type of changes your PR introduces:
What is the current behavior?
Currently, Madara does not implement the
starknet_subscribeEvents
WebSocket RPC method from Starknet RPC v0.8.0. This feature is needed to allow clients to subscribe to and receive real-time Starknet events with filtering capabilities.Resolves: #337
What is the new behavior?
starknet_subscribeEvents
WebSocket endpoint following v0.8.0 specificationsstarknet_subscribeEvents
in the supported JSON-RPC methods sectionDoes this introduce a breaking change?
No
Other information