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: add support for eip5792 getCapabilities and sendCalls #2576

Merged
merged 61 commits into from
Aug 6, 2024
Merged

Conversation

tomiir
Copy link
Collaborator

@tomiir tomiir commented Jul 22, 2024

Description

  • Adds new RPC request format with better dev-ex and fixes for concurrent requests
  • Adds new EIP 5792 RPC request methods
  • Adds RPC tests

Supersedes
#2527
#2320

Type of change

  • Chore (non-breaking change that addresses non-functional tasks, maintenance, or code quality improvements)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • Code in this PR is covered by automated tests (Unit tests, E2E tests) => Wallet calls need to spend gas :|
  • My changes generate no new warnings
  • I have reviewed my own code
  • I have filled out all required sections

@@ -47,6 +48,10 @@ export function EthersGetCallsStatusTest() {
}
}
function isGetCallsStatusSupported(): boolean {
// Replace with capability check
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it a todo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added a proper comment for following prs

@@ -98,7 +112,8 @@ export function EthersSendCallsWithPaymasterServiceTest() {
)
}

return false
// Replace with capability check
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, is it a todo? Should be replaced with something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

If it requires more polishing or would be handled in another PR etc, would it be better to give more context to devs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually a todo yes. Ty

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually there's a typing issue. I'll change this in a different PR 🙏
Would like to get this merged

@tomiir tomiir merged commit ccd6147 into main Aug 6, 2024
11 checks passed
@tomiir tomiir deleted the feat/eip-5792 branch August 6, 2024 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants