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

SMA-392: Session Keys V2 #180

Merged
merged 41 commits into from
Mar 2, 2024
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
5cdea88
feat: Stateless Session Key Manager Module
ankurdubey521 Nov 20, 2023
f4b3772
feat: Statefull Session Key Manager Module + Bundler Compliance
ankurdubey521 Nov 20, 2023
ced1fc2
feat: Hybrid Validation
ankurdubey521 Nov 20, 2023
b6574b8
chore: add more console.logs
ankurdubey521 Nov 20, 2023
30da23c
fix: dependencies and wallet utils
ankurdubey521 Dec 4, 2023
5cc20b1
chore: remove session comparision tests
ankurdubey521 Dec 4, 2023
6acc584
chore: move common stateful session functionality to StatefulSessionK…
ankurdubey521 Dec 4, 2023
2e71194
Merge branch 'develop' into feat/sma-392-session-keys-v2
ankurdubey521 Dec 4, 2023
6a84e57
test: Foundry Tests for Stateful Session Key Manager
ankurdubey521 Dec 18, 2023
089e7cb
test: Basic Foundry Tests for Hybrid Session Key Manager
ankurdubey521 Dec 18, 2023
d6451d1
test: Foundry Tests for Hybrid Session Key Manager
ankurdubey521 Dec 18, 2023
edf5d3e
feat: SessionKeyManagerHybrid Gas Optimisation
ankurdubey521 Dec 18, 2023
9166014
feat: SessionKeyManagerStateful Gas Optimisation
ankurdubey521 Dec 18, 2023
a75fa43
feat: Hybrid Session Key Manager Gas Optimisations + Refactor for Bat…
ankurdubey521 Dec 19, 2023
57ceed6
feat: Hybrid Session Key Manager - Batched Execution
ankurdubey521 Dec 19, 2023
0575f17
fix: Hybrid SKM: add callSpecificData
ankurdubey521 Dec 20, 2023
8f5ea61
test: Hyrbid SKM Single Call Parsers
ankurdubey521 Dec 20, 2023
6ca3e6a
test: Stateful SKM Single Call Parsers
ankurdubey521 Dec 20, 2023
2dca0d4
test: Hybrid SKM Batch Call Parsers
ankurdubey521 Dec 20, 2023
a5f0d22
test: Hybrid SKM Batch Call - Single Item and Tests Refactor
ankurdubey521 Dec 20, 2023
3e2eb30
test: Hybrid SKM Batch Call - All Positive Tests
ankurdubey521 Dec 20, 2023
08e5aff
feat: Merge Stateful Session Key Manager into Hybrid Session Key Manager
ankurdubey521 Dec 23, 2023
eb6f21a
chore: Refactor Test Utils
ankurdubey521 Dec 23, 2023
8d9db63
test: Hybrid SKM Batch Call - Negative Cases
ankurdubey521 Dec 24, 2023
878d38e
fix: Restore Session Key Manager Module and Batched Session Router
ankurdubey521 Dec 24, 2023
5f3cfd5
fix: Bundler Integration Tests
ankurdubey521 Dec 24, 2023
c5fd4ad
chore: Bump Bundler version to accountabstraction/bundler:0.6.2
ankurdubey521 Dec 24, 2023
058f8dd
test: Bundler Integration Tests for Hybrid SKM in Single Mode
ankurdubey521 Dec 25, 2023
ed20f48
test: Bundler Integration Tests for Hybrid SKM in Batch Mode
ankurdubey521 Dec 25, 2023
b936ef4
Merge branch 'develop' into feat/sma-392-session-keys-v2
ankurdubey521 Dec 28, 2023
e41d075
Merge branch 'develop' into feat/sma-392-session-keys-v2
ankurdubey521 Dec 28, 2023
7453008
fix: Feedback #180 by @filmakarov
ankurdubey521 Dec 28, 2023
22bc7fc
fix: Feedback #180 by @livingrockrises
ankurdubey521 Jan 2, 2024
1c524f9
fix: Feedback #180 by @livingrockrises - 2
ankurdubey521 Jan 2, 2024
c5ca9de
Merge branch 'develop' into feat/sma-392-session-keys-v2
ankurdubey521 Jan 3, 2024
11cf3ad
fix: Feedback #180 by @AmanRaj1608
ankurdubey521 Jan 5, 2024
ac206f4
fix: Feedback #180 by @Aboudjem
ankurdubey521 Jan 5, 2024
3041d8f
feat: Solidity ^0.8.23 for SessionKeyManagerHybrid
ankurdubey521 Jan 5, 2024
f365630
fix: Feedback #180 by @tomarsachin2271
ankurdubey521 Jan 5, 2024
c3fe846
Merge branch 'develop' into feat/sma-392-session-keys-v2
ankurdubey521 Mar 2, 2024
50ccc11
feat: Kawach Audit for SKMv2
ankurdubey521 Mar 2, 2024
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
37 changes: 0 additions & 37 deletions .gas-snapshot

This file was deleted.

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ deployments/localhost
typings
forge-cache
out
.env.bak
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.env.*

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would also cover .env.example, which should not be ignored

2 changes: 1 addition & 1 deletion .solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"code-complexity": ["error", 7],
"function-max-lines": ["error", 80],
"max-line-length": ["warn", 120],
"max-states-count": ["error", 15],
"max-states-count": ["error", 20],
"no-empty-blocks": "error",
"no-unused-vars": "error",
"payable-fallback": "off",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,4 @@ import {ISignatureValidator} from "../../interfaces/ISignatureValidator.sol";
interface IBaseAuthorizationModule is
IAuthorizationModule,
ISignatureValidator
{

}
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;

/**
* @title Session Key Manager module for Biconomy Modular Smart Accounts.
* @dev Stores the Session Information explicity in the storage, instead of maintainting
* a merkle tree.
* This reduces the amount of calldata required to validate a session key, making it cheaper on
* L2s.
* Allows for a session to be enabled explicity, or being batched with the first usage of said session
* @author Ankur Dubey - <[email protected]>
*/
interface ISessionKeyManagerModuleHybrid {
struct SessionData {
uint48 validUntil;
uint48 validAfter;
address sessionValidationModule;
bytes sessionKeyData;
}

event SessionCreated(
address indexed sa,
bytes32 indexed sessionDataDigest,
SessionData data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would there be any benefit of this?
instead can emit skm address and on skm maybe emit specific event per choice of dev.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skm address + valid until + valid after separate

Copy link
Contributor Author

@ankurdubey521 ankurdubey521 Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for cheaply storing the session data corresponding to a sessionId on-chain, and primarily for analytics which Data Squad has been requesting.
This will also allow us to track useful metrics such as session usage per session enabled, etc.

instead can emit skm address and on skm maybe emit specific event per choice of dev.

why would we want to emit SKM address?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just like paymaster address is key component of AA tx. with every session key tx a skm will be involved and makes sense to emit it's address.

My concern was emitting entire session data and then decoding it may not be straight forward

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think decoding should be easy since it can be done directly with ABI, no?

);

event SessionDisabled(
address indexed sa,
bytes32 indexed sessionDataDigest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar thought process here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replied above.

);

/**
* @dev creates a session for a smart account
* @param sessionData session data
*/
function enableSession(SessionData calldata sessionData) external;

/**
* @dev creates multiple sessions for a smart account
* @param sessionDatas session data corresponding to multiple sessions to be enabled
*/
function enableSessions(SessionData[] calldata sessionDatas) external;

/**
* @notice Explicitly disable a session. Can be useful in situations where a session
* needs to be disabled before it expires.
* @param _sessionDigest digest of session key data
*/
function disableSession(bytes32 _sessionDigest) external;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from sdk pov would we / devs needs to store exact session information pieces to generate calldata for this?
we would give helper from sdk to build a userop/tx payload which does this and I am thinking about how we can make it simpler

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the Session Data (validUntil, validAfter, svm, data) needs to be maintained in the user's local storage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think adding a batch disableSession would be helpful. The users can definitely call execute batch multiple times to disableSession but having a method might be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


/**
* @notice Explicitly disable multiple sessions. Can be useful in situations where a session
* needs to be disabled before it expires.
* @param _sessionDigests digests of session key datas to be disabled
*/
function disableSessions(bytes32[] calldata _sessionDigests) external;

/**
* @notice Returns session data for a given session digest and smart account
* @param _sessionDataDigest digest of session key data
* @param _sa smart account for which session data is to be fetched
* @return data SessionData struct
*/
function enabledSessionsData(
bytes32 _sessionDataDigest,
address _sa
) external view returns (SessionData memory data);

/**
* @dev Returns session data digest
* @param _data session data
* @return digest of session data
*/
function sessionDataDigest(
SessionData calldata _data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming we would need to store data for a session id(offchain identifier) then call this view method to get digest when needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct, but since this is a pure function it can be done off-chain for performance reasons. This is mainly to provide a reference implementation.

) external pure returns (bytes32);
}
4 changes: 1 addition & 3 deletions contracts/smart-account/modules/BaseAuthorizationModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,4 @@ import {AuthorizationModulesConstants} from "./AuthorizationModulesConstants.sol
abstract contract BaseAuthorizationModule is
IBaseAuthorizationModule,
AuthorizationModulesConstants
{

}
{}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.23;

import {BaseAuthorizationModule} from "./BaseAuthorizationModule.sol";
import {ISessionValidationModule} from "../interfaces/modules/ISessionValidationModule.sol";
import {ISessionKeyManagerModule} from "../interfaces/modules/ISessionKeyManagerModule.sol";
import {ISessionKeyManagerModule} from "../interfaces/modules/SessionKeyManagers/ISessionKeyManagerModule.sol";
import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.sol";
import {UserOperation} from "@account-abstraction/contracts/interfaces/UserOperation.sol";
Expand Down
6 changes: 1 addition & 5 deletions contracts/smart-account/modules/PasskeyRegistryModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,7 @@ contract PasskeyRegistryModule is
if (passKeyId.pubKeyX != 0 && passKeyId.pubKeyY != 0)
revert AlreadyInitedForSmartAccount(msg.sender);

smartAccountPasskey[msg.sender] = PassKeyId(
_pubKeyX,
_pubKeyY,
_keyId
);
smartAccountPasskey[msg.sender] = PassKeyId(_pubKeyX, _pubKeyY, _keyId);

return address(this);
}
Expand Down
Loading
Loading