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

Update EIP-5792: Propose changes to EIP-5792 #8826

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+54 −23
Open

Update EIP-5792: Propose changes to EIP-5792 #8826

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40d79c7
Propose changes to EIP-5792
forshtat Aug 25, 2024
a97f3a2
Update EIPS/eip-5792.md
forshtat Aug 26, 2024
d14ce17
Rename status response field from 'capabilitiesData' to 'capabilities…
forshtat Sep 4, 2024
0ecea7b
Change batch identifier size from 32 bytes to 64 bytes
forshtat Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 52 additions & 21 deletions EIPS/eip-5792.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,33 +35,41 @@ We also define one capability expression for use with the fourth method, which f

### `wallet_sendCalls`

Requests that a wallet submits a batch of calls. `from` and `chainId` are top-level properties rather than per-call properties because all calls should be sent from the same sender and on the same chain, identified by [EIP-155](./eip-155.md) integers expressed in hexadecimal notation. The items in the `calls` field are only those that are shared by all transaction types. Any other fields that a wallet may need to submit a transaction should be handled by the wallet.
Requests that a wallet submits a batch of calls. `from` and `chainId` are top-level properties rather than per-call properties because all calls should be sent from the same sender and on the same chain, identified by [EIP-155](./eip-155.md) integers expressed in hexadecimal notation.
The items in the `calls` field are simple `{to, data, value}` tuples.

The capabilities field is how an app can communicate with a wallet about capabilities a wallet supports. For example, this is where an app can specify a paymaster service URL from which an [ERC-4337](./eip-4337.md) wallet can request a `paymasterAndData` input for a user operation.

Each capability defined in the "capabilities" member can define global or call specific fields.
These fields are set inside this capability's entry in the `capabilitiesData` object.
Each entity in the `calls` field may contain an optional `capabilitiesData` object.
This object allows the applications to attach a capability-specific metadata to individual calls.

The wallet:

* MUST send the calls in the order specified in the request
* MUST send the calls on the same chain identified by the call's `chainId` property
* MUST send the calls on the same chain identified by the request's `chainId` property
* MUST NOT await for any calls to be finalized to complete the batch
* MUST NOT send any calls from the request if the user rejects the request
* MAY revert all calls if any call fails
* MAY send all calls as part of one or more transactions, depending on wallet capability
* SHOULD stop executing the calls if any call fails
* MAY reject the request if the from address does not match the enabled account
* MAY reject the request if one or more calls in the batch is expected to fail, when simulated sequentially
* MUST reject the request if it contains a `capability` that is not supported by the wallet
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I wonder if this is best 🤔 . E.g. why not allow specifying a paymaster which the app would like to be used but is not a necessity.

Copy link
Contributor

Choose a reason for hiding this comment

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

How a wallet can tell if the cap (e.g. paymaster) is required by this app or optional?
The app can always query the wallet caps first, to decide how to continue.

Copy link
Contributor

Choose a reason for hiding this comment

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

capability specs should specify whether they're required or not / how optional ones are handled. paymaster, for example, notes that the app must not assume the provided paymaster service is the one that ends up being used.

so if an app wants the capability to be required, the capability spec should include a required field, and that is how a wallet can tell if the capability is required by the app. i dont think we should blanket all capabilities to be required by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two issues raised here:

  1. App providing a "capability" that is not supported by the wallet:

There is a wallet_getCapabilities method that provides the application with a list of supported "capabilities".
Shouldn't the app that ignores that list and submits a wallet_sendCalls with a "capability" that is not supported be considered to behave incorrectly? I think it should reject the call, which would prevent dapps from yoloing their sendCalls requests.

  1. Wallet discretion to use or ignore a supported "capability" requested by the dapp:

As the API defined here deals with real value and assets, making capabilities required by default makes a lot of sense to me. If using a paymaster for a transaction is indeed optional, it is trivial to specify optional: true, but forgetting to set required: true for a security-oriented capability may be catastrophic.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

So @forshtat is the conclusion here that each capability could have an optional: flag? But passing a capability the wallet doesn't have would be hard failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So @forshtat is the conclusion here that each capability could have an optional: flag? But passing a capability the wallet doesn't have would be hard failure?

In my personal opinion, yes, this seems pretty reasonable.


#### `wallet_sendCalls` RPC Specification

```typescript
type SendCallsParams = {
version: string;
from: `0x${string}`;
chainId: `0x${string}` | undefined; // Hex chain id
calls: {
to?: `0x${string}` | undefined;
data?: `0x${string}` | undefined;
value?: `0x${string}` | undefined; // Hex value
chainId?: `0x${string}` | undefined; // Hex chain id
capabilitiesData?: Record<string, any> | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, I don't love how this kind of muddles the simplicity of the calls struct. Is the complaint that capabilities makes it hard to specify info on a per call basis. Could we get a better example of when this would be needed 🤔

One example could be an "optional flag" capability that would allow a Smart Contract Wallet to proceed with the batch in case of a revert

This should be possible with a general capability? Or you're saying one call in particular can revert?

Copy link
Contributor

Choose a reason for hiding this comment

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

You don't have to implement it, if our wallet doesn't have any per-call capability.
Also, an app doesn't have to know about it, unless it needs that capability.
The idea that if someone ever want to add per-call capability, it doesn't mess directly with the "call" structure, or with such params of another capability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the complaint that capabilities makes it hard to specify info on a per call basis?

Yes, that pretty much sums it up. Of course, the capability could provide an array of details, which would have to match the calls array's length exactly and contain null values for calls that do not require extra parameters, which is functionally equivalent but seems like a much more quirky API to me.

Or you're saying one call in particular can revert?

In my example, yes, I was talking about an "atomic" batch with a single "optional" call that should not revert the batch;
It is hard to reason about potential future additional examples, so these are a little hypothetical, but in my opinion, worth considering:
If a "per-call gas limit capability" is defined, it can be used to provide a per-call gas limit;
In case a "multi-chain capability" is defined, the 'capabilitiesData' field can be used to specify a per-call 'chainId' and L2-specific parameters, something like gas_per_pubdata_limit;
In case a "conditional call capability" is defined, it can provide some description of a condition to include or exclude a call from the batch;
etc.

Does this make sense? Or should dapps never need to go to the trouble of defining per-call parameters for a sendCalls request?

}[];
capabilities?: Record<string, any> | undefined;
};
Expand All @@ -76,18 +84,17 @@ type SendCallsResult = string;
{
"version": "1.0",
"from": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
"chainId": "0x01",
"calls": [
{
"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
"value": "0x9184e72a",
"data": "0xd46e8dd67c5d32be8d46e8dd67c5d32be8058bb8eb970870f072445675058bb8eb970870f072445675",
"chainId": "0x01",
"data": "0xd46e8dd67c5d32be8d46e8dd67c5d32be8058bb8eb970870f072445675058bb8eb970870f072445675"
},
{
"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
"value": "0x182183",
"data": "0xfbadbaf01",
"chainId": "0x01",
"data": "0xfbadbaf01"
}
],
"capabilities": {
Expand All @@ -102,17 +109,24 @@ type SendCallsResult = string;

##### `wallet_sendCalls` Example Return Value

The identifier can be any string. The only requirement is that for a given session, users should be able to call `wallet_getCallsStatus` with this value and expect a call-batch status to be returned.
The identifier MUST be unique 32 bytes represented as a hex encoded string.
For a given session, users should be able to call `wallet_getCallsStatus` with this value and expect a call-batch status
to be returned.

```json
"0xe670ec64341771606e55d6b4ca35a1a6b75ee3d5145a99d05921026d1527331"
```

### `wallet_getCallsStatus`

Returns the status of a call batch that was sent via `wallet_sendCalls`. The identifier of the transaction is the value returned from the `wallet_sendCalls` RPC. Note that this method only returns a subset of the fields that `eth_getTransactionReceipt` returns, excluding any fields that may differ across wallet implementations.
Returns the status of a call batch that was sent via `wallet_sendCalls`.
The identifier of the batch is the value returned from the `wallet_sendCalls` RPC.
Note that the `receipts` objects of this method's response is a strict subset of the object returned by `eth_getTransactionReceipt`.

The `capabilitiesData` object allows the wallets to attach a capability-specific metadata to the response.

* If a wallet does not execute multiple calls atomically (i.e. in multiple transactions), the receipts in the `receipts` field MUST be in order of the calls sent.
* The receipts in the `receipts` field MUST be in order they are included on-chain.
* The `receipts` field MUST contain receipts for all calls in a batch that were included on-chain, including reverted ones.
* If a wallet executes multiple calls atomically (i.e. in a single transaction), `wallet_getCallsStatus` MUST return a single receipt, corresponding to the transaction in which the calls were included.
* The `logs` in the receipt objects MUST only include logs relevant to the calls submitted using `wallet_sendCalls`. For example, in the case of a transaction submitted onchain by an [ERC-4337](./eip-4337.md) bundler, the logs must only include those relevant to the user operation constructed using the calls submitted via `wallet_sendCalls`. I.e. the logs should not include those from other unrelated user operations submitted in the same bundle.

Expand All @@ -122,26 +136,41 @@ Returns the status of a call batch that was sent via `wallet_sendCalls`. The ide
type GetCallsParams = string;

type GetCallsResult = {
status: 'PENDING' | 'CONFIRMED';
batchId: `0x${string}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

callsId

Copy link
Contributor

Choose a reason for hiding this comment

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

should keep the name the same

chainId?: `0x${string}`;
batchStatus: string; // See "Status Codes"
receipts?: {
logs: {
address: `0x${string}`;
data: `0x${string}`;
topics: `0x${string}`[];
}[];
status: `0x${string}`; // Hex 1 or 0 for success or failure, respectively
chainId: `0x${string}`;
blockHash: `0x${string}`;
blockNumber: `0x${string}`;
gasUsed: `0x${string}`;
transactionHash: `0x${string}`;
}[];
capabilitiesData?: Record<string, any> | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

I find it kinda confusing that this is the same phrase as on the request, but would likely be different data?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it is per-capability result data, in the results structure. Should it be named capabilitiesResultsData ?
I think it is clear enough, simply because it is in the response .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fully support renaming this parameter. capabilitiesResultsData is a bit long, how about just capabilitiesResults or capabilitiesStatus?

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer clarity even if verbose so I think I like capabilitiesResultsData

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer clarity even if verbose so I think I like capabilitiesResultsData

Done.

};
```

##### Status Codes for `batchStatus` field
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should define what a batch is first? it's the first time we use it


The purpose of the `batchStatus` field is to provide a short summary of the current status of the batch.
It provides some off-chain context to the array of inner transaction `receipts`.

| Name | Description |
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this idea, thoughts @jxom @lukasrosario ?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah i think this makes sense if we're talking about single chain. would be tough if multichain.

also not clear to me where DISCARDED would come up. @forshtat can you share more on how you were thinking this might come up?

Copy link
Contributor Author

@forshtat forshtat Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, it sounds similar to "FAILURE" but the difference I had in mind was that a "FAILURE" means the action failed due to on-chain reverts, and "DISCARDED" means the wallet gave up on trying to send the batch to the network.
Maybe the names do not make the distinction clear enough, or do you think the distinction is not worth the extra status code?

One example could be a batch that specifies a paymasterService capability, and given the wallet need to go through these steps before sending anything on-chain:
Screenshot 2024-08-28 at 18 16 29

The wallet cannot block the wallet_sendCalls() for all this time, so returns some identifier and proceeds with preparing the UserOperation.
During the preparation phase the wallet_getCallsStatus returns PENDING as nothing really happened.
However, if the "Paymaster Service" fails with an error at some stage, wallet could keep retrying to reach it for some time, and the transaction has not technically "failed", but after 2 minutes of trying to reach the "Paymaster Service" it gives up. I thought that this action's status should be marked as "DISCARDED" instead of "FAILURE".

Another example is a UserOperation that has some condition in its validation function. Canonical ERC-4337 limits this use-case a lot, but does not completely prevent it. A wallet may want to accept a batch that is not yet valid, check if it has become valid during the next 2 hours, and mark the entire operation as "DISCARDED" after that.

Does this makes sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

yet another example of "discarded": assume you put a maxFeePerGas that is too low, and after sometime , because the network gas raised, the maxFee becomes too low below the baseFee, and the transaction is completely dropped. If we are sending a "sequence", it can happen to any transaction along the sequence.
Now this transaction didn't technically "fail", since it didn't get anything on-chain - but definitely there's no reason to continue and wait for it.

|-----------|----------------------------------------------------------------------------------------------------------------------|
| PENDING | Batch has been received by the wallet but has not completed execution on-chain |
Copy link

@ajhodges ajhodges Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

How do you feel about possibly expanding PENDING to capture more of the offchain context around the pending state? How can a caller know if it needs to retry a batch with a higher gas price, or if there is some transient infra issue with getting batches included?

For example

  • PENDING: Batch has been received by the wallet but has not completed execution on-chain
  • UNDERPRICED or PENDING_RETRYABLE: Batch has been received by the wallet but the gas prices are too low for the current chain conditions
  • STALLED: Batch has been received by the wallet but is stuck for non-pricing related reasons; i.e. ERC-4337 bundler is down or the chain has halted. The definition of 'stuck' is left up to the wallet, i.e. batch is priced appropriately but has not been included in >60s

Copy link
Contributor

Choose a reason for hiding this comment

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

@forshtat would be good to get your thoughts here. imo UNDERPRICED specifically seems like a reasonable addition, as it could be retried.

@ajhodges i'm not a huge fan of the ambiguity for STALLED though. to me this is effectively still PENDING, and each wallet should have their own interpretations of PENDING taking "too long". if we and others feel strongly about it being its own status, i would prefer to be more opinionated in the spec and define when a PENDING batch becomes STALLED, instead of leaving it open-ended.

Copy link
Contributor

@jxom jxom Nov 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What if we just left it as PENDING and CONFIRMED, and had a sibling statusCode or detail property that could be extensible enough to accommodate for what ever addition?
This would make it a lot easier to coerce on frontend apps.

| SUCCESS | Batch has been included on-chain without reverts, receipts array contains info of all calls |
| PARTIAL | Batch has been included on-chain only partially, the rest is either reverted or invalid. Receipts array contains info of all on-chain included calls |
| FAILURE | Batch has been invalid or reverted completely and only changes related to gas charge may have been included on-chain |
| DISCARDED | Batch has not been included on-chain and wallet will not retry |

##### `wallet_getCallsStatus` Example Parameters

As with the return value of `wallet_sendCalls`, the batch identifier may be any string.
The batch identifier is a unique 32 bytes represented as a hex encoded string returned from `wallet_sendCalls`.

```json
[
Expand All @@ -153,7 +182,9 @@ As with the return value of `wallet_sendCalls`, the batch identifier may be any

```json
{
"status": "CONFIRMED",
"chainId": "0x01",
"batchId": "0xe670ec64341771606e55d6b4ca35a1a6b75ee3d5145a99d05921026d1527331",
"batchStatus": "SUCCESS",
"receipts": [
{
"logs": [
Expand All @@ -166,7 +197,6 @@ As with the return value of `wallet_sendCalls`, the batch identifier may be any
}
],
"status": "0x1",
"chainId": "0x01",
"blockHash": "0xf19bbafd9fd0124ec110b848e8de4ab4f62bf60c189524e54213285e7f540d4a",
"blockNumber": "0xabcd",
"gasUsed": "0xdef",
Expand Down Expand Up @@ -198,7 +228,7 @@ This method accepts a call bundle identifier returned by a `wallet_sendCalls` ca

### `wallet_getCapabilities`

This RPC allows an application to request capabilities from a wallet (e.g. batch transactions, paymaster communication), without distinct discovery and permission requests. For more on the difference between requesting capabilities and discoverying features, see the ["Privacy Considerations" section](#privacy-considerations).
This RPC allows an application to request capabilities from a wallet (e.g. batch transactions, paymaster communication), without distinct discovery and permission requests. For more on the difference between requesting capabilities and discovering features, see the ["Privacy Considerations" section](#privacy-considerations).

This method SHOULD return an error if the user has not already authorized a connection between the application and the requested address.

Expand All @@ -212,17 +242,18 @@ If any of these supplemental expressions of capabilities are contradicted by cap

Capabilities are returned in key/value pairs, with the key naming a capability and a value conforming to a shape defined for that name, in an object keyed to the relevant [EIP-155](./eip-155.md) `chainId` expressed in hexadecimal notation.
Capabilities are nested in per-chain objects because wallets may support different capabilities across multiple chains authorized in a given session.
All values for the "capability" fields MUST contain Semantic Version value `version`.

```typescript
type GetCapabilitiesParams = [`0x${string}`]; // Wallet address
type GetCapabilitiesParams = [`0x${string}`, [`0x${string}`]]; // Wallet address, array of queried chain ids (optional)

type GetCapabilitiesResult = Record<`0x${string}`, <Record<string, any>>; // Hex chain id
```

##### `wallet_getCapabilities` Example Parameters

```json
["0xd46e8dd67c5d32be8058bb8eb970870f07244567"]
["0xd46e8dd67c5d32be8058bb8eb970870f07244567", ["0x2105", "0x14A34"]]
```

##### `wallet_getCapabilities` Example Return Value
Expand All @@ -233,15 +264,15 @@ The capabilities below are for illustrative purposes.
{
"0x2105": {
"paymasterService": {
"supported": true
"version": "1.0.0"
},
"sessionKeys": {
"supported": true
"version": "1.0.0"
}
},
"0x14A34": {
"paymasterService": {
"supported": true
"version": "1.0.0"
}
}
}
Expand Down
Loading