Skip to content

Commit

Permalink
Fully cover guard with tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@cristovaoth
cristovaoth committed Oct 12, 2023
1 parent b80933d commit 260ce31
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 56 deletions.
3 changes: 3 additions & 0 deletions contracts/core/Modifier.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,17 @@ abstract contract Modifier is
if (modules[msg.sender] == address(0)) {
(bytes32 hash, address signer) = moduleTxSignedBy();

// is the signer a module?
if (modules[signer] == address(0)) {
revert NotAuthorized(msg.sender);
}

// is the provided signature fresh?
if (executed[signer][hash]) {
revert HashAlreadyExecuted(hash);
}

// was the presented signature invalidated?
if (invalidated[signer][hash]) {
revert HashInvalidated(hash);
}
Expand Down
22 changes: 22 additions & 0 deletions contracts/test/TestGuard.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,25 @@ contract TestGuard is FactoryFriendly, BaseGuard {
module = _module;
}
}

contract TestNonCompliantGuard is IERC165 {
function supportsInterface(bytes4) external pure returns (bool) {
return false;
}

function checkTransaction(
address,
uint256,
bytes memory,
Enum.Operation,
uint256,
uint256,
uint256,
address,
address,
bytes memory,
address
) public {}

function checkAfterExecution(bytes32, bool) public {}
}
4 changes: 2 additions & 2 deletions contracts/test/TestModifier.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ contract TestModifier is Modifier {
}

function setUp(bytes memory initializeParams) public override initializer {
setupModules();
__Ownable_init(msg.sender);
(address _avatar, address _target) = abi.decode(
initializeParams,
(address, address)
);
setupModules();
_transferOwnership(msg.sender);
avatar = _avatar;
target = _target;
}
Expand Down
135 changes: 81 additions & 54 deletions test/04_Guard.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,45 +1,97 @@
import { AddressZero } from "@ethersproject/constants";
import { loadFixture } from "@nomicfoundation/hardhat-network-helpers";
import { expect } from "chai";
import hre from "hardhat";

describe("Guardable", async () => {
async function setupTests() {
const Avatar = await hre.ethers.getContractFactory("TestAvatar");
const avatar = await Avatar.deploy();
const iAvatar = await hre.ethers.getContractAt("IAvatar", avatar.address);
const Module = await hre.ethers.getContractFactory("TestModule");
const module = await Module.deploy(iAvatar.address, iAvatar.address);
await avatar.enableModule(module.address);
const Guard = await hre.ethers.getContractFactory("TestGuard");
const guard = await Guard.deploy(module.address);
return {
iAvatar,
guard,
module,
};
}
import { AddressZero } from "@ethersproject/constants";
import { loadFixture } from "@nomicfoundation/hardhat-network-helpers";
import { TestGuard__factory, TestModule__factory } from "../typechain-types";

async function setupTests() {
const [owner, other, relayer] = await hre.ethers.getSigners();
const Avatar = await hre.ethers.getContractFactory("TestAvatar");
const avatar = await Avatar.deploy();
const Module = await hre.ethers.getContractFactory("TestModule");
const module = TestModule__factory.connect(
(await Module.connect(owner).deploy(avatar.address, avatar.address))
.address,
owner
);
await avatar.enableModule(module.address);

const Guard = await hre.ethers.getContractFactory("TestGuard");
const guard = TestGuard__factory.connect(
(await Guard.deploy(module.address)).address,
relayer
);

const GuardNonCompliant = await hre.ethers.getContractFactory(
"TestNonCompliantGuard"
);
const guardNonCompliant = TestGuard__factory.connect(
(await GuardNonCompliant.deploy()).address,
hre.ethers.provider
);

const tx = {
to: avatar.address,
value: 0,
data: "0x",
operation: 0,
avatarTxGas: 0,
baseGas: 0,
gasPrice: 0,
gasToken: AddressZero,
refundReceiver: AddressZero,
signatures: "0x",
};

return {
owner,
other,
module,
guard,
guardNonCompliant,
tx,
};
}

describe("Guardable", async () => {
describe("setGuard", async () => {
it("reverts if reverts if caller is not the owner", async () => {
const { module } = await loadFixture(setupTests);
const [, user1] = await hre.ethers.getSigners();
await expect(module.connect(user1).setGuard(user1.address))
const { other, guard, module } = await loadFixture(setupTests);
await expect(module.connect(other).setGuard(guard.address))
.to.be.revertedWithCustomError(module, "OwnableUnauthorizedAccount")
.withArgs(user1.address);
.withArgs(other.address);
});

it("reverts if guard does not implement ERC165", async () => {
const { module } = await loadFixture(setupTests);
await expect(module.setGuard(module.address)).to.be.reverted;
});

it("reverts if guard implements ERC165 and returns false", async () => {
const { module, guardNonCompliant } = await loadFixture(setupTests);
await expect(module.setGuard(guardNonCompliant.address))
.to.be.revertedWithCustomError(module, "NotIERC165Compliant")
.withArgs(guardNonCompliant.address);
});

it("sets module and emits event", async () => {
const { module, guard } = await loadFixture(setupTests);
await expect(module.setGuard(guard.address))
.to.emit(module, "ChangedGuard")
.withArgs(guard.address);
});

it("sets guard back to zero", async () => {
const { module, guard } = await loadFixture(setupTests);
await expect(module.setGuard(guard.address))
.to.emit(module, "ChangedGuard")
.withArgs(guard.address);

await expect(module.setGuard(AddressZero))
.to.emit(module, "ChangedGuard")
.withArgs(AddressZero);
});
});

describe("getGuard", async () => {
Expand All @@ -54,39 +106,15 @@ describe("BaseGuard", async () => {
const txHash =
"0x0000000000000000000000000000000000000000000000000000000000000001";

async function setupTests() {
const Avatar = await hre.ethers.getContractFactory("TestAvatar");
const avatar = await Avatar.deploy();
const iAvatar = await hre.ethers.getContractAt("IAvatar", avatar.address);
const Module = await hre.ethers.getContractFactory("TestModule");
const module = await Module.deploy(iAvatar.address, iAvatar.address);
await avatar.enableModule(module.address);
const Guard = await hre.ethers.getContractFactory("TestGuard");
const guard = await Guard.deploy(module.address);
const tx = {
to: avatar.address,
value: 0,
data: "0x",
operation: 0,
avatarTxGas: 0,
baseGas: 0,
gasPrice: 0,
gasToken: AddressZero,
refundReceiver: AddressZero,
signatures: "0x",
};
return {
iAvatar,
guard,
module,
tx,
};
}
it("supports interface", async () => {
const { guard } = await loadFixture(setupTests);
expect(await guard.supportsInterface("0xe6d7a83a")).to.be.true;
expect(await guard.supportsInterface("0x01ffc9a7")).to.be.true;
});

describe("checkTransaction", async () => {
it("reverts if test fails", async () => {
const { guard, tx } = await loadFixture(setupTests);
const [user1] = await hre.ethers.getSigners();
await expect(
guard.checkTransaction(
tx.to,
Expand All @@ -99,13 +127,12 @@ describe("BaseGuard", async () => {
tx.gasToken,
tx.refundReceiver,
tx.signatures,
user1.address
AddressZero
)
).to.be.revertedWith("Cannot send 1337");
});
it("checks transaction", async () => {
const { guard, tx } = await loadFixture(setupTests);
const [user1] = await hre.ethers.getSigners();
await expect(
guard.checkTransaction(
tx.to,
Expand All @@ -118,7 +145,7 @@ describe("BaseGuard", async () => {
tx.gasToken,
tx.refundReceiver,
tx.signatures,
user1.address
AddressZero
)
).to.emit(guard, "PreChecked");
});
Expand Down

0 comments on commit 260ce31

Please sign in to comment.