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: cantina-443 #3

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,6 @@
[submodule "lib/pyth-sdk-solidity"]
path = lib/pyth-sdk-solidity
url = https://github.com/pyth-network/pyth-sdk-solidity
[submodule "lib/ethereum-vault-connector"]
path = lib/ethereum-vault-connector
url = https://github.com/euler-xyz/ethereum-vault-connector
1 change: 1 addition & 0 deletions lib/ethereum-vault-connector
3 changes: 2 additions & 1 deletion remappings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
@uniswap/v3-periphery/=lib/v3-periphery/
@redstone/evm-connector/=lib/redstone-oracles-monorepo/packages/evm-connector/contracts/
@openzeppelin/contracts=lib/openzeppelin-contracts/contracts/
@pyth/=lib/pyth-sdk-solidity/
@pyth/=lib/pyth-sdk-solidity/
ethereum-vault-connector/=lib/ethereum-vault-connector/src/
8 changes: 4 additions & 4 deletions src/EulerRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract EulerRouter is Governable, IPriceOracle {

/// @notice Deploy EulerRouter.
/// @param _governor The address of the governor.
constructor(address _governor) Governable(_governor) {
constructor(address _evc, address _governor) Governable(_evc, _governor) {
if (_governor == address(0)) revert Errors.PriceOracle_InvalidConfiguration();
}

Expand All @@ -53,7 +53,7 @@ contract EulerRouter is Governable, IPriceOracle {
/// @param quote The address of the quote token.
/// @param oracle The address of the PriceOracle to resolve the pair.
/// @dev Callable only by the governor.
function govSetConfig(address base, address quote, address oracle) external onlyGovernor {
function govSetConfig(address base, address quote, address oracle) external onlyGovernor onlyEVCAccountOwner {
// This case is handled by `resolveOracle`.
if (base == quote) revert Errors.PriceOracle_InvalidConfiguration();
(address asset0, address asset1) = _sort(base, quote);
Expand All @@ -66,7 +66,7 @@ contract EulerRouter is Governable, IPriceOracle {
/// @param set True to configure the vault, false to clear the record.
/// @dev Callable only by the governor. Vault must implement ERC4626.
/// Note: Before configuring a vault verify that its `convertToAssets` is secure.
function govSetResolvedVault(address vault, bool set) external onlyGovernor {
function govSetResolvedVault(address vault, bool set) external onlyGovernor onlyEVCAccountOwner {
address asset = set ? IERC4626(vault).asset() : address(0);
resolvedVaults[vault] = asset;
emit ResolvedVaultSet(vault, asset);
Expand All @@ -75,7 +75,7 @@ contract EulerRouter is Governable, IPriceOracle {
/// @notice Set a PriceOracle as a fallback resolver.
/// @param _fallbackOracle The address of the PriceOracle that is called when base/quote is not configured.
/// @dev Callable only by the governor. `address(0)` removes the fallback.
function govSetFallbackOracle(address _fallbackOracle) external onlyGovernor {
function govSetFallbackOracle(address _fallbackOracle) external onlyGovernor onlyEVCAccountOwner {
fallbackOracle = _fallbackOracle;
emit FallbackOracleSet(_fallbackOracle);
}
Expand Down
12 changes: 7 additions & 5 deletions src/lib/Governable.sol
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.0;

import {EVCUtil} from "ethereum-vault-connector/utils/EVCUtil.sol";
import {Errors} from "src/lib/Errors.sol";

/// @title Governable
/// @custom:security-contact [email protected]
/// @author Euler Labs (https://www.eulerlabs.com/)
/// @notice Contract mixin for governance.
abstract contract Governable {
/// @notice Contract mixin for governance, compatible with EVC.
abstract contract Governable is EVCUtil {
/// @notice The active governor address. If `address(0)` then the role is renounced.
address public governor;

Expand All @@ -16,20 +17,21 @@ abstract contract Governable {
/// @param newGovernor The address of the newly appointed governor.
event GovernorSet(address indexed oldGovernor, address indexed newGovernor);

constructor(address _governor) {
constructor(address _evc, address _governor) EVCUtil(_evc) {
_setGovernor(_governor);
}

/// @notice Transfer the governor role to another address.
/// @param newGovernor The address of the next governor.
/// @dev Can only be called by the current governor.
function transferGovernance(address newGovernor) external onlyGovernor {
function transferGovernance(address newGovernor) external onlyGovernor onlyEVCAccountOwner {
_setGovernor(newGovernor);
}

/// @notice Restrict access to the governor.
/// @dev Consider also adding `onlyEVCAccountOwner` for stricter caller checks.
modifier onlyGovernor() {
if (msg.sender != governor) {
if (_msgSender() != governor) {
revert Errors.Governance_CallerNotGovernor();
}
_;
Expand Down
107 changes: 101 additions & 6 deletions test/EulerRouter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ pragma solidity ^0.8.0;

import {Test} from "forge-std/Test.sol";
import {IERC4626} from "forge-std/interfaces/IERC4626.sol";
import {EVCUtil} from "ethereum-vault-connector/utils/EVCUtil.sol";
import {StubERC4626} from "test/StubERC4626.sol";
import {StubEVCAuth} from "test/StubEVCAuth.sol";
import {StubPriceOracle} from "test/adapter/StubPriceOracle.sol";
import {boundAddr, distinct} from "test/utils/TestUtils.sol";
import {IPriceOracle} from "src/interfaces/IPriceOracle.sol";
Expand All @@ -12,6 +14,7 @@ import {EulerRouter} from "src/EulerRouter.sol";

contract EulerRouterTest is Test {
address GOVERNOR = makeAddr("GOVERNOR");
StubEVCAuth evc;
EulerRouter router;

address WETH = makeAddr("WETH");
Expand All @@ -25,7 +28,8 @@ contract EulerRouterTest is Test {
StubPriceOracle eOracle;

function setUp() public {
router = new EulerRouter(GOVERNOR);
evc = new StubEVCAuth();
router = new EulerRouter(address(evc), GOVERNOR);
}

function test_Constructor_Integrity() public view {
Expand All @@ -34,7 +38,12 @@ contract EulerRouterTest is Test {

function test_Constructor_RevertsWhen_GovernorIsZeroAddress() public {
vm.expectRevert(Errors.PriceOracle_InvalidConfiguration.selector);
new EulerRouter(address(0));
new EulerRouter(address(evc), address(0));
}

function test_Constructor_RevertsWhen_EVCIsZeroAddress() public {
vm.expectRevert(EVCUtil.EVC_InvalidAddress.selector);
new EulerRouter(address(0), GOVERNOR);
}

function test_GovSetConfig_Integrity(address base, address quote, address oracle) public {
Expand All @@ -49,6 +58,21 @@ contract EulerRouterTest is Test {
assertEq(router.getConfiguredOracle(quote, base), oracle);
}

function test_GovSetConfig_Integrity_EVCAuth(address base, address quote, address oracle) public {
vm.assume(base != quote);
(address token0, address token1) = base < quote ? (base, quote) : (quote, base);

evc.setAccountOwner(GOVERNOR, GOVERNOR);
evc.setCurrentOnBehalfOfAccount(GOVERNOR);
vm.expectEmit();
emit EulerRouter.ConfigSet(token0, token1, oracle);

evc.makeCall(address(router), abi.encodeCall(router.govSetConfig, (base, quote, oracle)));

assertEq(router.getConfiguredOracle(base, quote), oracle);
assertEq(router.getConfiguredOracle(quote, base), oracle);
}

function test_GovSetConfig_Integrity_OverwriteOk(address base, address quote, address oracleA, address oracleB)
public
{
Expand All @@ -75,13 +99,27 @@ contract EulerRouterTest is Test {
address oracle
) public {
vm.assume(base != quote);
vm.assume(caller != GOVERNOR);
vm.assume(caller != GOVERNOR && caller != address(evc));

vm.expectRevert(Errors.Governance_CallerNotGovernor.selector);
vm.prank(caller);
router.govSetConfig(base, quote, oracle);
}

function test_GovSetConfig_RevertsWhen_CallerNotGovernor_EVCAuth(
address caller,
address base,
address quote,
address oracle
) public {
vm.assume(base != quote);
vm.assume(caller != GOVERNOR && caller != address(evc));

evc.setCurrentOnBehalfOfAccount(caller);
vm.expectRevert();
evc.makeCall(address(router), abi.encodeCall(router.govSetConfig, (base, quote, oracle)));
}

function test_GovSetConfig_RevertsWhen_BaseEqQuote(address base, address oracle) public {
vm.expectRevert(Errors.PriceOracle_InvalidConfiguration.selector);
vm.prank(GOVERNOR);
Expand All @@ -100,6 +138,19 @@ contract EulerRouterTest is Test {
assertEq(router.resolvedVaults(vault), asset);
}

function test_GovSetVaultResolver_Integrity_EVCAuth(address vault, address asset) public {
vault = boundAddr(vault);
evc.setAccountOwner(GOVERNOR, GOVERNOR);
evc.setCurrentOnBehalfOfAccount(GOVERNOR);

vm.mockCall(vault, abi.encodeWithSelector(IERC4626.asset.selector), abi.encode(asset));
vm.expectEmit();
emit EulerRouter.ResolvedVaultSet(vault, asset);
evc.makeCall(address(router), abi.encodeCall(router.govSetResolvedVault, (vault, true)));

assertEq(router.resolvedVaults(vault), asset);
}

function test_GovSetVaultResolver_Integrity_OverwriteOk(address vault, address assetA, address assetB) public {
vault = boundAddr(vault);
vm.mockCall(vault, abi.encodeWithSelector(IERC4626.asset.selector), abi.encode(assetA));
Expand All @@ -114,20 +165,36 @@ contract EulerRouterTest is Test {
}

function test_GovSetVaultResolver_RevertsWhen_CallerNotGovernor(address caller, address vault) public {
vm.assume(caller != GOVERNOR);
vm.assume(caller != GOVERNOR && caller != address(evc));

vm.expectRevert(Errors.Governance_CallerNotGovernor.selector);
vm.prank(caller);
router.govSetResolvedVault(vault, true);
}

function test_GovSetVaultResolver_RevertsWhen_CallerNotGovernor_EVCAuth(address caller, address vault) public {
vm.assume(caller != GOVERNOR && caller != address(evc));

evc.setCurrentOnBehalfOfAccount(caller);
vm.expectRevert();
evc.makeCall(address(router), abi.encodeCall(router.govSetResolvedVault, (vault, true)));
}

function test_GovSetFallbackOracle_Integrity(address fallbackOracle) public {
vm.prank(GOVERNOR);
router.govSetFallbackOracle(fallbackOracle);

assertEq(router.fallbackOracle(), fallbackOracle);
}

function test_GovSetFallbackOracle_Integrity_EVCAuth(address fallbackOracle) public {
evc.setAccountOwner(GOVERNOR, GOVERNOR);
evc.setCurrentOnBehalfOfAccount(GOVERNOR);
evc.makeCall(address(router), abi.encodeCall(router.govSetFallbackOracle, (fallbackOracle)));

assertEq(router.fallbackOracle(), fallbackOracle);
}

function test_GovSetFallbackOracle_OverwriteOk(address fallbackOracleA, address fallbackOracleB) public {
vm.prank(GOVERNOR);
router.govSetFallbackOracle(fallbackOracleA);
Expand All @@ -146,13 +213,23 @@ contract EulerRouterTest is Test {
}

function test_GovSetFallbackOracle_RevertsWhen_CallerNotGovernor(address caller, address fallbackOracle) public {
vm.assume(caller != GOVERNOR);
vm.assume(caller != GOVERNOR && caller != address(evc));

vm.expectRevert(Errors.Governance_CallerNotGovernor.selector);
vm.prank(caller);
router.govSetFallbackOracle(fallbackOracle);
}

function test_GovSetFallbackOracle_RevertsWhen_CallerNotGovernor_EVCAuth(address caller, address fallbackOracle)
public
{
vm.assume(caller != GOVERNOR && caller != address(evc));

evc.setCurrentOnBehalfOfAccount(caller);
vm.expectRevert();
evc.makeCall(address(router), abi.encodeCall(router.govSetFallbackOracle, (fallbackOracle)));
}

function test_Quote_Integrity_BaseEqQuote(uint256 inAmount, address base, address oracle) public view {
base = boundAddr(base);
oracle = boundAddr(oracle);
Expand Down Expand Up @@ -384,12 +461,21 @@ contract EulerRouterTest is Test {
}

function test_TransferGovernance_RevertsWhen_CallerNotGovernor(address caller, address newGovernor) public {
vm.assume(caller != GOVERNOR);
vm.assume(caller != GOVERNOR && caller != address(evc));
vm.expectRevert(Errors.Governance_CallerNotGovernor.selector);
vm.prank(caller);
router.transferGovernance(newGovernor);
}

function test_TransferGovernance_RevertsWhen_CallerNotGovernor_EVCAuth(address caller, address newGovernor)
public
{
vm.assume(caller != GOVERNOR && caller != address(evc));
evc.setCurrentOnBehalfOfAccount(caller);
vm.expectRevert();
evc.makeCall(address(router), abi.encodeCall(router.transferGovernance, (newGovernor)));
}

function test_TransferGovernance_Integrity(address newGovernor) public {
vm.assume(newGovernor != address(0));
vm.prank(GOVERNOR);
Expand All @@ -398,6 +484,15 @@ contract EulerRouterTest is Test {
assertEq(router.governor(), newGovernor);
}

function test_TransferGovernance_Integrity_EVCAuth(address newGovernor) public {
vm.assume(newGovernor != address(0));
evc.setAccountOwner(GOVERNOR, GOVERNOR);
evc.setCurrentOnBehalfOfAccount(GOVERNOR);
evc.makeCall(address(router), abi.encodeCall(router.transferGovernance, (newGovernor)));

assertEq(router.governor(), newGovernor);
}

function test_TransferGovernance_Integrity_ZeroAddress() public {
vm.prank(GOVERNOR);
router.transferGovernance(address(0));
Expand Down
64 changes: 64 additions & 0 deletions test/StubEVCAuth.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.0;

import {ExecutionContext, EC} from "ethereum-vault-connector/ExecutionContext.sol";

contract StubEVCAuth {
using ExecutionContext for EC;

EC executionContext;
address a;
mapping(address accountOwner => address) owners;

function getRawExecutionContext() external view returns (uint256) {
return EC.unwrap(executionContext);
}

function setCurrentOnBehalfOfAccount(address _a) external {
a = _a;
}

function getCurrentOnBehalfOfAccount(address) external view returns (address, bool) {
require(a != address(0), "OnBehalfOfAccountNotAuthenticated");
return (a, false);
}

function setAccountOwner(address account, address owner) external {
owners[account] = owner;
}

function getAccountOwner(address account) external view returns (address owner) {
return owners[account];
}

function setControlCollateralInProgress(bool inProgress) external {
if (inProgress) {
executionContext = executionContext.setControlCollateralInProgress();
} else {
executionContext =
EC.wrap(EC.unwrap(executionContext) & ~uint256(0xFF00000000000000000000000000000000000000000000));
}
}

function setChecksInProgress(bool inProgress) external {
if (inProgress) {
executionContext = executionContext.setChecksInProgress();
} else {
executionContext =
EC.wrap(EC.unwrap(executionContext) & ~uint256(0xFF000000000000000000000000000000000000000000));
}
}

function setOperatorAuthenticated(bool authenticated) external {
if (authenticated) {
executionContext = executionContext.setOperatorAuthenticated();
} else {
executionContext = executionContext.clearOperatorAuthenticated();
}
}

function makeCall(address to, bytes calldata data) external {
(bool success,) = to.call(data);
require(success);
}
}