feat(`provider`): apply `GetSubscription` to trait #2220

frankudoags · 2025-03-19T12:25:30Z

Motivation

Follow up on #2203
Use GetSubscription type for subscription functions in Provider trait and refractor affected areas

Solution

PR Checklist

Added Tests
Added Documentation
Breaking changes

frankudoags · 2025-03-19T12:27:05Z

@mattsse @yash-atreya
Kindly take a look

mattsse

ty, some suggestions

mattsse · 2025-03-19T22:53:51Z

crates/provider/src/provider/trait.rs

+    fn subscribe_pending_transactions(&self) -> GetSubscription<(&'static str,), B256, N> {
+        GetSubscription::new(self.root(), "eth_subscribe", Some(("newPendingTransactions",)))


we can replace the string with

alloy/crates/rpc-types-eth/src/pubsub.rs

Line 76 in ee7a19b

pub enum SubscriptionKind {

mattsse · 2025-03-19T22:55:32Z

crates/provider/src/ext/admin.rs

-        &self,
-    ) -> TransportResult<alloy_pubsub::Subscription<alloy_rpc_types_admin::PeerEvent>>;
+    fn subscribe_peer_events(&self) -> GetSubscription<(), alloy_rpc_types_admin::PeerEvent, N>;


I believe we can get rid of the N generic if we use WeakClient instead of the RootProvider in the GetSubscription type

Addressed in #2219

mattsse · 2025-03-19T23:01:08Z

@yash-atreya I believe #2219 does the weakclient change, should we get this one in first?

yash-atreya · 2025-03-20T06:33:37Z

@yash-atreya I believe #2219 does the weakclient change, should we get this one in first?

Yes we can get this one in and I can rebase #2219

crates/provider/src/provider/subscription.rs

crates/provider/src/provider/trait.rs

frankudoags · 2025-03-20T06:59:27Z

@yash-atreya I like the weak client change to drop the network generic and passing just the Rpc Call directly. I think we can merge #2219 in and I follow that to upgrade other subscription functions.

Seeing that if this gets merged, there'd still be clean up which can be avoided if that gets merged first. Wdyt?
cc @mattsse

yash-atreya · 2025-03-20T12:38:53Z

Hey @frankudoags, #2219 was merged.

Please rebase your PR and let me know if you need any help.

Thanks!

… trait and refractor affected areas

…nts method

…where clause

frankudoags · 2025-03-20T15:17:42Z

having some issues @yash-atreya

The subscribe_logs is failing due to some lifetime issues and wants me to introduce a lifetime which I do not think I should(fails with "lifetime may not live long enough" for the &Filter in the response type
Trying to use GetSubscription for admin_peerEvents_subscribe is failing

alloy/crates/provider/src/provider/subscription.rs

Line 17 in e45a0f9

call: RpcCall<P, B256>,

expected RpcCall<(), FixedBytes<32>>, found RpcCall<[(); 0], _, _, fn(_) -> _>

frankudoags · 2025-03-20T15:44:48Z

our call method inside GetSubscription is RpcCall<P, B256>, does that mean it must always have params? if that is the case, it would not work for admin_peerEvents_subscribe or any subscription with no params @yash-atreya

crates/provider/src/ext/admin.rs

crates/provider/src/provider/trait.rs

…ption

frankudoags · 2025-03-20T16:11:33Z

All done now, @yash-atreya , ptal

crates/provider/src/provider/trait.rs

yash-atreya

send it!

frankudoags · 2025-03-21T09:21:25Z

thank you all @mattsse @yash-atreya @DaniPopes

github-project-automation bot added this to Alloy Mar 19, 2025

mattsse requested changes Mar 19, 2025

View reviewed changes

github-project-automation bot moved this to In Progress in Alloy Mar 19, 2025

DaniPopes requested changes Mar 20, 2025

View reviewed changes

crates/provider/src/provider/subscription.rs Outdated Show resolved Hide resolved

crates/provider/src/provider/trait.rs Show resolved Hide resolved

yash-atreya marked this pull request as ready for review March 20, 2025 07:40

yash-atreya requested a review from klkvr as a code owner March 20, 2025 07:40

mattsse mentioned this pull request Mar 20, 2025

chore(provider): use WeakClient in GetSubscription #2219

Merged

3 tasks

frankudoags added 3 commits March 20, 2025 15:58

feat: use GetSubscription type for subscription functions in Provider…

910d925

… trait and refractor affected areas

feat: refactor AdminApi to use GetSubscription for subscribe_peer_eve…

52e2353

…nts method

fmt

454b6da

frankudoags force-pushed the feat-add-subscription-type-to-subscription-fns branch from 3c16bdf to 454b6da Compare March 20, 2025 15:00

frankudoags added 2 commits March 20, 2025 16:00

fmt

1146a6b

refactor: simplify AdminApi trait definition by removing unnecessary …

2c827c2

…where clause

frankudoags requested a review from yash-atreya March 20, 2025 15:03

fix: clippy

c911437

yash-atreya reviewed Mar 20, 2025

View reviewed changes

crates/provider/src/ext/admin.rs Outdated Show resolved Hide resolved

yash-atreya reviewed Mar 20, 2025

View reviewed changes

crates/provider/src/provider/trait.rs Outdated Show resolved Hide resolved

crates/provider/src/provider/trait.rs Outdated Show resolved Hide resolved

fix: make Filter owned, add return type of NoParams for admin subscri…

33994c4

…ption

yash-atreya requested changes Mar 20, 2025

View reviewed changes

crates/provider/src/provider/trait.rs Outdated Show resolved Hide resolved

use SubscriptionKind and Param rpc_type_eth

00cff1e

frankudoags requested a review from yash-atreya March 20, 2025 18:26

fmt

237b681

fix: doc test

a03bb2f

frankudoags requested review from DaniPopes and mattsse March 20, 2025 18:45

DaniPopes requested changes Mar 21, 2025

View reviewed changes

crates/provider/src/provider/trait.rs Show resolved Hide resolved

yash-atreya requested changes Mar 21, 2025

View reviewed changes

crates/provider/src/provider/trait.rs Outdated Show resolved Hide resolved

crates/provider/src/provider/trait.rs Outdated Show resolved Hide resolved

crates/provider/src/provider/trait.rs Outdated Show resolved Hide resolved

yash-atreya changed the title ~~feat: use GetSubscription type for subscription functions in Provider…~~ feat(provider): apply GetSubscription to trait Mar 21, 2025

fixes

fde52e2

frankudoags requested review from DaniPopes and yash-atreya March 21, 2025 06:51

DaniPopes approved these changes Mar 21, 2025

View reviewed changes

yash-atreya approved these changes Mar 21, 2025

View reviewed changes

yash-atreya moved this from In Progress to Reviewed in Alloy Mar 21, 2025

DaniPopes enabled auto-merge (squash) March 21, 2025 09:12

mattsse approved these changes Mar 21, 2025

View reviewed changes

DaniPopes merged commit 84eb058 into alloy-rs:main Mar 21, 2025
27 checks passed

github-project-automation bot moved this from Reviewed to Done in Alloy Mar 21, 2025

		fn subscribe_pending_transactions(&self) -> GetSubscription<(&'static str,), B256, N> {
		GetSubscription::new(self.root(), "eth_subscribe", Some(("newPendingTransactions",)))

feat(provider): apply GetSubscription to trait #2220

feat(provider): apply GetSubscription to trait #2220

Uh oh!

Conversation

frankudoags commented Mar 19, 2025

Motivation

Solution

PR Checklist

Uh oh!

frankudoags commented Mar 19, 2025

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

mattsse Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

mattsse Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

yash-atreya Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattsse commented Mar 19, 2025

Uh oh!

yash-atreya commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

frankudoags commented Mar 20, 2025

Uh oh!

yash-atreya commented Mar 20, 2025

Uh oh!

frankudoags commented Mar 20, 2025

Uh oh!

frankudoags commented Mar 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

frankudoags commented Mar 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yash-atreya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

frankudoags commented Mar 21, 2025

Uh oh!

Uh oh!

feat(`provider`): apply `GetSubscription` to trait #2220

feat(`provider`): apply `GetSubscription` to trait #2220

yash-atreya Mar 20, 2025 •

edited

Loading

yash-atreya commented Mar 20, 2025 •

edited

Loading