Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ set -euo pipefail
# Generate the snapshots
pnpm snapshots

# Check if the generated snapshots are different from the committed snapshots
if git diff --exit-code snapshots > /dev/null; then
[ -z "$(git ls-files --others --exclude-standard snapshots)" ] || exit 1
# Check if the generated `snapshots` or `test/kontrol` files are different from the committed versions
if git diff --exit-code snapshots test/kontrol > /dev/null; then
[ -z "$(git ls-files --others --exclude-standard snapshots test/kontrol)" ] || exit 1
else
exit 1
fi
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@
"sourceCodeHash": "0xea3872d8f196ae3c863363dfa4b57803cb2a24b0c100244d8f861891e901e03f"
},
"src/Safe/LivenessModule.sol": {
"initCodeHash": "0xdf37345a47e176266f0ece4c624c7e9c2bd81661675f53cf07706dc44dafad27",
"sourceCodeHash": "0xdf17b5a6c068b2cf8fd24383066f0cc8e4ab0002f2470476beae594ca86879f3"
"initCodeHash": "0xde144889fe7d98dbf300a98f5331edd535086a4af8ae6d88ca190c7f4c754a2d",
"sourceCodeHash": "0x3ff4a3f21202478935412d47fd5ef7f94a170402ddc50e5c062013ce5544c83f"
},
"src/cannon/MIPS.sol": {
"initCodeHash": "0xa5d36fc67170ad87322f358f612695f642757bbf5280800d5d878da21402579a",
Expand Down
13 changes: 13 additions & 0 deletions packages/contracts-bedrock/snapshots/abi/LivenessModule.json
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,19 @@
"stateMutability": "view",
"type": "function"
},
{
"inputs": [],
"name": "ownershipTransferredToFallback",
"outputs": [
{
"internalType": "bool",
"name": "",
"type": "bool"
}
],
"stateMutability": "view",
"type": "function"
},
{
"inputs": [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
[]
[
{
"bytes": "1",
"label": "ownershipTransferredToFallback",
"offset": 0,
"slot": "0",
"type": "bool"
}
]
15 changes: 13 additions & 2 deletions packages/contracts-bedrock/src/Safe/LivenessModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ contract LivenessModule is ISemver {
/// @notice Emitted when the fallback owner takes ownership
event OwnershipTransferredToFallback();

/// @notice Flag to indicate if the module has been deactivated
bool public ownershipTransferredToFallback;

/// @notice The Safe contract instance
Safe internal immutable SAFE;

Expand Down Expand Up @@ -51,7 +54,7 @@ contract LivenessModule is ISemver {

/// @notice Semantic version.
/// @custom:semver 1.1.0
string public constant version = "1.1.0";
string public constant version = "1.2.0";

// Constructor to initialize the Safe and baseModule instances
constructor(
Expand Down Expand Up @@ -131,11 +134,16 @@ contract LivenessModule is ISemver {
/// @param _ownersToRemove The owners to remove
function removeOwners(address[] memory _previousOwners, address[] memory _ownersToRemove) external {
require(_previousOwners.length == _ownersToRemove.length, "LivenessModule: arrays must be the same length");
address[] memory currentOwners = SAFE.getOwners();
require(
!ownershipTransferredToFallback,
"LivenessModule: The safe has been shutdown, the LivenessModule and LivenessGuard should be removed or replaced."
);

// Initialize the ownersCount count to the current number of owners, so that we can track the number of
// owners in the Safe after each removal. The Safe will revert if an owner cannot be removed, so it is safe
// keep a local count of the number of owners this way.
uint256 ownersCount = SAFE.getOwners().length;
uint256 ownersCount = currentOwners.length;
for (uint256 i = 0; i < _previousOwners.length; i++) {
// Validate that the owner can be removed, which means that either:
// 1. the ownersCount is now less than MIN_OWNERS, in which case all owners should be removed regardless
Expand Down Expand Up @@ -196,6 +204,9 @@ contract LivenessModule is ISemver {
if (!success) {
revert OwnerRemovalFailed(string(returnData));
}

// Deactivate the module to prevent unintended behavior after the fallback owner has taken ownership.
ownershipTransferredToFallback = true;
emit OwnershipTransferredToFallback();
}

Expand Down
14 changes: 14 additions & 0 deletions packages/contracts-bedrock/test/Safe/LivenessModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ contract LivenessModule_Getters_Test is LivenessModule_TestInit {
assertEq(livenessModule.thresholdPercentage(), THRESHOLD_PERCENTAGE);
assertEq(safeInstance.safe.getThreshold(), livenessModule.getRequiredThreshold(safeInstance.owners.length));
assertEq(livenessModule.fallbackOwner(), fallbackOwner);
assertFalse(livenessModule.ownershipTransferredToFallback());
}
}

Expand Down Expand Up @@ -400,6 +401,13 @@ contract LivenessModule_RemoveOwners_Test is LivenessModule_TestInit {
assertEq(safeInstance.safe.getOwners().length, 1);
assertEq(safeInstance.safe.getOwners()[0], fallbackOwner);
assertEq(safeInstance.safe.getThreshold(), 1);

// Ensure that the LivenessModule's removeOwners function is now disabled
assertTrue(livenessModule.ownershipTransferredToFallback());
vm.expectRevert(
"LivenessModule: The safe has been shutdown, the LivenessModule and LivenessGuard should be removed or replaced."
);
livenessModule.removeOwners(prevOwners, ownersToRemove);
}
}

Expand Down Expand Up @@ -546,6 +554,12 @@ contract LivenessModule_RemoveOwnersFuzz_Test is LivenessModule_TestInit {
assertEq(safeInstance.safe.getOwners().length, 1);
assertEq(safeInstance.safe.getOwners()[0], fallbackOwner);
assertEq(safeInstance.safe.getThreshold(), 1);
// Ensure that the LivenessModule's removeOwners function is now disabled
assertTrue(livenessModule.ownershipTransferredToFallback());
vm.expectRevert(
"LivenessModule: The safe has been shutdown, the LivenessModule and LivenessGuard should be removed or replaced."
);
livenessModule.removeOwners(prevOwners, ownersToRemove);
} else {
// For both of the incorrect behaviors, we need to calculate the number of owners to remove to
// trigger that behavior. We initialize that value here then set it in the if statements below.
Expand Down
Loading