Skip to content
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

feature: ProviderCall #788

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feature: ProviderCall #788

wants to merge 11 commits into from

Conversation

prestwich
Copy link
Member

@prestwich prestwich commented May 27, 2024

Closes #736

Motivation

Allow providers to get outputs from arbitrary sources, including synchronous

Solution

  • Add a ProviderCall future that wraps 4 data sources (rpc, batch rpc, other future, ready)
  • change provider return types
  • change inner type of eth call and with_block

drive-by:

  • generalize into_owned_params
  • alphabetize some mod defs
  • improve mapping internal to rpccall
  • add map to waiter

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@prestwich prestwich added enhancement New feature or request debt Tech debt which needs to be addressed labels May 27, 2024
@prestwich prestwich self-assigned this May 27, 2024
@prestwich prestwich force-pushed the prestwich/provider-call-fut branch from b465c20 to b1496cb Compare May 28, 2024 09:02
Copy link
Contributor

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense,

this is still easy to follow imo

@prestwich prestwich force-pushed the prestwich/provider-call-fut branch from b1496cb to 21b11f1 Compare May 31, 2024 10:23
fix: rename to prov_call for disambiguation
@prestwich prestwich force-pushed the prestwich/provider-call-fut branch from 068c2e4 to f14aa77 Compare May 31, 2024 11:48
@prestwich
Copy link
Member Author

the main problem right now is figuring out how to integrate it into WithBlock and EthCall 🤔

@yash-atreya
Copy link
Contributor

hey @prestwich

Taking this over, need it for #954

@yash-atreya
Copy link
Contributor

yash-atreya commented Jun 27, 2024

hey @prestwich

Taking this over, need it for #954

Would appreciate it if you have any pointers on how to move forward from here.

I think only integrating ProviderCall with RpcWithBlock and EthCall remains?

@yash-atreya yash-atreya marked this pull request as ready for review July 2, 2024 07:48
Copy link
Contributor

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, this solution forces us to duplicate the trait functions and therefor doesn't solve the problem because ethcall still only supports rpccall.

the critical part is this:

let fut = client.request(method, params).map_resp(map);

basically a transformer that takes the ethcall and converts it into a providercall, currently this is hardcoded to the rpc client, but ideally this is anything that can fn(ethcallparams) -> ProviderCall

this needs a different approach so that the ethcall and rpcwith block can be converted to any providercall.

Comment on lines +145 to +151
fn call<'req>(
&'req self,
tx: &'req N::TransactionRequest,
) -> ProviderCall<T, EthCallParams<'req, 'req, N>, Bytes, Bytes> {
let call = self.call_internal(tx);
call.into_provider_call()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a breaking api change, that forces usage of call_internal if you want to use EthCall functions. I don't think this is what we want here, because this is very counter-intuitive.

Comment on lines +304 to +306
let call = RpcWithBlock::new(self.weak_client(), "eth_getBalance", address);

ProviderCall::from(call)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not really work, because this now erases the RpcWithBlock functions making this type a bit redundant.

Comment on lines +40 to +41
/// RpcWithBlock
RpcWithBlock(RpcWithBlock<Conn, Params, Resp, Output, Map>),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like the same approach, just slightly modified.

all of this is a bit weird, because imo we need the inverse here, because rpcwithblock is just a helper type that drives something to completion. otherwise this doesn't solve the problem described in the issue. because this is restricted to rpcall:

#788 (review)

Comment on lines +201 to +204
/// Set the block id to "pending".
pub fn pending(self) -> Self {
self.block_id(BlockId::pending())
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this leaks rpcwithblock features to provider call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Tech debt which needs to be addressed enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Allow custom implementation for Provider methods
4 participants