Update EIP-8141: allow payer to approve before sender#11575
Update EIP-8141: allow payer to approve before sender#11575eth-bot merged 1 commit intoethereum:masterfrom
Conversation
github-actions
Bot
added
c-update
|
✅ All reviewers have approved. |
eth-bot
changed the title
eth-bot
left a comment
eth-bot
left a comment
There was a problem hiding this comment.
All Reviewers Have Approved; Performing Automatic Merge...
|
|
||
| - `P256VERIFY` must reject invalid public keys, including points that are not on the P256 curve. | ||
| - For the P256 (r1) signature type, the sender address is `keccak(P256_ADDRESS_DOMAIN|qx|qy)[12:]`. | ||
| - The default-code payment path does not maintain payer-specific replay protection. A default-code account that approves payment before sender execution approval is replay-safe only if the same transaction later consumes the sender nonce, or if the payer accepts replay risk through some external mechanism. |
There was a problem hiding this comment.
The default code is not safe to use as the guarantor because it uses the canonical sig hash instead of the frame sig hash, so it doesn't know if the transaction will "later consume the sender nonce" since someone can replace the sender signature with an invalid one without invalidating the payer signature.
There was a problem hiding this comment.
Yeah that seems fine? Payer first seems like a quite specialized role, they can deploy an account to do this?
| ##### `VERIFY` Mode | ||
|
|
||
| Identifies the frame as a validation frame. Its purpose is to *verify* that a sender and/or payer authorized the transaction. It must call `APPROVE` during execution. Failure to do so will result in the whole transaction being invalid. | ||
| Identifies the frame as a validation frame. Its purpose is to *verify* that a sender and/or payer authorized the transaction. It must call `APPROVE` during execution. Failure to do so before payment has been approved will result in the whole transaction being invalid after payment has been approved, the frame is treated as a paid failure. |
There was a problem hiding this comment.
I think you meant to break this sentence?
Failure to do so before payment has been approved will result in the whole transaction being invalid after payment has been approved, the frame is treated as a paid failure.
There was a problem hiding this comment.
Also, if a VERIFY frame after payment has been approved does NOT call APPROVE at all, is the transaction valid or not? The rest of the spec would suggest that VERIFY frames MUST call approve, but the way this is written makes it sound like it's OK to not call APPROVE as long as payment has been approved.
My suggestion would be:
- Before payment is approved, VERIFY frames MUST successfully call
APPROVEfor the transaction to be valid - After payment is approved, VERIFY frames MUST call
APPROVEfor the transaction to be valid, butAPPROVEmay fail without invalidating the transaction.
Not sure if that's clear or confusing.
There was a problem hiding this comment.
Yeah that makes sense and should be the intention!
| 4. `pay` must execute in `VERIFY` mode and successfully call `APPROVE(APPROVE_PAYMENT)`. | ||
| 4. `pay` must execute in `VERIFY` or `DEFAULT` mode and successfully call `APPROVE(APPROVE_PAYMENT)`. | ||
| - If `pay` targets a canonical paymaster instance, the validation prefix must be a sender-first paymaster prefix and `pay` must execute in `VERIFY` mode. | ||
| - If `pay` appears in a payer-first prefix, it must not target a canonical paymaster instance. |
There was a problem hiding this comment.
Shouldn't we update the canonical paymaster code so it can be safely used in a payer-first txn? Otherwise the usefulness of this update is severely limited, since the public mempool only allows 1 pending txn per non-canonical paymaster.
I implemented a replay protection mechanism here: #11555. Though since we don't have an explicit approval scope for guarantors in this PR, the guarantor code path would need to have the additional logic of checking its frame position.
There was a problem hiding this comment.
I'm not against adding it to the canonical paymaster, but I think there should be a strong reason to do so at this stage. We can also always add later after 8141 is live if there is demand.
| - execution uses a banned opcode | ||
| - execution performs a state write, except deterministic deployment performed by the first `deploy` frame through a known deployer | ||
| - execution reads storage outside `tx.sender` | ||
| - execution performs a state write, except deterministic deployment performed by the first `deploy` frame through a known deployer, or payer-owned replay-protection writes performed by an admitted `pay` frame |
There was a problem hiding this comment.
It's not helpful to specify "replay-protection writes" right? Since it's not clear how the client would enforce that. I think we just need to make the canonical paymaster handle replay, and canonical paymaster can do state writes because of the canonical paymaster exception.
| - execution performs a state write, except deterministic deployment performed by the first `deploy` frame through a known deployer | ||
| - execution reads storage outside `tx.sender` | ||
| - execution performs a state write, except deterministic deployment performed by the first `deploy` frame through a known deployer, or payer-owned replay-protection writes performed by an admitted `pay` frame | ||
| - execution reads storage outside `tx.sender`, except payer-owned storage read by an admitted `pay` frame for payment authentication and replay protection |
| - TSTORE (0x5D) | ||
|
|
||
| `SLOAD` can be used only to access `tx.sender` storage, including when reached transitively via `CALL*` or `DELEGATECALL`. | ||
| `SLOAD` can be used only to access `tx.sender` storage, including when reached transitively via `CALL*` or `DELEGATECALL`. A `pay` frame admitted under the paymaster rules may additionally read the paymaster's own storage needed to authenticate and replay-protect payment. |
| The canonical paymaster in this draft authorizes with a single secp256k1 signer via `ecrecover`, does not support contract-signature schemes, and may change in later specifications, in which case a new canonical implementation version would be required. | ||
|
|
||
| Because the canonical paymaster implementation is explicitly standardized to be safe for public mempool use, nodes do not need to apply the generic validation trace and opcode rules to that `pay` frame. Instead, they identify it by runtime code match and apply the paymaster-specific accounting and revalidation rules in this section. | ||
| The canonical paymaster is sender-first only for public mempool purposes. Its `pay` frame executes in `VERIFY` mode after an `only_verify` frame has approved execution. Its calldata is `r || s || v`, where `r` and `s` are 32 bytes each and `v` is one byte. The canonical paymaster signature is checked against the canonical signature hash (`TXPARAM(0x08)`). It does not maintain payer-owned replay protection; replay protection for the canonical sender-first flow comes from the sender nonce consumed by the preceding execution approval. |
There was a problem hiding this comment.
Per comments above, once we update the canonical paymaster to support the guarantor pattern, we don't need all these mentions of sender-first throughout the PR.
| ##### Non-canonical paymaster | ||
|
|
||
| For non-canonical paymasters, `pending_withdrawal_amount` is not meaningful since they may not support timelocked withdrawals. Instead, we keep the mempool safe by enforcing that each non-canonical paymaster can only be used with no more than `MAX_PENDING_TXS_USING_NON_CANONICAL_PAYMASTER` pending transactions. | ||
| For non-canonical paymasters, `pending_withdrawal_amount` is not meaningful since they may not support timelocked withdrawals. Instead, we keep the mempool safe by enforcing that each non-canonical paymaster can only be used with no more than `MAX_PENDING_TXS_USING_NON_CANONICAL_PAYMASTER` pending transactions. This limit also bounds the effect of non-canonical paymaster replay-state dependencies. |
There was a problem hiding this comment.
Per comments above, I think these mentions of replay-state may be unhelpful since it's not clear how they can be enforced.
|
|
||
| `FRAMEPARAM`, `FRAMEDATALOAD`, and `FRAMEDATACOPY` allow validation code to inspect other frames, including later `SENDER` frames and their `value`s. As a result, paymasters and other `VERIFY` frames can observe user operation parameters before approval and may condition their behavior on that information. Users should therefore treat non-`VERIFY` frame parameters and data as visible to validation logic and should not rely on untrusted paymasters or verifiers to keep such information private. | ||
|
|
||
| #### Payer-First Replay Protection |
There was a problem hiding this comment.
There's already a very similar section above; might wanna consolidate it.
|
reopened as draft #11580 |
3 participants
Alternative to #11555
I think it is simpler to just allow the payer to approve before the sender instead of adding the full guarantor role.