Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Draft] feat: add guardian recovery #261

Draft
wants to merge 42 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
e9c2b5e
feat: empty GuardianRecoveryValidator
calvogenerico Jan 9, 2025
df852cf
feat: methods to add a guardian
calvogenerico Jan 9, 2025
3972ec2
fix: reverting when guardian not found
calvogenerico Jan 9, 2025
1a6def0
fix: uint to uint256
calvogenerico Jan 9, 2025
b1478cd
feat: add validateTransaction implementation to GuardianRecoveryValid…
MiniRoman Jan 17, 2025
f01162c
chore: refactor tests
MiniRoman Jan 17, 2025
a7eec81
chore: clean up code
MiniRoman Jan 17, 2025
3290826
feat: improve init method
MiniRoman Jan 17, 2025
fde86e3
feat: simplify initRecovery method
MiniRoman Jan 22, 2025
731ffd7
chore: resolve build issues
MiniRoman Jan 22, 2025
d9dc82b
chore: resolve build issues
MiniRoman Jan 22, 2025
94ffc8b
chore: resolve pr comments
MiniRoman Jan 23, 2025
771c586
feat: restore guardiansFor method
MiniRoman Jan 23, 2025
b36bcb2
chore: remove unused access to accountGuardians
MiniRoman Jan 23, 2025
24f34e8
feat: make guardian recovery validator contract proxy-able
MiniRoman Jan 23, 2025
4c73093
chore: simplify initializer function name
MiniRoman Jan 24, 2025
4ae13ba
Merge pull request #1 from Moonsong-Labs/feat/guardian-module
aon Jan 24, 2025
064764b
feat: add function to retrieve guarded accounts
MiniRoman Jan 24, 2025
afb9c70
fix: improve recovery validator logic
aon Jan 24, 2025
a09d7e2
Merge pull request #2 from Moonsong-Labs/feat/guardian-module
aon Jan 24, 2025
460446c
feat: allow paymaster calls to GuardianRecoveryValidator
MiniRoman Jan 28, 2025
b408afc
Merge pull request #3 from Moonsong-Labs/feat/guardian-module
aon Jan 28, 2025
ddac18b
feat: merge from working branch
aon Jan 30, 2025
0ec4f89
feat: fix guardian recovery validator compilation
MiniRoman Jan 30, 2025
59cff84
fix: add compiler version and remove unwanted comments
aon Jan 30, 2025
b5e95c6
fix: bugs and jsdoc format to match rest of package
aon Jan 30, 2025
5f4feea
fix: test that included guardian contract
aon Jan 30, 2025
a759787
feat: add passkey to account relation
aon Jan 31, 2025
303827d
feat: prevent account overlap
aon Jan 31, 2025
9768a19
feat: improve registered accounts logic
aon Feb 3, 2025
2d4d3f1
fix: tests
aon Feb 3, 2025
e14f484
fix: unknown accounts
aon Feb 3, 2025
b147d63
fix: discard recovery bug
aon Feb 3, 2025
62eb5fe
fix: move account verifications
aon Feb 3, 2025
cae4e89
feat: add guardian added time to guardian information
MiniRoman Jan 31, 2025
ae25098
fix: deployment
aon Feb 3, 2025
b8fe05b
fix: address to account id is not empty when initiating recovery
MiniRoman Feb 4, 2025
ac2d360
fix: remove double save on guardedAccounts
aon Feb 4, 2025
b41bed1
Add OidcKeyRegistry
matias-gonz Feb 6, 2025
ecde01c
Update deploy script
matias-gonz Feb 6, 2025
fb48235
Fix/paymaster-recovery-validator (#291)
aon Feb 13, 2025
c248dae
Merge branch 'feat/oidc-account-recovery' into guardian-recovery
matias-gonz Feb 14, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: clean up code
MiniRoman committed Jan 17, 2025
commit a7eec81ef2b85b1eabf32612a07d88826a8d4a3b
31 changes: 20 additions & 11 deletions src/validators/GuardianRecoveryValidator.sol
Original file line number Diff line number Diff line change
@@ -21,14 +21,17 @@ contract GuardianRecoveryValidator is IGuardianRecoveryValidator {

error GuardianNotFound(address guardian);
error GuardianNotProposed(address guardian);
error PasskeyNotMatched();
error CooldownPerionNotPassed();
error ExpiredRequest();

event RecoveryInitiated();

uint256 constant REQUEST_VALIDITY_TIME = 72 * 60 * 60; // 72 hours
uint256 constant REQUEST_DELAY_TIME = 24 * 60 * 60; // 24 hours

mapping(address account => Guardian[]) public accountGuardians;
mapping(address account => mapping(address validator => RecoveryRequest)) public pendingRecoveryData;
mapping(address account => RecoveryRequest) public pendingRecoveryData;

address public webAuthValidator;

@@ -113,7 +116,8 @@ contract GuardianRecoveryValidator is IGuardianRecoveryValidator {
// This method has to start the recovery.
// It's called by the sso account.
function initRecovery(bytes memory passkey) external {
pendingRecoveryData[msg.sender][webAuthValidator] = RecoveryRequest(passkey, block.timestamp);
pendingRecoveryData[msg.sender] = RecoveryRequest(passkey, block.timestamp);

emit RecoveryInitiated();
}

@@ -142,6 +146,7 @@ contract GuardianRecoveryValidator is IGuardianRecoveryValidator {
require(transaction.to <= type(uint160).max, "Overflow");
address target = address(uint160(transaction.to));

// Check for calling "initRecovery" method by guardian on GuardianRecoveryValidator contract
if (target == address(this)) {
require(selector == this.initRecovery.selector, "Unsupported function call");

@@ -153,21 +158,25 @@ contract GuardianRecoveryValidator is IGuardianRecoveryValidator {
if (accountGuardians[msg.sender][i].addr == recoveredAddress && accountGuardians[msg.sender][i].isReady)
return true;
}
revert GuardianNotFound(recoveredAddress);
} else if (target == address(webAuthValidator)) {
// Check for calling "addValidationKey" method by anyone on WebAuthValidator contract
require(selector == WebAuthValidator.addValidationKey.selector, "Unsupported function call");
bytes memory validationKeyData = abi.decode(transaction.data[4:], (bytes));

require(
pendingRecoveryData[msg.sender][target].passkey.length == validationKeyData.length &&
keccak256(pendingRecoveryData[msg.sender][target].passkey) == keccak256(validationKeyData),
"New Passkey not matched with recent request"
);
// Verify that current request matches pending one
if (
pendingRecoveryData[msg.sender].passkey.length != validationKeyData.length ||
keccak256(pendingRecoveryData[msg.sender].passkey) != keccak256(validationKeyData)
) revert PasskeyNotMatched();

uint256 timePassedSinceRequest = block.timestamp - pendingRecoveryData[msg.sender][target].timestamp;
require(timePassedSinceRequest > REQUEST_DELAY_TIME, "Cooldown period not passed");
require(timePassedSinceRequest < REQUEST_VALIDITY_TIME, "Request not valid anymore");
// Ensure time constraints
uint256 timePassedSinceRequest = block.timestamp - pendingRecoveryData[msg.sender].timestamp;
if (timePassedSinceRequest < REQUEST_DELAY_TIME) revert CooldownPerionNotPassed();
if (timePassedSinceRequest > REQUEST_VALIDITY_TIME) revert ExpiredRequest();

delete pendingRecoveryData[msg.sender][target];
// Cleanup currently processed recovery data
delete pendingRecoveryData[msg.sender];

return true;
}
87 changes: 50 additions & 37 deletions test/GuardianRecoveryValidatorTest.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Address, encodeAbiParameters, Hex, parseEther } from "viem";
import { Address, encodeAbiParameters, Hex, parseEther, zeroHash } from "viem";
import { cacheBeforeEach, ContractFixtures, getProvider } from "./utils";
import { expect } from "chai";
import { Provider, SmartAccount, utils, Wallet } from "zksync-ethers";
@@ -17,18 +17,18 @@ describe("GuardianRecoveryValidator", function () {
let ssoAccountInstance: SsoAccount;
let newGuardianConnectedSsoAccount: SmartAccount;
let ownerConnectedSsoAccount: SmartAccount;
let externalUserConnectedSsoAccount: SmartAccount;
let guardianWallet: Wallet;
let ownerWallet: Wallet;
let externalUserWallet: Wallet;
let webauthn: WebAuthValidator;
let guardianValidator: GuardianRecoveryValidator;


cacheBeforeEach(async () => {
guardianWallet = new Wallet(Wallet.createRandom().privateKey, provider);
ownerWallet = new Wallet(Wallet.createRandom().privateKey, provider);
console.log("guardianWallet")

console.log(guardianWallet.address)
externalUserWallet = new Wallet(Wallet.createRandom().privateKey, provider);

const generatedKey = await generatePassKey();

@@ -68,8 +68,8 @@ describe("GuardianRecoveryValidator", function () {
guardianWallet.signingKey.sign(hash).serialized,
guardiansValidatorAddr,
abiCoder.encode(
[],
[]
['uint256'],
[123]
),
],
);
@@ -82,6 +82,10 @@ describe("GuardianRecoveryValidator", function () {
address: await ssoAccountInstance.getAddress(),
secret: ownerWallet.privateKey,
}, provider);
externalUserConnectedSsoAccount = new SmartAccount({
address: await ssoAccountInstance.getAddress(),
secret: externalUserWallet.privateKey,
}, provider);
})

async function randomWallet(): Promise<[HDNodeWallet, GuardianRecoveryValidator]> {
@@ -226,11 +230,10 @@ describe("GuardianRecoveryValidator", function () {

cacheBeforeEach(async () => {
newKey = await generatePassKey();
refTimestamp = (await provider.getBlock('latest')).timestamp + 240;
await helpers.time.setNextBlockTimestamp(refTimestamp)
refTimestamp = (await provider.getBlock('latest')).timestamp;
})
const sut = async () => {
const sut = async (ssoAccount: SmartAccount = newGuardianConnectedSsoAccount) => {

const functionData = guardianValidator.interface.encodeFunctionData(
'initRecovery',
[newKey]
@@ -241,30 +244,29 @@ describe("GuardianRecoveryValidator", function () {
data: functionData
};
txToSign.gasLimit = await provider.estimateGas(txToSign);
const txData = await newGuardianConnectedSsoAccount.signTransaction(txToSign)
const txData = await ssoAccount.signTransaction(txToSign)
const tx = await provider.broadcastTransaction(txData);
await tx.wait()
return tx;
}
it("it creates new recovery process.", async function () {
await sut();

const request = (await guardianValidator.pendingRecoveryData(
newGuardianConnectedSsoAccount.address,
webauthn
newGuardianConnectedSsoAccount.address
));
expect(request.passkey).to.eq(newKey)
expect(request.timestamp).to.eq(refTimestamp);
expect(Math.abs(Number(request.timestamp) - refTimestamp)).to.lt(10);
})
it("it prohibits non guardian from starting recovery process", async function () {
await expect(sut(externalUserConnectedSsoAccount)).to.be.reverted
})
})
describe('And has active recovery process and trying to execute', () => {
let newKey: string;
let refTimestamp: number;

cacheBeforeEach(async () => {
newKey = await generatePassKey();
refTimestamp = (await provider.getBlock('latest')).timestamp + 480;
await helpers.time.setNextBlockTimestamp(refTimestamp)


const functionData = guardianValidator.interface.encodeFunctionData(
'initRecovery',
[newKey]
@@ -277,35 +279,46 @@ describe("GuardianRecoveryValidator", function () {
txToSign.gasLimit = await provider.estimateGas(txToSign);
const txData = await newGuardianConnectedSsoAccount.signTransaction(txToSign)
const tx = await provider.broadcastTransaction(txData);
await tx.wait()
})
const sut = async () => {
const sut = async (keyToAdd: string, ssoAccount: SmartAccount = newGuardianConnectedSsoAccount) => {
const functionData = webauthn.interface.encodeFunctionData(
'addValidationKey',
[newKey]
[keyToAdd]
);
const txToSign = {
...(await aaTxTemplate(await ssoAccountInstance.getAddress(), provider)),
to: await webauthn.getAddress(),
data: functionData
};
txToSign.gasLimit = await provider.estimateGas(txToSign);
const txData = await newGuardianConnectedSsoAccount.signTransaction(txToSign)
const tx = await provider.broadcastTransaction(txData);
await tx.wait()
return await ssoAccount.sendTransaction(txToSign)
}
it("it should clean up pending request.", async function () {

await helpers.time.increase(2*24*60*60)
await sut();

const request = (await guardianValidator.pendingRecoveryData(
newGuardianConnectedSsoAccount.address,
webauthn
));
expect(request.passkey).to.eq('0x')
expect(request.timestamp).to.eq(0);
})
describe('but not enough time has passed', () => {
it("it should not accept transaction.", async function () {
await helpers.time.increase(12 * 60 * 60);
await expect(sut(newKey)).to.be.reverted
})
});
describe('but passing wrong new key', () => {
it("it should revert.", async function () {
const wrongKey = await generatePassKey();
await helpers.time.increase(1 * 24 * 60 * 60 + 60);
expect(sut(wrongKey)).to.be.reverted
})
});
describe('and passing correct new key', () => {
it("it should clean up pending request.", async function () {

await helpers.time.increase(2 * 24 * 60 * 60)
await sut(newKey);

const request = (await guardianValidator.pendingRecoveryData(
newGuardianConnectedSsoAccount.address
));
expect(request.passkey).to.eq('0x')
expect(request.timestamp).to.eq(0);
})
});
})
})
})