Skip to content

Commit

Permalink
Switch to isReplayable
Browse files Browse the repository at this point in the history 
This patch fixes some items in regards to a user signing `isReplayable`. Specifically, when a user signs an operation, they need to specify if the quark operation is replayable or not. When not, the state manager nonce _must be_ `bytes32(uint256(-1))`, but when it is replayable, the first play _must be_ `op.nonce`. If the user doesn't sign `isReplayable`, then we'd need to enforce that the first play is `replayToken = op.nonce`, which is bad since it that script could (if a bad nonce is chosen) end up replayable, which is an attack vector for a semi-bad client decision. Thus, we add this check to the user's signature and all things should be good in the world.

Also, we start to fix a bit of verbiage to make things a bit nicer.

Techncially, while it's missing about 200 new tests, this should be a complete implementation.
  • Loading branch information
@hayesgm
hayesgm committed Sep 9, 2024
1 parent 5908b7e commit 4f02f25
Show file tree
Hide file tree
Showing 10 changed files with 174 additions and 110 deletions.
22 changes: 15 additions & 7 deletions src/quark-core/src/QuarkNonceManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {IQuarkWallet} from "quark-core/src/interfaces/IQuarkWallet.sol";
* @author Compound Labs, Inc.
*/
contract QuarkNonceManager {
error NonReplayableNonce(address wallet, bytes32 nonce, bytes32 submissionToken);
error NonReplayableNonce(address wallet, bytes32 nonce, bytes32 submissionToken, bool exhausted);
error InvalidSubmissionToken(address wallet, bytes32 nonce, bytes32 submissionToken);

event NonceSubmitted(address wallet, bytes32 nonce, bytes32 submissionToken);
Expand All @@ -36,17 +36,25 @@ contract QuarkNonceManager {
/**
* @notice Attempts a first or subsequent submission of a given nonce from a wallet.
* @param nonce The nonce of the chain to submit.
* @param submissionToken The submission token of the submission. For single-use operations, set `submissionToken` to `uint256(-1)`. For first-use replayable operations, set `submissionToken` = `nonce`.
* @param isReplayable True only if the operation has been marked as replayable. Otherwise, submission token must be the EXHAUSTED value.
* @param submissionToken The token for this submission. For single-use operations, set `submissionToken` to `uint256(-1)`. For first-use replayable operations, set `submissionToken` = `nonce`. Otherwise, the next submission token from the nonce-chain.
*/
function submitNonceToken(bytes32 nonce, bytes32 submissionToken) external {
function submitNonceToken(bytes32 nonce, bool isReplayable, bytes32 submissionToken) external {
bytes32 lastTokenSubmission = nonceSubmissions[msg.sender][nonce];
if (lastTokenSubmission == EXHAUSTED) {
revert NonReplayableNonce(msg.sender, nonce, submissionToken);
revert NonReplayableNonce(msg.sender, nonce, submissionToken, true);
}
// Defense-in-depth check for non-replayable operations
if (!isReplayable && lastTokenSubmission != FREE) {
revert NonReplayableNonce(msg.sender, nonce, submissionToken, false);
}

bool validFirstPlay =
lastTokenSubmission == FREE && (isReplayable ? submissionToken == nonce : submissionToken == EXHAUSTED);

bool validFirstPlayOrReplay =
lastTokenSubmission == FREE || keccak256(abi.encodePacked(submissionToken)) == lastTokenSubmission;
if (!validFirstPlayOrReplay) {
/* validToken = validFirstPlay or ( validReplay ); */
bool validToken = validFirstPlay || keccak256(abi.encodePacked(submissionToken)) == lastTokenSubmission;
if (!validToken) {
revert InvalidSubmissionToken(msg.sender, nonce, submissionToken);
}

Expand Down
97 changes: 56 additions & 41 deletions src/quark-core/src/QuarkWallet.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ library QuarkWalletMetadata {

/// @notice The EIP-712 typehash for authorizing an operation for this version of QuarkWallet
bytes32 internal constant QUARK_OPERATION_TYPEHASH = keccak256(
"QuarkOperation(bytes32 nonce,address scriptAddress,bytes[] scriptSources,bytes scriptCalldata,uint256 expiry)"
"QuarkOperation(bytes32 nonce,bool isReplayable,address scriptAddress,bytes[] scriptSources,bytes scriptCalldata,uint256 expiry)"
);

/// @notice The EIP-712 typehash for authorizing a MultiQuarkOperation for this version of QuarkWallet
Expand Down Expand Up @@ -71,7 +71,7 @@ contract QuarkWallet is IERC1271 {
/// @notice Address of CodeJar contract used to deploy transaction script source code
CodeJar public immutable codeJar;

/// @notice Address of QuarkNonceManager contract that manages nonces and nonce-namespaced transaction script storage
/// @notice Address of QuarkNonceManager contract that manages nonces for this quark wallet
QuarkNonceManager public immutable nonceManager;

/// @notice Name of contract
Expand Down Expand Up @@ -123,6 +123,8 @@ contract QuarkWallet is IERC1271 {
struct QuarkOperation {
/// @notice Nonce identifier for the operation
bytes32 nonce;
/// @notice Whether this script is replayable or not.
bool isReplayable;
/// @notice The address of the transaction script to run
address scriptAddress;
/// @notice Creation codes Quark must ensure are deployed before executing this operation
Expand All @@ -136,7 +138,7 @@ contract QuarkWallet is IERC1271 {
/**
* @notice Construct a new QuarkWalletImplementation
* @param codeJar_ The CodeJar contract used to deploy scripts
* @param nonceManager_ The QuarkNonceManager contract used to write/read nonces and storage for this wallet
* @param nonceManager_ The QuarkNonceManager contract used to write/read nonces for this wallet
*/
constructor(CodeJar codeJar_, QuarkNonceManager nonceManager_) {
codeJar = codeJar_;
Expand All @@ -156,9 +158,29 @@ contract QuarkWallet is IERC1271 {
external
returns (bytes memory)
{
return executeQuarkOperationWithSubmissionToken(op, getInitialSubmissionToken(op), v, r, s);
}

/**
* @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 A submission token. For replayable operations, initial value should be `submissionToken = op.nonce`, for non-replayable operations, `submissionToken = bytes32(type(uint256).max)`.
* @param v EIP-712 signature v value
* @param r EIP-712 signature r value
* @param s EIP-712 signature s value
* @return Return value from the executed operation
*/
function executeQuarkOperationWithSubmissionToken(
QuarkOperation calldata op,
bytes32 submissionToken,
uint8 v,
bytes32 r,
bytes32 s
) public returns (bytes memory) {
bytes32 opDigest = getDigestForQuarkOperation(op);

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

/**
Expand All @@ -178,56 +200,43 @@ contract QuarkWallet is IERC1271 {
bytes32 r,
bytes32 s
) public returns (bytes memory) {
bytes32 opDigest = getDigestForQuarkOperation(op);

bool isValidOp = false;
for (uint256 i = 0; i < opDigests.length; ++i) {
if (opDigest == opDigests[i]) {
isValidOp = true;
break;
}
}
if (!isValidOp) {
revert InvalidMultiQuarkOperation();
}
bytes32 multiOpDigest = getDigestForMultiQuarkOperation(opDigests);

return verifySigAndExecuteQuarkOperation(op, multiOpDigest, v, r, s);
return executeMultiQuarkOperationWithReplayToken(op, getInitialSubmissionToken(op), opDigests, v, r, s);
}

/**
* @notice Verify a signature and execute a single-use QuarkOperation
* @notice Executes a first play or a replay of 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 digest A EIP-712 digest for either a QuarkOperation or MultiQuarkOperation to verify the signature against
* @param replayToken A replay token. For replayables, initial value should be `replayToken = op.nonce`, for non-replayables, `replayToken = bytes32(type(uint256).max)`
* @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
* @return Return value from the executed operation
*/
function verifySigAndExecuteQuarkOperation(
function executeMultiQuarkOperationWithReplayToken(
QuarkOperation calldata op,
bytes32 digest,
bytes32 replayToken,
bytes32[] memory opDigests,
uint8 v,
bytes32 r,
bytes32 s
) 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);
) public returns (bytes memory) {
bytes32 opDigest = getDigestForQuarkOperation(op);

// guarantee every script in scriptSources is deployed
for (uint256 i = 0; i < op.scriptSources.length; ++i) {
codeJar.saveCode(op.scriptSources[i]);
bool isValidOp = false;
for (uint256 i = 0; i < opDigests.length; ++i) {
if (opDigest == opDigests[i]) {
isValidOp = true;
break;
}
}
if (!isValidOp) {
revert InvalidMultiQuarkOperation();
}
bytes32 multiOpDigest = getDigestForMultiQuarkOperation(opDigests);

nonceManager.submitNonceToken(op.nonce, EXHAUSTED_TOKEN);

emit ExecuteQuarkScript(msg.sender, op.scriptAddress, op.nonce, ExecutionType.Signature);

return executeScriptInternal(op.scriptAddress, op.scriptCalldata);
return verifySigAndExecuteQuarkOperation(op, replayToken, multiOpDigest, v, r, s);
}

/**
Expand All @@ -240,7 +249,7 @@ contract QuarkWallet is IERC1271 {
* @param s EIP-712 signature s value
* @return Return value from the executed operation
*/
function verifySigAndExecuteReplayableQuarkOperation(
function verifySigAndExecuteQuarkOperation(
QuarkOperation calldata op,
bytes32 submissionToken,
bytes32 digest,
Expand All @@ -260,7 +269,7 @@ contract QuarkWallet is IERC1271 {
codeJar.saveCode(op.scriptSources[i]);
}

nonceManager.submitNonceToken(op.nonce, submissionToken);
nonceManager.submitNonceToken(op.nonce, op.isReplayable, submissionToken);

emit ExecuteQuarkScript(msg.sender, op.scriptAddress, op.nonce, ExecutionType.Signature);

Expand Down Expand Up @@ -292,7 +301,7 @@ contract QuarkWallet is IERC1271 {
codeJar.saveCode(scriptSources[i]);
}

nonceManager.submitNonceToken(nonce, EXHAUSTED_TOKEN);
nonceManager.submitNonceToken(nonce, false, EXHAUSTED_TOKEN);

emit ExecuteQuarkScript(msg.sender, scriptAddress, nonce, ExecutionType.Direct);

Expand Down Expand Up @@ -324,6 +333,7 @@ contract QuarkWallet is IERC1271 {
abi.encode(
QUARK_OPERATION_TYPEHASH,
op.nonce,
op.isReplayable,
op.scriptAddress,
keccak256(encodedScriptSources),
keccak256(op.scriptCalldata),
Expand Down Expand Up @@ -499,4 +509,9 @@ contract QuarkWallet is IERC1271 {

/// @notice Fallback for receiving native token
receive() external payable {}

/// @dev Returns the expected initial submission token for an operation, which is either `op.nonce` for a replayable operation, or `bytes32(type(uint256).max)` (the "exhausted" token) for a non-replayable operation.
function getInitialSubmissionToken(QuarkOperation memory op) internal pure returns (bytes32) {
return op.isReplayable ? op.nonce : EXHAUSTED_TOKEN;
}
}
2 changes: 1 addition & 1 deletion src/quark-core/src/QuarkWalletStandalone.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ contract QuarkWalletStandalone is QuarkWallet, IHasSignerExecutor {
* @param signer_ The address that is allowed to sign QuarkOperations for this wallet
* @param executor_ The address that is allowed to directly execute Quark scripts for this wallet
* @param codeJar_ The CodeJar contract used to deploy scripts
* @param nonceManager_ The QuarkNonceManager contract used to write/read nonces and storage for this wallet
* @param nonceManager_ The QuarkNonceManager contract used to write/read nonces for this wallet
*/
constructor(address signer_, address executor_, CodeJar codeJar_, QuarkNonceManager nonceManager_)
QuarkWallet(codeJar_, nonceManager_)
Expand Down
3 changes: 2 additions & 1 deletion test/lib/CancelOtherScript.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import "quark-core/src/QuarkWallet.sol";

contract CancelOtherScript {
function run(bytes32 nonce) public {
return QuarkWallet(payable(address(this))).nonceManager().submitNonceToken(nonce, bytes32(type(uint256).max));
return
QuarkWallet(payable(address(this))).nonceManager().submitNonceToken(nonce, true, bytes32(type(uint256).max));
}
}
2 changes: 2 additions & 0 deletions test/lib/QuarkOperationHelper.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ contract QuarkOperationHelper is Test {
scriptSources: ensureScripts,
scriptCalldata: scriptCalldata,
nonce: semiRandomNonce(wallet),
isReplayable: false,
expiry: block.timestamp + 1000
});
} else {
Expand All @@ -53,6 +54,7 @@ contract QuarkOperationHelper is Test {
scriptSources: ensureScripts,
scriptCalldata: scriptCalldata,
nonce: semiRandomNonce(wallet),
isReplayable: false,
expiry: block.timestamp + 1000
});
}
Expand Down
1 change: 1 addition & 0 deletions test/lib/SignatureHelper.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ contract SignatureHelper is Test {
abi.encode(
QuarkWalletMetadata.QUARK_OPERATION_TYPEHASH,
op.nonce,
op.isReplayable,
op.scriptAddress,
keccak256(encodedArray),
keccak256(op.scriptCalldata),
Expand Down
15 changes: 12 additions & 3 deletions test/quark-core/EIP712.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ contract EIP712Test is Test {

QuarkWallet.QuarkOperation memory op = QuarkWallet.QuarkOperation({
nonce: nextNonce,
isReplayable: true,
scriptAddress: incrementerAddress,
scriptSources: scriptSources,
scriptCalldata: scriptCalldata,
Expand All @@ -138,28 +139,32 @@ contract EIP712Test is Test {
},
{ QuarkOperation: [
{ name: 'nonce', type: 'bytes32' },
{ name: 'isReplayable', type: 'bool' },
{ name: 'scriptAddress', type: 'address' },
{ name: 'scriptSources', type: 'bytes[]' },
{ name: 'scriptCalldata', type: 'bytes' },
{ name: 'expiry', type: 'uint256' }
]},
{
nonce: '0x0000000000000000000000000000000000000000000000000000000000000000',
isReplayable: true,
scriptAddress: '0x5cB7957c702bB6BB8F22aCcf66657F0defd4550b',
scriptSources: ['0x608060405234801561001057600080fd5b506102a7806100206000396000f3fe608060405234801561001057600080fd5b50600436106100365760003560e01c80636b582b7614610056578063e5910ae714610069575b73f62849f9a0b5bf2913b396098f7c7019b51a820a61005481610077565b005b610054610064366004610230565b610173565b610054610077366004610230565b806001600160a01b031663d09de08a6040518163ffffffff1660e01b8152600401600060405180830381600087803b1580156100b257600080fd5b505af11580156100c6573d6000803e3d6000fd5b50505050806001600160a01b031663d09de08a6040518163ffffffff1660e01b8152600401600060405180830381600087803b15801561010557600080fd5b505af1158015610119573d6000803e3d6000fd5b50505050806001600160a01b031663d09de08a6040518163ffffffff1660e01b8152600401600060405180830381600087803b15801561015857600080fd5b505af115801561016c573d6000803e3d6000fd5b5050505050565b61017c81610077565b306001600160a01b0316632e716fb16040518163ffffffff1660e01b8152600401602060405180830381865afa1580156101ba573d6000803e3d6000fd5b505050506040513d601f19601f820116820180604052508101906101de9190610254565b6001600160a01b0316631913592a6040518163ffffffff1660e01b8152600401600060405180830381600087803b15801561015857600080fd5b6001600160a01b038116811461022d57600080fd5b50565b60006020828403121561024257600080fd5b813561024d81610218565b9392505050565b60006020828403121561026657600080fd5b815161024d8161021856fea26469706673582212200d71f9cd831b3c67d6f6131f807ee7fc47d21f07fe8f7b90a01dab56abb8403464736f6c63430008170033'],
scriptCalldata: '0xe5910ae7000000000000000000000000f62849f9a0b5bf2913b396098f7c7019b51a820a',
expiry: 9999999999999
}
)
0x1901ce5fced5138ae147492ff6ba56247e9d6f30bbbe45ae60eb0a0135d528a94be437302412583af420731c67963b8628682b151f38070c3c9142fc40054158666e
0x1901
ce5fced5138ae147492ff6ba56247e9d6f30bbbe45ae60eb0a0135d528a94be4
115a39f16a8c9e3e390e94dc858a17eba53b5358382af38b02f1ac31c2b5f9b0
*/

bytes32 domainHash = new SignatureHelper().domainSeparator(wallet_);
assertEq(domainHash, hex"ce5fced5138ae147492ff6ba56247e9d6f30bbbe45ae60eb0a0135d528a94be4");

bytes32 structHash = new SignatureHelper().opStructHash(op);
assertEq(structHash, hex"37302412583af420731c67963b8628682b151f38070c3c9142fc40054158666e");
assertEq(structHash, hex"115a39f16a8c9e3e390e94dc858a17eba53b5358382af38b02f1ac31c2b5f9b0");
}

function testRevertsForBadCalldata() public {
Expand Down Expand Up @@ -227,7 +232,11 @@ contract EIP712Test is Test {
// submitter tries to reuse the same signature twice, for a non-replayable operation
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.NonReplayableNonce.selector, address(wallet), op.nonce, bytes32(type(uint256).max)
QuarkNonceManager.NonReplayableNonce.selector,
address(wallet),
op.nonce,
bytes32(type(uint256).max),
true
)
);
wallet.executeQuarkOperation(op, v, r, s);
Expand Down
Loading

0 comments on commit 4f02f25

Please sign in to comment.