-
Notifications
You must be signed in to change notification settings - Fork 2
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: new userOp structure for EPv0.7 #677
Conversation
@veljkovranic please add instructions how do I run the Nexus SDK and test the new v7 endpoints. |
$match: { | ||
$and: [ | ||
{ "metaData.dappAPIKey": bundlerApiKey }, | ||
{ creationTime: { $gte: startTime } }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check if we have Mongo indexes on these fields?
It's also a good opportunity to setup a connection to our MongoDB and browse the data if you didn't do it so far.
@TakGN can give you instructions how to connect to the prod database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this is a lot of work an a great job.
I left some less important "style" related comments that you can optionally take into account.
Only two things that worry me:
- The relayer manager issue I mentioned
- I need instructions how to test this locally with Nexus
if (!userOp.paymasterAndData) { | ||
userOp.paymasterAndData = "0x"; | ||
} | ||
// // for userOp completeness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we commenting this out instead of deleting?
@@ -19,12 +19,6 @@ | |||
59144, 421614, 11155111, 1115, 3441005, 84532, 168587773, 80085, 534351, | |||
56400, 11155420, 80002 | |||
], | |||
"nonRM2SupportedNetworks": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@veljkovranic I think that we can safely delete the whole statis-config.json
file, it was used in the past and was left in the codebase just for reference.
log.info( | ||
`No user op found for transactionId: ${transactionId} on chainId: ${this.chainId}`, | ||
); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is a bit unreadable and looks like a 🎄 but it's not that important, we can leave it for later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the good comments in code and docstrings! 🏆
📖 Context
Type of change
Why are we doing this?
ERC-4337 got a new update with EntryPoint v0.7, which affected the UserOps structure.
The new object doesn't contain initCode and paymasterAndData fields (as in used to up to EPv0.6),
and it introduced new fields: factor, factoryData, paymaster, paymasterVerificationGasLimit, paymasterPostOpGasLimit, and paymasterData.
All this can be found at https://eips.ethereum.org/EIPS/eip-4337#useroperation
Related issues:
link to issue
What did we do?
How Has This Been Tested?
Tested this manually by sending a user op on the Base Sepolia network
(here is a link to the successful transactions against EPv0.7 contract - https://sepolia.basescan.org/tx/0xb48b098665b1d74a8d26e02a8549af270113cf147232de08088406de28fa87e3 )
More thorough testing is still pending (tbd)
👀 How do I review this?
Instructions for calling new set of endpoints are here:
git clone https://github.com/bcnmy/biconomy-client-sdk.git
git checkout feat/nexus-pm
setup
in .env
bun install
bun run build
bun run scripts/send:userOp.ts