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

feat: temporary provider trait #20

Merged
merged 43 commits into from
Dec 18, 2023
Merged

Conversation

onbjerg
Copy link
Member

@onbjerg onbjerg commented Nov 5, 2023

Motivation

I am currently migrating Foundry to use Alloy's providers instead of Ether's middleware. To make migration easier later on when we have the final API down I am introducing a temporary trait for the current Provider struct.

Additionally, some RPC methods were either incomplete or missing.

Solution

  • Add the missing RPC methods
  • Adjust the incomplete RPC methods
  • Add a temporary provider trait

Todo

  • Make remaining adjustments
  • Add docs
  • Adjust/add tests

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Depends on #28

@onbjerg onbjerg added the enhancement New feature or request label Nov 5, 2023
@onbjerg onbjerg changed the title wip feat: temporary provider trait Nov 5, 2023
@onbjerg onbjerg force-pushed the onbjerg/alloy-temp-provider-trait branch from 342e2d9 to 9a060cd Compare November 17, 2023 14:25
@onbjerg onbjerg changed the base branch from main to prestwich/new-new-rpc-result November 17, 2023 14:25
@onbjerg onbjerg changed the base branch from prestwich/new-new-rpc-result to main November 17, 2023 15:33
@onbjerg onbjerg force-pushed the onbjerg/alloy-temp-provider-trait branch from 6b8901b to b32a3d8 Compare November 17, 2023 15:36
@onbjerg onbjerg force-pushed the onbjerg/alloy-temp-provider-trait branch from f597612 to 0702253 Compare November 25, 2023 07:50
onbjerg and others added 15 commits November 25, 2023 08:50
* workaround

* fix: actual fix

* fix: actual fix part 2
* workaround

* fix: actual fix

* fix: actual fix part 2

* fix access list keys

---------

Co-authored-by: Enrique Ortiz <hi@enriqueortiz.dev>
* chore: start removing cows

* chore: uncow tempProvider

* Apply suggestions from code review

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>

* switch to single element tuple

---------

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
* feat: add rlp encoding/decoding to tx types

* feat: add encodable/decodable traits to tx

* chore: remove out-of-scope func

* chore: remove bad links on comments

* chore: fix docs

* clippy
* feat(): raw_request

* chore: remove unneeded async block
@Evalir Evalir force-pushed the onbjerg/alloy-temp-provider-trait branch from 0ccb869 to bc87178 Compare December 14, 2023 02:36
@Evalir Evalir force-pushed the onbjerg/alloy-temp-provider-trait branch from bc87178 to 1d825ec Compare December 14, 2023 03:02
@gakonst gakonst marked this pull request as ready for review December 18, 2023 10:02
crates/providers/src/provider.rs Outdated Show resolved Hide resolved
crates/providers/src/provider.rs Outdated Show resolved Hide resolved
crates/providers/src/provider.rs Outdated Show resolved Hide resolved
Self: Sync;

#[cfg(feature = "anvil")]
async fn set_code(&self, address: Address, code: &'static str) -> TransportResult<()>
Copy link
Member

Choose a reason for hiding this comment

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

AnvilProvider extension trait? instead of cfg in this trait

where
Self: Sync;

async fn raw_request<P, R>(&self, method: &'static str, params: P) -> TransportResult<R>
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this in the trait? Because it breaks object safety unless Self:Sized

Copy link
Contributor

Choose a reason for hiding this comment

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

Need it for the RPC cheatcode—maybe we could add an extension trait for this too?

@@ -36,6 +113,131 @@ pub struct LegacyTransactionRequest {
pub chain_id: Option<u64>,
}

impl Encodable for LegacyTransactionRequest {
Copy link
Member

Choose a reason for hiding this comment

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

Should this even implement rlp traits? This is the same as encode_fields

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO we should remove these RLP impls—we originally did this to migrate cast but decided against it later. RLP-able types should go on the alloy-consensus-types crate

len += self.gas_limit.length();
len += self.kind.length();
len += self.value.length();
len += self.input.0.length();
Copy link
Member

Choose a reason for hiding this comment

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

this can be in the derive macro and the method in encodable trait

///
/// See [Self::encode_for_signing] for more information on the encoding format.
pub fn signature_hash(&self) -> B256 {
let mut buf = bytes::BytesMut::with_capacity(self.payload_len_for_signature());
Copy link
Member

Choose a reason for hiding this comment

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

This should just be Vec

@@ -49,6 +251,175 @@ pub struct EIP2930TransactionRequest {
pub access_list: AccessList,
}

impl Encodable for EIP2930TransactionRequest {
fn encode(&self, out: &mut dyn BufMut) {
self.chain_id.encode(out);
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

@@ -63,6 +434,183 @@ pub struct EIP1559TransactionRequest {
pub access_list: AccessList,
}

impl Encodable for EIP1559TransactionRequest {
fn encode(&self, out: &mut dyn BufMut) {
Copy link
Member

Choose a reason for hiding this comment

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

same

DaniPopes and others added 4 commits December 18, 2023 14:21
* chore: remove self: sync from defs

* chore: remove most generics

* chore: default for BlockId

* chore: remove unnecesary reassignment

* chore: auto impl on mut/rc

* chore: unnecesary assignment
Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

Alright let's send it. @Evalir you wanna take any follow-ups to this that we didn't address yet?

@gakonst gakonst merged commit 4198132 into main Dec 18, 2023
17 checks passed
@gakonst gakonst deleted the onbjerg/alloy-temp-provider-trait branch December 18, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants