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.

Sign up for GitHub

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

Nonce Cancel by New Op #213

Merged
merged 3 commits into from
Sep 12, 2024
Merged

Nonce Cancel by New Op #213

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56f2e1d
Nonce Cancel by New Op
hayesgm Sep 11, 2024
d24faf0
TStore for Transients (#214)
hayesgm Sep 12, 2024
13d17be
Address feedback
hayesgm Sep 12, 2024
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
10 changes: 10 additions & 0 deletions src/quark-core/src/QuarkNonceManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ contract QuarkNonceManager {
error InvalidSubmissionToken(address wallet, bytes32 nonce, bytes32 submissionToken);

event NonceSubmitted(address wallet, bytes32 nonce, bytes32 submissionToken);
event NonceCanceled(address wallet, bytes32 nonce);

/// @notice Represents the unclaimed bytes32 value.
bytes32 public constant FREE = QuarkNonceManagerMetadata.FREE;
Expand All @@ -32,6 +33,15 @@ contract QuarkNonceManager {
/// @notice Mapping from nonces to last used submission token.
mapping(address wallet => mapping(bytes32 nonce => bytes32 lastToken)) public submissions;

/**
* @notice Ensures a given nonce is canceled for sender. An un-used nonce will not be usable in the future, and a replayable nonce will no longer be replayable. This is a no-op for already canceled operations.
* @param nonce The nonce of the chain to cancel.
*/
function cancel(bytes32 nonce) external {
submissions[msg.sender][nonce] = EXHAUSTED;
emit NonceCanceled(msg.sender, nonce);
}

/**
* @notice Attempts a first or subsequent submission of a given nonce from a wallet.
* @param nonce The nonce of the chain to submit.
Expand Down
8 changes: 6 additions & 2 deletions src/quark-core/src/QuarkScript.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// SPDX-License-Identifier: BSD-3-Clause
pragma solidity 0.8.23;

import {QuarkWallet, QuarkWalletMetadata, IHasSignerExecutor} from "quark-core/src/QuarkWallet.sol";
import {QuarkNonceManagerMetadata} from "quark-core/src/QuarkNonceManager.sol";
import {QuarkWallet, QuarkWalletMetadata, IHasSignerExecutor, IQuarkWallet} from "quark-core/src/QuarkWallet.sol";
import {QuarkNonceManager, QuarkNonceManagerMetadata} from "quark-core/src/QuarkNonceManager.sol";

/**
* @title Quark Script
Expand Down Expand Up @@ -70,6 +70,10 @@ abstract contract QuarkScript {
return IHasSignerExecutor(address(this)).executor();
}

function nonceManager() internal view returns (QuarkNonceManager) {
return QuarkNonceManager(IQuarkWallet(address(this)).nonceManager());
}

function allowCallback() internal {
bytes32 callbackSlot = QuarkWalletMetadata.CALLBACK_SLOT;
bytes32 activeScriptSlot = QuarkWalletMetadata.ACTIVE_SCRIPT_SLOT;
Expand Down
1 change: 1 addition & 0 deletions src/quark-core/src/QuarkWallet.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {CodeJar} from "codejar/src/CodeJar.sol";

import {QuarkNonceManager} from "quark-core/src/QuarkNonceManager.sol";
import {IHasSignerExecutor} from "quark-core/src/interfaces/IHasSignerExecutor.sol";
import {IQuarkWallet} from "quark-core/src/interfaces/IQuarkWallet.sol";
hayesgm marked this conversation as resolved.
Show resolved Hide resolved

/**
* @title Quark Wallet Metadata
Expand Down
1 change: 1 addition & 0 deletions src/quark-core/src/interfaces/IQuarkWallet.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ interface IQuarkWallet {
uint256 expiry;
}

function nonceManager() external view returns (address);
function executeQuarkOperation(QuarkOperation calldata op, uint8 v, bytes32 r, bytes32 s)
external
returns (bytes memory);
Expand Down
12 changes: 9 additions & 3 deletions test/lib/CancelOtherScript.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,16 @@ import "quark-core/src/QuarkWallet.sol";
import "quark-core/src/QuarkScript.sol";

contract CancelOtherScript is QuarkScript {
hayesgm marked this conversation as resolved.
Show resolved Hide resolved
event CancelNonce();
event Nop();
event CancelNonce(bytes32 nonce);

function run() public {
emit CancelNonce();
function nop() public {
emit Nop();
}

function run(bytes32 nonce) public {
nonceManager().cancel(nonce);
emit CancelNonce(nonce);
}

function checkNonce() public view returns (bytes32) {
Expand Down
24 changes: 16 additions & 8 deletions test/lib/QuarkOperationHelper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,26 @@ contract QuarkOperationHelper is Test {
return (operation, submissionTokens);
}

function cancelReplayable(QuarkWallet wallet, QuarkWallet.QuarkOperation memory quarkOperation)
function cancelReplayableByNop(QuarkWallet wallet, QuarkWallet.QuarkOperation memory quarkOperation)
public
returns (QuarkWallet.QuarkOperation memory)
{
return cancelReplayable(wallet, quarkOperation, abi.encodeWithSignature("run()"));
return getCancelOperation(wallet, quarkOperation.nonce, abi.encodeWithSignature("nop()"));
}

function cancelReplayable(
QuarkWallet wallet,
QuarkWallet.QuarkOperation memory quarkOperation,
bytes memory callData
) public returns (QuarkWallet.QuarkOperation memory) {
function cancelReplayableByNewOp(QuarkWallet wallet, QuarkWallet.QuarkOperation memory quarkOperation)
public
returns (QuarkWallet.QuarkOperation memory)
{
return getCancelOperation(
wallet, semiRandomNonce(wallet), abi.encodeWithSignature("run(bytes32)", quarkOperation.nonce)
);
}

function getCancelOperation(QuarkWallet wallet, bytes32 selfNonce, bytes memory callData)
public
returns (QuarkWallet.QuarkOperation memory)
{
bytes memory cancelOtherScript = new YulHelper().getCode("CancelOtherScript.sol/CancelOtherScript.json");
address scriptAddress = wallet.codeJar().saveCode(cancelOtherScript);
bytes[] memory scriptSources = new bytes[](1);
Expand All @@ -152,7 +160,7 @@ contract QuarkOperationHelper is Test {
scriptAddress: scriptAddress,
scriptSources: scriptSources,
scriptCalldata: callData,
nonce: quarkOperation.nonce,
nonce: selfNonce,
isReplayable: false,
expiry: block.timestamp + 1000
});
Expand Down
5 changes: 3 additions & 2 deletions test/quark-core/Noncer.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,9 @@ contract NoncerTest is Test {
);
(uint8 v, bytes32 r, bytes32 s) = new SignatureHelper().signOp(alicePrivateKey, aliceWallet, op);

QuarkWallet.QuarkOperation memory cancelOp =
new QuarkOperationHelper().cancelReplayable(aliceWallet, op, abi.encodeWithSignature("checkReplayCount()"));
QuarkWallet.QuarkOperation memory cancelOp = new QuarkOperationHelper().getCancelOperation(
aliceWallet, op.nonce, abi.encodeWithSignature("checkReplayCount()")
hayesgm marked this conversation as resolved.
Show resolved Hide resolved
);
(uint8 cancelV, bytes32 cancelR, bytes32 cancelS) =
new SignatureHelper().signOp(alicePrivateKey, aliceWallet, cancelOp);

Expand Down
149 changes: 117 additions & 32 deletions test/quark-core/QuarkNonceManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ contract QuarkNonceManagerTest is Test {
function testNonceOneIsValid() public {
bytes32 nonce = bytes32(uint256(1));

// by default, nonce 0 is not set
// by default, nonce 1 is not set
assertEq(nonceManager.submissions(address(0x123), nonce), nonceManager.FREE());

// nonce 0 can be set manually
// nonce 1 can be set manually
vm.prank(address(0x123));
nonceManager.submit(nonce, false, nonce);
assertEq(nonceManager.submissions(address(0x123), nonce), nonceManager.EXHAUSTED());
Expand Down Expand Up @@ -226,26 +226,25 @@ contract QuarkNonceManagerTest is Test {
// nonce values are not incremental; you can use a random number as
// long as it has not been set
bytes32 nonceSecret = bytes32(uint256(99));
bytes32 replayToken2 = keccak256(abi.encodePacked(nonceSecret));
bytes32 replayToken1 = keccak256(abi.encodePacked(replayToken2));
bytes32 rootHash = keccak256(abi.encodePacked(replayToken1));
bytes32 nonce = rootHash;
bytes32 submissionToken2 = keccak256(abi.encodePacked(nonceSecret));
bytes32 submissionToken1 = keccak256(abi.encodePacked(submissionToken2));
bytes32 nonce = keccak256(abi.encodePacked(submissionToken1));

assertEq(nonceManager.submissions(address(this), nonce), FREE_TOKEN);

nonceManager.submit(nonce, true, rootHash);
assertEq(nonceManager.submissions(address(this), nonce), rootHash);
nonceManager.submit(nonce, true, nonce);
assertEq(nonceManager.submissions(address(this), nonce), nonce);

vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, rootHash)
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, nonce)
);
nonceManager.submit(nonce, true, rootHash);
nonceManager.submit(nonce, true, nonce);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, replayToken2
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, submissionToken2
)
);
nonceManager.submit(nonce, true, replayToken2);
nonceManager.submit(nonce, true, submissionToken2);
vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, nonceSecret)
);
Expand All @@ -261,19 +260,19 @@ contract QuarkNonceManagerTest is Test {
);
nonceManager.submit(nonce, true, EXHAUSTED_TOKEN);

nonceManager.submit(nonce, true, replayToken1);
assertEq(nonceManager.submissions(address(this), nonce), replayToken1);
nonceManager.submit(nonce, true, submissionToken1);
assertEq(nonceManager.submissions(address(this), nonce), submissionToken1);

vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, rootHash)
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, nonce)
);
nonceManager.submit(nonce, true, rootHash);
nonceManager.submit(nonce, true, nonce);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, replayToken1
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, submissionToken1
)
);
nonceManager.submit(nonce, true, replayToken1);
nonceManager.submit(nonce, true, submissionToken1);
vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, nonceSecret)
);
Expand All @@ -289,25 +288,25 @@ contract QuarkNonceManagerTest is Test {
);
nonceManager.submit(nonce, true, EXHAUSTED_TOKEN);

nonceManager.submit(nonce, true, replayToken2);
assertEq(nonceManager.submissions(address(this), nonce), replayToken2);
nonceManager.submit(nonce, true, submissionToken2);
assertEq(nonceManager.submissions(address(this), nonce), submissionToken2);

vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, rootHash)
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, nonce)
);
nonceManager.submit(nonce, true, rootHash);
nonceManager.submit(nonce, true, nonce);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, replayToken1
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, submissionToken1
)
);
nonceManager.submit(nonce, true, replayToken1);
nonceManager.submit(nonce, true, submissionToken1);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, replayToken2
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, submissionToken2
)
);
nonceManager.submit(nonce, true, replayToken2);
nonceManager.submit(nonce, true, submissionToken2);
vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, FREE_TOKEN)
);
Expand All @@ -323,21 +322,21 @@ contract QuarkNonceManagerTest is Test {
assertEq(nonceManager.submissions(address(this), nonce), nonceSecret);

vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, rootHash)
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, nonce)
);
nonceManager.submit(nonce, true, rootHash);
nonceManager.submit(nonce, true, nonce);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, replayToken1
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, submissionToken1
)
);
nonceManager.submit(nonce, true, replayToken1);
nonceManager.submit(nonce, true, submissionToken1);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, replayToken2
QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, submissionToken2
)
);
nonceManager.submit(nonce, true, replayToken2);
nonceManager.submit(nonce, true, submissionToken2);
vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.InvalidSubmissionToken.selector, address(this), nonce, nonceSecret)
);
Expand All @@ -353,4 +352,90 @@ contract QuarkNonceManagerTest is Test {
);
nonceManager.submit(nonce, true, EXHAUSTED_TOKEN);
}

function testCancelChain() public {
hayesgm marked this conversation as resolved.
Show resolved Hide resolved
bytes32 nonceSecret = bytes32(uint256(99));
bytes32 submissionToken2 = keccak256(abi.encodePacked(nonceSecret));
bytes32 submissionToken1 = keccak256(abi.encodePacked(submissionToken2));
bytes32 nonce = keccak256(abi.encodePacked(submissionToken1));

assertEq(nonceManager.submissions(address(this), nonce), FREE_TOKEN);

nonceManager.submit(nonce, true, nonce);
assertEq(nonceManager.submissions(address(this), nonce), nonce);

nonceManager.cancel(nonce);
hayesgm marked this conversation as resolved.
Show resolved Hide resolved
assertEq(nonceManager.submissions(address(this), nonce), EXHAUSTED_TOKEN);

vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.NonReplayableNonce.selector, address(this), nonce, nonce, true)
);
nonceManager.submit(nonce, true, nonce);

vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.NonReplayableNonce.selector, address(this), nonce, submissionToken2, true
)
);
nonceManager.submit(nonce, true, submissionToken2);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.NonReplayableNonce.selector, address(this), nonce, submissionToken1, true
)
);
nonceManager.submit(nonce, true, submissionToken1);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.NonReplayableNonce.selector, address(this), nonce, nonceSecret, true
)
);
nonceManager.submit(nonce, true, nonceSecret);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.NonReplayableNonce.selector, address(this), nonce, EXHAUSTED_TOKEN, true
)
);
nonceManager.submit(nonce, true, EXHAUSTED_TOKEN);
vm.expectRevert(
abi.encodeWithSelector(
QuarkNonceManager.NonReplayableNonce.selector, address(this), nonce, FREE_TOKEN, true
)
);
nonceManager.submit(nonce, true, FREE_TOKEN);
}

function testPrecancelNonce() public {
bytes32 nonce = bytes32(uint256(1));

vm.prank(address(0x123));
nonceManager.cancel(nonce);

// by default, nonce 0 is not set
hayesgm marked this conversation as resolved.
Show resolved Hide resolved
assertEq(nonceManager.submissions(address(0x123), nonce), nonceManager.EXHAUSTED());

// nonce 0 can be set manually
hayesgm marked this conversation as resolved.
Show resolved Hide resolved
vm.prank(address(0x123));
vm.expectRevert(
abi.encodeWithSelector(QuarkNonceManager.NonReplayableNonce.selector, address(0x123), nonce, nonce, true)
);
nonceManager.submit(nonce, false, nonce);
assertEq(nonceManager.submissions(address(0x123), nonce), nonceManager.EXHAUSTED());
}

function testCancelExhaustedIsNoOp() public {
bytes32 nonce = bytes32(uint256(1));

// by default, nonce 1 is not set
assertEq(nonceManager.submissions(address(0x123), nonce), nonceManager.FREE());

// nonce 1 can be set manually
vm.prank(address(0x123));
nonceManager.submit(nonce, false, nonce);
assertEq(nonceManager.submissions(address(0x123), nonce), nonceManager.EXHAUSTED());
Comment on lines +431 to +434
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we already have this and I didn't see it -- but is there also a case for manually doing nonceManager.submit(nonce, true, nonce)? That is, making it replayable before you ever actually execute an op with that nonce.

one weird case would be:

  1. manually nonceManager.submit(nonce, true, nonce)
  2. submit an op that has isReplayable=false

if chain length is 1, you're in a weird spot: it'll revert the first time you try to submit an op with that nonce. So you can never execute a script with that nonce (except if you find a preimage). You can clean it up by doing cancel(nonce) from a new op, but otherwise it's kind of a weird edge case. Convoluted, but perhaps worth documenting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting-- I guess i'm thinking "if a script sends a weird message to nonce manager is it that important?" as a rule of thumb. Totally smoething to consider, but can't a script also call like submit(nonce, true, 5) or whatever that's also weird?


vm.prank(address(0x123));
nonceManager.cancel(nonce);

assertEq(nonceManager.submissions(address(0x123), nonce), nonceManager.EXHAUSTED());
}
}
Loading
Loading