Combined updates by Vectorized · Pull Request #11 · ithacaxyz/account

Vectorized · 2025-01-31T11:05:54Z

Combine user ops gas args into 1.
Make compensate function on Delegation for direct payment.
Make DS-style delegatecall, via ERC-7579.
Allow ERC-1271 isValidSignature to allow certain keys to validate, if msg.sender is appropriate. If the key is revoked, this entire mapping is discarded (we'll bump some storage pointer).
Make EIP-712 not require chain id or authorize only and revoke only.
Update Solady.
Ensure that we can sweep funds from EntryPoint.
Delegation and EntryPoint deployments.
Refactor to use Foundry scripting for deployments.

Vectorized · 2025-01-31T19:58:33Z

New Idea for UserOp.

I'm thinking of having a bytes32 paymentPriority field, which is given by

abi.encodePacked(
    address(priorityRecipient),
    uint40(priorityGatedUntil),
    uint40(lerpStart), 
    uint16(lerpDuration) 
)

When the block.timestamp <= priorityGatedUntil, only the priorityRecipient can get payment. If the paymentRecipient != priorityRecipient, the paymentRecipient will be overwritten to priorityRecipient.

The effectivePaymentMaxAmount will be linearly interpolated from 0 to paymentMaxAmount from lerpStart to lerpStart + lerpDuration. In a competitive filling environment, fillers might want to compete to be the first to fill a user op, and might overpay in gas. Also fillers would have no incentive to charge anything less than paymentMaxAmount.

Having a Reverse Dutch Auction will make fillers consider if they want to frontrun other fillers and accept a lower payment, incentivizing lower filling fees for users.

gakonst · 2025-02-01T00:39:08Z

+        bytes32 structHash = EfficientHashLib.hash(
+            uint256(EXECUTE_TYPEHASH),
+            nonce & 1,
+            uint256(a.hash()),
+            nonce,
+            _getDelegationStorage().nonceSalt
        );
+        return nonce & 1 > 0 ? _hashTypedDataSansChainId(structHash) : _hashTypedData(structHash);


Love this and we should document this technique really well IMO!

gakonst · 2025-02-01T00:49:12Z

+    function execute(bytes32 mode, bytes calldata executionData) public payable virtual override {
+        if (bytes1(mode) == 0xff) {
+            address target = address(bytes20(LibBytes.loadCalldata(executionData, 0x00)));
+            if (!_getDelegationStorage().approvedImplementations.contains(target)) {
+                revert Unauthorized();
+            }
+            bytes calldata data = LibBytes.sliceCalldata(executionData, 0x14);
+            assembly ("memory-safe") {
+                let m := mload(0x40)
+                calldatacopy(m, data.offset, data.length)
+                if iszero(delegatecall(gas(), target, m, data.length, codesize(), 0x00)) {
+                    returndatacopy(m, 0x00, returndatasize())
+                    revert(m, returndatasize())
+                }
+            }
+            return;
+        }
+        super.execute(mode, executionData);
+    }
+
+    function supportsExecutionMode(bytes32 mode) public view virtual override returns (bool) {
+        return LibBit.or(bytes1(mode) == 0xff, super.supportsExecutionMode(mode));
+    }


Is the goal of doing it with ERC7579 interface here to make it compatible with https://github.com/zkemail/email-recovery/blob/65b525af2e9dfbc8338b08b708b8912f1b81725e/src/modules/EmailRecoveryModule.sol#L97? Otherwise why do we need the mode flag? I originally thought this might be execute(address target,bytes calldata)

And we're basically reserving the 0xff mode for code delegation?

The ERC7579 standard has reserved 0xff it for delegate calls.

gakonst · 2025-02-01T00:54:50Z

+        /// @dev The combined gas limit for payment, verification, and calling the EOA.
+        uint256 combinedGas;


cleaner

Suggested change

/// @dev The combined gas limit for payment, verification, and calling the EOA.

uint256 combinedGas;

/// @dev The combined gas limit for payment, verification, and calling the EOA.

uint256 gasLimit;

gakonst · 2025-02-01T00:55:46Z

        uint256 paymentMaxAmount;
-        /// @dev The gas limit for the payment.
-        uint256 paymentGas;
-        /// @dev The gas limit for the verification.
-        uint256 verificationGas;
-        /// @dev The gas limit for calling the EOA.
-        uint256 callGas;
+        /// @dev The amount of ERC20 to pay per gas spent. For calculation of refunds.
+        /// If this is left at zero, it will be treated as infinity (i.e. no refunds).
+        uint256 paymentPerGas;


I was thinking we could remove the need for paymentAmount / paymentMaxAmount and only have the exchange rate. Still thinking about it though.

having a max is for safety reasons. Suppose someone wants to call a contract that uses very little gas in the general case, but spikes in gas usage in certain edge cases. (e.g. a sale contract that does a expensive migration once a certain target has been hit).

gakonst · 2025-02-01T00:57:40Z

+        // `_verifyAndCall()`.
+        if (s == 0xe235a92a) {
            require(msg.sender == address(this));
-            _execute(_calldataUserOp(), _calldataKeyHash());
-            assembly ("memory-safe") {
-                mstore(0x00, 1)
-                return(0x00, 0x20)
-            }
-        }
-        // `_computeDigest()`.
-        if (s == 0x693e53c3) {
-            bytes32 digest = _computeDigest(_calldataUserOp());
-            assembly ("memory-safe") {
-                mstore(0x00, digest)
-                return(0x00, 0x20)
-            }
+            (bool isValid, bytes32 keyHash) = _verify(userOp);
+            if (!isValid) revert VerificationError();
+            _execute(userOp, keyHash);
+            return;
        }


i like this: userop gives back "is this OK and the keyhash" and then you execute the user op with the keyhash

gakonst · 2025-02-01T00:59:36Z

+                // 1. Pay.
+                mstore(m, 0x1a3de5c3) // `_pay()`.
+                mstore(0x00, 0) // Zeroize the return slot.
+                let gCapped := xor(g, mul(xor(g, _PAYMENT_GAS_CAP), lt(_PAYMENT_GAS_CAP, g))) // `min`.
+                if iszero(call(gCapped, address(), 0, s, n, 0x00, 0x20)) {
+                    err := mload(0x00)
+                    if iszero(returndatasize()) { err := shl(224, 0xbff2584f) } // `PaymentError()`.
+                    break
+                }


I was wondering from the Slack thread if instead we can just move the _pay() call after _verifyAndCall(), and skip doing any refund logic altogether, wdyt?

we run the risk that the gas has been burned and the payer doesn’t have enough tokens to pay.

Right but then we'd just revert the UserOp? Can't we do that?

gakonst · 2025-02-01T01:01:52Z

+        uint256 gUsed = Math.rawSub(gStart, gasleft());
+        uint256 paymentPerGas = u.paymentPerGas;
+        if (paymentPerGas == uint256(0)) paymentPerGas = type(uint256).max;
+        uint256 finalPaymentAmount = Math.min(
+            paymentAmount,
+            Math.saturatingMul(paymentPerGas, Math.saturatingAdd(gUsed, _REFUND_GAS))
+        );
+        address paymentRecipient = u.paymentRecipient;
+        if (paymentRecipient == address(0)) paymentRecipient = address(this);
+        if (LibBit.and(finalPaymentAmount != 0, paymentRecipient != address(this))) {
+            TokenTransferLib.safeTransfer(u.paymentToken, paymentRecipient, finalPaymentAmount);   
+        }
+        if (paymentAmount > finalPaymentAmount) {
+            uint256 toRefund = Math.rawSub(paymentAmount, finalPaymentAmount);
+            TokenTransferLib.safeTransfer(u.paymentToken, u.eoa, toRefund);    
+        }


So instead of doing this, the amount you're _pay()-ing is always the finalPaymentAmount? paymentPerGas * gasUsed.

gakonst · 2025-02-01T01:02:18Z

+            // To prevent griefing, we need to do two non-reverting gas-limited calls.
+            // Even if the verify and call fails, which the gas will be burned, 
+            // the payment has already been made and can't be reverted.


I don't understand, why? I was thnking as I write below "you execute, measure the gas executed, multiply it by the exchange rate, and pay in erc20 only the exact amount needed at the end"

gakonst

Merging to unblock work at the key rotation & gas estimation, have more thoughts for me & Vectorized to go over for the gas sponsorship model

gakonst · 2025-02-03T19:39:12Z

+    /// If the nonce is odd, the digest will be computed without the chain ID.
+    /// Otherwise, the digest will be computed with the chain ID.


@jxom we should bake this in the client-side API

gakonst · 2025-02-03T19:41:35Z

-    {
+    /// @dev Pays `paymentAmount` of `paymentToken` to the Entry Point.
+    /// The last address argument is `onBehalfOf`, which we don't use here.
+    function payEntryPoint(address paymentToken, uint256 paymentAmount, address) public virtual {


Can this 3rd address arg be removed? What do we need it for?

gakonst · 2025-02-03T19:43:22Z


+    /// @dev Override to allow for a delegate call workflow.
+    /// Any implementation contract used in the delegate call workflow must be approved first.
+    function execute(bytes32 mode, bytes calldata executionData) public payable virtual override {


Interesting that this is not onlyThis, do you envision a third party calling into this to e.g. trigger some kind of recurring operation on behalf of the user? Which is why we need to gate the delegates?

gakonst · 2025-02-03T19:50:18Z

+            let s := add(m, 0x1c) // Start of the calldata in memory to pass to the self call.
+            let n := add(encodedUserOp.length, 0x24) // Length of the calldata to the self call.
+
+            // To prevent griefing, we need to do two non-reverting gas-limited calls.


What's the griefing vector?

gakonst · 2025-02-03T19:52:15Z

+            // 1. Pay.
+            mstore(m, 0x1a3de5c3) // `_pay()`.
+            mstore(0x00, 0) // Zeroize the return slot.
+            let gCapped := xor(g, mul(xor(g, _PAYMENT_GAS_CAP), lt(_PAYMENT_GAS_CAP, g))) // `min`.
+            // Perform the gas-limited self call.
+            switch call(gCapped, address(), 0, s, n, 0x00, 0x20)
+            case 0 {
+                err := mload(0x00)
+                if iszero(returndatasize()) { err := shl(224, 0xbff2584f) } // `PaymentError()`.
+            }


Maybe dumb Q but why aren't we just calling the internal _pay() function here and doing it with a gas-limited self-call via the fallback?

gakonst · 2025-02-03T19:52:43Z

+        uint256 paymentAmount;
+        assembly ("memory-safe") {
+            // Check if there's sufficient gas left for the gas-limited self calls
+            // via the 63/64 rule. This is for gas estimation. If the total amount of gas


What is for gas estimation?

gakonst · 2025-02-03T19:55:11Z

+                    if iszero(returndatasize()) { err := shl(224, 0xad4db224) } // `VerifiedCallError()`.
+                }
+            }
+        }


So all of the above is doing is _pay(op); _verify(op); but in assembly and with the self-call vs internal calls.

gakonst · 2025-02-03T20:03:20Z

+        if (LibBit.and(finalPaymentAmount != 0, paymentRecipient != address(this))) {
+            TokenTransferLib.safeTransfer(u.paymentToken, paymentRecipient, finalPaymentAmount);
+        }


I don't understand, given we are doing this stuff, why do we even call _pay(u) above? Let's just call payEntrypoint(...) here with finalPaymentAmount to paymentRecipient instead of first pulling the funds in from the account to the entrypoint, and then from the entrypoint to the payment recipient?

jenpaff · 2025-02-03T23:07:56Z

#12

Vectorized added 6 commits January 31, 2025 11:03

Combine user op gas args into 1

878001b

Add withdraw function on EntryPoint

5a240dd

Normify EntryPoint

d67855e

Tidy EntryPoint

5c729d5

Add delegatecall 7579

2b54eb0

Tidy

1131fe9

Vectorized added 3 commits January 31, 2025 22:51

Prevent EntryPoint payment revert if verification or call fails

7535a4f

Add todo comment on refund strategy

cf3aa71

Add refunds

24d4511

gakonst reviewed Feb 1, 2025

View reviewed changes

Vectorized added 3 commits February 1, 2025 02:38

Tidy

080a7bf

Edit comments

4b3db4c

Edit comments

610e6cf

gakonst approved these changes Feb 3, 2025

View reviewed changes

gakonst merged commit dc29dc9 into main Feb 3, 2025

Vectorized deleted the combined-updates-1 branch April 8, 2025 00:08

Dargon789 mentioned this pull request Apr 11, 2026

feat: add ERC20 transfer benchmark for Porto with passkey #440

Open

		/// @dev The combined gas limit for payment, verification, and calling the EOA.
		uint256 combinedGas;

		/// If the nonce is odd, the digest will be computed without the chain ID.
		/// Otherwise, the digest will be computed with the chain ID.

Conversation

Vectorized commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vectorized commented Jan 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gakonst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jenpaff commented Feb 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Vectorized commented Jan 31, 2025 •

edited

Loading