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.

Sign up for GitHub

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

Jump to bottom

feat: add erc4337 endpoint methods to provider #1176

Merged
merged 21 commits into from
Aug 26, 2024
Merged

feat: add erc4337 endpoint methods to provider #1176

merged 21 commits into from
Aug 26, 2024
+281 −45

Conversation

royvardhan
Copy link
Contributor

@royvardhan royvardhan commented Aug 22, 2024 •
edited
Loading

Motivation

Closes: #1111

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

mattsse
mattsse requested changes Aug 22, 2024
View reviewed changes
Copy link
Member

@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.

cool, I think the only thing missing is serde rename all camelcase

crates/rpc-types-eth/src/eip4337.rs Outdated Show resolved Hide resolved
crates/rpc-types-eth/src/eip4337.rs Outdated Show resolved Hide resolved
crates/rpc-types-eth/src/eip4337.rs Outdated Show resolved Hide resolved
@mattsse mattsse marked this pull request as ready for review August 22, 2024 08:23
mattsse
mattsse approved these changes Aug 22, 2024
View reviewed changes
Copy link
Member

@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.

lgtm, if you want you can add additional endpoints defined in the eip

@royvardhan
Copy link
Contributor Author

lgtm, if you want you can add additional endpoints defined in the eip

Thanks for approving, the test that I have added is a filler one and needs some touchups.

Okay, I will the additional endpoints. Thanks 🫡

@mattsse
Copy link
Member

mattsse commented Aug 23, 2024

this doesn't fully compile yet

@royvardhan
Copy link
Contributor Author

this doesn't fully compile yet

pushing fix

@onbjerg onbjerg changed the title feat: add eip4337 eth_sendUserOperation method to provider feat: add erc4337 eth_sendUserOperation method to provider Aug 23, 2024
onbjerg
onbjerg requested changes Aug 23, 2024
View reviewed changes
Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

please rename it from eip to erc

crates/provider/src/ext/eip4337.rs Outdated Show resolved Hide resolved
onbjerg
onbjerg requested changes Aug 23, 2024
View reviewed changes
Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

a more concrete set of changes needed - we need to distinguish that anything related to this is an erc. there is not an eip-4337. thanks for taking this on:)

crates/provider/Cargo.toml Outdated Show resolved Hide resolved
crates/provider/src/ext/eip4337.rs Outdated Show resolved Hide resolved
crates/provider/src/ext/eip4337.rs Outdated Show resolved Hide resolved
crates/provider/src/ext/eip4337.rs Outdated Show resolved Hide resolved
crates/provider/src/ext/eip4337.rs Outdated Show resolved Hide resolved
crates/provider/src/ext/mod.rs Outdated Show resolved Hide resolved
crates/rpc-types-eth/src/lib.rs Outdated Show resolved Hide resolved
@royvardhan
Copy link
Contributor Author

a more concrete set of changes needed - we need to distinguish that anything related to this is an erc. there is not an eip-4337. thanks for taking this on:)

Thank you for suggesting these changes. Actually, I saw a eip4337 file in the main branch and followed that naming. I have now renamed it as well.

onbjerg
onbjerg approved these changes Aug 23, 2024
View reviewed changes
Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

lgtm!

@onbjerg
Copy link
Member

onbjerg commented Aug 23, 2024

looks like the tests are failing - ptal

mattsse
mattsse requested changes Aug 24, 2024
View reviewed changes
crates/provider/src/ext/erc4337.rs Outdated Show resolved Hide resolved
@royvardhan
Copy link
Contributor Author

I have run these tests on a different RPC to check if the responses in the tests are valid and they are! Hopefully, the tests pass in the CI, otherwise, we remove them? Because people will be connecting alloy provider to RPCs that support the operation? Let’s see!

@royvardhan royvardhan changed the title feat: add erc4337 eth_sendUserOperation method to provider feat: add erc4337 endpoint methods to provider Aug 24, 2024
@royvardhan
Copy link
Contributor Author

@mattsse Build working.

mattsse
mattsse requested changes Aug 26, 2024
View reviewed changes
crates/provider/src/ext/erc4337.rs Outdated Show resolved Hide resolved
@onbjerg
Copy link
Member

onbjerg commented Aug 26, 2024

great! thanks for this

@mattsse mattsse merged commit 155914b into alloy-rs:main Aug 26, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@onbjerg onbjerg onbjerg approved these changes

@mattsse mattsse mattsse approved these changes

@zeroknots zeroknots zeroknots approved these changes

@DaniPopes DaniPopes Awaiting requested review from DaniPopes DaniPopes is a code owner

@gakonst gakonst Awaiting requested review from gakonst gakonst is a code owner

@prestwich prestwich Awaiting requested review from prestwich prestwich is a code owner

@emhane emhane Awaiting requested review from emhane emhane is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature] add RPC eth_sendUserOperation for ERC-4337 Bunder
4 participants
@royvardhan @mattsse @onbjerg @zeroknots