Skip to content
This repository has been archived by the owner on Oct 6, 2023. It is now read-only.

Commit

Permalink
Refactors for PR #250 ("Added GasManager facet...") (#258)
Browse files Browse the repository at this point in the history 
* Use Validator.canChange in AccountsGasManager

* Validate caller for correct vault type

* Update tests

* Update delegateIsValid to check envTime is strictly smaller than exp. date
  • Loading branch information
Nenad Misic authored Aug 2, 2023
1 parent 7af3568 commit f1e8159
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 58 deletions.
30 changes: 21 additions & 9 deletions contracts/core/accounts/facets/AccountsGasManager.sol
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.16;

import {LibAccounts} from "../lib/LibAccounts.sol";
import {AccountStorage} from "../storage.sol";
import {Validator} from "../../validator.sol";
Expand Down Expand Up @@ -68,7 +69,7 @@ contract AccountsGasManager is ReentrancyGuardFacet, IAccountsGasManager {
/// @dev Only callable by the owner, liquidInvestmentManager, or lockedInvestmentManager
/// Sends the balance specified from the specified token and vault type to the endow's gasFwd contract
function addGas(uint32 id, IVault.VaultType vault, address token, uint256 amount) external {
if (!_validateCaller(id)) {
if (!_validateCaller(id, vault)) {
revert Unauthorized();
}

Expand All @@ -91,23 +92,34 @@ contract AccountsGasManager is ReentrancyGuardFacet, IAccountsGasManager {
IERC20(token).safeTransfer(state.ENDOWMENTS[id].gasFwd, amount);
}

function _validateCaller(uint32 id) internal view returns (bool) {
function _validateCaller(uint32 id, IVault.VaultType vault) internal view returns (bool) {
AccountStorage.State storage state = LibAccounts.diamondStorage();
AccountStorage.Endowment memory tempEndowment = state.ENDOWMENTS[id];

if (
Validator.canCall(
state.ENDOWMENTS[id].settingsController.lockedInvestmentManagement,
vault == IVault.VaultType.LOCKED &&
Validator.canChange(
tempEndowment.settingsController.lockedInvestmentManagement,
msg.sender,
tempEndowment.owner,
block.timestamp
) ||
Validator.canCall(
state.ENDOWMENTS[id].settingsController.liquidInvestmentManagement,
)
) {
return true;
}

if (
vault == IVault.VaultType.LIQUID &&
Validator.canChange(
tempEndowment.settingsController.liquidInvestmentManagement,
msg.sender,
tempEndowment.owner,
block.timestamp
) ||
msg.sender == state.ENDOWMENTS[id].owner
)
) {
return true;
}

return false;
}
}
23 changes: 1 addition & 22 deletions contracts/core/validator.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ library Validator {
) internal pure returns (bool) {
return (delegate.addr != address(0) &&
sender == delegate.addr &&
(delegate.expires == 0 || envTime <= delegate.expires));
(delegate.expires == 0 || envTime < delegate.expires));
}

function canChange(
Expand All @@ -52,27 +52,6 @@ library Validator {
(delegateIsValid(permissions.delegate, sender, envTime) || sender == owner));
}

function canCall(
LibAccounts.SettingsPermission memory permissions,
address sender,
uint256 envTime
) internal pure returns (bool) {
// return true if:
// Caller is the specified delegate address AND
// the delegate hasn't expired OR doesn't expire
bool approved;
if (sender == permissions.delegate.addr) {
if (permissions.delegate.expires > 0) {
if (permissions.delegate.expires > envTime) {
approved = true;
}
} else {
approved = true;
}
}
return approved;
}

function validateFee(LibAccounts.FeeSetting memory fee) internal pure {
if (fee.bps > 0 && fee.payoutAddress == address(0)) {
revert("Invalid fee payout zero address given");
Expand Down
55 changes: 28 additions & 27 deletions test/core/accounts/AccountsGasManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import {
import {
AccountsGasManager,
AccountsGasManager__factory,
GasFwd,
GasFwd__factory,
DummyERC20,
DummyERC20__factory,
GasFwd,
GasFwd__factory,
TestFacetProxyContract,
} from "typechain-types";
import {genWallet, getSigners, VaultType} from "utils";
import {VaultType, getSigners} from "utils";
import {deployFacetAsProxy} from "./utils";

use(smock.matchers);
Expand All @@ -30,6 +30,7 @@ describe("AccountsGasManager", function () {
let token: FakeContract<DummyERC20>;
let gasFwd: FakeContract<GasFwd>;
const ACCOUNT_ID = 1;
const BALANCE = 1000;

before(async function () {
const signers = await getSigners(hre);
Expand All @@ -43,6 +44,10 @@ describe("AccountsGasManager", function () {
beforeEach(async () => {
token = await smock.fake<DummyERC20>(new DummyERC20__factory());
gasFwd = await smock.fake<GasFwd>(new GasFwd__factory());

token.transfer.returns(true);
token.transferFrom.returns(true);
gasFwd.sweep.returns(BALANCE);
});

describe("upon `sweepForClosure`", async function () {
Expand All @@ -58,9 +63,6 @@ describe("AccountsGasManager", function () {
gasFwd: gasFwd.address,
};
await state.setEndowmentDetails(ACCOUNT_ID, endowment);

token.transfer.returns(true);
token.transferFrom.returns(true);
});

it("reverts if not called by self", async function () {
Expand All @@ -75,15 +77,15 @@ describe("AccountsGasManager", function () {
ACCOUNT_ID,
token.address,
]);
expect(await state.callSelf(0, calldata)).to.emit(gasFwd, "Sweep");
await expect(state.callSelf(0, calldata)).to.not.be.reverted;

expect(gasFwd.sweep).to.have.been.calledWith(token.address);
});
});

describe("upon `sweepForEndowment`", async function () {
let facet: AccountsGasManager;
let state: TestFacetProxyContract;
const BALANCE = 1000;

beforeEach(async function () {
state = await deployFacetAsProxy(hre, owner, proxyAdmin, impl.address);
Expand All @@ -101,10 +103,6 @@ describe("AccountsGasManager", function () {
gasFwd: gasFwd.address,
};
await state.setEndowmentDetails(ACCOUNT_ID, endowment);

token.transfer.returns(true);
token.transferFrom.returns(true);
gasFwd.sweep.returns(BALANCE);
});

it("reverts if not called by admin", async function () {
Expand All @@ -114,9 +112,12 @@ describe("AccountsGasManager", function () {
});

it("calls the sweep method and updates the appropriate balance", async function () {
expect(
await facet.connect(owner).sweepForEndowment(ACCOUNT_ID, VaultType.LIQUID, token.address)
).to.emit(gasFwd, "Sweep");
await expect(
facet.connect(owner).sweepForEndowment(ACCOUNT_ID, VaultType.LIQUID, token.address)
).to.not.be.reverted;

expect(gasFwd.sweep).to.have.been.calledWith(token.address);

const [locked, liquid] = await state.getEndowmentTokenBalance(ACCOUNT_ID, token.address);
expect(liquid).to.equal(BALANCE);
expect(locked).to.equal(0);
Expand All @@ -132,9 +133,6 @@ describe("AccountsGasManager", function () {
beforeEach(async function () {
state = await deployFacetAsProxy(hre, owner, proxyAdmin, impl.address);
facet = AccountsGasManager__factory.connect(state.address, owner);
token.transfer.returns(true);
token.transferFrom.returns(true);
gasFwd.sweep.returns(BALANCE);
});

it("reverts if not called by an approved caller", async function () {
Expand Down Expand Up @@ -163,9 +161,10 @@ describe("AccountsGasManager", function () {
await state.setEndowmentDetails(ACCOUNT_ID, endowment);
await state.setEndowmentTokenBalance(ACCOUNT_ID, token.address, BALANCE, BALANCE);

expect(
await facet.connect(user).addGas(ACCOUNT_ID, VaultType.LOCKED, token.address, GAS_COST)
);
await expect(
facet.connect(user).addGas(ACCOUNT_ID, VaultType.LOCKED, token.address, GAS_COST)
).to.not.be.reverted;

const [locked, liquid] = await state.getEndowmentTokenBalance(ACCOUNT_ID, token.address);
expect(locked).to.equal(BALANCE - GAS_COST);
expect(liquid).to.equal(BALANCE);
Expand All @@ -191,9 +190,10 @@ describe("AccountsGasManager", function () {
await state.setEndowmentDetails(ACCOUNT_ID, endowment);
await state.setEndowmentTokenBalance(ACCOUNT_ID, token.address, BALANCE, BALANCE);

expect(
await facet.connect(user).addGas(ACCOUNT_ID, VaultType.LIQUID, token.address, GAS_COST)
);
await expect(
facet.connect(user).addGas(ACCOUNT_ID, VaultType.LIQUID, token.address, GAS_COST)
).to.not.be.reverted;

const [locked, liquid] = await state.getEndowmentTokenBalance(ACCOUNT_ID, token.address);
expect(locked).to.equal(BALANCE);
expect(liquid).to.equal(BALANCE - GAS_COST);
Expand All @@ -208,9 +208,10 @@ describe("AccountsGasManager", function () {
await state.setEndowmentDetails(ACCOUNT_ID, endowment);
await state.setEndowmentTokenBalance(ACCOUNT_ID, token.address, BALANCE, BALANCE);

expect(
await facet.connect(user).addGas(ACCOUNT_ID, VaultType.LIQUID, token.address, GAS_COST)
);
await expect(
facet.connect(user).addGas(ACCOUNT_ID, VaultType.LIQUID, token.address, GAS_COST)
).to.not.be.reverted;

const [locked, liquid] = await state.getEndowmentTokenBalance(ACCOUNT_ID, token.address);
expect(locked).to.equal(BALANCE);
expect(liquid).to.equal(BALANCE - GAS_COST);
Expand Down

0 comments on commit f1e8159

Please sign in to comment.