Skip to content

Commit

Permalink
removeGuardian tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Filipp Makarov authored and Filipp Makarov committed Oct 14, 2023
1 parent 276d53d commit 1225a91
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 33 deletions.
2 changes: 1 addition & 1 deletion contracts/smart-account/modules/AccountRecoveryModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ contract AccountRecoveryModule is BaseAuthorizationModule {
validUntil == 0 ? type(uint48).max : validUntil,
validAfter
);
// don't increment guardiansCount as we haven't decremented it when deleting previous one
// don't increment guardiansCount as we haven't decremented it when deleting previous one
emit GuardianAdded(
msg.sender,
newGuardian,
Expand Down
195 changes: 163 additions & 32 deletions test/module/AccountRecovery.Module.specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2068,12 +2068,8 @@ describe("Account Recovery Module: ", async () => {
});

it("Can add a guardian and proper event is emitted", async () => {
const {
entryPoint,
userSA,
accountRecoveryModule,
ecdsaModule,
} = await setupTests();
const { entryPoint, userSA, accountRecoveryModule, ecdsaModule } =
await setupTests();

const guardiansBefore = (
await accountRecoveryModule.getSmartAccountSettings(userSA.address)
Expand Down Expand Up @@ -2262,7 +2258,6 @@ describe("Account Recovery Module: ", async () => {
});
});


describe("replaceGuardian", async () => {
let newGuardian: string;

Expand All @@ -2283,7 +2278,7 @@ describe("Account Recovery Module: ", async () => {
userSA,
accountRecoveryModule,
ecdsaModule,
guardians
guardians,
} = await setupTests();

const guardianToRemove = guardians[1];
Expand Down Expand Up @@ -2338,61 +2333,197 @@ describe("Account Recovery Module: ", async () => {
.to.emit(accountRecoveryModule, "GuardianRemoved")
.withArgs(userSA.address, guardianToRemove);

const newGuardianTimeFrame = await accountRecoveryModule.getGuardianParams(
newGuardian,
userSA.address
const newGuardianTimeFrame =
await accountRecoveryModule.getGuardianParams(
newGuardian,
userSA.address
);
const removedGuardianTimeFrame =
await accountRecoveryModule.getGuardianParams(
guardianToRemove,
userSA.address
);
expect(newGuardianTimeFrame.validUntil).to.equal(
expectedTimeFrame.validUntil
);
const removedGuardianTimeFrame = await accountRecoveryModule.getGuardianParams(
guardianToRemove,
userSA.address
expect(newGuardianTimeFrame.validAfter).to.equal(
expectedTimeFrame.validAfter
);
expect(newGuardianTimeFrame.validUntil).to.equal(expectedTimeFrame.validUntil);
expect(newGuardianTimeFrame.validAfter).to.equal(expectedTimeFrame.validAfter);
expect(removedGuardianTimeFrame.validUntil).to.equal(0);
expect(removedGuardianTimeFrame.validAfter).to.equal(0);
expect(guardiansAfter).to.equal(guardiansBefore);

});

it("reverts if guardian has not been set", async () => {
const { accountRecoveryModule, guardians } =
await setupTests();
const { accountRecoveryModule, guardians } = await setupTests();

// no guardians are set for deployer
const guardianToReplace = guardians[1];

await expect(accountRecoveryModule.replaceGuardian(guardianToReplace, newGuardian, 16741936496, 0)).
to.be.revertedWith("GuardianNotSet").
withArgs(guardianToReplace, deployer.address);
await expect(
accountRecoveryModule.replaceGuardian(
guardianToReplace,
newGuardian,
16741936496,
0
)
)
.to.be.revertedWith("GuardianNotSet")
.withArgs(guardianToReplace, deployer.address);
});

it("reverts if guardian to replace and new guardian are identical", async () => {
const { accountRecoveryModule, guardians } =
await setupTests();
const { accountRecoveryModule, guardians } = await setupTests();
await accountRecoveryModule.addGuardian(newGuardian, 16741936496, 0);

await expect(accountRecoveryModule.replaceGuardian(newGuardian, newGuardian, 16741936496, 0)).
to.be.revertedWith("GuardiansAreIdentical")
await expect(
accountRecoveryModule.replaceGuardian(
newGuardian,
newGuardian,
16741936496,
0
)
).to.be.revertedWith("GuardiansAreIdentical");
});

it("reverts if new guardian is zero", async () => {
const { accountRecoveryModule, guardians } =
await setupTests();
const { accountRecoveryModule, guardians } = await setupTests();

await accountRecoveryModule.addGuardian(guardians[1], 16741936496, 0);

const zeroGuardian = ethers.utils.hexZeroPad("0x", 32);

await expect(accountRecoveryModule.replaceGuardian(guardians[1], zeroGuardian, 16741936496, 0)).
to.be.revertedWith("ZeroGuardian");
await expect(
accountRecoveryModule.replaceGuardian(
guardians[1],
zeroGuardian,
16741936496,
0
)
).to.be.revertedWith("ZeroGuardian");
});
});

/*
describe("removeGuardian", async () => {
it("_________", async () => {});
it("Can remove guardian and it is removed and the event is emitted", async () => {
const {
entryPoint,
userSA,
accountRecoveryModule,
ecdsaModule,
guardians,
} = await setupTests();

const guardianToRemove = guardians[1];
const guardiansBefore = (
await accountRecoveryModule.getSmartAccountSettings(userSA.address)
).guardiansCount;

const removeGuardianData =
accountRecoveryModule.interface.encodeFunctionData("removeGuardian", [
guardianToRemove,
]);

const replaceGuardianUserOp = await makeEcdsaModuleUserOp(
"execute",
[
accountRecoveryModule.address,
ethers.utils.parseEther("0"),
removeGuardianData,
],
userSA.address,
smartAccountOwner,
entryPoint,
ecdsaModule.address
);
const handleOpsTxn = await entryPoint.handleOps(
[replaceGuardianUserOp],
alice.address,
{ gasLimit: 10000000 }
);
expect(handleOpsTxn)
.to.emit(accountRecoveryModule, "GuardianRemoved")
.withArgs(userSA.address, guardianToRemove);

const userSASettings = await accountRecoveryModule.getSmartAccountSettings(

Check failure on line 2448 in test/module/AccountRecovery.Module.specs.ts

View workflow job for this annotation

GitHub Actions / Lint sources (18.x)

Replace `·await·accountRecoveryModule.getSmartAccountSettings(⏎········userSA.address⏎······` with `⏎········await·accountRecoveryModule.getSmartAccountSettings(userSA.address`
userSA.address
);
const guardianTimeFrame = await accountRecoveryModule.getGuardianParams(
guardianToRemove,
userSA.address
);
const guardiansAfter = userSASettings.guardiansCount;
expect(guardianTimeFrame.validUntil).to.equal(0);
expect(guardianTimeFrame.validAfter).to.equal(0);
expect(guardiansAfter).to.equal(guardiansBefore - 1);
});

it("The threshold is adjusted if needed and event is emitted", async () => {
const {
entryPoint,
userSA,
accountRecoveryModule,
ecdsaModule,
guardians,
} = await setupTests();

const guardianToRemove = guardians[1];
const guardiansBefore = (
await accountRecoveryModule.getSmartAccountSettings(userSA.address)
).guardiansCount;
const thresholdBefore = (
await accountRecoveryModule.getSmartAccountSettings(userSA.address)
).recoveryThreshold;
expect(thresholdBefore).to.equal(guardiansBefore);

const removeGuardianData =
accountRecoveryModule.interface.encodeFunctionData("removeGuardian", [
guardianToRemove,
]);

const replaceGuardianUserOp = await makeEcdsaModuleUserOp(
"execute",
[
accountRecoveryModule.address,
ethers.utils.parseEther("0"),
removeGuardianData,
],
userSA.address,
smartAccountOwner,
entryPoint,
ecdsaModule.address
);
const handleOpsTxn = await entryPoint.handleOps(
[replaceGuardianUserOp],
alice.address,
{ gasLimit: 10000000 }
);
expect(handleOpsTxn)
.to.emit(accountRecoveryModule, "ThresholdChanged")
.withArgs(userSA.address, thresholdBefore-1);

Check failure on line 2503 in test/module/AccountRecovery.Module.specs.ts

View workflow job for this annotation

GitHub Actions / Lint sources (18.x)

Replace `-` with `·-·`

const userSASettings = await accountRecoveryModule.getSmartAccountSettings(

Check failure on line 2505 in test/module/AccountRecovery.Module.specs.ts

View workflow job for this annotation

GitHub Actions / Lint sources (18.x)

Replace `·await·accountRecoveryModule.getSmartAccountSettings(⏎········userSA.address⏎······` with `⏎········await·accountRecoveryModule.getSmartAccountSettings(userSA.address`
userSA.address
);

Check failure on line 2508 in test/module/AccountRecovery.Module.specs.ts

View workflow job for this annotation

GitHub Actions / Lint sources (18.x)

Delete `······`
expect(userSASettings.recoveryThreshold).to.equal(thresholdBefore-1);

Check failure on line 2509 in test/module/AccountRecovery.Module.specs.ts

View workflow job for this annotation

GitHub Actions / Lint sources (18.x)

Replace `-` with `·-·`
});

it("Reverts if the guardian has not been set", async () => {
const { accountRecoveryModule, guardians } = await setupTests();

// no guardians are set for deployer
const guardianToRemove = guardians[1];

await expect(

Check failure on line 2518 in test/module/AccountRecovery.Module.specs.ts

View workflow job for this annotation

GitHub Actions / Lint sources (18.x)

Replace `⏎········accountRecoveryModule.removeGuardian(guardianToRemove)⏎······` with `accountRecoveryModule.removeGuardian(guardianToRemove)`
accountRecoveryModule.removeGuardian(guardianToRemove)
)
.to.be.revertedWith("GuardianNotSet")
.withArgs(guardianToRemove, deployer.address);
});
});

/*
describe("removeExpiredGuardian", async () => {
it("_________", async () => {});
});
Expand Down

0 comments on commit 1225a91

Please sign in to comment.