From c241dea5661010b93a74074e7c80144cba0e74d2 Mon Sep 17 00:00:00 2001 From: kevincheng96 Date: Tue, 15 Oct 2024 12:11:29 -0700 Subject: [PATCH] CS6.3 - Fix Inaccurate and Incomplete NatSpec --- src/quark-core/src/QuarkNonceManager.sol | 6 +-- src/quark-core/src/QuarkScript.sol | 57 ++++++++++++++++++++---- src/quark-core/src/QuarkWallet.sol | 24 +++++----- 3 files changed, 63 insertions(+), 24 deletions(-) diff --git a/src/quark-core/src/QuarkNonceManager.sol b/src/quark-core/src/QuarkNonceManager.sol index 748d527e..896b06e1 100644 --- a/src/quark-core/src/QuarkNonceManager.sol +++ b/src/quark-core/src/QuarkNonceManager.sol @@ -43,9 +43,9 @@ 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 nonce The nonce of the Quark operation to submit. This value is the root of the nonce-chain that the submissionToken is a part of. * @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. + * @param submissionToken The token for this submission. For single-use operations and first-use replayable operations, set `submissionToken` = `nonce`. Otherwise, the next submission token from the nonce-chain. */ function submit(bytes32 nonce, bool isReplayable, bytes32 submissionToken) external { bytes32 lastTokenSubmission = submissions[msg.sender][nonce]; @@ -71,7 +71,7 @@ contract QuarkNonceManager { revert InvalidSubmissionToken(msg.sender, nonce, submissionToken); } - // Note: even with a valid submission token, we always set non-replayables to exhausted (e.g. for cancellations) + // Note: Even with a valid submission token, we always set non-replayables to exhausted (e.g. for cancellations) submissions[msg.sender][nonce] = isReplayable ? submissionToken : EXHAUSTED; emit NonceSubmitted(msg.sender, nonce, submissionToken); } diff --git a/src/quark-core/src/QuarkScript.sol b/src/quark-core/src/QuarkScript.sol index 4c4a062b..520d82f1 100644 --- a/src/quark-core/src/QuarkScript.sol +++ b/src/quark-core/src/QuarkScript.sol @@ -14,11 +14,11 @@ abstract contract QuarkScript { error InvalidActiveNonce(); error InvalidActiveSubmissionToken(); - /// @notice Storage location for the re-entrancy guard + /// @notice Transient storage location for the re-entrancy guard bytes32 internal constant REENTRANCY_FLAG_SLOT = bytes32(uint256(keccak256("quark.scripts.reentrancy.guard.v1")) - 1); - /// @notice A safer, but gassier reentrancy guard that writes the flag to the QuarkNonceManager + /// @notice Reentrancy guard that writes the flag to the wallet's transient storage modifier nonReentrant() { bytes32 slot = REENTRANCY_FLAG_SLOT; bytes32 flag; @@ -60,18 +60,22 @@ abstract contract QuarkScript { _; } + /// @notice Returns the `signer` of the wallet function signer() internal view returns (address) { return IHasSignerExecutor(address(this)).signer(); } + /// @notice Returns the `executor` of the wallet function executor() internal view returns (address) { return IHasSignerExecutor(address(this)).executor(); } + /// @notice Returns the `NonceManager` of the wallet function nonceManager() internal view returns (QuarkNonceManager) { return QuarkNonceManager(IQuarkWallet(address(this)).nonceManager()); } + /// @notice Enables callbacks to the wallet function allowCallback() internal { bytes32 callbackSlot = QuarkWalletMetadata.CALLBACK_SLOT; bytes32 activeScriptSlot = QuarkWalletMetadata.ACTIVE_SCRIPT_SLOT; @@ -81,6 +85,7 @@ abstract contract QuarkScript { } } + /// @notice Disables callbacks to the wallet function clearCallback() internal { bytes32 callbackSlot = QuarkWalletMetadata.CALLBACK_SLOT; assembly { @@ -88,14 +93,29 @@ abstract contract QuarkScript { } } + /** + * @notice Reads a uint256 from the wallet's storage + * @param key The key to read the value from + * @return The uint256 stored at the key + */ function readU256(string memory key) internal view returns (uint256) { return uint256(read(key)); } + /** + * @notice Reads a bytes32 from the wallet's storage + * @param key The key to read the value from + * @return The bytes32 stored at the key + */ function read(string memory key) internal view returns (bytes32) { return read(keccak256(bytes(key))); } + /** + * @notice Reads a bytes32 from the wallet's storage + * @param key The key to read the value from + * @return The bytes32 stored at the key + */ function read(bytes32 key) internal view returns (bytes32) { bytes32 value; bytes32 isolatedKey = getNonceIsolatedKey(key); @@ -105,14 +125,29 @@ abstract contract QuarkScript { return value; } + /** + * @notice Writes a uint256 to the wallet's storage + * @param key The key to write the value to + * @param value The value to write to storage + */ function writeU256(string memory key, uint256 value) internal { return write(key, bytes32(value)); } + /** + * @notice Writes a bytes32 to the wallet's storage + * @param key The key to write the value to + * @param value The value to write to storage + */ function write(string memory key, bytes32 value) internal { return write(keccak256(bytes(key)), value); } + /** + * @notice Writes a bytes32 to the wallet's storage + * @param key The key to write the value to + * @param value The value to write to storage + */ function write(bytes32 key, bytes32 value) internal { bytes32 isolatedKey = getNonceIsolatedKey(key); assembly { @@ -120,14 +155,16 @@ abstract contract QuarkScript { } } - // Returns a key isolated to the active nonce of a script - // This provide cooperative isolation of storage between scripts. + /** + * @notice Returns a key isolated to the active nonce of a script. This provides cooperative isolation of storage between scripts. + * @param key The key to create an nonce-isolated version of + */ function getNonceIsolatedKey(bytes32 key) internal view returns (bytes32) { bytes32 nonce = getActiveNonce(); return keccak256(abi.encodePacked(nonce, key)); } - // Note: this may not be accurate after any nested calls from a script + /// @notice Returns the active nonce of the wallet function getActiveNonce() internal view returns (bytes32) { bytes32 activeNonceSlot = QuarkWalletMetadata.ACTIVE_NONCE_SLOT; bytes32 value; @@ -138,7 +175,7 @@ abstract contract QuarkScript { return value; } - // Note: this may not be accurate after any nested calls from a script + /// @notice Returns the active submission token of the wallet function getActiveSubmissionToken() internal view returns (bytes32) { bytes32 activeSubmissionTokenSlot = QuarkWalletMetadata.ACTIVE_SUBMISSION_TOKEN_SLOT; bytes32 value; @@ -148,9 +185,11 @@ abstract contract QuarkScript { return value; } - // Note: this may not be accurate after any nested calls from a script - // Returns the active replay count of this script. Thus, the first submission should return 0, - // the second submission 1, and so on. This must be called before the script makes any external calls. + /** + * @notice Returns the active replay count of this script. Thus, the first submission should return 0, + * the second submission 1, and so on. + * @param key The key to create an nonce-isolated version of + */ function getActiveReplayCount() internal view returns (uint256) { bytes32 nonce = getActiveNonce(); bytes32 submissionToken = getActiveSubmissionToken(); diff --git a/src/quark-core/src/QuarkWallet.sol b/src/quark-core/src/QuarkWallet.sol index 1aebe699..5a908deb 100644 --- a/src/quark-core/src/QuarkWallet.sol +++ b/src/quark-core/src/QuarkWallet.sol @@ -41,16 +41,16 @@ library QuarkWalletMetadata { bytes32 internal constant MULTI_QUARK_OPERATION_DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,string version)"); - /// @notice Well-known storage slot for the currently executing script's callback address (if any) + /// @notice Well-known transient storage slot for the currently executing script's callback address (if any) bytes32 internal constant CALLBACK_SLOT = bytes32(uint256(keccak256("quark.v1.callback")) - 1); - /// @notice Well-known storage slot for the currently executing script's address (if any) + /// @notice Well-known transient storage slot for the currently executing script's address (if any) bytes32 internal constant ACTIVE_SCRIPT_SLOT = bytes32(uint256(keccak256("quark.v1.active.script")) - 1); - /// @notice Well-known storage slot for the nonce of the script that's currently executing. + /// @notice Well-known transient storage slot for the nonce of the script that's currently executing. bytes32 internal constant ACTIVE_NONCE_SLOT = bytes32(uint256(keccak256("quark.v1.active.nonce")) - 1); - /// @notice Well-known storage slot for the submission token of the script that's currently executing. + /// @notice Well-known transient storage slot for the submission token of the script that's currently executing. bytes32 internal constant ACTIVE_SUBMISSION_TOKEN_SLOT = bytes32(uint256(keccak256("quark.v1.active.submissionToken")) - 1); } @@ -126,16 +126,16 @@ contract QuarkWallet is IERC1271 { ) ); - /// @notice Well-known storage slot for the currently executing script's callback address (if any) + /// @notice Well-known transient storage slot for the currently executing script's callback address (if any) bytes32 public constant CALLBACK_SLOT = QuarkWalletMetadata.CALLBACK_SLOT; - /// @notice Well-known storage slot for the currently executing script's address (if any) + /// @notice Well-known transient storage slot for the currently executing script's address (if any) bytes32 public constant ACTIVE_SCRIPT_SLOT = QuarkWalletMetadata.ACTIVE_SCRIPT_SLOT; - /// @notice Well-known storage slot for the nonce of the script that's currently executing. + /// @notice Well-known transient storage slot for the nonce of the script that's currently executing. bytes32 public constant ACTIVE_NONCE_SLOT = QuarkWalletMetadata.ACTIVE_NONCE_SLOT; - /// @notice Well-known storage slot for the submission token of the script that's currently executing. + /// @notice Well-known transient storage slot for the submission token of the script that's currently executing. bytes32 public constant ACTIVE_SUBMISSION_TOKEN_SLOT = QuarkWalletMetadata.ACTIVE_SUBMISSION_TOKEN_SLOT; /// @notice The magic value to return for valid ERC1271 signature @@ -333,7 +333,7 @@ contract QuarkWallet is IERC1271 { } /** - * @dev Returns the domain separator for this Quark wallet + * @notice Returns the domain separator for this Quark wallet * @return Domain separator */ function getDomainSeparator() internal view returns (bytes32) { @@ -343,7 +343,7 @@ contract QuarkWallet is IERC1271 { } /** - * @dev Returns the EIP-712 digest for a QuarkOperation + * @notice Returns the EIP-712 digest for a QuarkOperation * @param op A QuarkOperation struct * @return EIP-712 digest */ @@ -368,7 +368,7 @@ contract QuarkWallet is IERC1271 { } /** - * @dev Returns the EIP-712 digest for a MultiQuarkOperation + * @notice Returns the EIP-712 digest for a MultiQuarkOperation * @param opDigests A list of EIP-712 digests for the operations in a MultiQuarkOperation * @return EIP-712 digest */ @@ -379,7 +379,7 @@ contract QuarkWallet is IERC1271 { } /** - * @dev Returns the EIP-712 digest of a QuarkMessage that can be signed by `signer` + * @notice Returns the EIP-712 digest of a QuarkMessage that can be signed by `signer` * @param message Message that should be hashed * @return Message hash */