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

fix: [spearbit-77] Refactor session key loading util functions #93

Merged
merged 1 commit into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/plugins/session/permissions/SessionKeyPermissions.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ abstract contract SessionKeyPermissions is ISessionKeyPlugin, SessionKeyPermissi

/// @inheritdoc ISessionKeyPlugin
function updateKeyPermissions(address sessionKey, bytes[] calldata updates) public override {
(SessionKeyData storage sessionKeyData, SessionKeyId keyId) = _loadSessionKey(msg.sender, sessionKey);
(SessionKeyData storage sessionKeyData, SessionKeyId keyId) = _loadSessionKeyData(msg.sender, sessionKey);

uint256 length = updates.length;
for (uint256 i = 0; i < length; ++i) {
Expand All @@ -34,7 +34,7 @@ abstract contract SessionKeyPermissions is ISessionKeyPlugin, SessionKeyPermissi

/// @inheritdoc ISessionKeyPlugin
function resetSessionKeyGasLimitTimestamp(address account, address sessionKey) external override {
(SessionKeyData storage sessionKeyData,) = _loadSessionKey(account, sessionKey);
(SessionKeyData storage sessionKeyData,) = _loadSessionKeyData(account, sessionKey);
if (sessionKeyData.gasLimitResetThisBundle) {
sessionKeyData.gasLimitResetThisBundle = false;
sessionKeyData.gasLimitTimeInfo.lastUsed = uint48(block.timestamp);
Expand Down
24 changes: 15 additions & 9 deletions src/plugins/session/permissions/SessionKeyPermissionsBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,6 @@ abstract contract SessionKeyPermissionsBase is ISessionKeyPlugin {

// Internal Functions

function _assertKeyExists(SessionKeyId id, address sessionKey) internal pure {
if (SessionKeyId.unwrap(id) == bytes32(0)) {
revert InvalidSessionKey(sessionKey);
}
}

function _sessionKeyIdOf(address associated, address sessionKey) internal view returns (SessionKeyId keyId) {
uint256 prefixAndBatchIndex = uint256(bytes32(SESSION_KEY_ID_PREFIX));
bytes memory associatedStorageKey =
Expand All @@ -120,6 +114,19 @@ abstract contract SessionKeyPermissionsBase is ISessionKeyPlugin {
}
}

/// @dev Helper function that loads the session key id and asserts it is registered.
function _loadSessionKeyId(address associated, address sessionKey)
internal
view
returns (SessionKeyId keyId)
{
SessionKeyId id = _sessionKeyIdOf(associated, sessionKey);
if (SessionKeyId.unwrap(id) == bytes32(0)) {
revert InvalidSessionKey(sessionKey);
}
return id;
}

function _updateSessionKeyId(address associated, address sessionKey, SessionKeyId newId) internal {
uint256 prefixAndBatchIndex = uint256(bytes32(SESSION_KEY_ID_PREFIX));
bytes memory associatedStorageKey =
Expand All @@ -146,13 +153,12 @@ abstract contract SessionKeyPermissionsBase is ISessionKeyPlugin {

/// @dev Helper function that loads the session key id, asserts it is registered, and returns the session key
/// data and the key id.
function _loadSessionKey(address associated, address sessionKey)
function _loadSessionKeyData(address associated, address sessionKey)
internal
view
returns (SessionKeyData storage sessionKeyData, SessionKeyId keyId)
{
SessionKeyId id = _sessionKeyIdOf(associated, sessionKey);
_assertKeyExists(id, sessionKey);
SessionKeyId id = _loadSessionKeyId(associated, sessionKey);
return (_sessionKeyDataOf(associated, id), id);
}

Expand Down
20 changes: 8 additions & 12 deletions src/plugins/session/permissions/SessionKeyPermissionsLoupe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ abstract contract SessionKeyPermissionsLoupe is SessionKeyPermissionsBase {
view
returns (ContractAccessControlType)
{
(SessionKeyData storage sessionKeyData,) = _loadSessionKey(account, sessionKey);
(SessionKeyData storage sessionKeyData,) = _loadSessionKeyData(account, sessionKey);
return sessionKeyData.contractAccessControlType;
}

Expand All @@ -21,8 +21,7 @@ abstract contract SessionKeyPermissionsLoupe is SessionKeyPermissionsBase {
view
returns (bool isOnList, bool checkSelectors)
{
SessionKeyId keyId = _sessionKeyIdOf(account, sessionKey);
_assertKeyExists(keyId, sessionKey);
SessionKeyId keyId = _loadSessionKeyId(account, sessionKey);
ContractData storage contractData = _contractDataOf(account, keyId, contractAddress);
return (contractData.isOnList, contractData.checkSelectors);
}
Expand All @@ -34,8 +33,7 @@ abstract contract SessionKeyPermissionsLoupe is SessionKeyPermissionsBase {
address contractAddress,
bytes4 selector
) external view returns (bool isOnList) {
SessionKeyId keyId = _sessionKeyIdOf(account, sessionKey);
_assertKeyExists(keyId, sessionKey);
SessionKeyId keyId = _loadSessionKeyId(account, sessionKey);
FunctionData storage functionData = _functionDataOf(account, keyId, contractAddress, selector);
return functionData.isOnList;
}
Expand All @@ -46,7 +44,7 @@ abstract contract SessionKeyPermissionsLoupe is SessionKeyPermissionsBase {
view
returns (uint48 validAfter, uint48 validUntil)
{
(SessionKeyData storage sessionKeyData,) = _loadSessionKey(account, sessionKey);
(SessionKeyData storage sessionKeyData,) = _loadSessionKeyData(account, sessionKey);
return (sessionKeyData.validAfter, sessionKeyData.validUntil);
}

Expand All @@ -56,7 +54,7 @@ abstract contract SessionKeyPermissionsLoupe is SessionKeyPermissionsBase {
view
returns (SpendLimitInfo memory)
{
(SessionKeyData storage sessionKeyData,) = _loadSessionKey(account, sessionKey);
(SessionKeyData storage sessionKeyData,) = _loadSessionKeyData(account, sessionKey);

bool hasLimit = !sessionKeyData.nativeTokenSpendLimitBypassed;

Expand All @@ -80,7 +78,7 @@ abstract contract SessionKeyPermissionsLoupe is SessionKeyPermissionsBase {
view
returns (SpendLimitInfo memory)
{
(, SessionKeyId keyId) = _loadSessionKey(account, sessionKey);
SessionKeyId keyId = _loadSessionKeyId(account, sessionKey);
ContractData storage tokenContractData = _contractDataOf(account, keyId, token);
return SpendLimitInfo({
hasLimit: tokenContractData.isERC20WithSpendLimit,
Expand All @@ -93,9 +91,7 @@ abstract contract SessionKeyPermissionsLoupe is SessionKeyPermissionsBase {

/// @inheritdoc ISessionKeyPlugin
function getRequiredPaymaster(address account, address sessionKey) external view returns (address) {
SessionKeyId id = _sessionKeyIdOf(account, sessionKey);
_assertKeyExists(id, sessionKey);
SessionKeyData storage sessionKeyData = _sessionKeyDataOf(account, id);
(SessionKeyData storage sessionKeyData,) = _loadSessionKeyData(account, sessionKey);
return sessionKeyData.hasRequiredPaymaster ? sessionKeyData.requiredPaymaster : address(0);
}

Expand All @@ -106,7 +102,7 @@ abstract contract SessionKeyPermissionsLoupe is SessionKeyPermissionsBase {
override
returns (SpendLimitInfo memory, bool)
{
(SessionKeyData storage sessionKeyData,) = _loadSessionKey(account, sessionKey);
(SessionKeyData storage sessionKeyData,) = _loadSessionKeyData(account, sessionKey);

bool hasLimit = sessionKeyData.hasGasLimit;
bool shouldReset = sessionKeyData.gasLimitResetThisBundle;
Expand Down