Skip to content

Commit

Permalink
CS6.3 - Fix Inaccurate and Incomplete NatSpec
Browse files Browse the repository at this point in the history
  • Loading branch information
@kevincheng96
kevincheng96 committed Oct 15, 2024
1 parent 7d4a98c commit c241dea
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 24 deletions.
6 changes: 3 additions & 3 deletions src/quark-core/src/QuarkNonceManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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);
}
Expand Down
57 changes: 48 additions & 9 deletions src/quark-core/src/QuarkScript.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -81,21 +85,37 @@ abstract contract QuarkScript {
}
}

/// @notice Disables callbacks to the wallet
function clearCallback() internal {
bytes32 callbackSlot = QuarkWalletMetadata.CALLBACK_SLOT;
assembly {
tstore(callbackSlot, 0)
}
}

/**
* @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);
Expand All @@ -105,29 +125,46 @@ 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 {
sstore(isolatedKey, value)
}
}

// 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;
Expand All @@ -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;
Expand All @@ -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();
Expand Down
24 changes: 12 additions & 12 deletions src/quark-core/src/QuarkWallet.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
*/
Expand All @@ -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
*/
Expand All @@ -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
*/
Expand Down

0 comments on commit c241dea

Please sign in to comment.