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
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": "0x6c3d47fdce3dcd5b8499da08061f3ee4ddd126b726bedec601210667bf73db1d",
"sourceCodeHash": "0x6bdfa1a9fe8a7bc6e032c2448b911ca3bd55ece0856866042e29a20e270dd0a3"
"initCodeHash": "0xdf37345a47e176266f0ece4c624c7e9c2bd81661675f53cf07706dc44dafad27",
"sourceCodeHash": "0xdf17b5a6c068b2cf8fd24383066f0cc8e4ab0002f2470476beae594ca86879f3"
},
"src/cannon/MIPS.sol": {
"initCodeHash": "0xa5d36fc67170ad87322f358f612695f642757bbf5280800d5d878da21402579a",
Expand Down
11 changes: 11 additions & 0 deletions packages/contracts-bedrock/snapshots/abi/LivenessModule.json
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,16 @@
],
"name": "RemovedOwner",
"type": "event"
},
{
"inputs": [
{
"internalType": "string",
"name": "",
"type": "string"
}
],
"name": "OwnerRemovalFailed",
"type": "error"
}
]
39 changes: 21 additions & 18 deletions packages/contracts-bedrock/src/Safe/LivenessModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import { ISemver } from "src/universal/ISemver.sol";
/// If the number of owners falls below the minimum number of owners, the ownership of the
/// safe will be transferred to the fallback owner.
contract LivenessModule is ISemver {
/// @notice Error message for failed owner removal.
error OwnerRemovalFailed(string);

/// @notice Emitted when an owner is removed due to insufficient liveness
event RemovedOwner(address indexed owner);

Expand Down Expand Up @@ -184,15 +187,15 @@ contract LivenessModule is ISemver {
/// @param _prevOwner Owner that pointed to the owner to be replaced in the linked list
/// @param _oldOwner Owner address to be replaced.
function _swapToFallbackOwnerSafeCall(address _prevOwner, address _oldOwner) internal {
require(
SAFE.execTransactionFromModule({
to: address(SAFE),
value: 0,
operation: Enum.Operation.Call,
data: abi.encodeCall(OwnerManager.swapOwner, (_prevOwner, _oldOwner, FALLBACK_OWNER))
}),
"LivenessModule: failed to swap to fallback owner"
);
(bool success, bytes memory returnData) = SAFE.execTransactionFromModuleReturnData({
to: address(SAFE),
value: 0,
operation: Enum.Operation.Call,
data: abi.encodeCall(OwnerManager.swapOwner, (_prevOwner, _oldOwner, FALLBACK_OWNER))
});
if (!success) {
revert OwnerRemovalFailed(string(returnData));
}
emit OwnershipTransferredToFallback();
}

Expand All @@ -201,15 +204,15 @@ contract LivenessModule is ISemver {
/// @param _owner Owner address to be removed.
/// @param _threshold New threshold.
function _removeOwnerSafeCall(address _prevOwner, address _owner, uint256 _threshold) internal {
require(
SAFE.execTransactionFromModule({
to: address(SAFE),
value: 0,
operation: Enum.Operation.Call,
data: abi.encodeCall(OwnerManager.removeOwner, (_prevOwner, _owner, _threshold))
}),
"LivenessModule: failed to remove owner"
);
(bool success, bytes memory returnData) = SAFE.execTransactionFromModuleReturnData({
to: address(SAFE),
value: 0,
operation: Enum.Operation.Call,
data: abi.encodeCall(OwnerManager.removeOwner, (_prevOwner, _owner, _threshold))
});
if (!success) {
revert OwnerRemovalFailed(string(returnData));
}
emit RemovedOwner(_owner);
}

Expand Down
15 changes: 11 additions & 4 deletions packages/contracts-bedrock/test/Safe/LivenessModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { LivenessGuard } from "src/Safe/LivenessGuard.sol";
contract LivenessModule_TestInit is Test, SafeTestTools {
using SafeTestLib for SafeInstance;

error OwnerRemovalFailed(string reason);

// LivenessModule events
event SignersRecorded(bytes32 indexed txHash, address[] signers);
event RemovedOwner(address indexed owner);
Expand Down Expand Up @@ -261,10 +263,13 @@ contract LivenessModule_RemoveOwners_TestFail is LivenessModule_TestInit {
address[] memory prevOwners = new address[](1);
address[] memory ownersToRemove = new address[](1);
ownersToRemove[0] = safeInstance.owners[0];
prevOwners[0] = ownersToRemove[0]; // incorrect.
// Incorrectly set the previous owner as equal to the owner to remove, which will cause the Safe to revert.
prevOwners[0] = ownersToRemove[0];

_warpPastLivenessInterval();
vm.expectRevert("LivenessModule: failed to remove owner");
vm.expectRevert(
abi.encodeWithSelector(OwnerRemovalFailed.selector, (abi.encodeWithSignature("Error(string)", "GS205")))
);
livenessModule.removeOwners(prevOwners, ownersToRemove);
}

Expand All @@ -278,11 +283,13 @@ contract LivenessModule_RemoveOwners_TestFail is LivenessModule_TestInit {
}
address[] memory prevOwners = safeInstance.getPrevOwners(ownersToRemove);

// Incorrectly set the final owner to address(0)
// Incorrectly set the final owner to address(0), causing the Safe to revert.
ownersToRemove[ownersToRemove.length - 1] = address(0);

_warpPastLivenessInterval();
vm.expectRevert("LivenessModule: failed to swap to fallback owner");
vm.expectRevert(
abi.encodeWithSelector(OwnerRemovalFailed.selector, (abi.encodeWithSignature("Error(string)", "GS203")))
);
livenessModule.removeOwners(prevOwners, ownersToRemove);
}

Expand Down
Loading