From 38f828d054668ebdcddaa36e4961a16e7e849bab Mon Sep 17 00:00:00 2001 From: Maurelian <maurelian@protonmail.ch> Date: Mon, 23 Oct 2023 13:09:24 -0400 Subject: [PATCH] feat(ctb): Rename lastSigned to more accurate lastLive This is more accurate because of the fact that showLiveness can be used without approving a transaction. --- packages/contracts-bedrock/src/Safe/LivenessGuard.sol | 10 +++++----- packages/contracts-bedrock/src/Safe/LivenessModule.sol | 2 +- packages/contracts-bedrock/test/LivenessGuard.t.sol | 4 ++-- specs/safe-liveness-checking.md | 7 +++---- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/contracts-bedrock/src/Safe/LivenessGuard.sol b/packages/contracts-bedrock/src/Safe/LivenessGuard.sol index 39da08c27..310e771f2 100644 --- a/packages/contracts-bedrock/src/Safe/LivenessGuard.sol +++ b/packages/contracts-bedrock/src/Safe/LivenessGuard.sol @@ -21,8 +21,8 @@ contract LivenessGuard is ISemver, GetSigners, BaseGuard { string public constant version = "1.0.0"; Safe public immutable safe; - mapping(address => uint256) public lastSigned; + mapping(address => uint256) public lastLive; EnumerableSet.AddressSet private ownersBefore; constructor(Safe _safe) { @@ -42,14 +42,14 @@ contract LivenessGuard is ISemver, GetSigners, BaseGuard { // If the value was present, remove() returns true. if (ownersBefore.remove(ownersAfter[i]) == false) { // This address was not already an owner, add it to the lastSigned mapping - lastSigned[ownersAfter[i]] = block.timestamp; + lastLive[ownersAfter[i]] = block.timestamp; } } // Now iterate over the remaining ownersBefore entries. Any remaining addresses are no longer an owner, so we // delete them from the lastSigned mapping. for (uint256 j = 0; j < ownersBefore.length(); j++) { address owner = ownersBefore.at(j); - delete lastSigned[owner]; + delete lastLive[owner]; } return; } @@ -104,7 +104,7 @@ contract LivenessGuard is ISemver, GetSigners, BaseGuard { _getNSigners({ dataHash: txHash, signatures: signatures, requiredSignatures: threshold }); for (uint256 i = 0; i < signers.length; i++) { - lastSigned[signers[i]] = block.timestamp; + lastLive[signers[i]] = block.timestamp; } emit SignersRecorded(txHash, signers); } @@ -113,7 +113,7 @@ contract LivenessGuard is ISemver, GetSigners, BaseGuard { /// This is useful for owners who have not recently signed a transaction via the Safe. function showLiveness() external { require(safe.isOwner(msg.sender), "LivenessGuard: only Safe owners may demontstrate liveness"); - lastSigned[msg.sender] = block.timestamp; + lastLive[msg.sender] = block.timestamp; address[] memory signers = new address[](1); signers[0] = msg.sender; diff --git a/packages/contracts-bedrock/src/Safe/LivenessModule.sol b/packages/contracts-bedrock/src/Safe/LivenessModule.sol index 20c6bf72e..a7ccbf42c 100644 --- a/packages/contracts-bedrock/src/Safe/LivenessModule.sol +++ b/packages/contracts-bedrock/src/Safe/LivenessModule.sol @@ -56,7 +56,7 @@ contract LivenessModule is ISemver { function removeOwner(address owner) external { // Check that the owner has not signed a transaction in the last 30 days require( - livenessGuard.lastSigned(owner) < block.timestamp - livenessInterval, + livenessGuard.lastLive(owner) < block.timestamp - livenessInterval, "LivenessModule: owner has signed recently" ); diff --git a/packages/contracts-bedrock/test/LivenessGuard.t.sol b/packages/contracts-bedrock/test/LivenessGuard.t.sol index d85466081..8046fb01b 100644 --- a/packages/contracts-bedrock/test/LivenessGuard.t.sol +++ b/packages/contracts-bedrock/test/LivenessGuard.t.sol @@ -51,7 +51,7 @@ contract LivnessGuard_CheckTx_Test is LivnessGuard_TestInit { safeInstance.execTransaction({ to: address(1111), value: 0, data: hex"abba" }); for (uint256 i; i < safeInstance.threshold; i++) { - assertEq(livenessGuard.lastSigned(safeInstance.owners[i]), block.timestamp); + assertEq(livenessGuard.lastLive(safeInstance.owners[i]), block.timestamp); } } } @@ -70,6 +70,6 @@ contract LivenessGuard_ShowLiveness_Test is LivnessGuard_TestInit { vm.prank(caller); livenessGuard.showLiveness(); - assertEq(livenessGuard.lastSigned(caller), block.timestamp); + assertEq(livenessGuard.lastLive(caller), block.timestamp); } } diff --git a/specs/safe-liveness-checking.md b/specs/safe-liveness-checking.md index 8a891d2b6..1a71cfee8 100644 --- a/specs/safe-liveness-checking.md +++ b/specs/safe-liveness-checking.md @@ -32,7 +32,7 @@ This is achieved using two types of contracts which the Safe contract has built- For implementing liveness checks a `LivenessGuard` is created which receives the signatures from each executed transaction, and tracks the latest time at which a transaction was signed by each -signer. This time is made publicly available by calling a `lastSigned(address)(Timestamp)` method. +signer. This time is made publicly available by calling a `lastLive(address)(Timestamp)` method. Signers may also call the contract directly in order to prove liveness. @@ -42,13 +42,12 @@ A `LivenessModule` is also created which does the following: 1. Has a function `removeOwner()` that anyone may call to specify an owner to be removed from the Safe. -1. The Module would then check the `LivenessGuard.lastSigned()` to determine if the signer is +1. The Module would then check the `LivenessGuard.lastLive()` to determine if the signer is eligible for removal. 1. If so, it will call the Safe's `removeSigner()` to remove the non-live signer, and if necessary reduce the threshold. 1. When a member is removed, the signing parameters are modified such that `M/N` is the lowest ratio - which remains above 75%. These ratios are (9 of 12, 9 of 11, 8 of 10, 7 of 9, 6 of 8). Using - integer math, this can be expressed as `M = (N * 75 + 99) / 100`. + which remains above 75%. Using integer math, this can be expressed as `M = (N * 75 + 99) / 100`. ### Shutdown -- 2.23.0