Skip to content

Commit

Permalink
Switch from v,r,s to bytes for signature
Browse files Browse the repository at this point in the history
  • Loading branch information
kevincheng96 committed Sep 27, 2024
1 parent bffc5ee commit f5502a5
Show file tree
Hide file tree
Showing 28 changed files with 545 additions and 704 deletions.
100 changes: 40 additions & 60 deletions src/quark-core/src/QuarkWallet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -171,58 +171,48 @@ contract QuarkWallet is IERC1271 {
* @notice Execute a QuarkOperation via signature
* @dev Can only be called with signatures from the wallet's signer
* @param op A QuarkOperation struct
* @param v EIP-712 signature v value
* @param r EIP-712 signature r value
* @param s EIP-712 signature s value
* @param signature A EIP-712 signature
* @return Return value from the executed operation
*/
function executeQuarkOperation(QuarkOperation calldata op, uint8 v, bytes32 r, bytes32 s)
function executeQuarkOperation(QuarkOperation calldata op, bytes calldata signature)
external
returns (bytes memory)
{
return executeQuarkOperationWithSubmissionToken(op, op.nonce, v, r, s);
return executeQuarkOperationWithSubmissionToken(op, op.nonce, signature);
}

/**
* @notice Executes a first play or a replay of a QuarkOperation via signature
* @dev Can only be called with signatures from the wallet's signer
* @param op A QuarkOperation struct
* @param submissionToken The submission token for the replayable quark operation for QuarkNonceManager. This is initially the `op.nonce`, and for replayable operations, it is the next token in the nonce chain.
* @param v EIP-712 signature v value
* @param r EIP-712 signature r value
* @param s EIP-712 signature s value
* @param signature A EIP-712 signature
* @return Return value from the executed operation
*/
function executeQuarkOperationWithSubmissionToken(
QuarkOperation calldata op,
bytes32 submissionToken,
uint8 v,
bytes32 r,
bytes32 s
bytes calldata signature
) public returns (bytes memory) {
bytes32 opDigest = getDigestForQuarkOperation(op);

return verifySigAndExecuteQuarkOperation(op, submissionToken, opDigest, v, r, s);
return verifySigAndExecuteQuarkOperation(op, submissionToken, opDigest, signature);
}

/**
* @notice Execute a QuarkOperation that is part of a MultiQuarkOperation via signature
* @dev Can only be called with signatures from the wallet's signer
* @param op A QuarkOperation struct
* @param opDigests A list of EIP-712 digests for the operations in a MultiQuarkOperation
* @param v EIP-712 signature v value
* @param r EIP-712 signature r value
* @param s EIP-712 signature s value
* @param signature A EIP-712 signature
* @return Return value from the executed operation
*/
function executeMultiQuarkOperation(
QuarkOperation calldata op,
bytes32[] memory opDigests,
uint8 v,
bytes32 r,
bytes32 s
bytes32[] calldata opDigests,
bytes calldata signature
) public returns (bytes memory) {
return executeMultiQuarkOperationWithSubmissionToken(op, op.nonce, opDigests, v, r, s);
return executeMultiQuarkOperationWithSubmissionToken(op, op.nonce, opDigests, signature);
}

/**
Expand All @@ -231,18 +221,14 @@ contract QuarkWallet is IERC1271 {
* @param op A QuarkOperation struct
* @param submissionToken The submission token for the replayable quark operation for QuarkNonceManager. This is initially the `op.nonce`, and for replayable operations, it is the next token in the nonce chain.
* @param opDigests A list of EIP-712 digests for the operations in a MultiQuarkOperation
* @param v EIP-712 signature v value
* @param r EIP-712 signature r value
* @param s EIP-712 signature s value
* @param signature A EIP-712 signature
* @return Return value from the executed operation
*/
function executeMultiQuarkOperationWithSubmissionToken(
QuarkOperation calldata op,
bytes32 submissionToken,
bytes32[] memory opDigests,
uint8 v,
bytes32 r,
bytes32 s
bytes32[] calldata opDigests,
bytes calldata signature
) public returns (bytes memory) {
bytes32 opDigest = getDigestForQuarkOperation(op);

Expand All @@ -258,33 +244,29 @@ contract QuarkWallet is IERC1271 {
}
bytes32 multiOpDigest = getDigestForMultiQuarkOperation(opDigests);

return verifySigAndExecuteQuarkOperation(op, submissionToken, multiOpDigest, v, r, s);
return verifySigAndExecuteQuarkOperation(op, submissionToken, multiOpDigest, signature);
}

/**
* @notice Verify a signature and execute a replayable QuarkOperation
* @param op A QuarkOperation struct
* @param submissionToken The submission token for the replayable quark operation for QuarkNonceManager. This is initially the `op.nonce`, and for replayable operations, it is the next token in the nonce chain.
* @param digest A EIP-712 digest for either a QuarkOperation or MultiQuarkOperation to verify the signature against
* @param v EIP-712 signature v value
* @param r EIP-712 signature r value
* @param s EIP-712 signature s value
* @param signature A EIP-712 signature
* @return Return value from the executed operation
*/
function verifySigAndExecuteQuarkOperation(
QuarkOperation calldata op,
bytes32 submissionToken,
bytes32 digest,
uint8 v,
bytes32 r,
bytes32 s
bytes calldata signature
) internal returns (bytes memory) {
if (block.timestamp >= op.expiry) {
revert SignatureExpired();
}

// if the signature check does not revert, the signature is valid
checkValidSignatureInternal(IHasSignerExecutor(address(this)).signer(), digest, v, r, s);
checkValidSignatureInternal(IHasSignerExecutor(address(this)).signer(), digest, signature);

// guarantee every script in scriptSources is deployed
for (uint256 i = 0; i < op.scriptSources.length; ++i) {
Expand Down Expand Up @@ -399,29 +381,11 @@ contract QuarkWallet is IERC1271 {
* @return The ERC-1271 "magic value" that indicates the signature is valid
*/
function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4) {
/*
* Code taken directly from OpenZeppelin ECDSA.tryRecover; see:
* https://github.com/OpenZeppelin/openzeppelin-contracts/blob/HEAD/contracts/utils/cryptography/ECDSA.sol#L64-L68
*
* This is effectively an optimized variant of the Reference Implementation; see:
* https://eips.ethereum.org/EIPS/eip-1271#reference-implementation
*/
if (signature.length != 65) {
revert InvalidSignature();
}
bytes32 r;
bytes32 s;
uint8 v;
assembly {
r := mload(add(signature, 0x20))
s := mload(add(signature, 0x40))
v := byte(0, mload(add(signature, 0x60)))
}
// Note: The following logic further encodes the provided `hash` with the wallet's domain
// to prevent signature replayability for Quark wallets owned by the same `signer`
bytes32 digest = getDigestForQuarkMessage(abi.encode(hash));
// If the signature check does not revert, the signature is valid
checkValidSignatureInternal(IHasSignerExecutor(address(this)).signer(), digest, v, r, s);
checkValidSignatureInternal(IHasSignerExecutor(address(this)).signer(), digest, signature);
return EIP_1271_MAGIC_VALUE;
}

Expand All @@ -432,12 +396,9 @@ contract QuarkWallet is IERC1271 {
* to the smart contract; if the smart contract that owns the wallet has no
* code, the signature will be treated as an EIP-712 signature and revert
*/
function checkValidSignatureInternal(address signatory, bytes32 digest, uint8 v, bytes32 r, bytes32 s)
internal
view
{
function checkValidSignatureInternal(address signatory, bytes32 digest, bytes memory signature) internal view {
if (signatory.code.length > 0) {
bytes memory signature = abi.encodePacked(r, s, v);
// For EIP-1271 smart contract signers, the signature can be of any signature scheme (e.g. BLS, Passkey)
(bool success, bytes memory data) =
signatory.staticcall(abi.encodeWithSelector(EIP_1271_MAGIC_VALUE, digest, signature));
if (!success) {
Expand All @@ -448,7 +409,26 @@ contract QuarkWallet is IERC1271 {
revert InvalidEIP1271Signature();
}
} else {
(address recoveredSigner, ECDSA.RecoverError recoverError) = ECDSA.tryRecover(digest, v, r, s);
// For EOA signers, this implementation of the QuarkWallet only supports ECDSA signatures
/*
* Code taken directly from OpenZeppelin ECDSA.tryRecover; see:
* https://github.com/OpenZeppelin/openzeppelin-contracts/blob/HEAD/contracts/utils/cryptography/ECDSA.sol#L64-L68
*
* This is effectively an optimized variant of the Reference Implementation; see:
* https://eips.ethereum.org/EIPS/eip-1271#reference-implementation
*/
if (signature.length != 65) {
revert InvalidSignature();
}
bytes32 r;
bytes32 s;
uint8 v;
assembly {
r := mload(add(signature, 0x20))
s := mload(add(signature, 0x40))
v := byte(0, mload(add(signature, 0x60)))
}
(address recoveredSigner, ECDSA.RecoverError recoverError) = ECDSA.tryRecover(digest, signature);
if (recoverError != ECDSA.RecoverError.NoError) {
revert InvalidSignature();
}
Expand Down
6 changes: 2 additions & 4 deletions src/quark-core/src/periphery/BatchExecutor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ contract BatchExecutor {
struct OperationParams {
address account;
QuarkWallet.QuarkOperation op;
uint8 v;
bytes32 r;
bytes32 s;
bytes signature;
uint256 gasLimit;
}

Expand All @@ -42,7 +40,7 @@ contract BatchExecutor {
* @return Success and return value from the executed operation
*/
function executeOperation(OperationParams memory op) internal returns (bool, bytes memory) {
bytes memory data = abi.encodeWithSelector(QuarkWallet.executeQuarkOperation.selector, op.op, op.v, op.r, op.s);
bytes memory data = abi.encodeWithSelector(QuarkWallet.executeQuarkOperation.selector, op.op, op.signature);
// We purposely ignore success and return values since the BatchExecutor will most likely be called by an EOA
// Lower-level call is used to avoid reverting on failure
(bool success, bytes memory retData) = op.account.call{gas: op.gasLimit}(data);
Expand Down
44 changes: 14 additions & 30 deletions src/quark-proxy/src/QuarkWalletProxyFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,20 +87,16 @@ contract QuarkWalletProxyFactory {
* @param signer Address to set as the signer of the QuarkWallet
* @param executor Address to set as the executor of the QuarkWallet
* @param op The QuarkOperation to execute on the wallet
* @param v EIP-712 Signature `v` value
* @param r EIP-712 Signature `r` value
* @param s EIP-712 Signature `s` value
* @param signature A EIP-712 signature
* @return bytes Return value of executing the operation
*/
function createAndExecute(
address signer,
address executor,
QuarkWallet.QuarkOperation memory op,
uint8 v,
bytes32 r,
bytes32 s
QuarkWallet.QuarkOperation calldata op,
bytes calldata signature
) external returns (bytes memory) {
return createAndExecute(signer, executor, DEFAULT_SALT, op, v, r, s);
return createAndExecute(signer, executor, DEFAULT_SALT, op, signature);
}

/**
Expand All @@ -109,26 +105,22 @@ contract QuarkWalletProxyFactory {
* @param executor Address to set as the executor of the QuarkWallet
* @param salt Salt value of QuarkWallet to create and execute operation with
* @param op The QuarkOperation to execute on the wallet
* @param v EIP-712 Signature `v` value
* @param r EIP-712 Signature `r` value
* @param s EIP-712 Signature `s` value
* @param signature A EIP-712 signature
* @return bytes Return value of executing the operation
*/
function createAndExecute(
address signer,
address executor,
bytes32 salt,
QuarkWallet.QuarkOperation memory op,
uint8 v,
bytes32 r,
bytes32 s
QuarkWallet.QuarkOperation calldata op,
bytes calldata signature
) public returns (bytes memory) {
address payable walletAddress = walletAddressForSalt(signer, executor, salt);
if (walletAddress.code.length == 0) {
create(signer, executor, salt);
}

return QuarkWallet(walletAddress).executeQuarkOperation(op, v, r, s);
return QuarkWallet(walletAddress).executeQuarkOperation(op, signature);
}

/**
Expand All @@ -137,21 +129,17 @@ contract QuarkWalletProxyFactory {
* @param executor Address to set as the executor of the QuarkWallet
* @param op The QuarkOperation to execute on the wallet
* @param opDigests A list of EIP-712 digests for the operations in a MultiQuarkOperation
* @param v EIP-712 Signature `v` value
* @param r EIP-712 Signature `r` value
* @param s EIP-712 Signature `s` value
* @param signature A EIP-712 signature
* @return bytes Return value of executing the operation
*/
function createAndExecuteMulti(
address signer,
address executor,
QuarkWallet.QuarkOperation calldata op,
bytes32[] calldata opDigests,
uint8 v,
bytes32 r,
bytes32 s
bytes calldata signature
) external returns (bytes memory) {
return createAndExecuteMulti(signer, executor, DEFAULT_SALT, op, opDigests, v, r, s);
return createAndExecuteMulti(signer, executor, DEFAULT_SALT, op, opDigests, signature);
}

/**
Expand All @@ -161,9 +149,7 @@ contract QuarkWalletProxyFactory {
* @param salt Salt value of QuarkWallet to create and execute operation with
* @param op The QuarkOperation to execute on the wallet
* @param opDigests A list of EIP-712 digests for the operations in a MultiQuarkOperation
* @param v EIP-712 Signature `v` value
* @param r EIP-712 Signature `r` value
* @param s EIP-712 Signature `s` value
* @param signature A EIP-712 signature
* @return bytes Return value of executing the operation
*/
function createAndExecuteMulti(
Expand All @@ -172,16 +158,14 @@ contract QuarkWalletProxyFactory {
bytes32 salt,
QuarkWallet.QuarkOperation calldata op,
bytes32[] calldata opDigests,
uint8 v,
bytes32 r,
bytes32 s
bytes calldata signature
) public returns (bytes memory) {
address payable walletAddress = walletAddressForSalt(signer, executor, salt);
if (walletAddress.code.length == 0) {
create(signer, executor, salt);
}

return QuarkWallet(walletAddress).executeMultiQuarkOperation(op, opDigests, v, r, s);
return QuarkWallet(walletAddress).executeMultiQuarkOperation(op, opDigests, signature);
}

/**
Expand Down
Loading

0 comments on commit f5502a5

Please sign in to comment.