nativeTokenLimitPerTransaction is checked per batch item instead of per transaction

[chenshj73]

nativeTokenLimitPerTransaction is checked per item in executeBatch, so a batch can transfer more native token than the documented per-transaction limit

Hi thirdweb team — I was reviewing the smart-wallet account permission logic and noticed a possible semantic mismatch in the handling of nativeTokenLimitPerTransaction for executeBatch.

The interface documents nativeTokenLimitPerTransaction as:

/**
 *  @notice The payload that must be signed by an authorized wallet to set permissions for a signer to use the smart wallet.
 *
 *  @param signer The addres of the signer to give permissions.
 *  @param approvedTargets The list of approved targets that a role holder can call using the smart wallet.
 *  @param nativeTokenLimitPerTransaction The maximum value that can be transferred by a role holder in a single transaction.
 *  @param permissionStartTimestamp The UNIX timestamp at and after which a signer has permission to use the smart wallet.
 *  @param permissionEndTimestamp The UNIX timestamp at and after which a signer no longer has permission to use the smart wallet.
 *  @param reqValidityStartTimestamp The UNIX timestamp at and after which a signature is valid.
 *  @param reqValidityEndTimestamp The UNIX timestamp at and after which a signature is invalid/expired.
 *  @param uid A unique non-repeatable ID for the payload.
 *  @param isAdmin Whether the signer should be an admin.
 */
struct SignerPermissionRequest {
    address signer;
    uint8 isAdmin;
    address[] approvedTargets;
    uint256 nativeTokenLimitPerTransaction;
    uint128 permissionStartTimestamp;
    uint128 permissionEndTimestamp;
    uint128 reqValidityStartTimestamp;
    uint128 reqValidityEndTimestamp;
    bytes32 uid;
}

In AccountCore.isValidSigner, a single execute call is checked against the limit as expected:

// checking target and value for `execute`
if (sig == AccountExtension.execute.selector) {
    // Extract the `target` and `value` arguments from the calldata for `execute`.
    (address target, uint256 value) = decodeExecuteCalldata(_userOp.callData);

    // if wildcard target is not approved, check that the target is in the approvedTargets set.
    if (!isWildCard) {
        // Check if the target is approved.
        if (!approvedTargets.contains(target)) {
            // Account: target not approved.
            return false;
        }
    }

    // Check if the value is within the allowed range.
    if (permissions.nativeTokenLimitPerTransaction < value) {
        // Account: value too high OR Account: target not approved.
        return false;
    }
}

For executeBatch, however, each item is checked independently:

// checking target and value for `executeBatch`
else if (sig == AccountExtension.executeBatch.selector) {
    // Extract the `target` and `value` array arguments from the calldata for `executeBatch`.
    (address[] memory targets, uint256[] memory values, ) = decodeExecuteBatchCalldata(_userOp.callData);

    // if wildcard target is not approved, check that the targets are in the approvedTargets set.
    if (!isWildCard) {
        for (uint256 i = 0; i < targets.length; i++) {
            if (!approvedTargets.contains(targets[i])) {
                // If any target is not approved, break the loop.
                return false;
            }
        }
    }

    // For each target+value pair, check if the value is within the allowed range.
    for (uint256 i = 0; i < targets.length; i++) {
        if (permissions.nativeTokenLimitPerTransaction < values[i]) {
            // Account: value too high OR Account: target not approved.
            return false;
        }
    }
} else {
    // Account: calling invalid fn.
    return false;
}

return true;

This means a signer with nativeTokenLimitPerTransaction = 20 can submit an executeBatch with three transfers of 10 each. Every item is within the per-item check, so isValidSigner returns true, but the single UserOperation / account transaction transfers 30 total native token.

Suggested regression test:

function test_isValidSigner_executeBatch_totalNativeValueCanExceedLimit()
    public
    whenNotAdmin
    whenValidTimestamps
    whenCorrectTarget
{
    address[] memory _approvedTargets = new address[](1);
    _approvedTargets[0] = address(numberContract);
    account.setApprovedTargetsForSigner(opSigner, _approvedTargets);

    uint256 count = 3;
    address[] memory targets = new address[](count);
    uint256[] memory values = new uint256[](count);
    bytes[] memory callData = new bytes[](count);

    for (uint256 i = 0; i < count; i += 1) {
        targets[i] = address(numberContract);
        values[i] = 10;
        callData[i] = abi.encodeWithSignature("incrementNum()", i);
    }

    op = _setupUserOpExecuteBatch(accountSignerPKey, bytes(""), targets, values, callData);

    // Each individual value is <= 20, but the batch total is 30.
    nativeTokenLimit = 20;
    account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp);

    bool isValid = account.isValidSigner(opSigner, op);

    // If the documented limit is meant to be per transaction / UserOperation,
    // this should be false.
    assertFalse(isValid);
}

Existing tests cover the case where at least one batch item exceeds the limit:

function test_isValidSigner_executeBatch_breachNativeTokenLimit()
    public
    whenNotAdmin
    whenValidTimestamps
    whenCorrectTarget
{
    // set wildcard
    address[] memory _approvedTargets = new address[](1);
    _approvedTargets[0] = address(numberContract);
    account.setApprovedTargetsForSigner(opSigner, _approvedTargets);

    // user op execute
    uint256 count = 3;
    address[] memory targets = new address[](count);
    uint256[] memory values = new uint256[](count);
    bytes[] memory callData = new bytes[](count);

    for (uint256 i = 0; i < count; i += 1) {
        targets[i] = address(numberContract);
        values[i] = 10;
        callData[i] = abi.encodeWithSignature("incrementNum()", i);
    }

    op = _setupUserOpExecuteBatch(accountSignerPKey, bytes(""), targets, values, callData);

    account.setPermissionsForSigner(opSigner, nativeTokenLimit, startTimestamp, endTimestamp);

    bool isValid = account.isValidSigner(opSigner, op);

    assertFalse(isValid);
}

But in that test nativeTokenLimit is left at 0, so each item individually exceeds the limit. It does not cover the cumulative case where every item is allowed individually but the transaction total exceeds the configured limit.

If the intended semantics are actually “per call inside a batch” rather than “per transaction/UserOperation”, then this may only need documentation/field-name clarification. If the intended semantics are the documented one, executeBatch should probably accumulate values[i] and compare the total against nativeTokenLimitPerTransaction.

Reviewed commit: 0da770f27209.