Update EIP-5792: Propose changes to EIP-5792

[forshtat]

List of changes proposed in this PR and their motivations:

1. Rephrase "The items in the calls field are only those that are shared by all transaction types" and "method only returns a subset of the fields that eth_getTransactionReceipt returns, excluding any fields that may differ across wallet implementations".

There is a rapidly growing list of transaction types, and the definition of the "shared by all" can become confusing.
Specifically, most transaction fields are removed in RIP-7560 transaction type which is part of the Account Abstraction roadmap.

2. Add optional 'capabilitiesData' field to individual 'calls' entities.

It seems likely that more advanced capabilities will need to provide some data on a per-call basis. 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, which would otherwise default to requiring success from all calls. Whether this particular capability is needed or not, it serves as an example of why such an option might be needed in the future.

3. Specify that an unsupported 'capability' cannot be ignored and the operation must be rejected.

I did not see it in the text anywhere, and ignoring data provided in the capabilities field is obviously extremely dangerous.

4. Add 'chainId' back to the 'SendCallsParams'.

For now, it is defined as optional, but probably shouldn't be. This has been debated lately and I don't know what the conclusion is.

5. Specify that the 'identifier' must be a 32 byte hex.

One reason is that "any string" seems to be too vague of a definition for an ERC. What about "", "ለከበሩጓደኛመርህመንገድይሆናል", or "😎🤏🕶😳", are these strings okay?
Also, in other programming languages it could be more convenient to use the same identifier type as for transactions, blocks and all other things in Ethereum. If there is a motivation for leaving it as "any string", please at least specify it in the ERC.

6. Added 'capabilitiesData' object to the 'wallet_getCallsStatus' response.

It seems very plausible that 'capabilities' will need to provide some additional information to the response. This was previously impossible, which could lead to wallets creating non-standard ways of querying that information.

7. Made the 'receipts' field requirements stricter.

These rules were probably implied anyway, but now they are explicit.

8. Rename 'status' to 'batchStatus'

There is a 'status' field in each 'receipt', which is a hex string. Having another 'status' field, that is not of the same type, appears to be confusing.

9. Specified 5 separate 'batchStatus' values

It seems like the previous two values did not cover the entire set of possible 'sendCalls' outcomes. The new status values are: PENDING, SUCCESS, PARTIAL, FAILURE and DISCARDED.

10. Added 'chainId' array to the 'wallet_getCapabilities' request

It seems like a wallet that supports a very long list of chains would be forced to return an unusually large JSON object as a response. However, as most dapps know which chains they may run on, it makes sense for them to limit the scope of the request.

11. Added SemVer 'version' parameter to capabilities.

It is not likely that all 'capabilities' will reach their final version from day one.
As there was no built-in mechanism for versioning, and having no centralized "capabilities authority", it would be pretty likely to see capabilities like 'paymasterServiceV2' and 'atomicBatch_3' proliferate, creating a lot of confusion.

[eth-bot]

File EIPS/eip-5792.md

Requires 1 more reviewers from @arein, @drortirosh, @jxom, @lukasrosario, @moodysalem, @wilsoncusack

[shahafn]

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

[wilsoncusack]

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?

[drortirosh]

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.

[forshtat]

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?

[wilsoncusack]

I like this idea, thoughts @jxom @lukasrosario ?

[lukasrosario]

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?

[forshtat]

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:

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?

[drortirosh]

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.

[wilsoncusack]

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.

[drortirosh]

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.

[lukasrosario]

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.

[forshtat]

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.

2. 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?

[wilsoncusack]

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?

[forshtat]

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.

[wilsoncusack]

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

[drortirosh]

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 .

[forshtat]

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

[wilsoncusack]

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

[forshtat]

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

Done.

[lukasrosario]

Specify that the 'identifier' must be a 32 byte hex.

@forshtat I'm ok with constraining it to hex, but 32 bytes makes things difficult. For example, in our case the calls id packs the chain id and user op hash together, which we wouldn't be able to do with 32 bytes. Especially with conversations around allowing calls across multiple chains, it would be inevitable that wallets would need to have some more complex state to store + report statuses, which they would not be too happy about.

how would you feel about hex of any length?

[forshtat]

Specify that the 'identifier' must be a 32 byte hex.

@forshtat I'm ok with constraining it to hex, but 32 bytes makes things difficult. For example, in our case the calls id packs the chain id and user op hash together, which we wouldn't be able to do with 32 bytes. Especially with conversations around allowing calls across multiple chains, it would be inevitable that wallets would need to have some more complex state to store + report statuses, which they would not be too happy about.

how would you feel about hex of any length?

Hmm. I assumed the identifier was just some kind of a hash of the request, so there is never a need to parse the identifier to learn any information, but to simply look it up in memory.
I guess the identifier could be used to contain some data, but intuitively it feels like a bit of an anti-pattern.
In your case the identity of a batch is {chainId, userOpHash}, right? Does it create a lot of overhead if you had to "remember" the chainId-of-the-userOpHash mapping for the period that the wallet_getCallsStatus is expected to remember the identifier?
If I recall correctly, the userOpHash includes a chainId anyways so there is basically no chance of hash collision between two User Operations on different chains.

Anyways, maybe instead of "any length" we should just pick a length that makes most sense and just left-pad it with zeroes?

[lukasrosario]

Does it create a lot of overhead if you had to "remember" the chainId-of-the-userOpHash mapping for the period that the wallet_getCallsStatus is expected to remember the identifier?

It does. Because in our case, and in the case of some other wallet teams we've been chatting with, getCallsStatus will route to a server so an app can get a status without opening an extension / popup / app. So either:

1. The relevant information needs to be packed into the identifier or
2. This server needs to store a mapping of an arbitrary identifier to the relevant ops.

Maybe it's inevitable, but we (and presumably other wallets) would like to avoid (2) if possible, as it would require introducing some storage into these servers.

64 bytes would be sufficient if:

1. We keep calls on a single chain and
2. We are talking about 4337 wallets.

While user op hashes hash over the chain id, we would still need to pack the chain id and user op hash together in the identifier so we know which bundler to call.

This fails about if we start to talk about making the RPC multichain though, as we could never guarantee all the info we need would fit into whatever max length we specify. It also does not work well for EOAs that would need to send multiple transactions for multiple calls and keep track of multiple transaction hashes.

[forshtat]

RE: 64 bytes would be sufficient if... This fails about if we start to talk about making the RPC multichain though, as we could never guarantee all the info we need would fit into whatever max length we specify.

Doesn't it raise a concern that the identifier size may end up growing out of control as long as extra information has to be injected into it? That basically the ability to use unrestrictedly large byte arrays as identifiers would be seriously abused?
Like, technically, Buffer.from(JSON.stringify(userOperationsArray)).toString('hex') would also be a valid "any length hex identifier", wouldn't it?

[lukasrosario]

Doesn't it raise a concern that the identifier size may end up growing out of control as long as extra information has to be injected into it?

Yeah, that's why I'm starting to think it's inevitable that wallets will need to have some type of storage in their backends. Could alternatively tag on extra calldata with relevant info (unfortunately at the cost of the user).

For now though assuming we stay with one chain as you have here can we say 64 bytes? This would be sufficient for wallets that can do atomic batching.

[forshtat]

For now though assuming we stay with one chain as you have here can we say 64 bytes?

Done.

[lukasrosario]

callsId

[lukasrosario]

should keep the name the same

[ajhodges]

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

[lukasrosario]

@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.

[jxom]

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.