did some more contracts

Author: nlordell

contracts/Safe.sol

@@ -438,7 +438,7 @@ contract Safe is
         //
         // WARNING: We do not clean potential dirty bits in types that are less than 256 bits (addresses and `Enum.Operation`)
         // Solidity types that are smaller than 256 bit can have dirty high bits when accessed in assembly according to the spec
-        // (see the warning in <https://docs.soliditylang.org/en/latest/assembly.html#access-to-external-variables-functions-and-libraries>).
+        // (see the warning in <https://docs.soliditylang.org/en/v0.7.6/assembly.html#access-to-external-variables-functions-and-libraries>).
         // 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`.
         // This is not a problem, however, we must consider this for potential future changes.
         /* solhint-disable no-inline-assembly */

contracts/base/FallbackManager.sol

@@ -65,7 +65,7 @@ abstract contract FallbackManager is SelfAuthorized, IFallbackManager {
         assembly {
             // When compiled with the optimizer, the compiler relies on certain assumptions on how the
             // memory is used, therefore we need to guarantee memory safety (keeping the free memory pointer
-            // at memory offset 0x40 intact, not going beyond the scratch space, etc)
+            // at memory offset 0x40 intact, not going beyond the scratch space, etc).
             // See: <https://docs.soliditylang.org/en/latest/assembly.html#memory-safety>
 
             let handler := sload(FALLBACK_HANDLER_STORAGE_SLOT)

contracts/common/NativeCurrencyPaymentFallback.sol

@@ -2,14 +2,20 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 /**
- * @title NativeCurrencyPaymentFallback - A contract that has a fallback to accept native currency payments.
+ * @title Native Currency Payment Fallback
+ * @notice A contract that has a fallback to accept native token payments.
  * @author Richard Meissner - @rmeissner
  */
 abstract contract NativeCurrencyPaymentFallback {
+    /**
+     * @notice Native tokens were received.
+     * @param sender The address that sent the tokens.
+     * @param value The native token value that was received.
+     */
     event SafeReceived(address indexed sender, uint256 value);
 
     /**
-     * @notice Receive function accepts native currency transactions.
+     * @notice Receive function that accepts native token transactions.
      * @dev Emits an event with sender and received value.
      */
     receive() external payable {

contracts/common/SecuredTokenTransfer.sol

@@ -2,27 +2,28 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 /**
- * @title SecuredTokenTransfer - Secure token transfer.
+ * @title Secured Token Transfer
+ * @notice Securely transfer tokens, handling non-standard ERC-20 tokens.
  * @author Richard Meissner - @rmeissner
  */
 abstract contract SecuredTokenTransfer {
     /**
-     * @notice Transfers a token and returns a boolean if it was a success
+     * @notice Transfers a token and returns `true` if it was successful, `false` otherwise.
      * @dev It checks the return data of the transfer call and returns true if the transfer was successful.
      *      It doesn't check if the `token` address is a contract or not.
-     * @param token Token that should be transferred
-     * @param receiver Receiver to whom the token should be transferred
-     * @param amount The amount of tokens that should be transferred
-     * @return transferred Returns true if the transfer was successful
+     * @param token Token that should be transferred.
+     * @param receiver Receiver to whom the token should be transferred.
+     * @param amount The amount of tokens that should be transferred.
+     * @return transferred Boolean indicating whether or not the transfer was successful.
      */
     function transferToken(address token, address receiver, uint256 amount) internal returns (bool transferred) {
-        // 0xa9059cbb - bytes4(keccak256("transfer(address,uint256)"))
+        // The transfer function selector `0xa9059cbb`, precomputed value of `bytes4(keccak256("transfer(address,uint256)"))`.
         bytes memory data = abi.encodeWithSelector(0xa9059cbb, receiver, amount);
         /* solhint-disable no-inline-assembly */
         /// @solidity memory-safe-assembly
         assembly {
             // We write the return value to scratch space.
-            // See https://docs.soliditylang.org/en/v0.7.6/internals/layout_in_memory.html#layout-in-memory
+            // See <https://docs.soliditylang.org/en/v0.7.6/internals/layout_in_memory.html#layout-in-memory>
             let success := call(sub(gas(), 10000), token, 0, add(data, 0x20), mload(data), 0, 0x20)
             switch returndatasize()
             case 0 {

contracts/common/SelfAuthorized.sol

@@ -4,16 +4,24 @@ pragma solidity >=0.7.0 <0.9.0;
 import {ErrorMessage} from "../libraries/ErrorMessage.sol";
 
 /**
- * @title SelfAuthorized - Authorizes current contract to perform actions to itself.
+ * @title Self Authorized
+ * @notice Authorizes current contract to perform actions on itself.
  * @author Richard Meissner - @rmeissner
  */
 abstract contract SelfAuthorized is ErrorMessage {
+    /**
+     * @dev Ensure that the `msg.sender` is the current contract.
+     */
     function requireSelfCall() private view {
         if (msg.sender != address(this)) revertWithError("GS031");
     }
 
+    /**
+     * @notice Ensure that a function is authorized.
+     * @dev This modifier authorizes calls by ensuring that the contract called itself.
+     */
     modifier authorized() {
-        // Modifiers are copied around during compilation. This is a function call as it minimized the bytecode size
+        // Modifiers are copied around during compilation. This is a function call to minimized the bytecode size.
         requireSelfCall();
         _;
     }

contracts/common/SignatureDecoder.sol

@@ -2,18 +2,19 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 /**
- * @title SignatureDecoder - Decodes signatures encoded as bytes
+ * @title Signature Decoder
+ * @notice Decodes encoded packed signatures bytes.
  * @author Richard Meissner - @rmeissner
  */
 abstract contract SignatureDecoder {
     /**
-     * @notice Splits signature bytes into `uint8 v, bytes32 r, bytes32 s`.
-     * @dev Make sure to perform a bounds check for @param pos, to avoid out of bounds access on @param signatures
-     *      The signature format is a compact form of {bytes32 r}{bytes32 s}{uint8 v}
-     *      Compact means uint8 is not padded to 32 bytes.
-     * @param pos Which signature to read.
-     *            A prior bounds check of this parameter should be performed, to avoid out of bounds access.
-     * @param signatures Concatenated {r, s, v} signatures.
+     * @notice Splits signature bytes into its `v`, `r` and `s` components, starting from `pos`.
+     * @dev Make sure to perform a bounds check for `pos`, to avoid out of bounds access on `signatures`.
+     *      The signature format is a compact form of `r:bytes32 || s:bytes32 || v:uint8`,
+     *      where `v` is a single byte and not padded to 32 bytes.
+     * @param pos Position to start reading the packed signature components.
+     *            Bounds check that `signatures[pos:pos+65]` is valid must be performed by the caller.
+     * @param signatures Encoded packed signatures bytes.
      * @return v Recovery ID or Safe signature type.
      * @return r Output value r of the signature.
      * @return s Output value s of the signature.

contracts/common/Singleton.sol

@@ -2,12 +2,16 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 /**
- * @title Singleton - Base for singleton contracts (should always be the first super contract)
- *        This contract is tightly coupled to our proxy contract (see `proxies/SafeProxy.sol`)
+ * @title Singleton
+ * @notice Base contract for singleton that can be used as implementations for {SafeProxy}s.
+ * @dev This must always be the first super contract.
+ *      This contract is tightly coupled to our {SafeProxy} contract (see `proxies/SafeProxy.sol`).
  * @author Richard Meissner - @rmeissner
  */
 abstract contract Singleton {
-    // singleton always has to be the first declared variable to ensure the same location as in the Proxy contract.
-    // It should also always be ensured the address is stored alone (uses a full word)
+    /**
+     * @dev `singleton` must be the first declared variable to ensure it has the same storage location as in the {SafeProxy} contract.
+     * The address must be stored alone: it must use a full 32-byte word and cannot be packed with other storage variables.
+     */
     address private singleton;
 }

contracts/common/StorageAccessible.sol

@@ -2,20 +2,21 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 /**
- * @title StorageAccessible - A generic base contract that allows callers to access all internal storage.
- * @notice See https://github.com/gnosis/util-contracts/blob/bb5fe5fb5df6d8400998094fb1b32a178a47c3a1/contracts/StorageAccessible.sol
- *         It removes a method from the original contract not needed for the Safe Smart Account contracts.
+ * @title Storage Accessible
+ * @notice A generic base contract that allows callers to access all internal storage.
+ * @dev See <https://github.com/gnosis/util-contracts/blob/bb5fe5fb5df6d8400998094fb1b32a178a47c3a1/contracts/StorageAccessible.sol>
+ *      It removes a method from the original contract not needed for the Safe Smart Account contracts.
  * @author Gnosis Developers
  */
 abstract contract StorageAccessible {
     /**
      * @notice Reads `length` bytes of storage in the current contract
-     * @param offset - the offset in the current contract's storage in words to start reading from
-     * @param length - the number of words (32 bytes) of data to read
-     * @return the bytes that were read.
+     * @param offset The offset in the current contract's storage in words to start reading from.
+     * @param length The number of words (32 bytes) of data to read.
+     * @return The bytes that were read.
      */
     function getStorageAt(uint256 offset, uint256 length) public view returns (bytes memory) {
-        // We use `<< 5` instead of `* 32` as SHR / SHL opcode only uses 3 gas, while DIV / MUL opcode uses 5 gas.
+        // We use `<< 5` instead of the equivalent `* 32` as `SHL` opcode only uses 3 gas, while the `MUL` opcode uses 5 gas.
         bytes memory result = new bytes(length << 5);
         for (uint256 index = 0; index < length; ++index) {
             /* solhint-disable no-inline-assembly */
@@ -30,13 +31,11 @@ abstract contract StorageAccessible {
     }
 
     /**
-     * @dev Performs a delegatecall on a targetContract in the context of self.
-     * Internally reverts execution to avoid side effects (making it effectively static).
-     *
-     * This method reverts with data equal to `abi.encodePacked(uint256(success), uint256(response.length), bytes(response))`.
-     * Specifically, the `returndata` after a call to this method will be:
-     * `success:uint256 || response.length:uint256 || response:bytes`.
-     *
+     * @notice Performs a `DELEGATECALL` to a `targetContract` in the context of self.
+     * @dev Internally reverts execution to avoid side effects (making it effectively static).
+     *      This method reverts with data equal to `abi.encodePacked(uint256(success), uint256(response.length), bytes(response))`.
+     *      Specifically, the return data after a call to this method will be:
+     *      `success:uint256 || response.length:uint256 || response:bytes`.
      * @param targetContract Address of the contract containing the code to execute.
      * @param calldataPayload Calldata that should be sent to the target contract (encoded method name and arguments).
      */
@@ -45,7 +44,7 @@ abstract contract StorageAccessible {
         /// @solidity memory-safe-assembly
         assembly {
             let success := delegatecall(gas(), targetContract, add(calldataPayload, 0x20), mload(calldataPayload), 0, 0)
-            // Load free memory location
+            // Load free memory location.
             let ptr := mload(0x40)
             mstore(ptr, success)
             mstore(add(ptr, 0x20), returndatasize())

contracts/external/SafeMath.sol

@@ -2,20 +2,20 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 /**
- * @title SafeMath
- * @notice Math operations with safety checks that revert on error (overflow/underflow)
+ * @title Safe Math
+ * @notice Math operations with safety checks that revert on error (overflow/underflow).
  */
 library SafeMath {
     /**
      * @notice Multiplies two numbers, reverts on overflow.
-     * @param a First number
-     * @param b Second number
-     * @return Product of a and b
+     * @param a First number.
+     * @param b Second number.
+     * @return Product of `a` and `b`.
      */
     function mul(uint256 a, uint256 b) internal pure returns (uint256) {
-        // Gas optimization: this is cheaper than requiring 'a' not to be zero, but the
-        // benefit is lost if 'b' is also tested.
-        // See: https://github.com/OpenZeppelin/openzeppelin-solidity/pull/522
+        // Gas optimization: this is cheaper than requiring 'a' not to be zero,
+        // but the benefit is lost if 'b' is also tested.
+        // See: <https://github.com/OpenZeppelin/openzeppelin-solidity/pull/522>
         if (a == 0) {
             return 0;
         }
@@ -28,9 +28,9 @@ library SafeMath {
 
     /**
      * @notice Subtracts two numbers, reverts on overflow (i.e. if subtrahend is greater than minuend).
-     * @param a First number
-     * @param b Second number
-     * @return Difference of a and b
+     * @param a First number.
+     * @param b Second number.
+     * @return Difference of `a` and `b`.
      */
     function sub(uint256 a, uint256 b) internal pure returns (uint256) {
         require(b <= a);
@@ -41,9 +41,9 @@ library SafeMath {
 
     /**
      * @notice Adds two numbers, reverts on overflow.
-     * @param a First number
-     * @param b Second number
-     * @return Sum of a and b
+     * @param a First number.
+     * @param b Second number.
+     * @return Sum of `a` and `b`.
      */
     function add(uint256 a, uint256 b) internal pure returns (uint256) {
         uint256 c = a + b;
@@ -54,9 +54,9 @@ library SafeMath {
 
     /**
      * @notice Returns the largest of two numbers.
-     * @param a First number
-     * @param b Second number
-     * @return Largest of a and b
+     * @param a First number.
+     * @param b Second number.
+     * @return Largest of `a` and `b`.
      */
     function max(uint256 a, uint256 b) internal pure returns (uint256) {
         return a >= b ? a : b;

docs/guidelines.md

@@ -52,13 +52,13 @@ We are manually reviewing the NatSpec documentation to make sure there aren't an
 - [x] base/GuardManager.sol
 - [x] base/ModuleManager.sol
 - [x] base/OwnerManager.sol
-- [ ] common/NativeCurrencyPaymentFallback.sol
-- [ ] common/SecuredTokenTransfer.sol
-- [ ] common/SelfAuthorized.sol
-- [ ] common/SignatureDecoder.sol
-- [ ] common/Singleton.sol
-- [ ] common/StorageAccessible.sol
-- [ ] external/SafeMath.sol
+- [x] common/NativeCurrencyPaymentFallback.sol
+- [x] common/SecuredTokenTransfer.sol
+- [x] common/SelfAuthorized.sol
+- [x] common/SignatureDecoder.sol
+- [x] common/Singleton.sol
+- [x] common/StorageAccessible.sol
+- [x] external/SafeMath.sol
 - [ ] handler/CompatibilityFallbackHandler.sol
 - [ ] handler/ExtensibleFallbackHandler.sol
 - [ ] handler/HandlerContext.sol