Skip to content

Commit

Permalink
Create framework to ensure newly added PCV Controllers are also set i…
Browse files Browse the repository at this point in the history
…n the PCV Guardian whitelist, delete outdated integration tests (#98)
  • Loading branch information
ElliotFriedman authored Aug 5, 2022
1 parent d437da9 commit 6b7c0f2
Show file tree
Hide file tree
Showing 9 changed files with 334 additions and 885 deletions.
438 changes: 0 additions & 438 deletions contracts/test/integration/ArbitrumTestVoltSystemOracle.t.sol

This file was deleted.

435 changes: 0 additions & 435 deletions contracts/test/integration/IntegrationTestVoltSystemOracle.t.sol

This file was deleted.

33 changes: 33 additions & 0 deletions contracts/test/integration/utils/ITimelockSimulation.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
pragma solidity =0.8.13;

import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";
import {IPCVGuardian} from "../../../pcv/IPCVGuardian.sol";
import {Vm} from "./../../unit/utils/Vm.sol";

interface ITimelockSimulation {
/// an array of actions makes up a proposal
struct action {
address target;
uint256 value;
bytes arguments;
string description;
}

/// @notice simulate timelock proposal
/// @param proposal an array of actions that compose a proposal
/// @param timelock to execute the proposal against
/// @param guardian to verify all transfers are authorized to hold PCV
/// @param executor account to execute the proposal on the timelock
/// @param proposer account to propose the proposal to the timelock
/// @param vm reference to a foundry vm instance
/// @param doLogging toggle to print out calldata and steps
function simulate(
action[] memory proposal,
TimelockController timelock,
IPCVGuardian guardian,
address executor,
address proposer,
Vm vm,
bool doLogging
) external;
}
80 changes: 80 additions & 0 deletions contracts/test/integration/utils/PCVGuardianWhitelist.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
pragma solidity =0.8.13;

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {IPCVGuardian} from "../../../pcv/IPCVGuardian.sol";
import {ITimelockSimulation} from "./ITimelockSimulation.sol";
import {IPermissions} from "./../../../core/IPermissions.sol";

/// Only allow approvals and transfers of PCV to addresses in PCV Guardian,
/// and only allow granting PCV controllers if they are subsequently added to
/// the PCV Guardian
contract PCVGuardianWhitelist {
mapping(bytes4 => bool) public functionDetectors;

constructor() {
functionDetectors[IERC20.transfer.selector] = true;
functionDetectors[IERC20.approve.selector] = true;
functionDetectors[IPermissions.grantPCVController.selector] = true;
}

/// @notice function to verify actions and ensure that granting a PCV Controller or transferring assets
/// only happens to addresses that are on the PCV Guardian whitelist
function verifyAction(
ITimelockSimulation.action[] memory proposal,
IPCVGuardian guardian
) public {
uint256 proposalLength = proposal.length;
for (uint256 i = 0; i < proposalLength; i++) {
bytes4 functionSig = bytesToBytes4(proposal[i].arguments);

if (functionDetectors[functionSig]) {
address recipient;
bytes memory payload = proposal[i].arguments;
assembly {
recipient := mload(add(payload, 36))
}

if (!guardian.isWhitelistAddress(recipient)) {
revert(
string(
abi.encodePacked(
"Address ",
toString(abi.encodePacked(recipient)),
" not in PCV Guardian whitelist"
)
)
);
}
}
}
}

/// @notice function to grab the first 4 bytes of calldata payload
function bytesToBytes4(bytes memory toSlice)
public
pure
returns (bytes4 functionSignature)
{
if (toSlice.length < 4) {
return bytes4(0);
}

assembly {
functionSignature := mload(add(toSlice, 0x20))
}
}

/// Credit ethereum stackexchange https://ethereum.stackexchange.com/a/58341
function toString(bytes memory data) public pure returns (string memory) {
bytes memory alphabet = "0123456789abcdef";

bytes memory str = new bytes(2 + data.length * 2);
str[0] = "0";
str[1] = "x";
for (uint256 i = 0; i < data.length; i++) {
str[2 + i * 2] = alphabet[uint256(uint8(data[i] >> 4))];
str[3 + i * 2] = alphabet[uint256(uint8(data[i] & 0x0f))];
}
return string(str);
}
}
23 changes: 13 additions & 10 deletions contracts/test/integration/utils/TimelockSimulation.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,34 @@ pragma solidity =0.8.13;
import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";
import {DSTest} from "./../../unit/utils/DSTest.sol";
import {Vm} from "./../../unit/utils/Vm.sol";
import {ITimelockSimulation} from "./ITimelockSimulation.sol";
import {PCVGuardianWhitelist} from "./PCVGuardianWhitelist.sol";
import {IPCVGuardian} from "../../../pcv/IPCVGuardian.sol";

import "hardhat/console.sol";

contract TimelockSimulation is DSTest {
/// an array of actions makes up a proposal
struct action {
address target;
uint256 value;
bytes arguments;
string description;
}

contract TimelockSimulation is
DSTest,
ITimelockSimulation,
PCVGuardianWhitelist
{
/// @notice simulate timelock proposal
/// @param proposal an array of actions that compose a proposal
/// @param timelock to execute the proposal against
/// @param guardian to verify all transfers are authorized to hold PCV
/// @param executor account to execute the proposal on the timelock
/// @param proposer account to propose the proposal to the timelock
/// @param vm reference to a foundry vm instance
/// @param doLogging toggle to print out calldata and steps
function simulate(
action[] memory proposal,
TimelockController timelock,
IPCVGuardian guardian,
address executor,
address proposer,
Vm vm,
bool doLogging
) public {
) public override {
uint256 delay = timelock.getMinDelay();
bytes32 salt = keccak256(abi.encode(proposal[0].description));

Expand Down Expand Up @@ -127,5 +128,7 @@ contract TimelockSimulation is DSTest {
} else if (doLogging) {
console.log("proposal already executed");
}

verifyAction(proposal, guardian);
}
}
14 changes: 12 additions & 2 deletions contracts/test/integration/vip/Runner.sol
Original file line number Diff line number Diff line change
@@ -1,21 +1,30 @@
pragma solidity =0.8.13;

// import {vipx} from "./vipx.sol";
import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";
import {TimelockSimulation} from "../utils/TimelockSimulation.sol";
import {MainnetAddresses} from "../fixtures/MainnetAddresses.sol";
import {ArbitrumAddresses} from "../fixtures/ArbitrumAddresses.sol";
import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";
import {PCVGuardian} from "./../../../pcv/PCVGuardian.sol";

/// @dev test harness for running and simulating VOLT Improvement Proposals
/// inherit the proposal to simulate
contract Runner is TimelockSimulation {
/// @notice mainnet PCV Guardian
PCVGuardian private immutable mainnetPCVGuardian =
PCVGuardian(MainnetAddresses.PCV_GUARDIAN);

/// @notice arbitrum PCV Guardian
PCVGuardian private immutable arbitrumPCVGuardian =
PCVGuardian(ArbitrumAddresses.PCV_GUARDIAN);

/// remove all function calls inside testProposal and don't inherit the VIP
/// once the proposal is live and passed
function testProposalMainnet() public {
// mainnetSetup();
// simulate(
// getMainnetProposal(),
// TimelockController(payable(MainnetAddresses.TIMELOCK_CONTROLLER)),
// mainnetPCVGuardian,
// MainnetAddresses.GOVERNOR,
// MainnetAddresses.EOA_1,
// vm,
Expand All @@ -29,6 +38,7 @@ contract Runner is TimelockSimulation {
// simulate(
// getArbitrumProposal(),
// TimelockController(payable(ArbitrumAddresses.TIMELOCK_CONTROLLER)),
// arbitrumPCVGuardian,
// ArbitrumAddresses.GOVERNOR,
// ArbitrumAddresses.EOA_1,
// vm,
Expand Down
60 changes: 60 additions & 0 deletions contracts/test/integration/vip/examples/vip_x_grant.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
pragma solidity =0.8.13;

import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";
import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";

import {TimelockSimulation} from "../../utils/TimelockSimulation.sol";
import {ArbitrumAddresses} from "../../fixtures/ArbitrumAddresses.sol";
import {MainnetAddresses} from "../../fixtures/MainnetAddresses.sol";
import {PriceBoundPSM} from "../../../../peg/PriceBoundPSM.sol";
import {AllRoles} from "./../../utils/AllRoles.sol";
import {DSTest} from "./../../../unit/utils/DSTest.sol";
import {Core} from "../../../../core/Core.sol";
import {Volt} from "../../../../volt/Volt.sol";
import {IVIP} from "./../IVIP.sol";
import {Vm} from "./../../../unit/utils/Vm.sol";

contract vip_x_grant is DSTest, IVIP {
using SafeCast for *;

Vm public constant vm = Vm(HEVM_ADDRESS);

/// --------------- Mainnet ---------------

/// this is an example proposal that will fail the PCV Guardian whitelist test
/// as PCV is being transferrred to a non authorized smart contract
function getMainnetProposal()
public
pure
override
returns (TimelockSimulation.action[] memory proposal)
{
proposal = new TimelockSimulation.action[](1);

proposal[0].target = MainnetAddresses.CORE;
proposal[0].value = 0;
proposal[0].arguments = abi.encodeWithSignature(
"grantPCVController(address)",
MainnetAddresses.REVOKED_EOA_1
);
proposal[0]
.description = "Grant PCV Controller and not setting in whitelist of PCV Guardian fails preflight checks";
}

function mainnetValidate() public override {}

function mainnetSetup() public override {}

/// --------------- Arbitrum ---------------

function getArbitrumProposal()
public
pure
override
returns (TimelockSimulation.action[] memory proposal)
{}

function arbitrumSetup() public override {}

function arbitrumValidate() public override {}
}
67 changes: 67 additions & 0 deletions contracts/test/integration/vip/examples/vip_x_grant_succeed.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
pragma solidity =0.8.13;

import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";
import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";

import {TimelockSimulation} from "../../utils/TimelockSimulation.sol";
import {ArbitrumAddresses} from "../../fixtures/ArbitrumAddresses.sol";
import {MainnetAddresses} from "../../fixtures/MainnetAddresses.sol";
import {PriceBoundPSM} from "../../../../peg/PriceBoundPSM.sol";
import {AllRoles} from "./../../utils/AllRoles.sol";
import {DSTest} from "./../../../unit/utils/DSTest.sol";
import {Core} from "../../../../core/Core.sol";
import {Volt} from "../../../../volt/Volt.sol";
import {IVIP} from "./../IVIP.sol";
import {Vm} from "./../../../unit/utils/Vm.sol";

contract vip_x_grant_succeed is DSTest, IVIP {
using SafeCast for *;

Vm public constant vm = Vm(HEVM_ADDRESS);

/// --------------- Mainnet ---------------

/// this is an example proposal that will fail the PCV Guardian whitelist test
/// as PCV is being transferrred to a non authorized smart contract
function getMainnetProposal()
public
pure
override
returns (TimelockSimulation.action[] memory proposal)
{
proposal = new TimelockSimulation.action[](2);

proposal[0].target = MainnetAddresses.CORE;
proposal[0].value = 0;
proposal[0].arguments = abi.encodeWithSignature(
"grantPCVController(address)",
MainnetAddresses.GOVERNOR
);
proposal[0].description = "Grant PCV Controller";

proposal[1].target = MainnetAddresses.PCV_GUARDIAN;
proposal[1].value = 0;
proposal[1].arguments = abi.encodeWithSignature(
"addWhitelistAddress(address)",
MainnetAddresses.GOVERNOR
);
proposal[1].description = "Add governor to whitelist in PCV Guardian";
}

function mainnetValidate() public override {}

function mainnetSetup() public override {}

/// --------------- Arbitrum ---------------

function getArbitrumProposal()
public
pure
override
returns (TimelockSimulation.action[] memory proposal)
{}

function arbitrumSetup() public override {}

function arbitrumValidate() public override {}
}
Loading

0 comments on commit 6b7c0f2

Please sign in to comment.