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 Aug 27, 2024
1 parent 5c5bd45 commit 16d4a98
Show file tree
Hide file tree
Showing 11 changed files with 135 additions and 93 deletions.
21 changes: 11 additions & 10 deletions src/quark-core/src/QuarkStateManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,41 +15,42 @@ contract QuarkStateManager {
event NonceSubmitted(address wallet, bytes32 nonce, bytes32 replayToken);

/// @notice Represents the unclaimed bytes32 value.
bytes32 public constant CLAIMABLE_TOKEN = bytes32(uint256(0));
bytes32 public constant UNCLAIMED_TOKEN = bytes32(uint256(0));

/// @notice A token that implies a Quark Operation is no longer replayable.
bytes32 public constant NO_REPLAY_TOKEN = bytes32(type(uint).max);

/// @notice Mapping from nonces to last used replay token.
mapping(address wallet => mapping(bytes32 nonce => bytes32 lastToken)) public nonceTokens;
mapping(address wallet => mapping(bytes32 nonce => bytes32 lastToken)) public nonceReplayTokens;

/**
* @notice Returns the nonce token (last replay token) for a given nonce. For finalized scripts, this will be `uint256(-1)`. For unclaimed nonces, this will be `uint256(0)`. Otherwise, it will be the next value in the replay chain.
* @param wallet The wallet for which to get the nonce token.
* @param nonce The nonce for the given request.
* @return replayToken The last used replay token, or 0 if unused or -1 if finalized.
*/
function getNonceToken(address wallet, bytes32 nonce) external view returns (bytes32 replayToken) {
return nonceTokens[wallet][nonce];
function getNonceReplayToken(address wallet, bytes32 nonce) external view returns (bytes32 replayToken) {
return nonceReplayTokens[wallet][nonce];
}

/**
* @notice Attempts a first or subsequent submission of a given nonce from a wallet.
* @param nonce The nonce of the chain to submit.
* @param replayToken The replay token of the submission. For single-use operations, set `replayToken` to `uint256(-1)`. For first-use replayable operations, set `replayToken` = `nonce`.
*/
function submitNonceToken(bytes32 nonce, bytes32 replayToken) external {
bytes32 lastToken = nonceTokens[msg.sender][nonce];
function submitNonceReplayToken(bytes32 nonce, bool isReplayable, bytes32 replayToken) external {
bytes32 lastToken = nonceReplayTokens[msg.sender][nonce];
if (lastToken == NO_REPLAY_TOKEN) {
revert NonReplayableNonce(msg.sender, nonce, replayToken);
}

bool validReplay = lastToken == CLAIMABLE_TOKEN || keccak256(abi.encodePacked(replayToken)) == lastToken;
if (!validReplay) {
bool validFirstPlay = lastToken == UNCLAIMED_TOKEN && (isReplayable ? replayToken == nonce : replayToken == NO_REPLAY_TOKEN);
/* validToken = validFirstPlay or ( validReplay ); */
bool validToken = validFirstPlay || keccak256(abi.encodePacked(replayToken)) == lastToken;
if (!validToken) {
revert InvalidReplayToken(msg.sender, nonce, replayToken);
}

nonceTokens[msg.sender][nonce] = replayToken;
nonceReplayTokens[msg.sender][nonce] = replayToken;
emit NonceSubmitted(msg.sender, nonce, replayToken);
}
}
92 changes: 52 additions & 40 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 @@ -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 Down Expand Up @@ -155,10 +157,27 @@ contract QuarkWallet is IERC1271 {
function executeQuarkOperation(QuarkOperation calldata op, uint8 v, bytes32 r, bytes32 s)
external
returns (bytes memory)
{
return executeQuarkOperationWithReplayToken(op, getInitialReplayToken(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 replayToken A replay token. For replayables, initial value should be `replayToken = op.nonce`, for non-replayables, `replayToken = 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 executeQuarkOperationWithReplayToken(QuarkOperation calldata op, bytes32 replayToken, uint8 v, bytes32 r, bytes32 s)
public
returns (bytes memory)
{
bytes32 opDigest = getDigestForQuarkOperation(op);

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

/**
Expand All @@ -178,60 +197,47 @@ 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, getInitialReplayToken(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);

stateManager.submitNonceToken(op.nonce, NO_REPLAY_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);
}

/**
* @notice Verify a signature and execute a replayable QuarkOperation
* @notice Verify a signature and execute a single-use QuarkOperation
* @param op A QuarkOperation struct
* @param replayToken The replay token for the replayable quark operation. For the first submission, this is generally the `rootHash`.
* @param digest A EIP-712 digest for either a QuarkOperation or MultiQuarkOperation to verify the signature against
Expand All @@ -240,7 +246,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 replayToken,
bytes32 digest,
Expand All @@ -260,7 +266,7 @@ contract QuarkWallet is IERC1271 {
codeJar.saveCode(op.scriptSources[i]);
}

stateManager.submitNonceToken(op.nonce, replayToken);
stateManager.submitNonceReplayToken(op.nonce, op.isReplayable, replayToken);

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

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

stateManager.submitNonceToken(nonce, NO_REPLAY_TOKEN);
stateManager.submitNonceReplayToken(nonce, false, NO_REPLAY_TOKEN);

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

Expand Down Expand Up @@ -324,6 +330,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 +506,9 @@ contract QuarkWallet is IERC1271 {

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

/// @dev Returns the expected initial replay token for an operation, which is either `op.nonce` for a replayable operation, or `bytes32(type(uint256).max)` (the no replay token) for a non-replayable operation.
function getInitialReplayToken(QuarkOperation memory op) internal pure returns (bytes32) {
return op.isReplayable ? op.nonce : NO_REPLAY_TOKEN;
}
}
2 changes: 1 addition & 1 deletion test/lib/CancelOtherScript.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ import "quark-core/src/QuarkWallet.sol";

contract CancelOtherScript {
function run(bytes32 nonce) public {
return QuarkWallet(payable(address(this))).stateManager().submitNonceToken(nonce, bytes32(type(uint).max));
return QuarkWallet(payable(address(this))).stateManager().submitNonceReplayToken(nonce, true, bytes32(type(uint).max));
}
}
2 changes: 1 addition & 1 deletion test/lib/ExecuteWithRequirements.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ contract ExecuteWithRequirements {
QuarkWallet wallet = QuarkWallet(payable(address(this)));
QuarkStateManager stateManager = wallet.stateManager();
for (uint256 i = 0; i < requirements.length; i++) {
if (stateManager.getNonceToken(address(wallet), requirements[i]) == bytes32(uint256(0))) {
if (stateManager.getNonceReplayToken(address(wallet), requirements[i]) == bytes32(uint256(0))) {
revert RequirementNotMet(requirements[i]);
}
}
Expand Down
4 changes: 3 additions & 1 deletion 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 All @@ -71,7 +73,7 @@ contract QuarkOperationHelper is Test {
function semiRandomNonce(QuarkStateManager quarkStateManager, QuarkWallet wallet) public view returns (bytes32) {
bytes32 nonce = bytes32(uint256(keccak256(abi.encodePacked(block.timestamp))) - 1);
while (true) {
if (quarkStateManager.getNonceToken(address(wallet), nonce) == bytes32(uint256(0))) {
if (quarkStateManager.getNonceReplayToken(address(wallet), nonce) == bytes32(uint256(0))) {
return nonce;
}

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
25 changes: 15 additions & 10 deletions test/quark-core/EIP712.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ contract EIP712Test is Test {
assertEq(counter.number(), 3);

// nonce is spent
assertEq(stateManager.getNonceToken(address(wallet), op.nonce), bytes32(type(uint256).max));
assertEq(stateManager.getNonceReplayToken(address(wallet), op.nonce), bytes32(type(uint256).max));
}

function testRevertsForBadCode() public {
Expand All @@ -99,7 +99,7 @@ contract EIP712Test is Test {
assertEq(counter.number(), 0);

// nonce is not spent
assertEq(stateManager.getNonceToken(address(wallet), op.nonce), bytes32(uint256(0)));
assertEq(stateManager.getNonceReplayToken(address(wallet), op.nonce), bytes32(uint256(0)));
}

function testStructHash() public {
Expand All @@ -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 All @@ -182,7 +187,7 @@ contract EIP712Test is Test {
assertEq(counter.number(), 0);

// nonce is not spent
assertEq(stateManager.getNonceToken(address(wallet), op.nonce), bytes32(uint256(0)));
assertEq(stateManager.getNonceReplayToken(address(wallet), op.nonce), bytes32(uint256(0)));
}

function testRevertsForBadExpiry() public {
Expand All @@ -205,7 +210,7 @@ contract EIP712Test is Test {
assertEq(counter.number(), 0);

// alice's nonce is not set
assertEq(stateManager.getNonceToken(address(wallet), op.nonce), bytes32(uint256(0)));
assertEq(stateManager.getNonceReplayToken(address(wallet), op.nonce), bytes32(uint256(0)));
}

function testRevertsOnReusedNonce() public {
Expand All @@ -222,7 +227,7 @@ contract EIP712Test is Test {
wallet.executeQuarkOperation(op, v, r, s);

assertEq(counter.number(), 3);
assertEq(stateManager.getNonceToken(address(wallet), op.nonce), bytes32(type(uint256).max));
assertEq(stateManager.getNonceReplayToken(address(wallet), op.nonce), bytes32(type(uint256).max));

// submitter tries to reuse the same signature twice, for a non-replayable operation
vm.expectRevert(abi.encodeWithSelector(QuarkStateManager.NonReplayableNonce.selector, address(wallet), op.nonce, bytes32(type(uint256).max)));
Expand All @@ -248,7 +253,7 @@ contract EIP712Test is Test {
wallet.executeQuarkOperation(op, v, r, s);

assertEq(counter.number(), 0);
assertEq(stateManager.getNonceToken(address(wallet), op.nonce), bytes32(uint256(0)));
assertEq(stateManager.getNonceReplayToken(address(wallet), op.nonce), bytes32(uint256(0)));
}

function testRevertsInvalidS() public {
Expand All @@ -270,7 +275,7 @@ contract EIP712Test is Test {
wallet.executeQuarkOperation(op, v, r, invalidS);

assertEq(counter.number(), 0);
assertEq(stateManager.getNonceToken(address(wallet), op.nonce), bytes32(uint256(0)));
assertEq(stateManager.getNonceReplayToken(address(wallet), op.nonce), bytes32(uint256(0)));
}

// TODO: Uncomment when replay tokens are supported
Expand Down Expand Up @@ -347,7 +352,7 @@ contract EIP712Test is Test {
wallet.executeQuarkOperation(op, v, r, s);

assertEq(counter.number(), 0);
assertEq(stateManager.getNonceToken(address(wallet), op.nonce), bytes32(uint256(0)));
assertEq(stateManager.getNonceReplayToken(address(wallet), op.nonce), bytes32(uint256(0)));
}

function testRequirements() public {
Expand Down
Loading

0 comments on commit 16d4a98

Please sign in to comment.