-
Notifications
You must be signed in to change notification settings - Fork 129
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
Add Main Paymaster Example Components #495
Conversation
85dfebd
to
4050d09
Compare
'<rootDir>/app/api/paymasterProxy', | ||
'<rootDir>/app/paymaster-bundler/utils/', |
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.
These are so specific because some unrelated tests are failing which will need to be fixed. Once the tests are fixed I can update this to be <rootDir>/app/
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.
Interesting..... if that's the case also open a public issue on Github so we don't forget to fix it.
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.
Tracking here #496
import { baseSepolia } from 'viem/chains'; | ||
|
||
const paymasterService = process.env.NEXT_PUBLIC_PAYMASTER_URL ?? ''; |
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.
Was this in the config?
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 was originally in the constants.ts
file. I figured that it wasn't necessary to assign a const to the env variable when I can just access the env whenever I needed it.
It looked like:
const PAYMASTER_SERVICE_URL = process.env.NEXT_PUBLIC_PAYMASTER_URL
chainId, | ||
entrypoint, | ||
userOp, | ||
}: { |
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.
Types are easier to read when are more explicit as their own variable.
like
type dude = { ... }
and than you use dude
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.
Updated to look like:
type ChainId = number;
type Entrypoint = string;
type UserOp = UserOperation<'v0.6'>;
type WillSponsorParams = {
}: {
chainId: ChainId;
entrypoint: Entrypoint;
userOp: UserOp;
};
export const willSponsor = async ({
chainId,
entrypoint,
userOp,
}: WillSponsorParams)
if (calldata.functionName !== 'executeBatch') return false; | ||
if (!calldata.args || calldata.args.length == 0) return false; | ||
|
||
const calls = calldata.args[0] as { |
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.
Each type should have his own variable.
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.
Updated to:
type CallDataTargetAddress = Address;
type CallDataValue = bigint;
type CallData = Hex;
const calls = calldata.args[0] as {
target: CallDataTargetAddress;
value: CallDataValue;
data: CallData;
}
* Checks if the call data in the user operation is valid and meets sponsorship criteria. | ||
* | ||
*/ | ||
function isValidCallData(userOp: UserOperation<'v0.6'>): boolean { |
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.
UserOperation<'v0.6'>
what's this?
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.
I know this looks weird but it's a type that UserOperation
extends called EntryPointVersion
...there is a type error if this is omitted.
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.
From the documentation - export type EntryPointVersion = "v0.6" | "v0.7"
|
||
/** | ||
* Checks if the call data in the user operation is valid and meets sponsorship criteria. | ||
* |
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 an empty string?
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.
I don't follow, should I add more to the function comment?
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.
Wait - Updated to remove the empty * line
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.
template/web/app/paymaster-bundler/_components/PaymasterBundlerDemo.tsx
Outdated
Show resolved
Hide resolved
template/web/app/paymaster-bundler/_components/PaymasterBundlerDemo.tsx
Outdated
Show resolved
Hide resolved
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.
7e7c9b6
to
8fd3286
Compare
8fd3286
to
44826a9
Compare
import { ENTRYPOINT_ADDRESS_V06 } from 'permissionless'; | ||
import { paymasterActionsEip7677 } from 'permissionless/experimental'; | ||
import { createClient, http } from 'viem'; | ||
import { createPublicClient } from 'viem'; |
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 two lines to import from viem
, just do one line.
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.
Whoops, fixed!
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.
- @
What changed? Why?
Notes to reviewers
How has it been tested?