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.

Sign up for GitHub

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

Add onlyEVCAccount modifier and _msgSenderOnlyEVCAccount, _msgSenderOnlyEVCAccountOwner functions #175

Merged
merged 9 commits into from
Oct 29, 2024
Merged

Add onlyEVCAccount modifier and _msgSenderOnlyEVCAccount, _msgSenderOnlyEVCAccountOwner functions #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e64ca4a
feat: add onlyEVCAccount modifier
kasperpawlowski Sep 9, 2024
650497d
chore: improve natspec
kasperpawlowski Oct 3, 2024
25585ae
fix: typo
kasperpawlowski Oct 3, 2024
fc8ff4f
chore: improve natspec
kasperpawlowski Oct 4, 2024
9006e67
chore: fix foundry version in the CI
kasperpawlowski Oct 11, 2024
3058f08
chore: forge fmt
kasperpawlowski Oct 11, 2024
83c5bb9
feat: add _msgSenderOnlyEVCAccountOwner
kasperpawlowski Oct 16, 2024
5025707
Merge branch 'master' into only-owner-or-subaccount
kasperpawlowski Oct 16, 2024
3686aea
feat: add _msgSenderOnlyEVCAccount
kasperpawlowski Oct 16, 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
62 changes: 53 additions & 9 deletions src/utils/EVCUtil.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ abstract contract EVCUtil {
_;
}

/// @notice Ensures a standard authentication path on the EVC allowing the account owner or any of its EVC accounts.
/// @dev This modifier checks if the caller is the EVC and if so, verifies the execution context.
/// It reverts if the operator is authenticated, control collateral is in progress, or checks are in progress.
/// @dev This modifier must not be used on functions utilized by liquidation flows, i.e. transfer or withdraw.
/// @dev This modifier must not be used on checkAccountStatus and checkVaultStatus functions.
/// @dev This modifier can be used on access controlled functions to prevent non-standard authentication paths on
/// the EVC.
modifier onlyEVCAccount() virtual {
_authenticateCallerWithStandardContextState(false);
_;
}

/// @notice Ensures a standard authentication path on the EVC.
/// @dev This modifier checks if the caller is the EVC and if so, verifies the execution context.
/// It reverts if the operator is authenticated, control collateral is in progress, or checks are in progress.
Expand All @@ -59,7 +71,7 @@ abstract contract EVCUtil {
/// @dev This modifier can be used on access controlled functions to prevent non-standard authentication paths on
/// the EVC.
modifier onlyEVCAccountOwner() virtual {
_onlyEVCAccountOwner();
_authenticateCallerWithStandardContextState(true);
_;
}

Expand Down Expand Up @@ -121,6 +133,29 @@ abstract contract EVCUtil {
return sender;
}

/// @notice Retrieves the message sender, ensuring it's any EVC account meaning that the execution context is in a
/// standard state (not operator authenticated, not control collateral in progress, not checks in progress).
/// @dev This function must not be used on functions utilized by liquidation flows, i.e. transfer or withdraw.
/// @dev This function must not be used on checkAccountStatus and checkVaultStatus functions.
/// @dev This function can be used on access controlled functions to prevent non-standard authentication paths on
/// the EVC.
/// @return The address of the message sender.
function _msgSenderOnlyEVCAccount() internal view returns (address) {
return _authenticateCallerWithStandardContextState(false);
}

/// @notice Retrieves the message sender, ensuring it's the EVC account owner and that the execution context is in a
/// standard state (not operator authenticated, not control collateral in progress, not checks in progress).
/// @dev It assumes that if the caller is not the EVC, the caller is the account owner.
/// @dev This function must not be used on functions utilized by liquidation flows, i.e. transfer or withdraw.
/// @dev This function must not be used on checkAccountStatus and checkVaultStatus functions.
/// @dev This function can be used on access controlled functions to prevent non-standard authentication paths on
/// the EVC.
/// @return The address of the message sender.
function _msgSenderOnlyEVCAccountOwner() internal view returns (address) {
return _authenticateCallerWithStandardContextState(true);
}

/// @notice Calls the current external function through the EVC.
/// @dev This function is used to route the current call through the EVC if it's not already coming from the EVC. It
/// makes the EVC set the execution context and call back this contract with unchanged calldata. msg.sender is used
Expand Down Expand Up @@ -159,12 +194,14 @@ abstract contract EVCUtil {
}
}

/// @notice Ensures that the function is called only by the EVC account owner
/// @notice Ensures that the function is called only by the EVC account owner or any of its EVC accounts
/// @dev This function checks if the caller is the EVC and if so, verifies that the execution context is not in a
/// special state (operator authenticated, collateral control in progress, or checks in progress). If the owner was
/// already registered on the EVC, it verifies that the onBehalfOfAccount is the owner.
/// @dev Reverts if the caller is not the EVC or if the execution context is in a special state.
function _onlyEVCAccountOwner() internal view {
/// special state (operator authenticated, collateral control in progress, or checks in progress). If
/// onlyAccountOwner is true and the owner was already registered on the EVC, it verifies that the onBehalfOfAccount
/// is the owner. If onlyAccountOwner is false, it allows any EVC account of the owner to call the function.
/// @param onlyAccountOwner If true, only allows the account owner; if false, allows any EVC account of the owner
/// @return The address of the message sender.
function _authenticateCallerWithStandardContextState(bool onlyAccountOwner) internal view returns (address) {
if (msg.sender == address(evc)) {
EC ec = EC.wrap(evc.getRawExecutionContext());

Expand All @@ -173,11 +210,18 @@ abstract contract EVCUtil {
}

address onBehalfOfAccount = ec.getOnBehalfOfAccount();
address owner = evc.getAccountOwner(onBehalfOfAccount);

if (owner != address(0) && owner != onBehalfOfAccount) {
revert NotAuthorized();
if (onlyAccountOwner) {
address owner = evc.getAccountOwner(onBehalfOfAccount);

if (owner != address(0) && owner != onBehalfOfAccount) {
revert NotAuthorized();
}
}

return onBehalfOfAccount;
}

return msg.sender;
}
}
96 changes: 95 additions & 1 deletion test/unit/EVCUtil/EVCUtil.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ contract EVCClient is EVCUtil {
// do nothing
}

function calledByEVCAccount() external onlyEVCAccount {
// do nothing
}

function calledByEVCAccountOwner() external onlyEVCAccountOwner {
// do nothing
}
Expand All @@ -53,6 +57,14 @@ contract EVCClient is EVCUtil {
return _msgSenderForBorrow();
}

function msgSenderOnlyEVCAccount() external view returns (address) {
return _msgSenderOnlyEVCAccount();
}

function msgSenderOnlyEVCAccountOwner() external view returns (address) {
return _msgSenderOnlyEVCAccountOwner();
}

fallback(bytes calldata) external returns (bytes memory) {
return abi.encode(IVault.checkAccountStatus.selector);
}
Expand Down Expand Up @@ -187,56 +199,138 @@ contract EVCUtilTest is Test {
evcClient.calledByEVCWithChecksInProgress();
}

function test_calledByEVCAccountOwner(address caller, uint8 id) external {
function test_calledByEVCAccount_calledByEVCAccountOwner_msgSenderOnlyEVCAccount_msgSenderOnlyEVCAccountOwner(
address caller,
uint8 id
) external {
vm.assume(!evc.haveCommonOwner(caller, address(0)) && !evc.haveCommonOwner(caller, address(evc)));
vm.assume(id != 0);

// msg.sender is not EVC
evc.setOnBehalfOfAccount(address(0));
vm.prank(caller);
evcClient.calledByEVCAccount();

vm.prank(caller);
evcClient.calledByEVCAccountOwner();

vm.prank(caller);
assertEq(evcClient.msgSenderOnlyEVCAccount(), caller);

vm.prank(caller);
assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller);

// msg.sender is EVC and operator is authenticated
evc.setOperatorAuthenticated(true);
vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.calledByEVCAccount();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.calledByEVCAccountOwner();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.msgSenderOnlyEVCAccount();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.msgSenderOnlyEVCAccountOwner();

// msg.sender is EVC and control collateral is in progress
evc.setOperatorAuthenticated(false);
evc.setControlCollateralInProgress(true);
vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.calledByEVCAccount();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.calledByEVCAccountOwner();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.msgSenderOnlyEVCAccount();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.msgSenderOnlyEVCAccountOwner();

// msg.sender is EVC and checks are in progress
evc.setControlCollateralInProgress(false);
evc.setChecksInProgress(true);
vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.calledByEVCAccount();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.calledByEVCAccountOwner();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.msgSenderOnlyEVCAccount();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.msgSenderOnlyEVCAccountOwner();

// msg.sender is EVC, the owner is not registered yet
evc.setChecksInProgress(false);
evc.setOnBehalfOfAccount(caller);
assertEq(evc.getAccountOwner(caller), address(0));
vm.prank(address(evc));
evcClient.calledByEVCAccount();

assertEq(evc.getAccountOwner(caller), address(0));
vm.prank(address(evc));
evcClient.calledByEVCAccountOwner();

assertEq(evc.getAccountOwner(caller), address(0));
vm.prank(address(evc));
assertEq(evcClient.msgSenderOnlyEVCAccount(), caller);

assertEq(evc.getAccountOwner(caller), address(0));
vm.prank(address(evc));
assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller);

// msg.sender is EVC, the owner is registered and the authenticated account is the owner
evc.setOnBehalfOfAccount(address(0));
vm.prank(caller);
evc.call(address(0), caller, 0, "");
assertEq(evc.getAccountOwner(caller), caller);
evc.setOnBehalfOfAccount(caller);
vm.prank(address(evc));
evcClient.calledByEVCAccount();

assertEq(evc.getAccountOwner(caller), caller);
vm.prank(address(evc));
evcClient.calledByEVCAccountOwner();

assertEq(evc.getAccountOwner(caller), caller);
vm.prank(address(evc));
assertEq(evcClient.msgSenderOnlyEVCAccount(), caller);

assertEq(evc.getAccountOwner(caller), caller);
vm.prank(address(evc));
assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller);

// msg.sender is EVC, the owner is registered but the authenticated account is not the owner
evc.setOnBehalfOfAccount(address(uint160(uint160(caller) ^ id)));
vm.prank(address(evc));
evcClient.calledByEVCAccount();

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.calledByEVCAccountOwner();

vm.prank(address(evc));
assertEq(evcClient.msgSenderOnlyEVCAccount(), address(uint160(uint160(caller) ^ id)));

vm.prank(address(evc));
vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
evcClient.msgSenderOnlyEVCAccountOwner();
}

function test_msgSender(address caller) external {
Expand Down
Loading