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.

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

feat: add upper limit to gas price in light user op hash #43

Merged
merged 8 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
22 changes: 22 additions & 0 deletions packages/smart-vaults/src/library/UserOperationLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ library UserOperationLib {

uint256 public constant INVALID_SIGNATURE = 1;

uint256 public constant PAYMASTER_VALIDATION_GAS_OFFSET = 20;

uint256 public constant PAYMASTER_POSTOP_GAS_OFFSET = 36;

uint256 public constant PAYMASTER_DATA_OFFSET = 52;

/* -------------------------------------------------------------------------- */
/* FUNCTIONS */
/* -------------------------------------------------------------------------- */
Expand Down Expand Up @@ -113,4 +119,20 @@ library UserOperationLib {
function hash(PackedUserOperation calldata userOp_) internal pure returns (bytes32) {
return keccak256(encode(userOp_));
}

function unpackUints(bytes32 packed) internal pure returns (uint256 high128, uint256 low128) {
return (uint128(bytes16(packed)), uint128(uint256(packed)));
r0ohafza marked this conversation as resolved.
Show resolved Hide resolved
}

function unpackPaymasterStaticFields(bytes calldata paymasterAndData)
internal
pure
returns (address paymaster, uint256 validationGasLimit, uint256 postOpGasLimit)
{
return (
address(bytes20(paymasterAndData[:PAYMASTER_VALIDATION_GAS_OFFSET])),
uint128(bytes16(paymasterAndData[PAYMASTER_VALIDATION_GAS_OFFSET:PAYMASTER_POSTOP_GAS_OFFSET])),
uint128(bytes16(paymasterAndData[PAYMASTER_POSTOP_GAS_OFFSET:PAYMASTER_DATA_OFFSET]))
);
}
}
79 changes: 75 additions & 4 deletions packages/smart-vaults/src/vault/SmartVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,35 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1
ERC1271
}

/// @notice Upper limits for maxPriorityFeePerGas, preVerificationGas, verificationGasLimit, callGasLimit,
/// paymasterValidationGasLimit and paymasterPostOpGasLimit that should be charged by the userOp. This is included
/// in the light userOp hash to ensure last signer does not exceed the specified gas price/limits. These values will
/// be ignored when threshold is 1. paymaster, paymasterValidationGasLimit and paymasterPostOpGasLimit will be
/// ignored if paymasterAndData is empty.
struct LightUserOpGasLimits {
Copy link
Contributor

Choose a reason for hiding this comment

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

vaguely tempted to pack a few of these to save the calldata but probably not worth it idk

Choose a reason for hiding this comment

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

If packing is added, one suggestion would be to pack the callGasLimit and verificationGasLimit values into a bytes32 value called accountGasLimits, since that would match the PackedUserOperation struct itself

I think it'd also be possible to pack preVerificationGas and maxPriorityFeePerGas together, but that pairing would be a bit less natural and wouldn't match PackedUserOperation

uint256 maxPriorityFeePerGas;
uint256 preVerificationGas;
uint256 callGasLimit;
uint256 verificationGasLimit;
r0ohafza marked this conversation as resolved.
Show resolved Hide resolved
address paymaster;
uint256 paymasterValidationGasLimit;
r0ohafza marked this conversation as resolved.
Show resolved Hide resolved
uint256 paymasterPostOpGasLimit;
}

/// @notice Single User Op Signature Scheme.
struct SingleUserOpSignature {
/// @notice light user op gas limits.
LightUserOpGasLimits gasLimits;
/// @notice list of signatures where threshold - 1
/// signatures will be verified against the light userOp hash and the final signature will be verified
/// against the userOp hash.
MultiSignerLib.SignatureWrapper[] signatures;
}

/// @notice Merkelized User Op Signature Scheme.
struct MerkelizedUserOpSignature {
/// @notice light user op gas limits.
LightUserOpGasLimits gasLimits;
/// @notice merkleRoot of all the light(userOp) in the Merkle Tree. If threshold is 1, this will be
/// bytes32(0).
bytes32 lightMerkleTreeRoot;
Expand Down Expand Up @@ -92,6 +114,12 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1
/// @notice Thrown when merkle root validation fails.
error InvalidMerkleProof();

/// @notice Thrown when LightUserOpGasLimits have been breached.
error InvalidGasLimits();

/// @notice Thrown when Paymaster LightUserOpGasLimits have been breached.
error InvalidPaymasterData();

/* -------------------------------------------------------------------------- */
/* MODIFIERS */
/* -------------------------------------------------------------------------- */
Expand Down Expand Up @@ -218,7 +246,10 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1
// if threshold is greater than 1, `threshold - 1` signers will sign over the light userOp hash. We lazily
// calculate light userOp hash based on number of signatures. If threshold is 1 then light userOp hash
// won't be needed.
if (signature.signatures.length > 1) lightHash = _getLightUserOpHash(userOp_);
if (signature.signatures.length > 1) {
_verifyGasLimits(signature.gasLimits, userOp_);
lightHash = _getLightUserOpHash(userOp_, signature.gasLimits);
}

return _validateSingleUserOp(lightHash, userOpHash_, signature);
} else if (signatureType == SignatureTypes.MerkelizedUserOp) {
Expand All @@ -227,7 +258,10 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1
// if threshold is greater than 1, `threshold - 1` signers will sign over the merkle tree root of light user
// op hash(s). We lazily calculate light userOp hash based on value of light merkle tree root. If threshold
// is 1 then light userOp hash won't be needed.
if (signature.lightMerkleTreeRoot != bytes32(0)) lightHash = _getLightUserOpHash(userOp_);
if (signature.lightMerkleTreeRoot != bytes32(0)) {
_verifyGasLimits(signature.gasLimits, userOp_);
lightHash = _getLightUserOpHash(userOp_, signature.gasLimits);
}

return _validateMerkelizedUserOp(lightHash, userOpHash_, signature);
} else {
Expand Down Expand Up @@ -329,8 +363,15 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1
}

/// @dev Get light userOp hash of the Packed user operation.
function _getLightUserOpHash(PackedUserOperation calldata userOp_) internal view returns (bytes32) {
return keccak256(abi.encode(userOp_.hashLight(), entryPoint(), block.chainid));
function _getLightUserOpHash(
PackedUserOperation calldata userOp_,
LightUserOpGasLimits memory gasLimits_
)
internal
view
returns (bytes32)
{
return keccak256(abi.encode(userOp_.hashLight(), gasLimits_, entryPoint(), block.chainid));
}

/// @dev validates if the given hash (ERC1271) was signed by the signers.
Expand Down Expand Up @@ -394,4 +435,34 @@ contract SmartVault is IAccount, Ownable, UUPSUpgradeable, MultiSignerAuth, ERC1
function _getSignatureType(bytes1 signatureType_) internal pure returns (SignatureTypes) {
return SignatureTypes(uint8(signatureType_));
}

function _verifyGasLimits(
LightUserOpGasLimits memory gasLimits_,
PackedUserOperation calldata userOp_
r0ohafza marked this conversation as resolved.
Show resolved Hide resolved
)
internal
pure
{
(uint256 userOpMaxPriorityFeePerGas,) = UserOperationLib.unpackUints(userOp_.gasFees);
(uint256 verificationGasLimit, uint256 callGasLimit) = UserOperationLib.unpackUints(userOp_.accountGasLimits);

if (
userOpMaxPriorityFeePerGas > gasLimits_.maxPriorityFeePerGas || callGasLimit > gasLimits_.callGasLimit
|| userOp_.preVerificationGas > gasLimits_.preVerificationGas
|| verificationGasLimit > gasLimits_.verificationGasLimit
) revert InvalidGasLimits();

if (userOp_.paymasterAndData.length > 0) {
(address paymaster, uint256 paymasterVerificationGasLimit, uint256 paymasterPostOpGasLimit) =
UserOperationLib.unpackPaymasterStaticFields(userOp_.paymasterAndData);

if (
gasLimits_.paymaster != paymaster
r0ohafza marked this conversation as resolved.
Show resolved Hide resolved
|| paymasterVerificationGasLimit > gasLimits_.paymasterValidationGasLimit
|| paymasterPostOpGasLimit > gasLimits_.paymasterPostOpGasLimit
) {
revert InvalidPaymasterData();
}
}
}
}
Loading