Skip to content

Commit

Permalink
FIX: fix all comments @jozer-rami
Browse files Browse the repository at this point in the history
  • Loading branch information
@alfredolopez80
alfredolopez80 committed May 21, 2024
1 parent 250c642 commit 4e54e9e
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 120 deletions.
2 changes: 1 addition & 1 deletion .gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ ModifySafeOwners:testRevertSafeNotRegisteredRemoveOwner_EOA_Caller() (gas: 50526
ModifySafeOwners:testRevertSafeNotRegisteredRemoveOwner_SAFE_Caller() (gas: 509494)
ModifySafeOwners:testRevertZeroAddressAddOwnerWithThreshold() (gas: 1672722)
ModifySafeOwners:testRevertZeroAddressProvidedRemoveOwner() (gas: 3080094)
TestDeploy:testDeploy() (gas: 6283203)
TestDeploy:testDeploy() (gas: 6285029)
TestDeploySafe:testTransferFundsSafe() (gas: 109672)
TestEnableGuard:testEnableKeyperGuard() (gas: 120880)
TestEnableGuard:testEnableKeyperModule() (gas: 122295)
Expand Down
102 changes: 44 additions & 58 deletions script/SkipExecutionOnBehalf.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
emptyData,
Enum.Operation(0)
);
// ttry to encode the end of the signature, from EOA random the internal data to the Wrapper of
// try to encode the end of the signature, from EOA random the internal data to the Wrapper of
// Execution On Behalf, with a rogue caller and that is secondary squad A1 over Root Safe
bytes memory internalData = abi.encodeWithSignature(
"execTransactionOnBehalf(bytes32,address,address,address,uint256,bytes,uint8,bytes)",
Expand Down Expand Up @@ -467,7 +467,7 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
Enum.Operation(0)
);

// vm.expectRevert("GS020"); // GS020: Invalid signatures provided
// GS020: Invalid signatures provided
keyperModule.execTransactionOnBehalf(
orgHash,
rootAddr,
Expand Down Expand Up @@ -523,7 +523,7 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
Enum.Operation(0)
);

// vm.expectRevert("GS026"); // GS026: Invalid signatures provided
// GS026: Invalid signatures provided
keyperModule.execTransactionOnBehalf(
orgHash,
rootAddr,
Expand Down Expand Up @@ -581,7 +581,7 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
);

setGnosisSafe(safeSubSquadA1Addr);
// vm.expectRevert(Errors.NotAuthorizedExecOnBehalf.selector); // NotAuthorizedExecOnBehalf: Caller is not authorized to execute on behalf
// NotAuthorizedExecOnBehalf: Caller is not authorized to execute on behalf
bool result = execTransactionOnBehalfTx(
orgHash,
safeSubSquadA1Addr,
Expand Down Expand Up @@ -625,7 +625,7 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
Enum.Operation(0)
);

// vm.expectRevert("GS013"); // GS013: Invalid signatures provided
// GS013: Invalid signatures provided
execTransactionOnBehalfTx(
orgHash,
rootAddr,
Expand All @@ -639,16 +639,16 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
vm.stopBroadcast();
}

// // Revert ZeroAddressProvided() execTransactionOnBehalf when arg "to" is address(0)
// // Scenario 1
// // Caller: rootAddr (org)
// // Caller Type: rootSafe
// // Caller Role: ROOT_SAFE
// // TargerSafe: safeSquadA1
// // TargetSafe Type: safe as a Child
// // rootSafe -----------
// // | |
// // safeSquadA1 <--------
// Revert ZeroAddressProvided() execTransactionOnBehalf when arg "to" is address(0)
// Scenario 1
// Caller: rootAddr (org)
// Caller Type: rootSafe
// Caller Role: ROOT_SAFE
// TargerSafe: safeSquadA1
// TargetSafe Type: safe as a Child
// rootSafe -----------
// | |
// safeSquadA1 <--------
function testRevertInvalidAddressProvidedExecTransactionOnBehalfScenarioOne(
) public {
vm.startBroadcast();
Expand All @@ -674,7 +674,6 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
Enum.Operation(0)
);
// Execute on behalf function with invalid receiver
// vm.expectRevert(Errors.InvalidAddressProvided.selector); // InvalidAddressProvided: Invalid address provided
keyperModule.execTransactionOnBehalf(
orgHash,
rootAddr,
Expand All @@ -688,16 +687,16 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
vm.stopBroadcast();
}

// // Revert ZeroAddressProvided() execTransactionOnBehalf when param "targetSafe" is address(0)
// // Scenario 2
// // Caller: rootAddr (org)
// // Caller Type: rootSafe
// // Caller Role: ROOT_SAFE
// // TargerSafe: safeSquadA1
// // TargetSafe Type: safe as a Child
// // rootSafe -----------
// // | |
// // safeSquadA1 <--------
// Revert ZeroAddressProvided() execTransactionOnBehalf when param "targetSafe" is address(0)
// Scenario 2
// Caller: rootAddr (org)
// Caller Type: rootSafe
// Caller Role: ROOT_SAFE
// TargerSafe: safeSquadA1
// TargetSafe Type: safe as a Child
// rootSafe -----------
// | |
// safeSquadA1 <--------
function testRevertZeroAddressProvidedExecTransactionOnBehalfScenarioTwo()
public
{
Expand All @@ -723,12 +722,7 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
Enum.Operation(0)
);

// Execute on behalf function from a not authorized caller
// vm.expectRevert(
// abi.encodeWithSelector(
// Errors.InvalidGnosisSafe.selector, address(0)
// )
// ); // InvalidGnosisSafe: Invalid Gnosis Safe
// InvalidGnosisSafe: Invalid Gnosis Safe
keyperModule.execTransactionOnBehalf(
orgHash,
rootAddr,
Expand All @@ -742,16 +736,16 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
vm.stopBroadcast();
}

// // Revert ZeroAddressProvided() execTransactionOnBehalf when param "org" is address(0)
// // Scenario 3
// // Caller: rootAddr (org)
// // Caller Type: rootSafe
// // Caller Role: ROOT_SAFE
// // TargerSafe: safeSquadA1
// // TargetSafe Type: safe as a Child
// // rootSafe -----------
// // | |
// // safeSquadA1 <--------
// Revert ZeroAddressProvided() execTransactionOnBehalf when param "org" is address(0)
// Scenario 3
// Caller: rootAddr (org)
// Caller Type: rootSafe
// Caller Role: ROOT_SAFE
// TargerSafe: safeSquadA1
// TargetSafe Type: safe as a Child
// rootSafe -----------
// | |
// safeSquadA1 <--------
function testRevertOrgNotRegisteredExecTransactionOnBehalfScenarioThree()
public
{
Expand All @@ -776,10 +770,7 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
emptyData,
Enum.Operation(0)
);
// Execute on behalf function from a not authorized caller
// vm.expectRevert(
// abi.encodeWithSelector(Errors.OrgNotRegistered.selector, address(0))
// ); // OrgNotRegistered: Org not registered
// OrgNotRegistered: Org not registered
keyperModule.execTransactionOnBehalf(
bytes32(0),
rootAddr,
Expand All @@ -793,12 +784,12 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
vm.stopBroadcast();
}

// // Revert InvalidGnosisSafe() execTransactionOnBehalf : when param "targetSafe" is not a safe
// // Caller: rootAddr (org)
// // Caller Type: rootSafe
// // Caller Role: ROOT_SAFE, SAFE_LEAD
// // TargerSafe: fakeTargetSafe
// // TargetSafe Type: EOA
// Revert InvalidGnosisSafe() execTransactionOnBehalf : when param "targetSafe" is not a safe
// Caller: rootAddr (org)
// Caller Type: rootSafe
// Caller Role: ROOT_SAFE, SAFE_LEAD
// TargerSafe: fakeTargetSafe
// TargetSafe Type: EOA
function testRevertInvalidGnosisSafeExecTransactionOnBehalf() public {
vm.startBroadcast();
(uint256 rootId, uint256 safeSquadA1) =
Expand All @@ -822,12 +813,7 @@ contract SkipSeveralScenarios is Script, SkipSetupEnv {
emptyData,
Enum.Operation(0)
);
// Execute on behalf function from a not authorized caller
// vm.expectRevert(
// abi.encodeWithSelector(
// Errors.InvalidGnosisSafe.selector, fakeTargetSafe
// )
// ); // InvalidGnosisSafe: Invalid Gnosis Safe
// InvalidGnosisSafe: Invalid Gnosis Safe
keyperModule.execTransactionOnBehalf(
orgHash,
rootAddr,
Expand Down
121 changes: 60 additions & 61 deletions test/ExecTransactionOnBehalf.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,17 +439,17 @@ contract ExecTransactionOnBehalf is DeployHelper, SignersHelper {

// // ! ********************** SUPER_SAFE ROLE ********************

// // execTransactionOnBehalf
// // Caller: safeSquadA1
// // Caller Type: safe
// // Caller Role: SUPER_SAFE of safeSubSquadA1
// // TargerSafe: safeSubSquadA1
// // TargetSafe Type: safe
// // rootSafe
// // |
// // safeSquadA1 as superSafe ---
// // | |
// // safeSubSquadA1 <------------
// execTransactionOnBehalf
// Caller: safeSquadA1
// Caller Type: safe
// Caller Role: SUPER_SAFE of safeSubSquadA1
// TargerSafe: safeSubSquadA1
// TargetSafe Type: safe
// rootSafe
// |
// safeSquadA1 as superSafe ---
// | |
// safeSubSquadA1 <------------
function testCan_ExecTransactionOnBehalf_SUPER_SAFE_as_SAFE_is_TARGETS_LEAD_SameTree(
) public {
(, uint256 safeSquadA1Id, uint256 safeSubSquadA1Id,,) =
Expand Down Expand Up @@ -700,19 +700,19 @@ contract ExecTransactionOnBehalf is DeployHelper, SignersHelper {
);
}

// // Revert NotAuthorizedExecOnBehalf() execTransactionOnBehalf (safeSubSquadA1 is attempting to execute on its superSafe)
// // Caller: safeSubSquadA1
// // Caller Type: safe
// // Caller Role: SUPER_SAFE
// // TargerSafe: safeSquadA1
// // TargetSafe Type: safe as lead
// // rootSafe
// // |
// // safeSquadA1 <----
// // | |
// // safeSubSquadA1 ---
// // |
// // safeSubSubSquadA1
// Revert NotAuthorizedExecOnBehalf() execTransactionOnBehalf (safeSubSquadA1 is attempting to execute on its superSafe)
// Caller: safeSubSquadA1
// Caller Type: safe
// Caller Role: SUPER_SAFE
// TargerSafe: safeSquadA1
// TargetSafe Type: safe as lead
// rootSafe
// |
// safeSquadA1 <----
// | |
// safeSubSquadA1 ---
// |
// safeSubSubSquadA1
function testRevertSuperSafeExecOnBehalf() public {
(uint256 rootId, uint256 squadIdA1, uint256 subSquadIdA1,) =
keyperSafeBuilder.setupOrgFourTiersTree(
Expand Down Expand Up @@ -798,16 +798,16 @@ contract ExecTransactionOnBehalf is DeployHelper, SignersHelper {
);
}

// // Revert ZeroAddressProvided() execTransactionOnBehalf when arg "to" is address(0)
// // Scenario 1
// // Caller: rootAddr (org)
// // Caller Type: rootSafe
// // Caller Role: ROOT_SAFE
// // TargerSafe: safeSquadA1
// // TargetSafe Type: safe as a Child
// // rootSafe -----------
// // | |
// // safeSquadA1 <--------
// Revert ZeroAddressProvided() execTransactionOnBehalf when arg "to" is address(0)
// Scenario 1
// Caller: rootAddr (org)
// Caller Type: rootSafe
// Caller Role: ROOT_SAFE
// TargerSafe: safeSquadA1
// TargetSafe Type: safe as a Child
// rootSafe -----------
// | |
// safeSquadA1 <--------
function testRevertInvalidAddressProvidedExecTransactionOnBehalfScenarioOne(
) public {
(uint256 rootId, uint256 safeSquadA1) =
Expand Down Expand Up @@ -846,16 +846,16 @@ contract ExecTransactionOnBehalf is DeployHelper, SignersHelper {
);
}

// // Revert ZeroAddressProvided() execTransactionOnBehalf when param "targetSafe" is address(0)
// // Scenario 2
// // Caller: rootAddr (org)
// // Caller Type: rootSafe
// // Caller Role: ROOT_SAFE
// // TargerSafe: safeSquadA1
// // TargetSafe Type: safe as a Child
// // rootSafe -----------
// // | |
// // safeSquadA1 <--------
// Revert ZeroAddressProvided() execTransactionOnBehalf when param "targetSafe" is address(0)
// Scenario 2
// Caller: rootAddr (org)
// Caller Type: rootSafe
// Caller Role: ROOT_SAFE
// TargerSafe: safeSquadA1
// TargetSafe Type: safe as a Child
// rootSafe -----------
// | |
// safeSquadA1 <--------
function testRevertZeroAddressProvidedExecTransactionOnBehalfScenarioTwo()
public
{
Expand Down Expand Up @@ -897,16 +897,16 @@ contract ExecTransactionOnBehalf is DeployHelper, SignersHelper {
);
}

// // Revert ZeroAddressProvided() execTransactionOnBehalf when param "org" is address(0)
// // Scenario 3
// // Caller: rootAddr (org)
// // Caller Type: rootSafe
// // Caller Role: ROOT_SAFE
// // TargerSafe: safeSquadA1
// // TargetSafe Type: safe as a Child
// // rootSafe -----------
// // | |
// // safeSquadA1 <--------
// Revert ZeroAddressProvided() execTransactionOnBehalf when param "org" is address(0)
// Scenario 3
// Caller: rootAddr (org)
// Caller Type: rootSafe
// Caller Role: ROOT_SAFE
// TargerSafe: safeSquadA1
// TargetSafe Type: safe as a Child
// rootSafe -----------
// | |
// safeSquadA1 <--------
function testRevertOrgNotRegisteredExecTransactionOnBehalfScenarioThree()
public
{
Expand Down Expand Up @@ -945,12 +945,12 @@ contract ExecTransactionOnBehalf is DeployHelper, SignersHelper {
);
}

// // Revert InvalidGnosisSafe() execTransactionOnBehalf : when param "targetSafe" is not a safe
// // Caller: rootAddr (org)
// // Caller Type: rootSafe
// // Caller Role: ROOT_SAFE, SAFE_LEAD
// // TargerSafe: fakeTargetSafe
// // TargetSafe Type: EOA
// Revert InvalidGnosisSafe() execTransactionOnBehalf : when param "targetSafe" is not a safe
// Caller: rootAddr (org)
// Caller Type: rootSafe
// Caller Role: ROOT_SAFE, SAFE_LEAD
// TargerSafe: fakeTargetSafe
// TargetSafe Type: EOA
function testRevertInvalidGnosisSafeExecTransactionOnBehalf() public {
(uint256 rootId, uint256 safeSquadA1) =
keyperSafeBuilder.setupRootOrgAndOneSquad(orgName, squadA1Name);
Expand Down Expand Up @@ -995,7 +995,6 @@ contract ExecTransactionOnBehalf is DeployHelper, SignersHelper {
// Squad A starts a executeOnBehalf tx for his children B
// -> The calldata for the function is another executeOnBehalfTx for children C
// -> Verify that this wrapped executeOnBehalf tx does not work
// TODO: test this scenario in Live Testnet
function testCannot_ExecTransactionOnBehalf_Wrapper_ExecTransactionOnBehalf_ChildSquad_over_RootSafe_With_SAFE(
) public {
(uint256 rootId, uint256 safeSquadA1, uint256 childSquadA1,,) =
Expand Down

0 comments on commit 4e54e9e

Please sign in to comment.