Skip to content

Commit 4309776

Browse files
authored
Add a warning comment about potential dirty bits in getTransactionHash (#872)
This commit adds a detailed warning in the `Safe.sol` contract regarding potential dirty bits in assembly code for types smaller than 256 bits. It emphasizes the importance of considering this for future changes while explaining the rationale behind using assembly for memory efficiency. No functional changes were made to the contract logic. This update aims to improve code clarity and maintainability. Originally reported by @jhoenicke
1 parent 489b2bc commit 4309776

File tree

1 file changed

+5
-0
lines changed

1 file changed

+5
-0
lines changed

contracts/Safe.sol

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,11 @@ contract Safe is
407407

408408
// We opted for using assembly code here, because the way Solidity compiler we use (0.7.6) allocates memory is
409409
// inefficient. We do not need to allocate memory for temporary variables to be used in the keccak256 call.
410+
//
411+
// WARNING: We do not clean potential dirty bits in types that are less than 256 bits (addresses and Enum.Operation)
412+
// The solidity assembly types that are smaller than 256 bit can have dirty high bits according to the spec (see the Warning in https://docs.soliditylang.org/en/latest/assembly.html#access-to-external-variables-functions-and-libraries).
413+
// However, we read most of the data from calldata, where the variables are not packed, and the only variable we read from storage is uint256 nonce.
414+
// This is not a problem, however, we must consider this for potential future changes.
410415
/* solhint-disable no-inline-assembly */
411416
/// @solidity memory-safe-assembly
412417
assembly {

0 commit comments

Comments
 (0)