Skip to content

Commit 650cd2b

Browse files
committed
Update onlyOwnersGuard, fix incorrect test
1 parent 52ce39c commit 650cd2b

File tree

2 files changed

+13
-16
lines changed

2 files changed

+13
-16
lines changed

contracts/examples/guards/OnlyOwnersGuard.sol

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,14 @@ import {Enum} from "../../common/Enum.sol";
66
import {BaseGuard} from "../../base/GuardManager.sol";
77

88
interface ISafe {
9-
function getOwners() external view returns (address[] memory);
9+
function isOwner(address owner) external view returns (bool);
1010
}
1111

1212
/**
1313
* @title OnlyOwnersGuard - Only allows owners to execute transactions.
1414
* @author Richard Meissner - @rmeissner
1515
*/
1616
contract OnlyOwnersGuard is BaseGuard {
17-
ISafe public safe;
18-
1917
constructor() {}
2018

2119
// solhint-disable-next-line payable-fallback
@@ -43,16 +41,7 @@ contract OnlyOwnersGuard is BaseGuard {
4341
bytes memory,
4442
address msgSender
4543
) external view override {
46-
// Only owners can exec
47-
address[] memory owners = ISafe(msg.sender).getOwners();
48-
for (uint256 i = 0; i < owners.length; i++) {
49-
if (owners[i] == msgSender) {
50-
return;
51-
}
52-
}
53-
54-
// msg sender is not an owner
55-
revert("msg sender is not allowed to exec");
44+
require(ISafe(msg.sender).isOwner(msgSender), "msg sender is not allowed to exec");
5645
}
5746

5847
function checkAfterExecution(bytes32, bool) external view override {}

test/guards/OnlyOwnersGuard.spec.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import { expect } from "chai";
22
import hre, { deployments, ethers } from "hardhat";
33
import { getMock, getSafeWithOwners } from "../utils/setup";
4-
import { buildSafeTransaction, executeContractCallWithSigners, executeTxWithSigners } from "../../src/utils/execution";
4+
import {
5+
buildSafeTransaction,
6+
executeContractCallWithSigners,
7+
executeTx,
8+
executeTxWithSigners,
9+
safeSignTypedData,
10+
} from "../../src/utils/execution";
511

612
describe("OnlyOwnersGuard", () => {
713
const setupTests = deployments.createFixture(async ({ deployments }) => {
@@ -40,13 +46,15 @@ describe("OnlyOwnersGuard", () => {
4046
const {
4147
safe,
4248
mock,
43-
signers: [, user2],
49+
signers: [user1, user2],
4450
} = await setupTests();
4551
const nonce = await safe.nonce();
4652
const mockAddress = await mock.getAddress();
4753
const safeTx = buildSafeTransaction({ to: mockAddress, data: "0xbaddad42", nonce });
54+
const signature = await safeSignTypedData(user1, await safe.getAddress(), safeTx);
55+
const safeUser2 = await safe.connect(user2);
4856

49-
await expect(executeTxWithSigners(safe, safeTx, [user2])).to.be.reverted;
57+
await expect(executeTx(safeUser2, safeTx, [signature])).to.be.revertedWith("msg sender is not allowed to exec");
5058
});
5159
});
5260
});

0 commit comments

Comments
 (0)