test(ctb): Add natspec to test functions

packages/contracts-bedrock/test/LivenessGuard.t.sol

@@ -10,14 +10,6 @@ import "test/safe-tools/SafeTestTools.sol";
 
 import { LivenessGuard } from "src/Safe/LivenessGuard.sol";
 
-// Todo(Maurelian):
-// Other tests needed:
-//   - EIP1271 signatures
-//   - Signatures from contracts
-//   - Signatures from non-owners
-//   - Signers may call directly to prove liveness (must be an owner).
-//   - Unexpected length of signature data
-
 contract LivenessGuard_TestInit is Test, SafeTestTools {
     using SafeTestLib for SafeInstance;
 
@@ -26,6 +18,7 @@ contract LivenessGuard_TestInit is Test, SafeTestTools {
     LivenessGuard livenessGuard;
     SafeInstance safeInstance;
 
+    /// @dev Sets up the test environment
     function setUp() public {
         safeInstance = _setupSafe();
         livenessGuard = new LivenessGuard(safeInstance.safe);
@@ -34,6 +27,7 @@ contract LivenessGuard_TestInit is Test, SafeTestTools {
 }
 
 contract LivenessGuard_Getters_Test is LivenessGuard_TestInit {
+    /// @dev Tests that the getters return the correct values
     function test_getters_works() external {
         assertEq(address(livenessGuard.safe()), address(safeInstance.safe));
         assertEq(livenessGuard.lastLive(address(0)), 0);
@@ -41,6 +35,7 @@ contract LivenessGuard_Getters_Test is LivenessGuard_TestInit {
 }
 
 contract LivenessGuard_CheckTx_TestFails is LivenessGuard_TestInit {
+    /// @dev Tests that the checkTransaction function reverts if the caller is not the Safe
     function test_checkTransaction_callerIsNotSafe_revert() external {
         vm.expectRevert("LivenessGuard: only Safe can call this function");
         livenessGuard.checkTransaction({
@@ -62,6 +57,7 @@ contract LivenessGuard_CheckTx_TestFails is LivenessGuard_TestInit {
 contract LivenessGuard_CheckTx_Test is LivenessGuard_TestInit {
     using SafeTestLib for SafeInstance;
 
+    /// @dev Tests that the checkTransaction function succeeds
     function test_checkTransaction_succeeds() external {
         // Create an array of the addresses who will sign the transaction. SafeTestTools
         // will generate these signatures up to the threshold by iterating over the owners array.
@@ -85,6 +81,7 @@ contract LivenessGuard_CheckTx_Test is LivenessGuard_TestInit {
 }
 
 contract LivenessGuard_CheckAfterExecution_TestFails is LivenessGuard_TestInit {
+    /// @dev Tests that the checkAfterExecution function reverts if the caller is not the Safe
     function test_checkAfterExecution_callerIsNotSafe_revert() external {
         vm.expectRevert("LivenessGuard: only Safe can call this function");
         livenessGuard.checkAfterExecution(bytes32(0), false);
@@ -93,7 +90,16 @@ contract LivenessGuard_CheckAfterExecution_TestFails is LivenessGuard_TestInit {
 
 contract LivenessGuard_CheckAfterExecution_Test is LivenessGuard_TestInit { }
 
+contract LivenessGuard_ShowLiveness_TestFail is LivenessGuard_TestInit {
+    /// @dev Tests that the showLiveness function reverts if the caller is not an owner
+    function test_showLiveness_callIsNotSafeOwner_reverts() external {
+        vm.expectRevert("LivenessGuard: only Safe owners can call this function");
+        livenessGuard.showLiveness();
+    }
+}
+
 contract LivenessGuard_ShowLiveness_Test is LivenessGuard_TestInit {
+    /// @dev Tests that the showLiveness function succeeds
     function test_showLiveness_succeeds() external {
         // Cache the caller
         address caller = safeInstance.owners[0];

packages/contracts-bedrock/test/LivenessModule.t.sol

@@ -12,6 +12,9 @@ import "test/safe-tools/SafeTestTools.sol";
 import { LivenessModule } from "src/Safe/LivenessModule.sol";
 import { LivenessGuard } from "src/Safe/LivenessGuard.sol";
 
+/// @dev A minimal wrapper around the OwnerManager contract. This contract is meant to be initialized with
+///      the same owners as a Safe instance, and then used to simulate the resulting owners list
+///      after an owner is removed.
 contract OwnerSimulator is OwnerManager {
     constructor(address[] memory _owners, uint256 _threshold) {
         setupOwners(_owners, _threshold);
@@ -25,7 +28,7 @@ contract OwnerSimulator is OwnerManager {
 contract LivenessModule_TestInit is Test, SafeTestTools {
     using SafeTestLib for SafeInstance;
 
-    /// @notice The address of the first owner in the linked list of owners
+    /// @dev The address of the first owner in the linked list of owners
     address internal constant SENTINEL_OWNERS = address(0x1);
 
     event SignersRecorded(bytes32 indexed txHash, address[] signers);
@@ -50,7 +53,7 @@ contract LivenessModule_TestInit is Test, SafeTestTools {
         }
     }
 
-    /// @notice Given an arrary of owners to remove, this function will return an array of the previous owners
+    /// @dev Given an array of owners to remove, this function will return an array of the previous owners
     ///         in the order that they must be provided to the LivenessMoules's removeOwners() function.
     ///         Because owners are removed one at a time, and not necessarily in order, we need to simulate
     ///         the owners list after each removal, in order to identify the correct previous owner.
@@ -67,6 +70,7 @@ contract LivenessModule_TestInit is Test, SafeTestTools {
         }
     }
 
+    /// @dev Sets up the test environment
     function setUp() public {
         // Create a Safe with 10 owners
         (, uint256[] memory keys) = makeAddrsAndKeys(10);
@@ -88,6 +92,7 @@ contract LivenessModule_TestInit is Test, SafeTestTools {
 }
 
 contract LivenessModule_Getters_Test is LivenessModule_TestInit {
+    /// @dev Tests if the getters work correctly
     function test_getters_works() external {
         assertEq(address(livenessModule.safe()), address(safeInstance.safe));
         assertEq(address(livenessModule.livenessGuard()), address(livenessGuard));
@@ -125,6 +130,7 @@ contract LivenessModule_Get75PercentThreshold_Test is LivenessModule_TestInit {
 }
 
 contract LivenessModule_RemoveOwner_Test is LivenessModule_TestInit {
+    /// @dev Tests if removing one owner works correctly
     function test_removeOwner_oneOwner_succeeds() external {
         uint256 ownersBefore = safeInstance.owners.length;
         address[] memory prevOwners = new address[](1);
@@ -138,6 +144,7 @@ contract LivenessModule_RemoveOwner_Test is LivenessModule_TestInit {
         assertEq(safeInstance.safe.getOwners().length, ownersBefore - 1);
     }
 
+    /// @dev Tests if removing all owners works correctly
     function test_removeOwner_allOwners_succeeds() external {
         uint256 numOwners = safeInstance.owners.length;