Skip to content

Commit

Permalink
further validation flow tests, add helper
Browse files Browse the repository at this point in the history
  • Loading branch information
Filipp Makarov authored and Filipp Makarov committed Oct 5, 2023
1 parent 6850c8f commit acbe76e
Show file tree
Hide file tree
Showing 5 changed files with 385 additions and 235 deletions.
16 changes: 10 additions & 6 deletions contracts/smart-account/modules/AccountRecoveryModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
* - It allows to _______________
* - ECDSA guardians only
* - For security reasons guardian address is not stored,
* instead its signature over CONTROL_HASH is used as
* instead its signature over CONTROL_HASH is used as
*
*
* @author Fil Makarov - <[email protected]>
Expand Down Expand Up @@ -184,11 +184,12 @@ contract AccountRecoveryModule is BaseAuthorizationModule {
.recoveryThreshold;
if (requiredSignatures == 0)
revert ThresholdNotSetForSmartAccount(userOp.sender);

(bytes memory signatures, ) = abi.decode(
userOp.signature,
(bytes, address)
);
if (signatures.length < requiredSignatures * 65)
if (signatures.length < requiredSignatures * 2*65)
revert InvalidSignaturesLength();

address lastGuardianAddress;
Expand Down Expand Up @@ -218,8 +219,12 @@ contract AccountRecoveryModule is BaseAuthorizationModule {
return SIG_VALIDATION_FAILED;
}

validAfter = _guardians[keccak256(currentGuardianSig)][userOp.sender].validAfter;
validUntil = _guardians[keccak256(currentGuardianSig)][userOp.sender].validUntil;
validAfter = _guardians[keccak256(currentGuardianSig)][
userOp.sender
].validAfter;
validUntil = _guardians[keccak256(currentGuardianSig)][
userOp.sender
].validUntil;

// 0,0 means the `currentGuardian` has not been set as guardian for the userOp.sender smartAccount
if (validUntil == 0 && validAfter == 0) {
Expand Down Expand Up @@ -335,8 +340,7 @@ contract AccountRecoveryModule is BaseAuthorizationModule {
uint48 validUntil,
uint48 validAfter
) external {
if (guardian == newGuardian)
revert GuardiansAreIdentical();
if (guardian == newGuardian) revert GuardiansAreIdentical();
if (guardian == bytes32(0)) revert ZeroGuardian();
if (newGuardian == bytes32(0)) revert ZeroGuardian();

Expand Down
10 changes: 6 additions & 4 deletions test/bundler-integration/module/AccountRecovery.Module.specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ describe("Account Recovery Module (via Bundler)", async () => {
}
);

const delay = async function(ms: number) : Promise<void> {
const delay = async function (ms: number): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, ms));
}
};

it("Can submit a recovery request and execute it after a proper delay", async () => {
const {
Expand Down Expand Up @@ -248,7 +248,10 @@ describe("Account Recovery Module (via Bundler)", async () => {
// await ethers.provider.send("evm_mine", []);

// USING THIS HACK INSTEAD
console.log("Waiting for the security delay of %i seconds to pass...", defaultSecurityDelay + 1);
console.log(
"Waiting for the security delay of %i seconds to pass...",
defaultSecurityDelay + 1
);
await delay((defaultSecurityDelay + 1) * 1000);

// can be non signed at all, just needs to be executed after the delay
Expand Down Expand Up @@ -281,6 +284,5 @@ describe("Account Recovery Module (via Bundler)", async () => {
expect(await ecdsaModule.getOwner(userSA.address)).to.not.equal(
smartAccountOwner.address
);

});
});
Loading

0 comments on commit acbe76e

Please sign in to comment.