Skip to content

Add ERC: Paymaster Web Service Capability - #360

Merged
eip-review-bot merged 30 commits into
ethereum:masterfrom
lukasrosario:paymaster-service-capability
Jun 6, 2024
Merged

eip-review-bot merged 30 commits into
ethereum:masterfrom
lukasrosario:paymaster-service-capability

Conversation

@lukasrosario

@lukasrosario lukasrosario commented Apr 4, 2024 •

Copy link
Copy Markdown
Contributor

Defines an EIP-5792 capability for paymaster web services along with a proposal for standardized paymaster web service RPC methods.

https://ethereum-magicians.org/t/add-erc-paymaster-web-service-capability/19530

@eip-review-bot

eip-review-bot commented Apr 4, 2024 •

Copy link
Copy Markdown
Collaborator

✅ All reviewers have approved.

@eip-review-bot eip-review-bot changed the title Add ERC: Paymaster Service Capability Add ERC: Paymaster Web Service Capability Apr 4, 2024
@lukasrosario
lukasrosario marked this pull request as ready for review April 4, 2024 02:39
@github-actions github-actions Bot added the w-ci label Apr 4, 2024
Comment thread ERCS/erc-draft_pm_capability.md Outdated
Comment thread ERCS/erc-draft_pm_capability.md Outdated
Comment thread ERCS/erc-draft_pm_capability.md Outdated
Comment thread ERCS/erc-draft_pm_capability.md Outdated
Comment thread ERCS/erc-draft_pm_capability.md Outdated
@github-actions github-actions Bot added w-ci and removed w-ci labels Apr 5, 2024
@github-actions

github-actions Bot commented Apr 5, 2024

Copy link
Copy Markdown

The commit cfc41e4 (as a parent of 45e5f03) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions Bot added w-ci and removed w-ci labels Apr 5, 2024
Comment thread ERCS/erc-7677.md
Comment thread ERCS/erc-7677.md Outdated
Comment thread ERCS/erc-7677.md
Comment thread ERCS/erc-7677.md Outdated
Comment thread ERCS/erc-7677.md Outdated
@lukasrosario
lukasrosario force-pushed the paymaster-service-capability branch from 7614e96 to d90e25f Compare April 9, 2024 14:17
Comment thread ERCS/erc-7677.md Outdated
Comment thread ERCS/erc-7677.md
Comment thread ERCS/erc-7677.md Outdated
Comment thread ERCS/erc-7677.md Outdated
Comment thread ERCS/erc-7677.md
Comment thread ERCS/erc-7677.md
}
```

###### `pm_getPaymasterData` Example Parameters

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Paymaster services are supposed to detect which account implementation is it from the sender's address?
otherwise how would one decode data based on policy-id?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

detecting the account implementation is not a strict requirement of paymaster services, and it is ultimately up to paymaster services to decide how to validate policy ids or whatever other abstractions they might have.

you could reliably do this without needing to know the implementation though by simulating the incoming user op, no need to decode the calldata (although you could if you know the implementation).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't this be a requirement as it is mentioned that
Paymaster services MUST detect which entrypoint version the account is using and return the correct fields.
how is a paymaster supposed to detect this without knowing the account implementation?
Afaik all smart accounts store the supported entry point on the smart account in a simple variable and reading that might be the best way to know this information.
To make that possible I think the paymaster services should have some knowledge about the implementation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

paymaster knows the version from the entrypoint address (in any case, a paymaster support explicit entrypoint instance, not "any".
it would still be nice if at the API level a "version" field is added..

@livingrockrises livingrockrises left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

review i

Comment thread ERCS/erc-7677.md
type GetPaymasterStubDataParams = [
// Below is specific to Entrypoint v0.6 but this API can be used with other entrypoint versions too
{
sender: `0x${string}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

these are the fields of UserOperation v0.6.
On v0.7, there are few differences (factory, factoryData, paymaster fields, packed gas fields).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

right, called out in the comment but can add the v0.7 fields to make it more clear that this is for any entrypoint version

Comment thread ERCS/erc-7677.md
maxFeePerGas: `0x${string}`;
maxPriorityFeePerGas: `0x${string}`;
}, // userOp
`0x${string}`, // EntryPoint

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it is better to have named parameters instead of indexed.
e.g.

{ userOp: {}, chainId: "0x..", entryPoint: "0x...", version: "0.7", context: {} }

And also be better to add "version", instead of rely on entrypoint address

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

curious about your reasoning here. i like the clarity of named arguments but most ethereum-related JSON-RPC methods i can think of use positional arguments. is there a reason you think we should stray from that loose convention other than clarity?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same question for the version arg, is that just for clarity?

Comment thread ERCS/erc-7677.md
"paymaster": "0x...",
"paymasterData": "0x...",
"paymasterVerificationGasLimit": "0x...",
"paymasterPostOpGasLimit": "0x..."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in v0.7, postOpGasLimit cannot be estimated by app/wallet, and must be returned by the serve
verificationGasLimit MAY be estimated, though if the service knows the value, it can place it anyway.

Comment thread ERCS/erc-7677.md
}
```

###### `pm_getPaymasterData` Example Parameters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

paymaster knows the version from the entrypoint address (in any case, a paymaster support explicit entrypoint instance, not "any".
it would still be nice if at the API level a "version" field is added..

Comment thread ERCS/erc-7677.md

```json
{
"paymaster": "0x...",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are cases where the stub/final separation is not needed, e.g: when the paymaster is a TokenPaymaster, and the paymasterData doesn't contain "signature".
In such cases, the 2nd RPC call to the paymaster MAY be skipped, by returning isFinal: true by this RPC call.

Comment thread ERCS/erc-7677.md

Returns values to be used in paymaster-related fields of a signed user operation. These are not stub values and will be used during user operation submission to a bundler. Similar to `pm_getPaymasterStubData`, accepts an unsigned user operation, entrypoint address, chain id, and a context object.

This method also returns a `sponsor` object with `name` and `icon` fields. The `name` field is the name of the party sponsoring the transaction, and the `icon` field is a URI pointing to an image. Wallet developers MAY choose to display sponsor information to users. The `icon` string MUST be a data URI as defined in [RFC-2397]. The image SHOULD be a square with 96x96px minimum resolution. The image format is RECOMMENDED to be either lossless or vector based such as PNG, WebP or SVG to make the image easy to render on the wallet. Since SVG images can execute Javascript, wallets MUST render SVG images using the `<img>` tag to ensure no untrusted Javascript execution can occur.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is the sponsor structure a "MUST" ?

@lukasrosario lukasrosario May 20, 2024 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is not a "MUST". will clarify.

Comment thread ERCS/erc-7677.md
maxPriorityFeePerGas: `0x${string}`;
}, // userOp
`0x${string}`, // EntryPoint
`0x${string}`, // Chain ID

@arpan-jain arpan-jain May 27, 2024 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a specific reason, to prefix 0x in front of chainId, instead of plain string or a number?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

chain ids are uint256 so all possible values could not be represented by a js number, and hex is pretty standardized in such cases vs the number as a decimal string

Comment thread assets/erc-7677/0.png

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If you can replace these with SVGs, we'd appreciate it

@eip-review-bot
eip-review-bot enabled auto-merge (squash) June 6, 2024 23:29

@eip-review-bot eip-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eip-review-bot
eip-review-bot merged commit 34e7e10 into ethereum:master Jun 6, 2024

@2kxj2q6n7d-pixel 2kxj2q6n7d-pixel left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

J

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.