From 05ecdd847cae06b3dbcf26c5ff5a5226a94a2b64 Mon Sep 17 00:00:00 2001 From: Ana Date: Tue, 14 May 2024 13:04:10 -0300 Subject: [PATCH] Update remediations (#70) * prompt proposal id * add deploy code * add make persistent * Check calldata * add run function with flags back * bravo match working * remove run function with flags * Bravo check calldata integration test * Timelock check onchain calldata * delete validate calldata script * Comments * Run tests on sepolia fork * move addresses test to a separate CI * update Addresses.json * remove default checkOnChainCalldata implementation * lint * debug * ci debug * Update Addresses.json * redeploy timelock * move from internal to public functions * functions visibility * print functions * review changes * rename DEV address to DEPLOYER_EOA * rename ci test * Remove examples and integration tests from this repo * remove calldata generation from CI * remove run-proposal.sh * merge conflicts * TimelockProposalUnitTest test_build pass * update ci * update foundry.toml * after deploy mock * fix state diff recording * test simulate working * Remove caller from Proposal constructor * test getCalldata * getExecuteCalldata * mock MockTimelockProposal * bravo mocks * setTimelock and setAddresses * Test Bravo Proposal * timelock description * fix timelock test_getCalldata * lint * compile * MockMultisigProposal * Multisig tests * add checkOnChainCalldata tests * do not broadcast afterDeployMock call * add validate function tests * remediate addresses.t.sol * unit -> integration tests * after deploy flag * remove multisig cast * simplify vault * simplify Vault and Token mocks * remove bravo mocks * lint * interfaces * delete interfaces/IGovernorBravo * compound proposal * Compound test * debug * run CI on mainnet * lint * add validateActionInclusion internal function * natspec * action hash * review changes * remove params from _simulateActions * Multisig Proposal example using Optmism * fix validate function * lint * Delete TimelockProposal * fix failing tests * review changes * compile --- .github/workflows/ci.yml | 6 +- addresses/Addresses.json | 128 ++- addresses/Addresses.sol | 4 +- addresses/AddressesDuplicated.json | 8 +- .../AddressesDuplicatedDifferentName.json | 8 +- docs/assets/diagram.svg | 2 +- docs/guides/governor-bravo-proposal.md | 24 +- docs/guides/multisig-proposal.md | 14 +- docs/guides/timelock-proposal.md | 14 +- docs/overview/architecture/README.md | 2 +- docs/overview/architecture/addresses.md | 2 +- .../architecture/internal-functions.md | 16 +- docs/type-check/example.md | 2 +- docs/type-check/type-check.md | 4 +- foundry.toml | 4 +- mocks/IERC20.sol | 53 ++ mocks/MockBravoProposal.sol | 64 ++ mocks/MockDuplicatedActionProposal.sol | 50 ++ mocks/MockERC20.sol | 145 +++ mocks/MockMultisigProposal.sol | 82 ++ mocks/MockTimelockProposal.sol | 97 ++ mocks/Token.sol | 15 + mocks/Vault.sol | 56 ++ mocks/bravo/GovernorBravoDelegate.sol | 830 ------------------ mocks/bravo/SafeMath.sol | 219 ----- mocks/bravo/Timelock.sol | 208 ----- src/interface/ICompoundConfigurator.sol | 45 + src/interface/IGovernorBravo.sol | 195 +--- src/interface/IVotes.sol | 76 -- src/proposals/GovernorBravoProposal.sol | 85 +- src/proposals/IProposal.sol | 56 +- src/proposals/MultisigProposal.sol | 27 +- src/proposals/Proposal.sol | 313 +++---- src/proposals/TimelockProposal.sol | 85 +- src/type-check/ExampleTypeCheck.sol | 1 - src/type-check/ExampleTypeCheck_02.sol | 1 - test/Addresses.t.sol | 78 +- test/BravoProposal.t.sol | 129 +++ test/DuplicatedAction.t.sol | 36 + test/MultisigProposal.t.sol | 137 +++ 40 files changed, 1397 insertions(+), 1924 deletions(-) create mode 100644 mocks/IERC20.sol create mode 100644 mocks/MockBravoProposal.sol create mode 100644 mocks/MockDuplicatedActionProposal.sol create mode 100644 mocks/MockERC20.sol create mode 100644 mocks/MockMultisigProposal.sol create mode 100644 mocks/MockTimelockProposal.sol create mode 100644 mocks/Token.sol create mode 100644 mocks/Vault.sol delete mode 100644 mocks/bravo/GovernorBravoDelegate.sol delete mode 100644 mocks/bravo/SafeMath.sol delete mode 100644 mocks/bravo/Timelock.sol create mode 100644 src/interface/ICompoundConfigurator.sol delete mode 100644 src/interface/IVotes.sol create mode 100644 test/BravoProposal.t.sol create mode 100644 test/DuplicatedAction.t.sol create mode 100644 test/MultisigProposal.t.sol diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 73173cb9..2af699e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,7 +43,7 @@ jobs: echo "✅ Passed or warnings found" >> $GITHUB_STEP_SUMMARY fi - unit-test: + integration-test: runs-on: "ubuntu-latest" steps: - name: "Check out the repo" @@ -68,8 +68,8 @@ jobs: - name: "Build the contracts and print their size" run: "forge build --sizes" - - name: "Proposal Simulator Test" - run: "forge test --nmc '(TypeCheck|TestAddresses)'" + - name: "Proposal Simulator Integration Tests" + run: "forge test --mc IntegrationTest --fork-url mainnet" - name: "Add test summary" run: | diff --git a/addresses/Addresses.json b/addresses/Addresses.json index 6c327a10..05552980 100644 --- a/addresses/Addresses.json +++ b/addresses/Addresses.json @@ -1,32 +1,14 @@ [ { - "addr": "0x3dd46846eed8D147841AE162C8425c08BD8E1b41", - "name": "DEV_MULTISIG", - "chainId": 31337, - "isContract": false - }, - { - "addr": "0x7da82C7AB4771ff031b66538D2fB9b0B047f6CF9", - "name": "TEAM_MULTISIG", - "chainId": 31337, - "isContract": false - }, - { - "addr": "0x1a9C8182C09F50C8318d769245beA52c32BE35BC", - "name": "PROTOCOL_TIMELOCK", - "chainId": 31337, - "isContract": false - }, - { - "addr": "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266", - "name": "PROTOCOL_GOVERNOR", + "addr": "0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739", + "name": "DEPLOYER_EOA", "chainId": 31337, "isContract": false }, { - "addr": "0x70997970C51812dc3A010C7d01b50e0d17dc79C8", - "name": "PROTOCOL_GOVERNANCE_TOKEN", - "chainId": 31337, + "addr": "0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739", + "name": "DEPLOYER_EOA", + "chainId": 1, "isContract": false }, { @@ -41,17 +23,47 @@ "chainId": 11155111, "isContract": false }, - { - "addr": "0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739", - "name": "DEPLOYER_EOA", - "chainId": 31337, - "isContract": false - }, { "addr": "0x2EE28b12156fCaa13Bb188347cE6635C18bc94ef", "chainId": 11155111, "isContract": true, - "name": "PROTOCOL_GOVERNOR" + "name": "GOVERNOR_BRAVO" + }, + { + "addr": "0xc0da02939e1441f497fd74f78ce7decb17b66529", + "chainId": 1, + "isContract": true, + "name": "COMPOUND_GOVERNOR_BRAVO" + }, + { + "addr": "0x316f9708bB98af7dA9c68C1C3b5e79039cD336E3", + "chainId": 1, + "isContract": true, + "name": "COMPOUND_CONFIGURATOR" + }, + { + "addr": "0xf603265f91f58F1EfA4fAd57694Fb3B77b25fC18", + "chainId": 1, + "isContract": false, + "name": "COMPOUND_PROPOSER" + }, + { + "addr": "0xa17581a9e3356d9a858b789d68b4d866e593ae94", + "chainId": 1, + "isContract": true, + "name": "COMPOUND_COMET" + }, + { + "addr": "0xc00e94cb662c3520282e6f5717214004a7f26888", + "chainId": 1, + "isContract": true, + "name": "COMP_TOKEN" + }, + { + "addr": "0x6d903f6003cca6255D85CcA4D3B5E5146dC33925", + "chainId": 1, + "isContract": true, + "name": "COMPOUND_TIMELOCK_BRAVO" }, { "addr": "0x37039662a2364Df8206691c1D1b9ff8d22389290", @@ -66,7 +78,7 @@ "name": "PROTOCOL_GOVERNANCE_TOKEN" }, { - "addr": "0xb7dd5fd38c6dc6af3dded96bbd8935a71eb04b21", + "addr": "0xAE96E645c7c7628BA9507B1e91A5fA3Cb1EB5DA1", "chainId": 11155111, "isContract": true, "name": "PROTOCOL_TIMELOCK" @@ -75,7 +87,31 @@ "addr": "0x35e292eA59C71caC368ecdf9046eF7C3ED0DF79E", "chainId": 11155111, "isContract": true, - "name": "PROTOCOL_GOVERNOR_ALPHA" + "name": "GOVERNOR_BRAVO_ALPHA" + }, + { + "addr": "0x7D26D5978cD4617C6c7a02C75a42927eF52f405C", + "chainId": 11155111, + "isContract": true, + "name": "TIMELOCK_VAULT" + }, + { + "addr": "0x861bB12491F453F48fE1A05e9C034588de638262", + "chainId": 11155111, + "isContract": true, + "name": "TIMELOCK_TOKEN" + }, + { + "addr": "0x0CB10d561C2a273AFBB81E535738BA3095E9dCF2", + "chainId": 11155111, + "isContract": true, + "name": "BRAVO_VAULT" + }, + { + "addr": "0x76126BDFC0893d9A27336b93824778432cA60808", + "chainId": 11155111, + "isContract": true, + "name": "BRAVO_VAULT_TOKEN" }, { "addr": "0x293cad26033577eb68137603a34d2dbfd05104d8", @@ -90,15 +126,27 @@ "isContract": true }, { - "addr": "0x9eF929330b3ABB6A814DF726A5834D43Df093E2d", - "chainId": 11155111, - "isContract": true, - "name": "VAULT" + "addr": "0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A", + "chainId": 1, + "name": "OPTIMISM_MULTISIG", + "isContract": true }, { - "addr": "0x8dfF5C5b38d65b36064226719F4839E288411376", - "chainId": 11155111, - "isContract": true, - "name": "TOKEN_1" + "addr": "0x543bA4AADBAb8f9025686Bd03993043599c6fB04", + "chainId": 1, + "name": "OPTIMISM_PROXY_ADMIN", + "isContract": true + }, + { + "addr": "0x5a7749f83b81b301cab5f48eb8516b986daef23d", + "chainId": 1, + "name": "OPTIMISM_L1_NFT_BRIDGE_PROXY", + "isContract": true + }, + { + "addr": "0xAE2AF01232a6c4a4d3012C5eC5b1b35059caF10d", + "chainId": 1, + "name": "OPTIMISM_L1_NFT_BRIDGE_IMPLEMENTATION", + "isContract": true } ] diff --git a/addresses/Addresses.sol b/addresses/Addresses.sol index 037d27b7..be489270 100644 --- a/addresses/Addresses.sol +++ b/addresses/Addresses.sol @@ -278,8 +278,8 @@ contract Addresses is IAddresses, Test { return _addresses[name][chainId].addr != address(0); } - /// @dev Print recorded addresses - function printRecordedAddresses() external view { + /// @dev Print new recorded and changed addresses + function printJSONChanges() external view { { ( string[] memory names, diff --git a/addresses/AddressesDuplicated.json b/addresses/AddressesDuplicated.json index 092d20d7..240580e8 100644 --- a/addresses/AddressesDuplicated.json +++ b/addresses/AddressesDuplicated.json @@ -1,13 +1,13 @@ [ { - "addr": "0x3dd46846eed8D147841AE162C8425c08BD8E1b41", - "name": "DEV_MULTISIG", + "addr": "0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739", + "name": "DEPLOYER_EOA", "chainId": 31337, "isContract": false }, { - "addr": "0x3dd46846eed8D147841AE162C8425c08BD8E1b41", - "name": "DEV_MULTISIG", + "addr": "0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739", + "name": "DEPLOYER_EOA", "chainId": 31337, "isContract": false } diff --git a/addresses/AddressesDuplicatedDifferentName.json b/addresses/AddressesDuplicatedDifferentName.json index bccb0976..d67f27f6 100644 --- a/addresses/AddressesDuplicatedDifferentName.json +++ b/addresses/AddressesDuplicatedDifferentName.json @@ -1,13 +1,13 @@ [ { - "addr": "0x3dd46846eed8D147841AE162C8425c08BD8E1b41", - "name": "DEV_MULTISIG", + "addr": "0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739", + "name": "DEPLOYER_EOA", "chainId": 31337, "isContract": false }, { - "addr": "0x3dd46846eed8D147841AE162C8425c08BD8E1b41", - "name": "DEV_MULTISIG2", + "addr": "0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739", + "name": "DEPLOYER_EOA2", "chainId": 31337, "isContract": false } diff --git a/docs/assets/diagram.svg b/docs/assets/diagram.svg index b46cc45d..169fc9d3 100644 --- a/docs/assets/diagram.svg +++ b/docs/assets/diagram.svg @@ -18,4 +18,4 @@ - TimelockProposalgetScheduleCaldata()getExecuteCalldata()_simulateActions()MultisigProposalgetCalldata()_simulateActions()Proposalname()...description()run()getCalldata()AddressesgetAddress()...addAddress()changeAddress()Forge Proposal SimulatorTimelock Governance ProtocolPROPOSAL_01_deploy()_afterDeploy()_build()_run()validate()PROPOSAL_02_build()_run()validate()TimelockPostProposalChecksetUp()Timelock01Testtest_01()test_02()test_03()TimelockPostProposalChecksetUp()Timelock02Testtest_01()test_02()test_03() \ No newline at end of file + TimelockProposalgetScheduleCaldata()getExecuteCalldata()_simulateActions()MultisigProposalgetCalldata()_simulateActions()Proposalname()...description()run()getCalldata()AddressesgetAddress()...addAddress()changeAddress()Forge Proposal SimulatorTimelock Governance ProtocolPROPOSAL_01_deploy()_afterDeployMock()_build()_run()validate()PROPOSAL_02_build()_run()validate()TimelockPostProposalChecksetUp()Timelock01Testtest_01()test_02()test_03()TimelockPostProposalChecksetUp()Timelock02Testtest_01()test_02()test_03() diff --git a/docs/guides/governor-bravo-proposal.md b/docs/guides/governor-bravo-proposal.md index d7dd939c..34b0c4fe 100644 --- a/docs/guides/governor-bravo-proposal.md +++ b/docs/guides/governor-bravo-proposal.md @@ -39,7 +39,7 @@ contract BRAVO_01 is GovernorBravoProposal { } /// @notice Deploys a vault contract and an ERC20 token contract. - function _deploy() internal override { + function deploy() public override { if (!addresses.isAddressSet("VAULT")) { Vault timelockVault = new Vault(); addresses.addAddress("VAULT", address(timelockVault), true); @@ -55,7 +55,7 @@ contract BRAVO_01 is GovernorBravoProposal { /// 1. Transfers vault ownership to timelock. /// 2. Transfer token ownership to timelock. /// 3. Transfers all tokens to timelock. - function _afterDeploy() internal override { + function afterDeployMock() public override { address timelock = addresses.getAddress("PROTOCOL_TIMELOCK"); Vault timelockVault = Vault(addresses.getAddress("VAULT")); MockToken token = MockToken(addresses.getAddress("TOKEN_1")); @@ -71,7 +71,7 @@ contract BRAVO_01 is GovernorBravoProposal { } /// @notice Sets up actions for the proposal, in this case, setting the MockToken to active. - function _build() internal override { + function build() public override { /// STATICCALL -- not recorded for the run stage address timelockVault = addresses.getAddress("VAULT"); address token = addresses.getAddress("TOKEN_1"); @@ -81,11 +81,11 @@ contract BRAVO_01 is GovernorBravoProposal { } /// @notice Executes the proposal actions. - function _run() internal override { + function simulate() public override { // Call parent _run function to check if there are actions to execute - super._run(); + super.simulate(); - address governor = addresses.getAddress("PROTOCOL_GOVERNOR"); + address governor = addresses.getAddress("GOVERNOR_BRAVO"); address govToken = addresses.getAddress("PROTOCOL_GOVERNANCE_TOKEN"); address proposer = addresses.getAddress("DEPLOYER_EOA"); @@ -93,7 +93,7 @@ contract BRAVO_01 is GovernorBravoProposal { } /// @notice Validates the post-execution state. - function _validate() internal override { + function validate() public override { address timelock = addresses.getAddress("PROTOCOL_TIMELOCK"); Vault timelockVault = Vault(addresses.getAddress("VAULT")); MockToken token = MockToken(addresses.getAddress("TOKEN_1")); @@ -117,7 +117,7 @@ Let's go through each of the functions that are overridden. they are added to the `Addresses` contract by calling `addAddress()`. - `_build()`: Set the necessary actions for your proposal. In this example, ERC20 token is whitelisted on the Vault contract. The actions should be written in solidity code and in the order they should be executed. Any calls (except to the Addresses object) will be recorded and stored as actions to execute in the run function. -- `_run()`: Execute the proposal actions outlined in the `_build()` step. This +- `simulate()`: Execute the proposal actions outlined in the `_build()` step. This function performs a call to `_simulateActions` from the inherited `GovernorBravoProposal` contract. Internally, `_simulateActions()` uses the calldata generated from the actions set up in the build step, and simulates the end-to-end workflow of a successful proposal submission, starting with a call to [propose](https://github.com/compound-finance/compound-governance/blob/5e581ef817464fdb71c4c7ef6bde4c552302d160/contracts/GovernorBravoDelegate.sol#L118). - `_validate()`: This final step is crucial for validating the post-execution state. It ensures that the timelock is the new owner of Vault and token, the tokens were transferred to timelock and the token was whitelisted on the Vault contract @@ -175,7 +175,7 @@ Copy the addresses of the timelock, governor, and governance token from the scri }, { "addr": "YOUR_GOVERNOR_ADDRESS", - "name": "PROTOCOL_GOVERNOR", + "name": "GOVERNOR_BRAVO", "chainId": 11155111, "isContract": true }, @@ -205,7 +205,7 @@ Run the script: forge script script/InitializeBravo.s.sol --rpc-url sepolia --broadcast -vvvv --slow --sender ${wallet_address} -vvvv --account ${wallet_name} -g 200 ``` -Copy the _PROTOCOL_GOVERNOR_ALPHA_ address from the script output and add it to +Copy the _GOVERNOR_BRAVO_ALPHA_ address from the script output and add it to the `Addresses.json` file. ### Setting Up the Addresses JSON @@ -223,7 +223,7 @@ to Address.json. The final Address.json file should be something like this: }, { "addr": "YOUR_GOVERNOR_ADDRESS", - "name": "PROTOCOL_GOVERNOR", + "name": "GOVERNOR_BRAVO", "chainId": 11155111, "isContract": true }, @@ -235,7 +235,7 @@ to Address.json. The final Address.json file should be something like this: }, { "addr": "YOUR_GOVERNOR_ALPHA_ADDRESS", - "name": "PROTOCOL_GOVERNOR_ALPHA", + "name": "GOVERNOR_BRAVO_ALPHA", "chainId": 11155111, "isContract": true }, diff --git a/docs/guides/multisig-proposal.md b/docs/guides/multisig-proposal.md index 28f9d97c..96858c1a 100644 --- a/docs/guides/multisig-proposal.md +++ b/docs/guides/multisig-proposal.md @@ -39,7 +39,7 @@ contract MULTISIG_01 is MultisigProposal { } /// @notice Deploys a vault contract and an ERC20 token contract. - function _deploy() internal override { + function deploy() public override { if (!addresses.isAddressSet("VAULT")) { Vault timelockVault = new Vault(); addresses.addAddress("VAULT", address(timelockVault), true); @@ -55,7 +55,7 @@ contract MULTISIG_01 is MultisigProposal { /// 1. Transfers vault ownership to dev multisig. /// 2. Transfer token ownership to dev multisig. /// 3. Transfers all tokens to dev multisig. - function _afterDeploy() internal override { + function afterDeployMock() public override { address devMultisig = addresses.getAddress("DEV_MULTISIG"); Vault timelockVault = Vault(addresses.getAddress("VAULT")); MockToken token = MockToken(addresses.getAddress("TOKEN_1")); @@ -70,7 +70,7 @@ contract MULTISIG_01 is MultisigProposal { } /// @notice Sets up actions for the proposal, in this case, setting the MockToken to active. - function _build() internal override { + function build() public override { /// STATICCALL -- not recorded for the run stage address timelockVault = addresses.getAddress("VAULT"); address token = addresses.getAddress("TOKEN_1"); @@ -80,9 +80,9 @@ contract MULTISIG_01 is MultisigProposal { } /// @notice Executes the proposal actions. - function _run() internal override { + function simulate() public override { /// Call parent _run function to check if there are actions to execute - super._run(); + super.simulate(); address multisig = addresses.getAddress("DEV_MULTISIG"); @@ -91,7 +91,7 @@ contract MULTISIG_01 is MultisigProposal { } /// @notice Validates the post-execution state. - function _validate() internal override { + function validate() public override { address devMultisig = addresses.getAddress("DEV_MULTISIG"); Vault timelockVault = Vault(addresses.getAddress("VAULT")); MockToken token = MockToken(addresses.getAddress("TOKEN_1")); @@ -116,7 +116,7 @@ Let's go through each of the functions that are overridden. - `_build()`: Set the necessary actions for your proposal. In this example, ERC20 token is whitelisted on the Vault contract. The actions should be written in solidity code and in the order they should be executed. Any calls (except to the Addresses object) will be recorded and stored as actions to execute in the run function. -- `_run()`: Execute the proposal actions outlined in the `_build()` step. This +- `simulate()`: Execute the proposal actions outlined in the `_build()` step. This function performs a call to `simulateActions()` from the inherited `MultisigProposal` contract. Internally, `_simulateActions()` simulates a call to the [Multicall3](https://www.multicall3.com/) contract with the calldata generated from the actions set up in the build step. - `_validate()`: This final step is crucial for validating the post-execution state. It ensures that the multisig is the new owner of Vault and token, the tokens were transferred to multisig and the token was whitelisted on the Vault contract diff --git a/docs/guides/timelock-proposal.md b/docs/guides/timelock-proposal.md index e87412ae..3765142c 100644 --- a/docs/guides/timelock-proposal.md +++ b/docs/guides/timelock-proposal.md @@ -38,7 +38,7 @@ contract TIMELOCK_01 is TimelockProposal { } /// @notice Deploys a vault contract and an ERC20 token contract. - function _deploy() internal override { + function deploy() public override { if (!addresses.isAddressSet("VAULT")) { Vault timelockVault = new Vault(); addresses.addAddress("VAULT", address(timelockVault), true); @@ -53,7 +53,7 @@ contract TIMELOCK_01 is TimelockProposal { // @notice Transfers vault ownership to timelock. // Transfer token ownership to timelock. // Transfers all tokens to timelock. - function _afterDeploy() internal override { + function afterDeployMock() public override { address timelock = addresses.getAddress("PROTOCOL_TIMELOCK"); Vault timelockVault = Vault(addresses.getAddress("VAULT")); MockToken token = MockToken(addresses.getAddress("TOKEN_1")); @@ -68,7 +68,7 @@ contract TIMELOCK_01 is TimelockProposal { } /// @notice Set up actions for the proposal, in this case, setting the MockToken to active. - function _build() internal override { + function build() public override { /// STATICCALL -- not recorded for the run stage address timelockVault = addresses.getAddress("VAULT"); address token = addresses.getAddress("TOKEN_1"); @@ -78,9 +78,9 @@ contract TIMELOCK_01 is TimelockProposal { } /// @notice Executes the proposal actions. - function _run() internal override { + function simulate() public override { /// Call parent _run function to check if there are actions to execute - super._run(); + super.simulate(); address dev = addresses.getAddress("DEPLOYER_EOA"); @@ -89,7 +89,7 @@ contract TIMELOCK_01 is TimelockProposal { } /// @notice Validates the post-execution state. - function _validate() internal override { + function validate() public override { address timelock = addresses.getAddress("PROTOCOL_TIMELOCK"); Vault timelockVault = Vault(addresses.getAddress("VAULT")); MockToken token = MockToken(addresses.getAddress("TOKEN_1")); @@ -114,7 +114,7 @@ Let's go through each of the functions that are overridden. - `_build()`: Set the necessary actions for your proposal. In this example, ERC20 token is whitelisted on the Vault contract. The actions should be written in solidity code and in the order they should be executed. Any calls (except to the Addresses object) will be recorded and stored as actions to execute in the run function. -- `_run()`: Execute the proposal actions outlined in the `_build()` step. This +- `simulate()`: Execute the proposal actions outlined in the `_build()` step. This function performs a call to `_simulateActions` from the inherited `TimelockProposal` contract. Internally, `_simulateActions()` simulates a call to Timelock [scheduleBatch](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/governance/TimelockController.sol#L291) and [executeBatch](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/governance/TimelockController.sol#L385) with the calldata generated from the actions set up in the build step. - `_validate()`: This final step is crucial for validating the post-execution state. It ensures that the timelock is the new owner of Vault and token, the tokens were transferred to timelock and the token was whitelisted on the Vault contract diff --git a/docs/overview/architecture/README.md b/docs/overview/architecture/README.md index 228b3b5e..7db21b4c 100644 --- a/docs/overview/architecture/README.md +++ b/docs/overview/architecture/README.md @@ -23,7 +23,7 @@ FPS accommodates different Governance types (e.g., Timelock, Multisig) through s ## Proposal Specific Contract -Protocols using FPS must create their own Proposal Specific Contracts, conforming to FPS standards. These contracts override functions relevant to the particular proposal, such as `_deploy()` and `_afterDeploy()` for proposals involving new contract deployments. For more details, refer to [internal functions](internal-functions.md). +Protocols using FPS must create their own Proposal Specific Contracts, conforming to FPS standards. These contracts override functions relevant to the particular proposal, such as `deploy()` and `_afterDeployMock()` for proposals involving new contract deployments. For more details, refer to [internal functions](internal-functions.md). ## Test Suite diff --git a/docs/overview/architecture/addresses.md b/docs/overview/architecture/addresses.md index 7b8320a0..b81cc677 100644 --- a/docs/overview/architecture/addresses.md +++ b/docs/overview/architecture/addresses.md @@ -175,7 +175,7 @@ contract PROPOSAL_01 is MultisigProposal { constructor() Proposal(ADDRESSES_PATH, "DEV_MULTISIG") {} - function _deploy() internal override { + function deploy() public override { if (!addresses.isAddressSet("CONTRACT_NAME")) { /// Deploy a new contract MyContract myContract = new MyContract(); diff --git a/docs/overview/architecture/internal-functions.md b/docs/overview/architecture/internal-functions.md index 4291db13..e54bc0af 100644 --- a/docs/overview/architecture/internal-functions.md +++ b/docs/overview/architecture/internal-functions.md @@ -4,17 +4,11 @@ This section details optional internal functions offered by FPS. These functions allow for a significant level of customization and can be overridden in your proposal contract as needed. -- `function _deploy() internal`: Defines new contract deployments. Newly deployed contracts must be added to the `Addresses` contract instance through the setters methods. -- `function _afterDeploy() internal`: Specifies post-deployment actions. Such actions can include wiring contracts together, transferring ownership rights, or invoking setter functions as the deployer. -- `function _build() internal`: Creates the proposal actions and saves them to storage in the proposal contract. -- `function _run() internal`: Executes the actions that were previously saved during the `_build` step. It's dependent on the successful execution of the `_build` function. Without calling `_build` first, the `_run` function becomes ineffectual as there would be no predefined actions to execute. -- `function _teardown() internal`: Defines the actions to be - taken post-proposal execution. For instance, in scenarios where a protocol - incorporates multiple timelocks and multisigs, this framework supports - executing only one proposal at a time. Hence, before executing the validate - function, this step can include pranking as another timelock to perform - necessary actions. -- `function _validate() internal`: Validates the state post-execution. It ensures that the contracts variables and proposal targets are set up correctly. +- `function deploy() public`: Defines new contract deployments. Newly deployed contracts must be added to the `Addresses` contract instance through the setters methods. +- `function afterDeployMock() public`: Specifies post-deployment actions. Such actions can include wiring contracts together, transferring ownership rights, or invoking setter functions as the deployer. +- `function build() public`: Creates the proposal actions and saves them to storage in the proposal contract. +- `function simulate() public`: Executes the actions that were previously saved during the `_build` step. It's dependent on the successful execution of the `_build` function. Without calling `_build` first, the `_run` function becomes ineffectual as there would be no predefined actions to execute. +- `function validate() public`: Validates the state post-execution. It ensures that the contracts variables and proposal targets are set up correctly. The actions in FPS are designed to be loosely coupled for flexible implementation, with the exception of the build and run functions, which require diff --git a/docs/type-check/example.md b/docs/type-check/example.md index b8d015c5..1f0d8140 100644 --- a/docs/type-check/example.md +++ b/docs/type-check/example.md @@ -13,7 +13,7 @@ Add file `ExampleTypeCheck.sol` in `src` of root repository: ```solidity // SPDX-License-Identifier: GPL-3.0 -pragma solidity =0.8.19; +pragma solidity ^0.8.0; contract ExampleTypeCheck { struct StructC { diff --git a/docs/type-check/type-check.md b/docs/type-check/type-check.md index 9f4e2919..d6b28a90 100644 --- a/docs/type-check/type-check.md +++ b/docs/type-check/type-check.md @@ -9,7 +9,7 @@ The `TypeCheckAddresses.json file` is a JSON file used for verifying the bytecod ExampleTypeCheck.sol ```solidity -pragma solidity =0.8.19; +pragma solidity ^0.8.0; contract ExampleTypeCheck { struct StructC { @@ -49,7 +49,7 @@ contract ExampleTypeCheck { ExampleTypeCheck_02.sol ```solidity -pragma solidity =0.8.19; +pragma solidity ^0.8.0; contract ExampleTypeCheck_02 { struct StructA { diff --git a/foundry.toml b/foundry.toml index 93f46308..c8ab2c6e 100644 --- a/foundry.toml +++ b/foundry.toml @@ -2,16 +2,16 @@ libs = ['lib'] test = 'test' auto_detect_solc = true - # Require read access for 'addresses' and 'out' folder on root repo for bytecode verification fs_permissions = [{ access = "read", path = "./"}] -evm_version = 'paris' optimizer = true optimizer_runs = 200 +evm_version="Cancun" [rpc_endpoints] localhost = "http://127.0.0.1:8545" sepolia = "https://sepolia.drpc.org" +mainnet = "https://mainnet.gateway.tenderly.co" #[etherscan] #sepolia = { key = "${ETHERSCAN_API_KEY}" } diff --git a/mocks/IERC20.sol b/mocks/IERC20.sol new file mode 100644 index 00000000..9c20dbfa --- /dev/null +++ b/mocks/IERC20.sol @@ -0,0 +1,53 @@ +pragma solidity ^0.8.0; + +/// @dev Interface of the ERC20 standard as defined in the EIP. +/// @dev This includes the optional name, symbol, and decimals metadata. +interface IERC20 { + /// @dev Emitted when `value` tokens are moved from one account (`from`) to another (`to`). + event Transfer(address indexed from, address indexed to, uint256 value); + + /// @dev Emitted when the allowance of a `spender` for an `owner` is set, where `value` + /// is the new allowance. + event Approval( + address indexed owner, + address indexed spender, + uint256 value + ); + + /// @notice Returns the amount of tokens in existence. + function totalSupply() external view returns (uint256); + + /// @notice Returns the amount of tokens owned by `account`. + function balanceOf(address account) external view returns (uint256); + + /// @notice Moves `amount` tokens from the caller's account to `to`. + function transfer(address to, uint256 amount) external returns (bool); + + /// @notice Returns the remaining number of tokens that `spender` is allowed + /// to spend on behalf of `owner` + function allowance( + address owner, + address spender + ) external view returns (uint256); + + /// @notice Sets `amount` as the allowance of `spender` over the caller's tokens. + /// @dev Be aware of front-running risks: https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729 + function approve(address spender, uint256 amount) external returns (bool); + + /// @notice Moves `amount` tokens from `from` to `to` using the allowance mechanism. + /// `amount` is then deducted from the caller's allowance. + function transferFrom( + address from, + address to, + uint256 amount + ) external returns (bool); + + /// @notice Returns the name of the token. + function name() external view returns (string memory); + + /// @notice Returns the symbol of the token. + function symbol() external view returns (string memory); + + /// @notice Returns the decimals places of the token. + function decimals() external view returns (uint8); +} diff --git a/mocks/MockBravoProposal.sol b/mocks/MockBravoProposal.sol new file mode 100644 index 00000000..d5aeafb2 --- /dev/null +++ b/mocks/MockBravoProposal.sol @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.0; + +import {GovernorBravoProposal} from "@proposals/GovernorBravoProposal.sol"; + +import {ICompoundConfigurator} from "@interface/ICompoundConfigurator.sol"; + +import {Addresses} from "@addresses/Addresses.sol"; + +contract MockBravoProposal is GovernorBravoProposal { + // @notice new kink value + uint64 public kink = 0.75 * 1e18; + + function name() public pure override returns (string memory) { + return "ADJUST_WETH_IR_CURVE"; + } + + function description() public pure override returns (string memory) { + return + "Mock proposal that adjust IR Curve for Compound v3 WETH on Mainnet"; + } + + function run() public override { + setAddresses( + new Addresses( + vm.envOr("ADDRESSES_PATH", string("./addresses/Addresses.json")) + ) + ); + + vm.makePersistent(address(addresses)); + + setGovernor(addresses.getAddress("COMPOUND_GOVERNOR_BRAVO")); + + super.run(); + } + + function build() + public + override + buildModifier(addresses.getAddress("COMPOUND_TIMELOCK_BRAVO")) + { + /// STATICCALL -- not recorded for the run stage + ICompoundConfigurator configurator = ICompoundConfigurator( + addresses.getAddress("COMPOUND_CONFIGURATOR") + ); + address comet = addresses.getAddress("COMPOUND_COMET"); + + /// CALLS -- mutative and recorded + configurator.setBorrowKink(comet, kink); + configurator.setSupplyKink(comet, kink); + } + + function validate() public view override { + ICompoundConfigurator configurator = ICompoundConfigurator( + addresses.getAddress("COMPOUND_CONFIGURATOR") + ); + address comet = addresses.getAddress("COMPOUND_COMET"); + + ICompoundConfigurator.Configuration memory config = configurator + .getConfiguration(comet); + assertEq(config.supplyKink, kink); + assertEq(config.borrowKink, kink); + } +} diff --git a/mocks/MockDuplicatedActionProposal.sol b/mocks/MockDuplicatedActionProposal.sol new file mode 100644 index 00000000..fa40f8c1 --- /dev/null +++ b/mocks/MockDuplicatedActionProposal.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.0; + +import {GovernorBravoProposal} from "@proposals/GovernorBravoProposal.sol"; + +import {ICompoundConfigurator} from "@interface/ICompoundConfigurator.sol"; + +import {Addresses} from "@addresses/Addresses.sol"; + +contract MockDuplicatedActionProposal is GovernorBravoProposal { + // @notice new kink value + uint64 public kink = 0.75 * 1e18; + + function name() public pure override returns (string memory) { + return "ADJUST_WETH_IR_CURVE"; + } + + function description() public pure override returns (string memory) { + return + "Mock proposal that adjust IR Curve for Compound v3 WETH on Mainnet"; + } + + function run() public override { + addresses = new Addresses( + vm.envOr("ADDRESSES_PATH", string("./addresses/Addresses.json")) + ); + vm.makePersistent(address(addresses)); + + setGovernor(addresses.getAddress("COMPOUND_GOVERNOR_BRAVO")); + + super.run(); + } + + function build() + public + override + buildModifier(addresses.getAddress("COMPOUND_TIMELOCK_BRAVO")) + { + /// STATICCALL -- not recorded for the run stage + + ICompoundConfigurator configurator = ICompoundConfigurator( + addresses.getAddress("COMPOUND_CONFIGURATOR") + ); + address comet = addresses.getAddress("COMPOUND_COMET"); + + /// CALLS -- mutative and recorded + configurator.setSupplyKink(comet, kink); + configurator.setSupplyKink(comet, kink); + } +} diff --git a/mocks/MockERC20.sol b/mocks/MockERC20.sol new file mode 100644 index 00000000..1e0914d0 --- /dev/null +++ b/mocks/MockERC20.sol @@ -0,0 +1,145 @@ +pragma solidity ^0.8.0; + +import {IERC20} from "./IERC20.sol"; + +/// @notice This is a mock contract of the ERC20 standard for testing purposes only, it SHOULD NOT be used in production. +/// @dev Forked from: https://github.com/transmissions11/solmate/blob/0384dbaaa4fcb5715738a9254a7c0a4cb62cf458/src/tokens/ERC20.sol +contract MockERC20 is IERC20 { + /*////////////////////////////////////////////////////////////// + METADATA STORAGE + //////////////////////////////////////////////////////////////*/ + + string internal _name; + + string internal _symbol; + + uint8 internal _decimals; + + function name() external view override returns (string memory) { + return _name; + } + + function symbol() external view override returns (string memory) { + return _symbol; + } + + function decimals() external view override returns (uint8) { + return _decimals; + } + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + uint256 internal _totalSupply; + + mapping(address owner => uint256 balance) internal _balanceOf; + + mapping(address owner => mapping(address spender => uint256 balance)) + internal _allowance; + + function totalSupply() external view override returns (uint256) { + return _totalSupply; + } + + function balanceOf(address owner) external view override returns (uint256) { + return _balanceOf[owner]; + } + + function allowance( + address owner, + address spender + ) external view override returns (uint256) { + return _allowance[owner][spender]; + } + + /*////////////////////////////////////////////////////////////// + INITIALIZE + //////////////////////////////////////////////////////////////*/ + + /// @dev A bool to track whether the contract has been initialized. + bool private initialized; + + /// @dev To hide constructor warnings across solc versions due to different constructor visibility requirements and + /// syntaxes, we add an initialization function that can be called only once. + function initialize( + string memory name_, + string memory symbol_, + uint8 decimals_ + ) public { + require(!initialized, "ALREADY_INITIALIZED"); + + _name = name_; + _symbol = symbol_; + _decimals = decimals_; + + initialized = true; + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function approve( + address spender, + uint256 amount + ) public virtual override returns (bool) { + _allowance[msg.sender][spender] = amount; + + emit Approval(msg.sender, spender, amount); + + return true; + } + + function transfer( + address to, + uint256 amount + ) public virtual override returns (bool) { + _balanceOf[msg.sender] = _sub(_balanceOf[msg.sender], amount); + _balanceOf[to] = _add(_balanceOf[to], amount); + + emit Transfer(msg.sender, to, amount); + + return true; + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public virtual override returns (bool) { + uint256 allowed = _allowance[from][msg.sender]; // Saves gas for limited approvals. + + if (allowed != ~uint256(0)) + _allowance[from][msg.sender] = _sub(allowed, amount); + + _balanceOf[from] = _sub(_balanceOf[from], amount); + _balanceOf[to] = _add(_balanceOf[to], amount); + + emit Transfer(from, to, amount); + + return true; + } + + function _mint(address to, uint256 amount) internal { + _totalSupply = _add(_totalSupply, amount); + _balanceOf[to] = _add(_balanceOf[to], amount); + + emit Transfer(address(0), to, amount); + } + + /*////////////////////////////////////////////////////////////// + INTERNAL SAFE MATH LOGIC + //////////////////////////////////////////////////////////////*/ + + function _add(uint256 a, uint256 b) internal pure returns (uint256) { + uint256 c = a + b; + require(c >= a, "ERC20: addition overflow"); + return c; + } + + function _sub(uint256 a, uint256 b) internal pure returns (uint256) { + require(a >= b, "ERC20: subtraction underflow"); + return a - b; + } +} diff --git a/mocks/MockMultisigProposal.sol b/mocks/MockMultisigProposal.sol new file mode 100644 index 00000000..a1d6b3ed --- /dev/null +++ b/mocks/MockMultisigProposal.sol @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.0; + +import {Addresses} from "@addresses/Addresses.sol"; + +import {MultisigProposal} from "@proposals/MultisigProposal.sol"; + +import {Vault} from "@mocks/Vault.sol"; + +interface ProxyAdmin { + function upgrade(address proxy, address implementation) external; +} + +interface Proxy { + function implementation() external view returns (address); +} + +contract MockMultisigProposal is MultisigProposal { + function name() public pure override returns (string memory) { + return "OPTMISM_MULTISIG_MOCK"; + } + + function description() public pure override returns (string memory) { + return "Mock proposal that upgrade the L1 NFT Bridge"; + } + + function run() public override { + addresses = new Addresses( + vm.envOr("ADDRESSES_PATH", string("./addresses/Addresses.json")) + ); + vm.makePersistent(address(addresses)); + + super.run(); + } + + function deploy() public override { + if (!addresses.isAddressSet("OPTIMISM_L1_NFT_BRIDGE_IMPLEMENTATION")) { + address l1NFTBridgeImplementation = address(new Vault()); + + addresses.addAddress( + "OPTIMISM_L1_NFT_BRIDGE_IMPLEMENTATION", + l1NFTBridgeImplementation, + true + ); + } + } + + function build() + public + override + buildModifier(addresses.getAddress("OPTIMISM_MULTISIG")) + { + ProxyAdmin proxy = ProxyAdmin( + addresses.getAddress("OPTIMISM_PROXY_ADMIN") + ); + + proxy.upgrade( + addresses.getAddress("OPTIMISM_L1_NFT_BRIDGE_PROXY"), + addresses.getAddress("OPTIMISM_L1_NFT_BRIDGE_IMPLEMENTATION") + ); + } + + function simulate() public override { + address multisig = addresses.getAddress("OPTIMISM_MULTISIG"); + + _simulateActions(multisig); + } + + function validate() public override { + Proxy proxy = Proxy( + addresses.getAddress("OPTIMISM_L1_NFT_BRIDGE_PROXY") + ); + + vm.startPrank(addresses.getAddress("OPTIMISM_PROXY_ADMIN")); + require( + proxy.implementation() == + addresses.getAddress("OPTIMISM_L1_NFT_BRIDGE_IMPLEMENTATION"), + "Proxy implementation not set" + ); + vm.stopPrank(); + } +} diff --git a/mocks/MockTimelockProposal.sol b/mocks/MockTimelockProposal.sol new file mode 100644 index 00000000..4d6451f1 --- /dev/null +++ b/mocks/MockTimelockProposal.sol @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.0; + +import {Addresses} from "@addresses/Addresses.sol"; + +import {TimelockProposal} from "@proposals/TimelockProposal.sol"; + +import {ITimelockController} from "@interface/ITimelockController.sol"; + +import {Vault} from "@mocks/Vault.sol"; +import {Token} from "@mocks/Token.sol"; + +// forge script mocks/MockTimelockProposal.sol --rpc-url sepolia --account +// ${CAST_WALLET} -vvvv --broadcast --slow +contract MockTimelockProposal is TimelockProposal { + function name() public pure override returns (string memory) { + return "TIMELOCK_MOCK"; + } + + function description() public pure override returns (string memory) { + return "Timelock proposal mock"; + } + + function run() public override { + addresses = new Addresses( + vm.envOr("ADDRESSES_PATH", string("./addresses/Addresses.json")) + ); + vm.makePersistent(address(addresses)); + + timelock = ITimelockController( + addresses.getAddress("PROTOCOL_TIMELOCK") + ); + + super.run(); + } + + function deploy() public override { + if (!addresses.isAddressSet("TIMELOCK_VAULT")) { + Vault timelockVault = new Vault(); + + addresses.addAddress( + "TIMELOCK_VAULT", + address(timelockVault), + true + ); + } + + if (!addresses.isAddressSet("TIMELOCK_TOKEN")) { + Token token = new Token(); + addresses.addAddress("TIMELOCK_TOKEN", address(token), true); + + // During forge script execution, the deployer of the contracts is + // the DEPLOYER_EOA. However, when running through forge test, the deployer of the contracts is this contract. + uint256 balance = token.balanceOf(address(this)) > 0 + ? token.balanceOf(address(this)) + : token.balanceOf(addresses.getAddress("DEPLOYER_EOA")); + + token.transfer(address(timelock), balance); + } + } + + function build() public override buildModifier(address(timelock)) { + /// STATICCALL -- not recorded for the run stage + address timelockVault = addresses.getAddress("TIMELOCK_VAULT"); + address token = addresses.getAddress("TIMELOCK_TOKEN"); + uint256 balance = Token(token).balanceOf(address(timelock)); + + Vault(timelockVault).whitelistToken(token, true); + + /// CALLS -- mutative and recorded + Token(token).approve(timelockVault, balance); + Vault(timelockVault).deposit(token, balance); + } + + function simulate() public override { + address dev = addresses.getAddress("DEPLOYER_EOA"); + + /// Dev is proposer and executor + _simulateActions(dev, dev); + } + + function validate() public view override { + Vault timelockVault = Vault(addresses.getAddress("TIMELOCK_VAULT")); + Token token = Token(addresses.getAddress("TIMELOCK_TOKEN")); + + uint256 balance = token.balanceOf(address(timelockVault)); + (uint256 amount, ) = timelockVault.deposits( + address(token), + address(timelock) + ); + assertEq(amount, balance); + + assertTrue(timelockVault.tokenWhitelist(address(token))); + + assertEq(token.balanceOf(address(timelockVault)), token.totalSupply()); + } +} diff --git a/mocks/Token.sol b/mocks/Token.sol new file mode 100644 index 00000000..424a48aa --- /dev/null +++ b/mocks/Token.sol @@ -0,0 +1,15 @@ +pragma solidity ^0.8.0; + +import {MockERC20} from "./MockERC20.sol"; + +contract Token is MockERC20 { + constructor() { + initialize("MockToken", "MTK", 18); + uint256 supply = 10_000_000e18; + _mint(msg.sender, supply); + } + + function mint(address to, uint256 amount) external { + _mint(to, amount); + } +} diff --git a/mocks/Vault.sol b/mocks/Vault.sol new file mode 100644 index 00000000..971a8d14 --- /dev/null +++ b/mocks/Vault.sol @@ -0,0 +1,56 @@ +pragma solidity ^0.8.0; + +import {IERC20} from "./IERC20.sol"; + +contract Vault { + uint256 public LOCK_PERIOD = 1 weeks; + + struct Deposit { + uint256 amount; + uint256 timestamp; + } + + mapping(address _token => mapping(address _user => Deposit _deposit)) + public deposits; + mapping(address _token => bool _isWhitelisted) public tokenWhitelist; + + function whitelistToken(address token, bool active) external { + tokenWhitelist[token] = active; + } + + function deposit(address token, uint256 amount) external { + require(tokenWhitelist[token], "Vault: token must be active"); + require(amount > 0, "Vault: amount must be greater than 0"); + require(token != address(0), "Vault: token must not be 0x0"); + + Deposit storage userDeposit = deposits[token][msg.sender]; + userDeposit.amount += amount; + userDeposit.timestamp = block.timestamp; + + IERC20(token).transferFrom(msg.sender, address(this), amount); + } + + function withdraw( + address token, + address payable to, + uint256 amount + ) external { + require(tokenWhitelist[token], "Vault: token must be active"); + require(amount > 0, "Vault: amount must be greater than 0"); + require(token != address(0), "Vault: token must not be 0x0"); + require( + deposits[token][msg.sender].amount >= amount, + "Vault: insufficient balance" + ); + require( + deposits[token][msg.sender].timestamp + LOCK_PERIOD < + block.timestamp, + "Vault: lock period has not passed" + ); + + Deposit storage userDeposit = deposits[token][msg.sender]; + userDeposit.amount -= amount; + + IERC20(token).transfer(to, amount); + } +} diff --git a/mocks/bravo/GovernorBravoDelegate.sol b/mocks/bravo/GovernorBravoDelegate.sol deleted file mode 100644 index 53af5d16..00000000 --- a/mocks/bravo/GovernorBravoDelegate.sol +++ /dev/null @@ -1,830 +0,0 @@ -// SPDX-License-Identifier: BSD-3-Clause -pragma solidity ^0.8.0; - -import {GovernorBravoDelegateStorageV2, GovernorBravoEvents, TimelockInterface, CompInterface, IGovernorAlpha} from "./IGovernorBravo.sol"; - -/// @custom:security-contact security@compound.finance -contract GovernorBravoDelegate is - GovernorBravoDelegateStorageV2, - GovernorBravoEvents -{ - /// @notice The name of this contract - string public constant NAME = "Compound Governor Bravo"; - - /// @notice The minimum settable proposal threshold - uint public constant MIN_PROPOSAL_THRESHOLD = 1000e18; // 1,000 Comp - - /// @notice The maximum settable proposal threshold - uint public constant MAX_PROPOSAL_THRESHOLD = 100000e18; //100,000 Comp - - /// @notice The minimum settable voting period - uint public constant MIN_VOTING_PERIOD = 60; // 1 minute - - /// @notice The max settable voting period - uint public constant MAX_VOTING_PERIOD = 80640; // About 2 weeks - - /// @notice The min settable voting delay - uint public constant MIN_VOTING_DELAY = 1; - - /// @notice The max settable voting delay - uint public constant MAX_VOTING_DELAY = 40320; // About 1 week - - /// @notice The number of votes in support of a proposal required in order for a quorum to be reached and for a vote to succeed - uint public constant quorumVotes = 400000e18; // 400,000 = 4% of Comp - - /// @notice The maximum number of actions that can be included in a proposal - uint public constant PROPOSAL_MAX_OPERATIONS = 10; // 10 actions - - /// @notice The EIP-712 typehash for the contract's domain - bytes32 public constant DOMAIN_TYPEHASH = - keccak256( - "EIP712Domain(string name,uint256 chainId,address verifyingContract)" - ); - - /// @notice The EIP-712 typehash for the ballot struct used by the contract - bytes32 public constant BALLOT_TYPEHASH = - keccak256("Ballot(uint256 proposalId,uint8 support)"); - - /// @notice The EIP-712 typehash for the ballot with reason struct used by the contract - bytes32 public constant BALLOT_WITH_REASON_TYPEHASH = - keccak256("Ballot(uint256 proposalId,uint8 support,string reason)"); - - /// @notice The EIP-712 typehash for the proposal struct used by the contract - bytes32 public constant PROPOSAL_TYPEHASH = - keccak256( - "Proposal(address[] targets,uint256[] values,string[] signatures,bytes[] calldatas,string description,uint256 proposalId)" - ); - - /** - * @notice Used to initialize the contract during delegator constructor - * @param timelock_ The address of the Timelock - * @param comp_ The address of the COMP token - * @param votingPeriod_ The initial voting period - * @param votingDelay_ The initial voting delay - * @param proposalThreshold_ The initial proposal threshold - */ - function initialize( - address timelock_, - address comp_, - uint votingPeriod_, - uint votingDelay_, - uint proposalThreshold_ - ) public virtual { - require( - address(timelock) == address(0), - "GovernorBravo::initialize: can only initialize once" - ); - require(msg.sender == admin, "GovernorBravo::initialize: admin only"); - require( - timelock_ != address(0), - "GovernorBravo::initialize: invalid timelock address" - ); - require( - comp_ != address(0), - "GovernorBravo::initialize: invalid comp address" - ); - require( - votingPeriod_ >= MIN_VOTING_PERIOD && - votingPeriod_ <= MAX_VOTING_PERIOD, - "GovernorBravo::initialize: invalid voting period" - ); - require( - votingDelay_ >= MIN_VOTING_DELAY && - votingDelay_ <= MAX_VOTING_DELAY, - "GovernorBravo::initialize: invalid voting delay" - ); - require( - proposalThreshold_ >= MIN_PROPOSAL_THRESHOLD && - proposalThreshold_ <= MAX_PROPOSAL_THRESHOLD, - "GovernorBravo::initialize: invalid proposal threshold" - ); - - timelock = TimelockInterface(timelock_); - comp = CompInterface(comp_); - votingPeriod = votingPeriod_; - votingDelay = votingDelay_; - proposalThreshold = proposalThreshold_; - } - - /** - * @notice Function used to propose a new proposal. Sender must have delegates above the proposal threshold - * @param targets Target addresses for proposal calls - * @param values Eth values for proposal calls - * @param signatures Function signatures for proposal calls - * @param calldatas Calldatas for proposal calls - * @param description String description of the proposal - * @return Proposal id of new proposal - */ - function propose( - address[] memory targets, - uint[] memory values, - string[] memory signatures, - bytes[] memory calldatas, - string memory description - ) public returns (uint) { - return - proposeInternal( - msg.sender, - targets, - values, - signatures, - calldatas, - description - ); - } - - /** - * @notice Function used to propose a new proposal. Sender must have delegates above the proposal threshold - * @param targets Target addresses for proposal calls - * @param values Eth values for proposal calls - * @param signatures Function signatures for proposal calls - * @param calldatas Calldatas for proposal calls - * @param description String description of the proposal - * @param proposalId The id of the proposal to propose (reverted if this isn't the next proposal id) - * @param v The recovery byte of the signature - * @param r Half of the ECDSA signature pair - * @param s Half of the ECDSA signature pair - * @return Proposal id of new proposal - */ - function proposeBySig( - address[] memory targets, - uint[] memory values, - string[] memory signatures, - bytes[] memory calldatas, - string memory description, - uint proposalId, - uint8 v, - bytes32 r, - bytes32 s - ) public returns (uint) { - require( - proposalId == proposalCount + 1, - "GovernorBravo::proposeBySig: invalid proposal id" - ); - address signatory; - { - bytes32 domainSeparator = keccak256( - abi.encode( - DOMAIN_TYPEHASH, - keccak256(bytes(NAME)), - getChainIdInternal(), - address(this) - ) - ); - - bytes32[] memory hashedCalldatas = new bytes32[](calldatas.length); - bytes32[] memory hashedSignatures = new bytes32[]( - signatures.length - ); - for (uint256 i = 0; i < calldatas.length; ++i) { - hashedCalldatas[i] = keccak256(calldatas[i]); - } - for (uint256 i = 0; i < signatures.length; ++i) { - hashedSignatures[i] = keccak256(bytes(signatures[i])); - } - - bytes32 structHash = keccak256( - abi.encode( - PROPOSAL_TYPEHASH, - keccak256(abi.encodePacked(targets)), - keccak256(abi.encodePacked(values)), - keccak256(abi.encodePacked(hashedSignatures)), - keccak256(abi.encodePacked(hashedCalldatas)), - keccak256(bytes(description)), - proposalId - ) - ); - bytes32 digest = keccak256( - abi.encodePacked("\x19\x01", domainSeparator, structHash) - ); - signatory = ecrecover(digest, v, r, s); - } - require( - signatory != address(0), - "GovernorBravo::proposeBySig: invalid signature" - ); - - return - proposeInternal( - signatory, - targets, - values, - signatures, - calldatas, - description - ); - } - - function proposeInternal( - address proposer, - address[] memory targets, - uint[] memory values, - string[] memory signatures, - bytes[] memory calldatas, - string memory description - ) internal returns (uint) { - // Reject proposals before initiating as Governor - require( - initialProposalId != 0, - "GovernorBravo::proposeInternal: Governor Bravo not active" - ); - // Allow addresses above proposal threshold and whitelisted addresses to propose - require( - comp.getPriorVotes(proposer, block.number - 1) > - proposalThreshold || - isWhitelisted(proposer), - "GovernorBravo::proposeInternal: proposer votes below proposal threshold" - ); - require( - targets.length == values.length && - targets.length == signatures.length && - targets.length == calldatas.length, - "GovernorBravo::proposeInternal: proposal function information arity mismatch" - ); - require( - targets.length != 0, - "GovernorBravo::proposeInternal: must provide actions" - ); - require( - targets.length <= PROPOSAL_MAX_OPERATIONS, - "GovernorBravo::proposeInternal: too many actions" - ); - - uint256 latestProposalId = latestProposalIds[proposer]; - if (latestProposalId != 0) { - ProposalState proposersLatestProposalState = state( - latestProposalId - ); - require( - proposersLatestProposalState != ProposalState.Active, - "GovernorBravo::proposeInternal: one live proposal per proposer, found an already active proposal" - ); - require( - proposersLatestProposalState != ProposalState.Pending, - "GovernorBravo::proposeInternal: one live proposal per proposer, found an already pending proposal" - ); - } - - uint startBlock = block.number + votingDelay; - uint endBlock = startBlock + votingPeriod; - - proposalCount++; - uint newProposalID = proposalCount; - Proposal storage newProposal = proposals[newProposalID]; - // This should never happen but add a check in case. - require( - newProposal.id == 0, - "GovernorBravo::proposeInternal: ProposalID collision" - ); - newProposal.id = newProposalID; - newProposal.proposer = proposer; - newProposal.eta = 0; - newProposal.targets = targets; - newProposal.values = values; - newProposal.signatures = signatures; - newProposal.calldatas = calldatas; - newProposal.startBlock = startBlock; - newProposal.endBlock = endBlock; - newProposal.forVotes = 0; - newProposal.againstVotes = 0; - newProposal.abstainVotes = 0; - newProposal.canceled = false; - newProposal.executed = false; - - latestProposalIds[newProposal.proposer] = newProposal.id; - - emit ProposalCreated( - newProposal.id, - proposer, - targets, - values, - signatures, - calldatas, - startBlock, - endBlock, - description - ); - return newProposal.id; - } - - /** - * @notice Queues a proposal of state succeeded - * @param proposalId The id of the proposal to queue - */ - function queue(uint proposalId) external { - require( - state(proposalId) == ProposalState.Succeeded, - "GovernorBravo::queue: proposal can only be queued if it is succeeded" - ); - Proposal storage proposal = proposals[proposalId]; - uint eta = block.timestamp + timelock.delay(); - for (uint i = 0; i < proposal.targets.length; i++) { - queueOrRevertInternal( - proposal.targets[i], - proposal.values[i], - proposal.signatures[i], - proposal.calldatas[i], - eta - ); - } - proposal.eta = eta; - emit ProposalQueued(proposalId, eta); - } - - function queueOrRevertInternal( - address target, - uint value, - string memory signature, - bytes memory data, - uint eta - ) internal { - require( - !timelock.queuedTransactions( - keccak256(abi.encode(target, value, signature, data, eta)) - ), - "GovernorBravo::queueOrRevertInternal: identical proposal action already queued at eta" - ); - timelock.queueTransaction(target, value, signature, data, eta); - } - - /** - * @notice Executes a queued proposal if eta has passed - * @param proposalId The id of the proposal to execute - */ - function execute(uint proposalId) external payable { - require( - state(proposalId) == ProposalState.Queued, - "GovernorBravo::execute: proposal can only be executed if it is queued" - ); - Proposal storage proposal = proposals[proposalId]; - proposal.executed = true; - for (uint i = 0; i < proposal.targets.length; i++) { - timelock.executeTransaction( - proposal.targets[i], - proposal.values[i], - proposal.signatures[i], - proposal.calldatas[i], - proposal.eta - ); - } - emit ProposalExecuted(proposalId); - } - - /** - * @notice Cancels a proposal only if sender is the proposer, or proposer delegates dropped below proposal threshold - * @param proposalId The id of the proposal to cancel - */ - function cancel(uint proposalId) external { - require( - state(proposalId) != ProposalState.Executed, - "GovernorBravo::cancel: cannot cancel executed proposal" - ); - - Proposal storage proposal = proposals[proposalId]; - - // Proposer can cancel - if (msg.sender != proposal.proposer) { - // Whitelisted proposers can't be canceled for falling below proposal threshold - if (isWhitelisted(proposal.proposer)) { - require( - (comp.getPriorVotes(proposal.proposer, block.number - 1) < - proposalThreshold) && msg.sender == whitelistGuardian, - "GovernorBravo::cancel: whitelisted proposer" - ); - } else { - require( - (comp.getPriorVotes(proposal.proposer, block.number - 1) < - proposalThreshold), - "GovernorBravo::cancel: proposer above threshold" - ); - } - } - - proposal.canceled = true; - for (uint i = 0; i < proposal.targets.length; i++) { - timelock.cancelTransaction( - proposal.targets[i], - proposal.values[i], - proposal.signatures[i], - proposal.calldatas[i], - proposal.eta - ); - } - - emit ProposalCanceled(proposalId); - } - - /** - * @notice Gets actions of a proposal - * @param proposalId the id of the proposal - * @return targets of the proposal actions - * @return values of the proposal actions - * @return signatures of the proposal actions - * @return calldatas of the proposal actions - */ - function getActions( - uint proposalId - ) - external - view - returns ( - address[] memory targets, - uint[] memory values, - string[] memory signatures, - bytes[] memory calldatas - ) - { - Proposal storage p = proposals[proposalId]; - return (p.targets, p.values, p.signatures, p.calldatas); - } - - /** - * @notice Gets the receipt for a voter on a given proposal - * @param proposalId the id of proposal - * @param voter The address of the voter - * @return The voting receipt - */ - function getReceipt( - uint proposalId, - address voter - ) external view returns (Receipt memory) { - return proposals[proposalId].receipts[voter]; - } - - /** - * @notice Gets the state of a proposal - * @param proposalId The id of the proposal - * @return Proposal state as a `ProposalState` enum - */ - function state(uint proposalId) public view returns (ProposalState) { - require( - proposalCount >= proposalId && proposalId > initialProposalId, - "GovernorBravo::state: invalid proposal id" - ); - Proposal storage proposal = proposals[proposalId]; - if (proposal.canceled) { - return ProposalState.Canceled; - } else if (block.number <= proposal.startBlock) { - return ProposalState.Pending; - } else if (block.number <= proposal.endBlock) { - return ProposalState.Active; - } else if ( - proposal.forVotes <= proposal.againstVotes || - proposal.forVotes < quorumVotes - ) { - return ProposalState.Defeated; - } else if (proposal.eta == 0) { - return ProposalState.Succeeded; - } else if (proposal.executed) { - return ProposalState.Executed; - } else if (block.timestamp >= proposal.eta + timelock.GRACE_PERIOD()) { - return ProposalState.Expired; - } else { - return ProposalState.Queued; - } - } - - /** - * @notice Cast a vote for a proposal - * @param proposalId The id of the proposal to vote on - * @param support The support value for the vote. 0=against, 1=for, 2=abstain - */ - function castVote(uint proposalId, uint8 support) external { - emit VoteCast( - msg.sender, - proposalId, - support, - castVoteInternal(msg.sender, proposalId, support), - "" - ); - } - - /** - * @notice Cast a vote for a proposal by signature - * @dev External function that accepts EIP-712 signatures for voting on proposals. - */ - function castVoteBySig( - uint proposalId, - uint8 support, - uint8 v, - bytes32 r, - bytes32 s - ) external { - bytes32 domainSeparator = keccak256( - abi.encode( - DOMAIN_TYPEHASH, - keccak256(bytes(NAME)), - getChainIdInternal(), - address(this) - ) - ); - bytes32 structHash = keccak256( - abi.encode(BALLOT_TYPEHASH, proposalId, support) - ); - bytes32 digest = keccak256( - abi.encodePacked("\x19\x01", domainSeparator, structHash) - ); - address signatory = ecrecover(digest, v, r, s); - require( - signatory != address(0), - "GovernorBravo::castVoteBySig: invalid signature" - ); - emit VoteCast( - signatory, - proposalId, - support, - castVoteInternal(signatory, proposalId, support), - "" - ); - } - - /** - * @notice Cast a vote for a proposal with a reason - * @param proposalId The id of the proposal to vote on - * @param support The support value for the vote. 0=against, 1=for, 2=abstain - * @param reason The reason given for the vote by the voter - */ - function castVoteWithReason( - uint proposalId, - uint8 support, - string calldata reason - ) external { - emit VoteCast( - msg.sender, - proposalId, - support, - castVoteInternal(msg.sender, proposalId, support), - reason - ); - } - - /** - * @notice Cast a vote for a proposal with a reason by `ERC712` signature - * @param proposalId The id of the proposal to vote on - * @param support The support value for the vote. 0=against, 1=for, 2=abstain - * @param reason The reason given for the vote by the voter - * @param v The recovery byte of the signature - * @param r Half of the ECDSA signature pair - * @param s Half of the ECDSA signature pair - */ - function castVoteWithReasonBySig( - uint proposalId, - uint8 support, - string calldata reason, - uint8 v, - bytes32 r, - bytes32 s - ) external { - address signatory; - { - bytes32 domainSeparator = keccak256( - abi.encode( - DOMAIN_TYPEHASH, - keccak256(bytes(NAME)), - getChainIdInternal(), - address(this) - ) - ); - bytes32 structHash = keccak256( - abi.encode( - BALLOT_WITH_REASON_TYPEHASH, - proposalId, - support, - keccak256(bytes(reason)) - ) - ); - bytes32 digest = keccak256( - abi.encodePacked("\x19\x01", domainSeparator, structHash) - ); - signatory = ecrecover(digest, v, r, s); - } - require( - signatory != address(0), - "GovernorBravo::castVoteWithReasonBySig: invalid signature" - ); - emit VoteCast( - signatory, - proposalId, - support, - castVoteInternal(signatory, proposalId, support), - reason - ); - } - - /** - * @notice Internal function that caries out voting logic - * @param voter The voter that is casting their vote - * @param proposalId The id of the proposal to vote on - * @param support The support value for the vote. 0=against, 1=for, 2=abstain - * @return The number of votes cast - */ - function castVoteInternal( - address voter, - uint proposalId, - uint8 support - ) internal returns (uint96) { - require( - state(proposalId) == ProposalState.Active, - "GovernorBravo::castVoteInternal: voting is closed" - ); - require( - support <= 2, - "GovernorBravo::castVoteInternal: invalid vote type" - ); - Proposal storage proposal = proposals[proposalId]; - Receipt storage receipt = proposal.receipts[voter]; - require( - receipt.hasVoted == false, - "GovernorBravo::castVoteInternal: voter already voted" - ); - uint96 votes = comp.getPriorVotes(voter, proposal.startBlock); - - if (support == 0) { - proposal.againstVotes = proposal.againstVotes + votes; - } else if (support == 1) { - proposal.forVotes = proposal.forVotes + votes; - } else if (support == 2) { - proposal.abstainVotes = proposal.abstainVotes + votes; - } - - receipt.hasVoted = true; - receipt.support = support; - receipt.votes = votes; - - return votes; - } - - /** - * @notice View function which returns if an account is whitelisted - * @param account Account to check white list status of - * @return If the account is whitelisted - */ - function isWhitelisted(address account) public view returns (bool) { - return (whitelistAccountExpirations[account] > block.timestamp); - } - - /** - * @notice Admin function for setting the voting delay - * @param newVotingDelay new voting delay, in blocks - */ - function _setVotingDelay(uint newVotingDelay) external { - require( - msg.sender == admin, - "GovernorBravo::_setVotingDelay: admin only" - ); - require( - newVotingDelay >= MIN_VOTING_DELAY && - newVotingDelay <= MAX_VOTING_DELAY, - "GovernorBravo::_setVotingDelay: invalid voting delay" - ); - uint oldVotingDelay = votingDelay; - votingDelay = newVotingDelay; - - emit VotingDelaySet(oldVotingDelay, votingDelay); - } - - /** - * @notice Admin function for setting the voting period - * @param newVotingPeriod new voting period, in blocks - */ - function _setVotingPeriod(uint newVotingPeriod) external { - require( - msg.sender == admin, - "GovernorBravo::_setVotingPeriod: admin only" - ); - require( - newVotingPeriod >= MIN_VOTING_PERIOD && - newVotingPeriod <= MAX_VOTING_PERIOD, - "GovernorBravo::_setVotingPeriod: invalid voting period" - ); - uint oldVotingPeriod = votingPeriod; - votingPeriod = newVotingPeriod; - - emit VotingPeriodSet(oldVotingPeriod, votingPeriod); - } - - /** - * @notice Admin function for setting the proposal threshold - * @dev newProposalThreshold must be greater than the hardcoded min - * @param newProposalThreshold new proposal threshold - */ - function _setProposalThreshold(uint newProposalThreshold) external { - require( - msg.sender == admin, - "GovernorBravo::_setProposalThreshold: admin only" - ); - require( - newProposalThreshold >= MIN_PROPOSAL_THRESHOLD && - newProposalThreshold <= MAX_PROPOSAL_THRESHOLD, - "GovernorBravo::_setProposalThreshold: invalid proposal threshold" - ); - uint oldProposalThreshold = proposalThreshold; - proposalThreshold = newProposalThreshold; - - emit ProposalThresholdSet(oldProposalThreshold, proposalThreshold); - } - - /** - * @notice Admin function for setting the whitelist expiration as a timestamp for an account. Whitelist status allows accounts to propose without meeting threshold - * @param account Account address to set whitelist expiration for - * @param expiration Expiration for account whitelist status as timestamp (if now < expiration, whitelisted) - */ - function _setWhitelistAccountExpiration( - address account, - uint expiration - ) external { - require( - msg.sender == admin || msg.sender == whitelistGuardian, - "GovernorBravo::_setWhitelistAccountExpiration: admin only" - ); - whitelistAccountExpirations[account] = expiration; - - emit WhitelistAccountExpirationSet(account, expiration); - } - - /** - * @notice Admin function for setting the whitelistGuardian. WhitelistGuardian can cancel proposals from whitelisted addresses - * @param account Account to set whitelistGuardian to (0x0 to remove whitelistGuardian) - */ - function _setWhitelistGuardian(address account) external { - require( - msg.sender == admin, - "GovernorBravo::_setWhitelistGuardian: admin only" - ); - address oldGuardian = whitelistGuardian; - whitelistGuardian = account; - - emit WhitelistGuardianSet(oldGuardian, whitelistGuardian); - } - - /** - * @notice Initiate the GovernorBravo contract - * @dev Admin only. Sets initial proposal id which initiates the contract, ensuring a continuous proposal id count - * @param governorAlpha The address for the Governor to continue the proposal id count from - */ - function _initiate(address governorAlpha) external { - require(msg.sender == admin, "GovernorBravo::_initiate: admin only"); - require( - initialProposalId == 0, - "GovernorBravo::_initiate: can only initiate once" - ); - proposalCount = IGovernorAlpha(governorAlpha).proposalCount(); - initialProposalId = proposalCount; - timelock.acceptAdmin(); - } - - /** - * @notice Begins transfer of admin rights. The newPendingAdmin must call `_acceptAdmin` to finalize the transfer. - * @dev Admin function to begin change of admin. The newPendingAdmin must call `_acceptAdmin` to finalize the transfer. - * @param newPendingAdmin New pending admin. - */ - function _setPendingAdmin(address newPendingAdmin) external { - // Check caller = admin - require( - msg.sender == admin, - "GovernorBravo:_setPendingAdmin: admin only" - ); - - // Save current value, if any, for inclusion in log - address oldPendingAdmin = pendingAdmin; - - // Store pendingAdmin with value newPendingAdmin - pendingAdmin = newPendingAdmin; - - // Emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin) - emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin); - } - - /** - * @notice Accepts transfer of admin rights. msg.sender must be pendingAdmin - * @dev Admin function for pending admin to accept role and update admin - */ - function _acceptAdmin() external { - // Check caller is pendingAdmin and pendingAdmin ≠ address(0) - require( - msg.sender == pendingAdmin && msg.sender != address(0), - "GovernorBravo:_acceptAdmin: pending admin only" - ); - - // Save current values for inclusion in log - address oldAdmin = admin; - address oldPendingAdmin = pendingAdmin; - - // Store admin with value pendingAdmin - admin = pendingAdmin; - - // Clear the pending value - pendingAdmin = address(0); - - emit NewAdmin(oldAdmin, admin); - emit NewPendingAdmin(oldPendingAdmin, pendingAdmin); - } - - function getChainIdInternal() internal view returns (uint) { - uint chainId; - assembly { - chainId := chainid() - } - return chainId; - } -} diff --git a/mocks/bravo/SafeMath.sol b/mocks/bravo/SafeMath.sol deleted file mode 100644 index 4525c5de..00000000 --- a/mocks/bravo/SafeMath.sol +++ /dev/null @@ -1,219 +0,0 @@ -// SPDX-License-Identifier: BSD-3-Clause -pragma solidity ^0.8.0; - -// From https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/math/Math.sol -// Subject to the MIT license. - -/** - * @dev Wrappers over Solidity's arithmetic operations with added overflow - * checks. - * - * Arithmetic operations in Solidity wrap on overflow. This can easily result - * in bugs, because programmers usually assume that an overflow raises an - * error, which is the standard behavior in high level programming languages. - * `SafeMath` restores this intuition by reverting the transaction when an - * operation overflows. - * - * Using this library instead of the unchecked operations eliminates an entire - * class of bugs, so it's recommended to use it always. - */ -library SafeMath { - /** - * @dev Returns the addition of two unsigned integers, reverting on overflow. - * - * Counterpart to Solidity's `+` operator. - * - * Requirements: - * - Addition cannot overflow. - */ - function add(uint256 a, uint256 b) internal pure returns (uint256) { - uint256 c; - unchecked { - c = a + b; - } - require(c >= a, "SafeMath: addition overflow"); - - return c; - } - - /** - * @dev Returns the addition of two unsigned integers, reverting with custom message on overflow. - * - * Counterpart to Solidity's `+` operator. - * - * Requirements: - * - Addition cannot overflow. - */ - function add( - uint256 a, - uint256 b, - string memory errorMessage - ) internal pure returns (uint256) { - uint256 c; - unchecked { - c = a + b; - } - require(c >= a, errorMessage); - - return c; - } - - /** - * @dev Returns the subtraction of two unsigned integers, reverting on underflow (when the result is negative). - * - * Counterpart to Solidity's `-` operator. - * - * Requirements: - * - Subtraction cannot underflow. - */ - function sub(uint256 a, uint256 b) internal pure returns (uint256) { - return sub(a, b, "SafeMath: subtraction underflow"); - } - - /** - * @dev Returns the subtraction of two unsigned integers, reverting with custom message on underflow (when the result is negative). - * - * Counterpart to Solidity's `-` operator. - * - * Requirements: - * - Subtraction cannot underflow. - */ - function sub( - uint256 a, - uint256 b, - string memory errorMessage - ) internal pure returns (uint256) { - require(b <= a, errorMessage); - uint256 c = a - b; - - return c; - } - - /** - * @dev Returns the multiplication of two unsigned integers, reverting on overflow. - * - * Counterpart to Solidity's `*` operator. - * - * Requirements: - * - Multiplication cannot overflow. - */ - function mul(uint256 a, uint256 b) internal pure returns (uint256) { - // Gas optimization: this is cheaper than requiring 'a' not being zero, but the - // benefit is lost if 'b' is also tested. - // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522 - if (a == 0) { - return 0; - } - - uint256 c; - unchecked { - c = a * b; - } - require(c / a == b, "SafeMath: multiplication overflow"); - - return c; - } - - /** - * @dev Returns the multiplication of two unsigned integers, reverting on overflow. - * - * Counterpart to Solidity's `*` operator. - * - * Requirements: - * - Multiplication cannot overflow. - */ - function mul( - uint256 a, - uint256 b, - string memory errorMessage - ) internal pure returns (uint256) { - // Gas optimization: this is cheaper than requiring 'a' not being zero, but the - // benefit is lost if 'b' is also tested. - // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522 - if (a == 0) { - return 0; - } - - uint256 c; - unchecked { - c = a * b; - } - require(c / a == b, errorMessage); - - return c; - } - - /** - * @dev Returns the integer division of two unsigned integers. - * Reverts on division by zero. The result is rounded towards zero. - * - * Counterpart to Solidity's `/` operator. Note: this function uses a - * `revert` opcode (which leaves remaining gas untouched) while Solidity - * uses an invalid opcode to revert (consuming all remaining gas). - * - * Requirements: - * - The divisor cannot be zero. - */ - function div(uint256 a, uint256 b) internal pure returns (uint256) { - return div(a, b, "SafeMath: division by zero"); - } - - /** - * @dev Returns the integer division of two unsigned integers. - * Reverts with custom message on division by zero. The result is rounded towards zero. - * - * Counterpart to Solidity's `/` operator. Note: this function uses a - * `revert` opcode (which leaves remaining gas untouched) while Solidity - * uses an invalid opcode to revert (consuming all remaining gas). - * - * Requirements: - * - The divisor cannot be zero. - */ - function div( - uint256 a, - uint256 b, - string memory errorMessage - ) internal pure returns (uint256) { - // Solidity only automatically asserts when dividing by 0 - require(b > 0, errorMessage); - uint256 c = a / b; - // assert(a == b * c + a % b); // There is no case in which this doesn't hold - - return c; - } - - /** - * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo), - * Reverts when dividing by zero. - * - * Counterpart to Solidity's `%` operator. This function uses a `revert` - * opcode (which leaves remaining gas untouched) while Solidity uses an - * invalid opcode to revert (consuming all remaining gas). - * - * Requirements: - * - The divisor cannot be zero. - */ - function mod(uint256 a, uint256 b) internal pure returns (uint256) { - return mod(a, b, "SafeMath: modulo by zero"); - } - - /** - * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo), - * Reverts with custom message when dividing by zero. - * - * Counterpart to Solidity's `%` operator. This function uses a `revert` - * opcode (which leaves remaining gas untouched) while Solidity uses an - * invalid opcode to revert (consuming all remaining gas). - * - * Requirements: - * - The divisor cannot be zero. - */ - function mod( - uint256 a, - uint256 b, - string memory errorMessage - ) internal pure returns (uint256) { - require(b != 0, errorMessage); - return a % b; - } -} diff --git a/mocks/bravo/Timelock.sol b/mocks/bravo/Timelock.sol deleted file mode 100644 index 90cbdaf7..00000000 --- a/mocks/bravo/Timelock.sol +++ /dev/null @@ -1,208 +0,0 @@ -// SPDX-License-Identifier: BSD-3-Clause -pragma solidity ^0.8.0; - -import "./SafeMath.sol"; - -contract Timelock { - using SafeMath for uint; - - event NewAdmin(address indexed newAdmin); - event NewPendingAdmin(address indexed newPendingAdmin); - event NewDelay(uint256 indexed newDelay); - event CancelTransaction( - bytes32 indexed txHash, - address indexed target, - uint256 value, - string signature, - bytes data, - uint256 eta - ); - event ExecuteTransaction( - bytes32 indexed txHash, - address indexed target, - uint256 value, - string signature, - bytes data, - uint256 eta - ); - event QueueTransaction( - bytes32 indexed txHash, - address indexed target, - uint256 value, - string signature, - bytes data, - uint256 eta - ); - - uint256 public constant GRACE_PERIOD = 14 days; - uint256 public constant MINIMUM_DELAY = 1; - uint256 public constant MAXIMUM_DELAY = 30 days; - - address public admin; - address public pendingAdmin; - uint256 public delay; - - mapping(bytes32 transaction => bool queued) public queuedTransactions; - - constructor(address admin_, uint256 delay_) { - require( - delay_ >= MINIMUM_DELAY, - "Timelock::constructor: Delay must exceed minimum delay." - ); - require( - delay_ <= MAXIMUM_DELAY, - "Timelock::setDelay: Delay must not exceed maximum delay." - ); - - admin = admin_; - delay = delay_; - } - - fallback() external payable {} - - receive() external payable {} - - function setDelay(uint256 delay_) public { - require( - msg.sender == address(this), - "Timelock::setDelay: Call must come from Timelock." - ); - require( - delay_ >= MINIMUM_DELAY, - "Timelock::setDelay: Delay must exceed minimum delay." - ); - require( - delay_ <= MAXIMUM_DELAY, - "Timelock::setDelay: Delay must not exceed maximum delay." - ); - delay = delay_; - - emit NewDelay(delay); - } - - function acceptAdmin() public { - require( - msg.sender == pendingAdmin, - "Timelock::acceptAdmin: Call must come from pendingAdmin." - ); - admin = msg.sender; - pendingAdmin = address(0); - - emit NewAdmin(admin); - } - - function setPendingAdmin(address pendingAdmin_) public { - require( - msg.sender == address(this), - "Timelock::setPendingAdmin: Call must come from Timelock." - ); - pendingAdmin = pendingAdmin_; - - emit NewPendingAdmin(pendingAdmin); - } - - function queueTransaction( - address target, - uint256 value, - string memory signature, - bytes memory data, - uint256 eta - ) public returns (bytes32) { - require( - msg.sender == admin, - "Timelock::queueTransaction: Call must come from admin." - ); - require( - eta >= getBlockTimestamp().add(delay), - "Timelock::queueTransaction: Estimated execution block must satisfy delay." - ); - - bytes32 txHash = keccak256( - abi.encode(target, value, signature, data, eta) - ); - queuedTransactions[txHash] = true; - - emit QueueTransaction(txHash, target, value, signature, data, eta); - return txHash; - } - - function cancelTransaction( - address target, - uint256 value, - string memory signature, - bytes memory data, - uint256 eta - ) public { - require( - msg.sender == admin, - "Timelock::cancelTransaction: Call must come from admin." - ); - - bytes32 txHash = keccak256( - abi.encode(target, value, signature, data, eta) - ); - queuedTransactions[txHash] = false; - - emit CancelTransaction(txHash, target, value, signature, data, eta); - } - - function executeTransaction( - address target, - uint256 value, - string memory signature, - bytes memory data, - uint256 eta - ) public payable returns (bytes memory) { - require( - msg.sender == admin, - "Timelock::executeTransaction: Call must come from admin." - ); - - bytes32 txHash = keccak256( - abi.encode(target, value, signature, data, eta) - ); - require( - queuedTransactions[txHash], - "Timelock::executeTransaction: Transaction hasn't been queued." - ); - require( - getBlockTimestamp() >= eta, - "Timelock::executeTransaction: Transaction hasn't surpassed time lock." - ); - require( - getBlockTimestamp() <= eta.add(GRACE_PERIOD), - "Timelock::executeTransaction: Transaction is stale." - ); - - queuedTransactions[txHash] = false; - - bytes memory callData; - - if (bytes(signature).length == 0) { - callData = data; - } else { - callData = abi.encodePacked( - bytes4(keccak256(bytes(signature))), - data - ); - } - - // solium-disable-next-line security/no-call-value - (bool success, bytes memory returnData) = target.call{value: value}( - callData - ); - require( - success, - "Timelock::executeTransaction: Transaction execution reverted." - ); - - emit ExecuteTransaction(txHash, target, value, signature, data, eta); - - return returnData; - } - - function getBlockTimestamp() internal view returns (uint256) { - // solium-disable-next-line security/no-block-members - return block.timestamp; - } -} diff --git a/src/interface/ICompoundConfigurator.sol b/src/interface/ICompoundConfigurator.sol new file mode 100644 index 00000000..e23e27cf --- /dev/null +++ b/src/interface/ICompoundConfigurator.sol @@ -0,0 +1,45 @@ +pragma solidity ^0.8.0; + +interface ICompoundConfigurator { + struct Configuration { + address governor; + address pauseGuardian; + address baseToken; + address baseTokenPriceFeed; + address extensionDelegate; + uint64 supplyKink; + uint64 supplyPerYearInterestRateSlopeLow; + uint64 supplyPerYearInterestRateSlopeHigh; + uint64 supplyPerYearInterestRateBase; + uint64 borrowKink; + uint64 borrowPerYearInterestRateSlopeLow; + uint64 borrowPerYearInterestRateSlopeHigh; + uint64 borrowPerYearInterestRateBase; + uint64 storeFrontPriceFactor; + uint64 trackingIndexScale; + uint64 baseTrackingSupplySpeed; + uint64 baseTrackingBorrowSpeed; + uint104 baseMinForRewards; + uint104 baseBorrowMin; + uint104 targetReserves; + AssetConfig[] assetConfigs; + } + + struct AssetConfig { + address asset; + address priceFeed; + uint8 decimals; + uint64 borrowCollateralFactor; + uint64 liquidateCollateralFactor; + uint64 liquidationFactor; + uint128 supplyCap; + } + + function getConfiguration( + address cometProxy + ) external view returns (Configuration memory); + + function setBorrowKink(address cometProxy, uint64 newBorrowKink) external; + + function setSupplyKink(address cometProxy, uint64 newSupplyKink) external; +} diff --git a/src/interface/IGovernorBravo.sol b/src/interface/IGovernorBravo.sol index b1234cc9..028254e6 100644 --- a/src/interface/IGovernorBravo.sol +++ b/src/interface/IGovernorBravo.sol @@ -1,166 +1,7 @@ // SPDX-License-Identifier: BSD-3-Clause pragma solidity ^0.8.0; -contract GovernorBravoEvents { - /// @notice An event emitted when a new proposal is created - event ProposalCreated( - uint id, - address proposer, - address[] targets, - uint[] values, - string[] signatures, - bytes[] calldatas, - uint startBlock, - uint endBlock, - string description - ); - - /** - * @notice An event emitted when a vote has been cast on a proposal - * @param voter The address which casted a vote - * @param proposalId The proposal id which was voted on - * @param support Support value for the vote. 0=against, 1=for, 2=abstain - * @param votes Number of votes which were cast by the voter - * @param reason The reason given for the vote by the voter - */ - event VoteCast( - address indexed voter, - uint proposalId, - uint8 support, - uint votes, - string reason - ); - - /// @notice An event emitted when a proposal has been canceled - event ProposalCanceled(uint id); - - /// @notice An event emitted when a proposal has been queued in the Timelock - event ProposalQueued(uint id, uint eta); - - /// @notice An event emitted when a proposal has been executed in the Timelock - event ProposalExecuted(uint id); - - /// @notice An event emitted when the voting delay is set - event VotingDelaySet(uint oldVotingDelay, uint newVotingDelay); - - /// @notice An event emitted when the voting period is set - event VotingPeriodSet(uint oldVotingPeriod, uint newVotingPeriod); - - /// @notice Emitted when implementation is changed - event NewImplementation( - address oldImplementation, - address newImplementation - ); - - /// @notice Emitted when proposal threshold is set - event ProposalThresholdSet( - uint oldProposalThreshold, - uint newProposalThreshold - ); - - /// @notice Emitted when pendingAdmin is changed - event NewPendingAdmin(address oldPendingAdmin, address newPendingAdmin); - - /// @notice Emitted when pendingAdmin is accepted, which means admin is updated - event NewAdmin(address oldAdmin, address newAdmin); - - /// @notice Emitted when whitelist account expiration is set - event WhitelistAccountExpirationSet(address account, uint expiration); - - /// @notice Emitted when the whitelistGuardian is set - event WhitelistGuardianSet(address oldGuardian, address newGuardian); -} - -contract GovernorBravoDelegatorStorage { - /// @notice Administrator for this contract - address public admin; - - /// @notice Pending administrator for this contract - address public pendingAdmin; - - /// @notice Active brains of Governor - address public implementation; -} - -/** - * @title Storage for Governor Bravo Delegate - * @notice For future upgrades, do not change GovernorBravoDelegateStorageV1. Create a new - * contract which implements GovernorBravoDelegateStorageV1 and following the naming convention - * GovernorBravoDelegateStorageVX. - */ -abstract contract GovernorBravoDelegateStorageV1 is - GovernorBravoDelegatorStorage -{ - /// @notice The delay before voting on a proposal may take place, once proposed, in blocks - uint public votingDelay; - - /// @notice The duration of voting on a proposal, in blocks - uint public votingPeriod; - - /// @notice The number of votes required in order for a voter to become a proposer - uint public proposalThreshold; - - /// @notice Initial proposal id set at become - uint public initialProposalId; - - /// @notice The total number of proposals - uint public proposalCount; - - /// @notice The address of the Compound Protocol Timelock - TimelockInterface public timelock; - - /// @notice The address of the Compound governance token - CompInterface public comp; - - /// @notice The official record of all proposals ever proposed - mapping(uint proposalId => Proposal proposal) public proposals; - - /// @notice The latest proposal for each proposer - mapping(address proposer => uint proposalId) public latestProposalIds; - - struct Proposal { - /// @notice Unique id for looking up a proposal - uint id; - /// @notice Creator of the proposal - address proposer; - /// @notice The timestamp that the proposal will be available for execution, set once the vote succeeds - uint eta; - /// @notice the ordered list of target addresses for calls to be made - address[] targets; - /// @notice The ordered list of values (i.e. msg.value) to be passed to the calls to be made - uint[] values; - /// @notice The ordered list of function signatures to be called - string[] signatures; - /// @notice The ordered list of calldata to be passed to each call - bytes[] calldatas; - /// @notice The block at which voting begins: holders must delegate their votes prior to this block - uint startBlock; - /// @notice The block at which voting ends: votes must be cast prior to this block - uint endBlock; - /// @notice Current number of votes in favor of this proposal - uint forVotes; - /// @notice Current number of votes in opposition to this proposal - uint againstVotes; - /// @notice Current number of votes for abstaining for this proposal - uint abstainVotes; - /// @notice Flag marking whether the proposal has been canceled - bool canceled; - /// @notice Flag marking whether the proposal has been executed - bool executed; - /// @notice Receipts of ballots for the entire set of voters - mapping(address => Receipt) receipts; - } - - /// @notice Ballot receipt record for a voter - struct Receipt { - /// @notice Whether or not a vote has been cast - bool hasVoted; - /// @notice Whether or not the voter supports the proposal or abstains - uint8 support; - /// @notice The number of votes the voter had, which were cast - uint96 votes; - } - +interface IGovernorBravo { /// @notice Possible states that a proposal may be in enum ProposalState { Pending, @@ -173,14 +14,18 @@ abstract contract GovernorBravoDelegateStorageV1 is Executed } - function state( - uint proposalId - ) external view virtual returns (ProposalState); -} + function votingDelay() external view returns (uint256); + + function votingPeriod() external view returns (uint256); + + function proposalThreshold() external view returns (uint256); + + function proposalCount() external view returns (uint256); -interface IGovernorAlpha { function quorumVotes() external view returns (uint256); + function timelock() external view returns (ITimelockBravo); + function getActions( uint256 proposalId ) @@ -194,23 +39,17 @@ interface IGovernorAlpha { ); function castVote(uint proposalId, uint8 support) external; + function queue(uint proposalId) external; + function execute(uint proposalId) external payable; -} -abstract contract GovernorBravoDelegateStorageV2 is - GovernorBravoDelegateStorageV1, - IGovernorAlpha -{ - /// @notice Stores the expiration of account whitelist status as a timestamp - mapping(address account => uint expiration) - public whitelistAccountExpirations; + function state(uint proposalId) external view returns (ProposalState); - /// @notice Address which manages whitelisted proposals and whitelist accounts - address public whitelistGuardian; + function comp() external view returns (IERC20VotesComp); } -interface TimelockInterface { +interface ITimelockBravo { function delay() external view returns (uint); function GRACE_PERIOD() external view returns (uint); function acceptAdmin() external; @@ -238,9 +77,11 @@ interface TimelockInterface { ) external payable returns (bytes memory); } -interface CompInterface { +interface IERC20VotesComp { function getPriorVotes( address account, uint blockNumber ) external view returns (uint96); + + function delegate(address delegatee) external; } diff --git a/src/interface/IVotes.sol b/src/interface/IVotes.sol deleted file mode 100644 index 99689622..00000000 --- a/src/interface/IVotes.sol +++ /dev/null @@ -1,76 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v4.9.0) (governance/utils/IVotes.sol) -pragma solidity ^0.8.0; - -/** - * @dev Common interface for {ERC20Votes}, {ERC721Votes}, and other {Votes}-enabled contracts. - * - * _Available since v4.5._ - */ -interface IVotes { - /** - * @dev Emitted when an account changes their delegate. - */ - event DelegateChanged( - address indexed delegator, - address indexed fromDelegate, - address indexed toDelegate - ); - - /** - * @dev Emitted when a token transfer or delegate change results in changes to a delegate's number of votes. - */ - event DelegateVotesChanged( - address indexed delegate, - uint256 previousBalance, - uint256 newBalance - ); - - /** - * @dev Returns the current amount of votes that `account` has. - */ - function getVotes(address account) external view returns (uint256); - - /** - * @dev Returns the amount of votes that `account` had at a specific moment in the past. If the `clock()` is - * configured to use block numbers, this will return the value at the end of the corresponding block. - */ - function getPastVotes( - address account, - uint256 timepoint - ) external view returns (uint256); - - /** - * @dev Returns the total supply of votes available at a specific moment in the past. If the `clock()` is - * configured to use block numbers, this will return the value at the end of the corresponding block. - * - * NOTE: This value is the sum of all available votes, which is not necessarily the sum of all delegated votes. - * Votes that have not been delegated are still part of total supply, even though they would not participate in a - * vote. - */ - function getPastTotalSupply( - uint256 timepoint - ) external view returns (uint256); - - /** - * @dev Returns the delegate that `account` has chosen. - */ - function delegates(address account) external view returns (address); - - /** - * @dev Delegates votes from the sender to `delegatee`. - */ - function delegate(address delegatee) external; - - /** - * @dev Delegates votes from signer to `delegatee`. - */ - function delegateBySig( - address delegatee, - uint256 nonce, - uint256 expiry, - uint8 v, - bytes32 r, - bytes32 s - ) external; -} diff --git a/src/proposals/GovernorBravoProposal.sol b/src/proposals/GovernorBravoProposal.sol index bc3acf39..5fdab41a 100644 --- a/src/proposals/GovernorBravoProposal.sol +++ b/src/proposals/GovernorBravoProposal.sol @@ -3,16 +3,24 @@ pragma solidity ^0.8.0; import "@forge-std/console.sol"; -import {TimelockInterface, GovernorBravoDelegateStorageV1 as Bravo} from "@interface/IGovernorBravo.sol"; -import {GovernorBravoDelegateStorageV2} from "@interface/IGovernorBravo.sol"; -import {IVotes} from "@interface/IVotes.sol"; +import {IGovernorBravo, ITimelockBravo, IERC20VotesComp} from "@interface/IGovernorBravo.sol"; import {Address} from "@utils/Address.sol"; + import {Proposal} from "./Proposal.sol"; abstract contract GovernorBravoProposal is Proposal { using Address for address; + /// @notice Governor Bravo contract + /// @dev must be set by the inheriting contract + IGovernorBravo public governor; + + /// @notice set the Governor Bravo contract + function setGovernor(address _governor) public { + governor = IGovernorBravo(_governor); + } + /// @notice Getter function for `GovernorBravoDelegate.propose()` calldata function getCalldata() public @@ -40,13 +48,12 @@ abstract contract GovernorBravoProposal is Proposal { /// @notice Check if there are any on-chain proposals that match the /// proposal calldata - function checkOnChainCalldata( - address governorAddress - ) public view override returns (bool calldataExist) { - GovernorBravoDelegateStorageV2 governor = GovernorBravoDelegateStorageV2( - governorAddress - ); - + function checkOnChainCalldata() + public + view + override + returns (bool calldataExist) + { uint256 proposalCount = governor.proposalCount(); while (proposalCount > 0) { @@ -56,7 +63,6 @@ abstract contract GovernorBravoProposal is Proposal { string[] memory signatures, bytes[] memory calldatas ) = governor.getActions(proposalCount); - proposalCount--; bytes memory onchainCalldata = abi.encodeWithSignature( "propose(address[],uint256[],string[],bytes[],string)", @@ -70,29 +76,24 @@ abstract contract GovernorBravoProposal is Proposal { bytes memory proposalCalldata = getCalldata(); if (keccak256(proposalCalldata) == keccak256(onchainCalldata)) { - console.log( - "Proposal calldata matches on-chain calldata with proposalId: ", - proposalCount - ); + if (DEBUG) { + console.log( + "Proposal calldata matches on-chain calldata with proposalId: ", + proposalCount + ); + } return true; } + + proposalCount--; } return false; } /// @notice Simulate governance proposal - /// @param governorAddress address of the Governor Bravo Delegator contract - /// @param governanceToken address of the governance token of the system - /// @param proposerAddress address of the proposer - function _simulateActions( - address governorAddress, - address governanceToken, - address proposerAddress - ) internal { - GovernorBravoDelegateStorageV2 governor = GovernorBravoDelegateStorageV2( - governorAddress - ); - + function simulate() public override { + address proposerAddress = address(1); + IERC20VotesComp governanceToken = governor.comp(); { // Ensure proposer has meets minimum proposal threshold and quorum votes to pass the proposal uint256 quorumVotes = governor.quorumVotes(); @@ -100,10 +101,10 @@ abstract contract GovernorBravoProposal is Proposal { uint256 votingPower = quorumVotes > proposalThreshold ? quorumVotes : proposalThreshold; - deal(governanceToken, proposerAddress, votingPower); + deal(address(governanceToken), proposerAddress, votingPower); // Delegate proposer's votes to itself vm.prank(proposerAddress); - IVotes(governanceToken).delegate(proposerAddress); + IERC20VotesComp(governanceToken).delegate(proposerAddress); vm.roll(block.number + 1); } @@ -111,17 +112,19 @@ abstract contract GovernorBravoProposal is Proposal { // Register the proposal vm.prank(proposerAddress); - bytes memory data = address(payable(governorAddress)).functionCall( - proposeCalldata - ); + bytes memory data = address(governor).functionCall(proposeCalldata); uint256 proposalId = abi.decode(data, (uint256)); // Check proposal is in Pending state - require(governor.state(proposalId) == Bravo.ProposalState.Pending); + require( + governor.state(proposalId) == IGovernorBravo.ProposalState.Pending + ); // Roll to Active state (voting period) vm.roll(block.number + governor.votingDelay() + 1); - require(governor.state(proposalId) == Bravo.ProposalState.Active); + require( + governor.state(proposalId) == IGovernorBravo.ProposalState.Active + ); // Vote YES vm.prank(proposerAddress); @@ -129,18 +132,24 @@ abstract contract GovernorBravoProposal is Proposal { // Roll to allow proposal state transitions vm.roll(block.number + governor.votingPeriod()); - require(governor.state(proposalId) == Bravo.ProposalState.Succeeded); + require( + governor.state(proposalId) == IGovernorBravo.ProposalState.Succeeded + ); // Queue the proposal governor.queue(proposalId); - require(governor.state(proposalId) == Bravo.ProposalState.Queued); + require( + governor.state(proposalId) == IGovernorBravo.ProposalState.Queued + ); // Warp to allow proposal execution on timelock - TimelockInterface timelock = TimelockInterface(governor.timelock()); + ITimelockBravo timelock = ITimelockBravo(governor.timelock()); vm.warp(block.timestamp + timelock.delay()); // Execute the proposal governor.execute(proposalId); - require(governor.state(proposalId) == Bravo.ProposalState.Executed); + require( + governor.state(proposalId) == IGovernorBravo.ProposalState.Executed + ); } } diff --git a/src/proposals/IProposal.sol b/src/proposals/IProposal.sol index 55134f35..bbca8896 100644 --- a/src/proposals/IProposal.sol +++ b/src/proposals/IProposal.sol @@ -3,20 +3,21 @@ pragma solidity ^0.8.0; import {Addresses} from "@addresses/Addresses.sol"; interface IProposal { - /// @notice proposal name, e.g. "BIP15" - /// @dev override this to set the proposal name + /// @notice proposal name, e.g. "BIP15". + /// @dev override this to set the proposal name. function name() external view returns (string memory); - /// @notice proposal description - /// @dev override this to set the proposal description + /// @notice proposal description. + /// @dev override this to set the proposal description. function description() external view returns (string memory); - /// @notice actually run the proposal - /// @dev review the implementation to determine which internal functions - /// might need overriding for you proposal + /// @notice function to be used by forge script. + /// @dev use flags to determine which actions to take + /// this function shoudn't be overriden. function run() external; - /// @notice Print proposal actions + /// @notice return proposal actions. + /// @dev this function shoudn't be overriden. function getProposalActions() external returns ( @@ -25,13 +26,44 @@ interface IProposal { bytes[] memory arguments ); - /// @notice Return proposal calldata + /// @notice return proposal calldata function getCalldata() external returns (bytes memory data); - /// @notice Check if there are any on-chain proposal that matches the + /// @notice check if there are any on-chain proposal that matches the /// proposal calldata - function checkOnChainCalldata(address addr) external view returns (bool); + function checkOnChainCalldata() external view returns (bool); - /// @notice Return Addresses object + /// @notice return Addresses object function addresses() external view returns (Addresses); + + /// @notice deploy any contracts needed for the proposal. + /// @dev contracts calls here are broadcast if the broadcast flag is set. + function deploy() external; + + /// @notice helper function to mock on-chain data after deployment + /// e.g. pranking, etching, etc. + function afterDeployMock() external; + + /// @notice build the proposal actions + /// @dev contract calls must be perfomed in plain solidity. + /// overriden requires using buildModifier modifier to leverage + /// foundry snapshot and state diff recording to populate the actions array. + function build() external; + + /// @notice actually simulates the proposal. + /// e.g. schedule and execute on Timelock Controller, + /// proposes, votes and execute on Governor Bravo, etc. + function simulate() external; + + /// @notice execute post-proposal checks. + /// e.g. read state variables of the deployed contracts to make + /// sure they are deployed and initialized correctly, or read + /// states that are expected to have changed during the simulate step. + function validate() external; + + /// @notice print proposal description, actions and calldata + function print() external; + + /// @notice set the Addresses contract + function setAddresses(Addresses _addresses) external; } diff --git a/src/proposals/MultisigProposal.sol b/src/proposals/MultisigProposal.sol index 3c779ada..c414134f 100644 --- a/src/proposals/MultisigProposal.sol +++ b/src/proposals/MultisigProposal.sol @@ -14,8 +14,10 @@ abstract contract MultisigProposal is Proposal { 0xb89c1b3bdf2cf8827818646bce9a8f6e372885f8c55e5c07acbd307cb133b000 ); - struct Call { + struct Call3Value { address target; + bool allowFailure; + uint256 value; bytes callData; } @@ -24,34 +26,37 @@ abstract contract MultisigProposal is Proposal { /// get proposal actions ( address[] memory targets, - , - /// ignore values + uint256[] memory values, bytes[] memory arguments ) = getProposalActions(); /// create calls array with targets and arguments - Call[] memory calls = new Call[](targets.length); + Call3Value[] memory calls = new Call3Value[](targets.length); for (uint256 i; i < calls.length; i++) { require(targets[i] != address(0), "Invalid target for multisig"); - calls[i] = Call({target: targets[i], callData: arguments[i]}); + calls[i] = Call3Value({ + target: targets[i], + allowFailure: false, + value: values[i], + callData: arguments[i] + }); } /// generate calldata - data = abi.encodeWithSignature("aggregate((address,bytes)[])", calls); + data = abi.encodeWithSignature( + "aggregate3Value((address,bool,uint256,bytes)[])", + calls + ); } /// @notice Check if there are any on-chain proposal that matches the /// proposal calldata - function checkOnChainCalldata(address) public pure override returns (bool) { + function checkOnChainCalldata() public pure override returns (bool) { revert("Not implemented"); } function _simulateActions(address multisig) internal { - require( - multisig.getContractHash() == MULTISIG_BYTECODE_HASH, - "Multisig address doesn't match Gnosis Safe contract bytecode" - ); vm.startPrank(multisig); /// this is a hack because multisig execTransaction requires owners signatures diff --git a/src/proposals/Proposal.sol b/src/proposals/Proposal.sol index 6ba6962c..68d371d4 100644 --- a/src/proposals/Proposal.sol +++ b/src/proposals/Proposal.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.8.0; +pragma solidity =0.8.25; import {Test} from "@forge-std/Test.sol"; import {VmSafe} from "@forge-std/Vm.sol"; @@ -23,84 +23,87 @@ abstract contract Proposal is Test, Script, IProposal { /// they all follow the same structure Action[] public actions; - /// @notice debug flag to print proposal actions, calldata, new addresses and changed addresses - /// @dev default is true - bool internal DEBUG = false; + /// @notice debug flag to print internal proposal logs + bool internal DEBUG; + bool internal DO_DEPLOY; + bool internal DO_AFTER_DEPLOY_MOCK; + bool internal DO_BUILD; + bool internal DO_SIMULATE; + bool internal DO_VALIDATE; + bool internal DO_PRINT; /// @notice Addresses contract Addresses public addresses; - /// @notice the actions caller name in the Addresses JSON - string public caller; - /// @notice primary fork id uint256 public primaryForkId; - modifier buildModifier() { - _startBuild(); + /// @notice buildModifier to be used by the build function to populate the + /// actions array + /// @param toPrank the address that will be used as the caller for the + /// actions, e.g. multisig address, timelock address, etc. + modifier buildModifier(address toPrank) { + _startBuild(toPrank); _; - _endBuild(); + _endBuild(toPrank); } - constructor(string memory addressesPath, string memory _caller) { - addresses = new Addresses(addressesPath); - vm.makePersistent(address(addresses)); - caller = _caller; + constructor() { DEBUG = vm.envOr("DEBUG", false); + + DO_DEPLOY = vm.envOr("DO_DEPLOY", true); + DO_AFTER_DEPLOY_MOCK = vm.envOr("DO_AFTER_DEPLOY_MOCK", true); + DO_BUILD = vm.envOr("DO_BUILD", true); + DO_SIMULATE = vm.envOr("DO_SIMULATE", true); + DO_VALIDATE = vm.envOr("DO_VALIDATE", true); + DO_PRINT = vm.envOr("DO_PRINT", true); } - /// @notice override this to set the proposal name + /// @notice proposal name, e.g. "BIP15". + /// @dev override this to set the proposal name. function name() external view virtual returns (string memory); - /// @notice override this to set the proposal description + /// @notice proposal description. + /// @dev override this to set the proposal description. function description() public view virtual returns (string memory); - /// @notice main function - /// @dev do not override - function run() external { + /// @notice function to be used by forge script. + /// @dev use flags to determine which actions to take + /// this function shoudn't be overriden. + function run() public virtual { vm.selectFork(primaryForkId); - address deployer = addresses.getAddress("DEPLOYER_EOA"); - - vm.startBroadcast(deployer); - _deploy(); - _afterDeploy(); - vm.stopBroadcast(); + if (DO_DEPLOY) { + /// DEPLOYER_EOA must be an unlocked account when running through forge script + /// use cast wallet to unlock the account + address deployer = addresses.getAddress("DEPLOYER_EOA"); - _outerBuild(); - _run(); - _teardown(); - _validate(); - - if (DEBUG) { - _printRecordedAddresses(); - _printActions(); - _printCalldata(); + vm.startBroadcast(deployer); + deploy(); + addresses.printJSONChanges(); + vm.stopBroadcast(); } - } - - function _outerBuild() private { - _startBuild(); - - _build(); - _endBuild(); + if (DO_AFTER_DEPLOY_MOCK) afterDeployMock(); + if (DO_BUILD) build(); + if (DO_SIMULATE) simulate(); + if (DO_VALIDATE) validate(); + if (DO_PRINT) print(); } - /// @notice Return proposal calldata + /// @notice return proposal calldata. function getCalldata() public virtual returns (bytes memory data); - /// @notice Check if there are any on-chain proposal that matches the + /// @notice check if there are any on-chain proposal that matches the /// proposal calldata - function checkOnChainCalldata( - address - ) public view virtual returns (bool calldataMatch); + function checkOnChainCalldata() public view virtual returns (bool matches); /// @notice get proposal actions /// @dev do not override function getProposalActions() public view + virtual override returns ( address[] memory targets, @@ -132,67 +135,55 @@ abstract contract Proposal is Test, Script, IProposal { } } - /// @notice Create actions for the proposal - /// @dev implementations should use buildModifier modifier - /// TODO remove implementation once move from internal to public functions - function build() public virtual {} - /// -------------------------------------------------------------------- /// -------------------------------------------------------------------- - /// ------------------ Internal functions to override ------------------ + /// --------------------------- Public functions ----------------------- /// -------------------------------------------------------------------- /// -------------------------------------------------------------------- - /// @dev Deploy contracts and add them to list of addresses - function _deploy() internal virtual {} + /// @notice set the Addresses contract + function setAddresses(Addresses _addresses) public { + addresses = _addresses; + } - /// @dev After deploying, call initializers and link contracts together - function _afterDeploy() internal virtual {} + /// @notice deploy any contracts needed for the proposal. + /// @dev contracts calls here are broadcast if the broadcast flag is set. + function deploy() public virtual {} - /// @dev After finishing deploy and deploy cleanup, build the proposal - function _build() internal virtual {} + /// @notice helper function to mock on-chain data after deployment + /// e.g. pranking, etching, etc. + function afterDeployMock() public virtual {} - /// @dev Actually run the proposal (e.g. queue actions in the Timelock, - /// or execute a serie of Multisig calls...). - /// See proposals for helper contracts. - /// address param is the address of the proposal executor - function _run() internal virtual { - /// Check if there are actions to run - uint256 actionsLength = actions.length; - require(actionsLength > 0, "No actions found"); + /// @notice build the proposal actions + /// @dev contract calls must be perfomed in plain solidity. + /// overriden requires using buildModifier modifier to leverage + /// foundry snapshot and state diff recording to populate the actions array. + function build() public virtual {} - for (uint256 i = 0; i < actionsLength; i++) { - for (uint256 j = i + 1; j < actionsLength; j++) { - // Check if either the target or the arguments are the same for any two actions - bool isDuplicateTarget = actions[i].target == actions[j].target; - bool isDuplicateArguments = keccak256(actions[i].arguments) == - keccak256(actions[j].arguments); + /// @notice actually simulates the proposal. + /// e.g. schedule and execute on Timelock Controller, + /// proposes, votes and execute on Governor Bravo, etc. + function simulate() public virtual {} - require( - !(isDuplicateTarget && isDuplicateArguments), - "Duplicate actions found" - ); - } + /// @notice execute post-proposal checks. + /// e.g. read state variables of the deployed contracts to make + /// sure they are deployed and initialized correctly, or read + /// states that are expected to have changed during the simulate step. + function validate() public virtual {} + + /// @notice print proposal description, actions and calldata + function print() public virtual { + console.log("\n---------------- Proposal Description ----------------"); + console.log(description()); + + console.log("\n------------------ Proposal Actions ------------------"); + for (uint256 i; i < actions.length; i++) { + console.log("%d). %s", i + 1, actions[i].description); + console.log("target: %s\npayload", actions[i].target); + console.logBytes(actions[i].arguments); + console.log("\n"); } - } - /// @dev After a proposal executed, if you mocked some behavior in the - /// afterDeploy step, you might want to tear down the mocks here. - /// For instance, in afterDeploy() you could impersonate the multisig - /// of another protocol to do actions in their protocol (in anticipation - /// of changes that must happen before your proposal execution), and here - /// you could revert these changes, to make sure the integration tests - /// run on a state that is as close to mainnet as possible. - function _teardown() internal virtual {} - - /// @dev For small post-proposal checks, e.g. read state variables of the - /// contracts you deployed, to make sure your deploy() and afterDeploy() - /// steps have deployed contracts in a correct configuration, or read - /// states that are expected to have change during your run() step. - function _validate() internal virtual {} - - /// @dev Print proposal calldata - function _printCalldata() internal virtual { console.log( "\n\n------------------ Proposal Calldata ------------------" ); @@ -201,94 +192,68 @@ abstract contract Proposal is Test, Script, IProposal { /// -------------------------------------------------------------------- /// -------------------------------------------------------------------- - /// -------------------------- Private functions ------------------------- + /// ------------------------- Internal functions ----------------------- /// -------------------------------------------------------------------- /// -------------------------------------------------------------------- - /// @dev Print proposal actions - function _printActions() private view { - console.log("\n---------------- Proposal Description ----------------"); - console.log(description()); - console.log("\n------------------ Proposal Actions ------------------"); - for (uint256 i; i < actions.length; i++) { - console.log("%d). %s", i + 1, actions[i].description); - console.log("target: %s\npayload", actions[i].target); - console.logBytes(actions[i].arguments); - console.log("\n"); - } - } + /// @notice validate actions inclusion + /// default implementation check for duplicate actions + function _validateAction( + address target, + uint256 value, + bytes memory data + ) internal virtual { + // uses transition storage to check for duplicate actions + bytes32 actionHash = keccak256(abi.encodePacked(target, value, data)); - /// @dev Print recorded addresses - function _printRecordedAddresses() private view { - ( - string[] memory recordedNames, - , - address[] memory recordedAddresses - ) = addresses.getRecordedAddresses(); - - if (recordedNames.length > 0) { - console.log( - "\n-------- Addresses added after running proposal --------" - ); - for (uint256 j = 0; j < recordedNames.length; j++) { - console.log( - "{\n 'addr': '%s', ", - recordedAddresses[j] - ); - console.log(" 'chainId': %d,", block.chainid); - console.log(" 'isContract': %s", true, ","); - console.log( - " 'name': '%s'\n}%s", - recordedNames[j], - j < recordedNames.length - 1 ? "," : "" - ); - } - } + uint256 found; - ( - string[] memory changedNames, - , - , - address[] memory changedAddresses - ) = addresses.getChangedAddresses(); + assembly { + found := tload(actionHash) + } - if (changedNames.length > 0) { - console.log( - "\n------- Addresses changed after running proposal --------" - ); + require(found == 0, "Duplicated action found"); - for (uint256 j = 0; j < changedNames.length; j++) { - console.log("{\n 'addr': '%s', ", changedAddresses[j]); - console.log(" 'chainId': %d,", block.chainid); - console.log(" 'isContract': %s", true, ","); - console.log( - " 'name': '%s'\n}%s", - changedNames[j], - j < changedNames.length - 1 ? "," : "" - ); - } + assembly { + tstore(actionHash, 1) } } + /// @notice validate actions + function _validateActions() internal virtual {} + + /// -------------------------------------------------------------------- + /// -------------------------------------------------------------------- + /// ------------------------- Private functions ------------------------ + /// -------------------------------------------------------------------- + /// -------------------------------------------------------------------- + /// @notice to be used by the build function to create a governance proposal /// kick off the process of creating a governance proposal by: /// 1). taking a snapshot of the current state of the contract /// 2). starting prank as the caller /// 3). starting a $recording of all calls created during the proposal - function _startBuild() private { + /// @param toPrank the address that will be used as the caller for the + /// actions, e.g. multisig address, timelock address, etc. + function _startBuild(address toPrank) private { + vm.startPrank(toPrank); + _startSnapshot = vm.snapshot(); - vm.startPrank(addresses.getAddress(caller)); + vm.startStateDiffRecording(); } /// @notice to be used at the end of the build function to snapshot /// the actions performed by the proposal and revert these changes /// then, stop the prank and record the actions that were taken by the proposal. - function _endBuild() private { - vm.stopPrank(); + /// @param caller the address that will be used as the caller for the + /// actions, e.g. multisig address, timelock address, etc. + function _endBuild(address caller) private { VmSafe.AccountAccess[] memory accountAccesses = vm .stopAndReturnStateDiff(); + vm.stopPrank(); + /// roll back all state changes made during the governance proposal require( vm.revertTo(_startSnapshot), @@ -305,9 +270,13 @@ abstract contract Proposal is Test, Script, IProposal { /// ignore calls to vm in the build function accountAccesses[i].accessor != address(addresses) && accountAccesses[i].kind == VmSafe.AccountAccessKind.Call && - accountAccesses[i].accessor == addresses.getAddress(caller) + accountAccesses[i].accessor == caller /// caller is correct, not a subcall ) { - /// caller is correct, not a subcall + _validateAction( + accountAccesses[i].account, + accountAccesses[i].value, + accountAccesses[i].data + ); actions.push( Action({ @@ -321,35 +290,15 @@ abstract contract Proposal is Test, Script, IProposal { " with ", vm.toString(accountAccesses[i].value), " eth and ", - _bytesToString(accountAccesses[i].data), + vm.toString(accountAccesses[i].data), " data." ) ) }) ); } - } - } - /// @notice convert bytes to a string - /// @param data the bytes to convert to a human readable string - function _bytesToString( - bytes memory data - ) private pure returns (string memory) { - /// Initialize an array of characters twice the length of data, - /// since each byte will be represented by two hexadecimal characters - bytes memory buffer = new bytes(data.length * 2); - - /// Characters for conversion - bytes memory characters = "0123456789abcdef"; - - for (uint256 i = 0; i < data.length; i++) { - /// For each byte, find the corresponding hexadecimal characters - buffer[i * 2] = characters[uint256(uint8(data[i] >> 4))]; - buffer[i * 2 + 1] = characters[uint256(uint8(data[i] & 0x0f))]; + _validateActions(); } - - /// Convert the bytes array to a string and return - return string(buffer); } } diff --git a/src/proposals/TimelockProposal.sol b/src/proposals/TimelockProposal.sol index 0163aefa..a764553e 100644 --- a/src/proposals/TimelockProposal.sol +++ b/src/proposals/TimelockProposal.sol @@ -10,8 +10,18 @@ import {Proposal} from "@proposals/Proposal.sol"; abstract contract TimelockProposal is Proposal { using Address for address; + /// @notice the predecessor timelock id - default is 0 but inherited bytes32 public predecessor = bytes32(0); + /// @notice the timelock controller + /// @dev must be set by the inheriting contract + ITimelockController public timelock; + + /// @notice set the timelock controller + function setTimelock(address _timelock) public { + timelock = ITimelockController(_timelock); + } + /// @notice get schedule calldata function getCalldata() public @@ -19,7 +29,7 @@ abstract contract TimelockProposal is Proposal { override returns (bytes memory scheduleCalldata) { - bytes32 salt = keccak256(abi.encode(actions[0].description)); + bytes32 salt = keccak256(abi.encode(description())); ( address[] memory targets, @@ -27,9 +37,7 @@ abstract contract TimelockProposal is Proposal { bytes[] memory payloads ) = getProposalActions(); - address addressCaller = addresses.getAddress(caller); - uint256 delay = ITimelockController(payable(addressCaller)) - .getMinDelay(); + uint256 delay = timelock.getMinDelay(); scheduleCalldata = abi.encodeWithSignature( "scheduleBatch(address[],uint256[],bytes[],bytes32,bytes32,uint256)", @@ -48,7 +56,7 @@ abstract contract TimelockProposal is Proposal { view returns (bytes memory executeCalldata) { - bytes32 salt = keccak256(abi.encode(actions[0].description)); + bytes32 salt = keccak256(abi.encode(description())); ( address[] memory targets, @@ -68,22 +76,21 @@ abstract contract TimelockProposal is Proposal { /// @notice Check if there are any on-chain proposal that matches the /// proposal calldata - function checkOnChainCalldata( - address timelockAddress - ) public view override returns (bool calldataExist) { - ITimelockController timelockController = ITimelockController( - payable(timelockAddress) - ); - + function checkOnChainCalldata() + public + view + override + returns (bool calldataExist) + { ( address[] memory targets, uint256[] memory values, bytes[] memory payloads ) = getProposalActions(); - bytes32 salt = keccak256(abi.encode(actions[0].description)); + bytes32 salt = keccak256(abi.encode(description())); - bytes32 hash = timelockController.hashOperationBatch( + bytes32 hash = timelock.hashOperationBatch( targets, values, payloads, @@ -91,9 +98,14 @@ abstract contract TimelockProposal is Proposal { salt ); - return - timelockController.isOperation(hash) || - timelockController.isOperationPending(hash); + if (DEBUG) { + console.log( + "Proposal calldata matches on-chain calldata with proposal hash: " + ); + console.logBytes32(hash); + } + + return timelock.isOperation(hash) || timelock.isOperationPending(hash); } /// @notice simulate timelock proposal @@ -103,7 +115,7 @@ abstract contract TimelockProposal is Proposal { address proposerAddress, address executorAddress ) internal { - bytes32 salt = keccak256(abi.encode(actions[0].description)); + bytes32 salt = keccak256(abi.encode(description())); if (DEBUG) { console.log("salt:"); @@ -113,17 +125,13 @@ abstract contract TimelockProposal is Proposal { bytes memory scheduleCalldata = getCalldata(); bytes memory executeCalldata = getExecuteCalldata(); - address addressCaller = addresses.getAddress(caller); - ITimelockController timelockController = ITimelockController( - payable(addressCaller) - ); ( address[] memory targets, uint256[] memory values, bytes[] memory payloads ) = getProposalActions(); - bytes32 proposalId = timelockController.hashOperationBatch( + bytes32 proposalId = timelock.hashOperationBatch( targets, values, payloads, @@ -132,14 +140,15 @@ abstract contract TimelockProposal is Proposal { ); if ( - !timelockController.isOperationPending(proposalId) && - !timelockController.isOperation(proposalId) + !timelock.isOperationPending(proposalId) && + !timelock.isOperation(proposalId) ) { vm.prank(proposerAddress); // Perform the low-level call - bytes memory returndata = address(payable(addressCaller)) - .functionCall(scheduleCalldata); + bytes memory returndata = address(timelock).functionCall( + scheduleCalldata + ); if (DEBUG && returndata.length > 0) { console.log("returndata"); @@ -150,15 +159,16 @@ abstract contract TimelockProposal is Proposal { console.logBytes32(proposalId); } - uint256 delay = timelockController.getMinDelay(); + uint256 delay = timelock.getMinDelay(); vm.warp(block.timestamp + delay); - if (!timelockController.isOperationDone(proposalId)) { + if (!timelock.isOperationDone(proposalId)) { vm.prank(executorAddress); // Perform the low-level call - bytes memory returndata = address(payable(addressCaller)) - .functionCall(executeCalldata); + bytes memory returndata = address(timelock).functionCall( + executeCalldata + ); if (DEBUG && returndata.length > 0) { console.log("returndata"); @@ -170,7 +180,18 @@ abstract contract TimelockProposal is Proposal { } /// @notice print schedule and execute calldata - function _printCalldata() internal view override { + function print() public view override { + console.log("\n---------------- Proposal Description ----------------"); + console.log(description()); + + console.log("\n------------------ Proposal Actions ------------------"); + for (uint256 i; i < actions.length; i++) { + console.log("%d). %s", i + 1, actions[i].description); + console.log("target: %s\npayload", actions[i].target); + console.logBytes(actions[i].arguments); + console.log("\n"); + } + console.log( "\n\n------------------ Schedule Calldata ------------------" ); diff --git a/src/type-check/ExampleTypeCheck.sol b/src/type-check/ExampleTypeCheck.sol index cb4619be..f58a3f5f 100644 --- a/src/type-check/ExampleTypeCheck.sol +++ b/src/type-check/ExampleTypeCheck.sol @@ -1,5 +1,4 @@ // SPDX-License-Identifier: GPL-3.0 - pragma solidity =0.8.19; contract ExampleTypeCheck { diff --git a/src/type-check/ExampleTypeCheck_02.sol b/src/type-check/ExampleTypeCheck_02.sol index c44dc68e..554a9cd9 100644 --- a/src/type-check/ExampleTypeCheck_02.sol +++ b/src/type-check/ExampleTypeCheck_02.sol @@ -1,5 +1,4 @@ // SPDX-License-Identifier: GPL-3.0 - pragma solidity =0.8.19; contract ExampleTypeCheck_02 { diff --git a/test/Addresses.t.sol b/test/Addresses.t.sol index 9a250565..08f5e4d2 100644 --- a/test/Addresses.t.sol +++ b/test/Addresses.t.sol @@ -32,29 +32,29 @@ contract TestAddresses is Test { } function test_getAddress() public { - address addr = addresses.getAddress("DEV_MULTISIG"); + address addr = addresses.getAddress("DEPLOYER_EOA"); - assertEq(addr, 0x3dd46846eed8D147841AE162C8425c08BD8E1b41); + assertEq(addr, 0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739); } function test_getAddressChainId() public { - address addr = addresses.getAddress("TEAM_MULTISIG", block.chainid); + address addr = addresses.getAddress("DEPLOYER_EOA", block.chainid); - assertEq(addr, 0x7da82C7AB4771ff031b66538D2fB9b0B047f6CF9); + assertEq(addr, 0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739); } function test_changeAddress() public { assertEq( - addresses.getAddress("DEV_MULTISIG"), - 0x3dd46846eed8D147841AE162C8425c08BD8E1b41, + addresses.getAddress("DEPLOYER_EOA"), + 0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739, "Wrong current address" ); address addr = vm.addr(1); - addresses.changeAddress("DEV_MULTISIG", addr, false); + addresses.changeAddress("DEPLOYER_EOA", addr, false); assertEq( - addresses.getAddress("DEV_MULTISIG"), + addresses.getAddress("DEPLOYER_EOA"), addr, "Not updated correclty" ); @@ -62,31 +62,31 @@ contract TestAddresses is Test { function test_changeAddressToSameAddressFails() public { assertEq( - addresses.getAddress("DEV_MULTISIG"), - 0x3dd46846eed8D147841AE162C8425c08BD8E1b41, + addresses.getAddress("DEPLOYER_EOA"), + 0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739, "Wrong current address" ); - address addr = addresses.getAddress("DEV_MULTISIG"); + address addr = addresses.getAddress("DEPLOYER_EOA"); vm.expectRevert( - "Address: DEV_MULTISIG already set to the same value on chain: 31337" + "Address: DEPLOYER_EOA already set to the same value on chain: 31337" ); - addresses.changeAddress("DEV_MULTISIG", addr, true); + addresses.changeAddress("DEPLOYER_EOA", addr, true); } function test_changeAddressChainId() public { assertEq( - addresses.getAddress("DEV_MULTISIG"), - 0x3dd46846eed8D147841AE162C8425c08BD8E1b41, + addresses.getAddress("DEPLOYER_EOA"), + 0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739, "Wrong current address" ); address addr = vm.addr(1); uint256 chainId = 31337; - addresses.changeAddress("DEV_MULTISIG", addr, chainId, false); + addresses.changeAddress("DEPLOYER_EOA", addr, chainId, false); assertEq( - addresses.getAddress("DEV_MULTISIG", chainId), + addresses.getAddress("DEPLOYER_EOA", chainId), addr, "Not updated correclty" ); @@ -110,13 +110,13 @@ contract TestAddresses is Test { function test_addAddressDifferentChain() public { address addr = vm.addr(1); uint256 chainId = 123; - addresses.addAddress("DEV_MULTISIG", addr, chainId, false); + addresses.addAddress("DEPLOYER_EOA", addr, chainId, false); - assertEq(addresses.getAddress("DEV_MULTISIG", chainId), addr); - // Validate that the 'DEV_MULTISIG' address for chain 31337 matches the address from Addresses.json. + assertEq(addresses.getAddress("DEPLOYER_EOA", chainId), addr); + // Validate that the 'DEPLOYER_EOA' address for chain 31337 matches the address from Addresses.json. assertEq( - addresses.getAddress("DEV_MULTISIG"), - 0x3dd46846eed8D147841AE162C8425c08BD8E1b41 + addresses.getAddress("DEPLOYER_EOA"), + 0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739 ); } @@ -172,7 +172,7 @@ contract TestAddresses is Test { function test_getChangedAddresses() public { address addr = vm.addr(1); - addresses.changeAddress("DEV_MULTISIG", addr, false); + addresses.changeAddress("DEPLOYER_EOA", addr, false); ( string[] memory names, uint256[] memory chainIds, @@ -198,7 +198,7 @@ contract TestAddresses is Test { function test_revertGetAddressChainZero() public { vm.expectRevert("ChainId cannot be 0"); - addresses.getAddress("DEV_MULTISIG", 0); + addresses.getAddress("DEPLOYER_EOA", 0); } function test_reverGetAddressNotSet() public { @@ -207,22 +207,22 @@ contract TestAddresses is Test { } function test_reverGetAddressNotSetOnChain() public { - vm.expectRevert("Address: DEV_MULTISIG not set on chain: 666"); - addresses.getAddress("DEV_MULTISIG", 666); + vm.expectRevert("Address: DEPLOYER_EOA not set on chain: 666"); + addresses.getAddress("DEPLOYER_EOA", 666); } function test_revertAddAddressAlreadySet() public { vm.expectRevert( - "Address with name: DEV_MULTISIG already set on chain: 31337" + "Address with name: DEPLOYER_EOA already set on chain: 31337" ); - addresses.addAddress("DEV_MULTISIG", vm.addr(1), false); + addresses.addAddress("DEPLOYER_EOA", vm.addr(1), false); } function test_revertAddAddressChainAlreadySet() public { vm.expectRevert( - "Address with name: DEV_MULTISIG already set on chain: 31337" + "Address with name: DEPLOYER_EOA already set on chain: 31337" ); - addresses.addAddress("DEV_MULTISIG", vm.addr(1), 31337, false); + addresses.addAddress("DEPLOYER_EOA", vm.addr(1), 31337, false); } function test_revertChangedAddressDoesNotExist() public { @@ -236,33 +236,33 @@ contract TestAddresses is Test { string memory addressesPath = "./addresses/AddressesDuplicated.json"; vm.expectRevert( - "Address with name: DEV_MULTISIG already set on chain: 31337" + "Address with name: DEPLOYER_EOA already set on chain: 31337" ); new Addresses(addressesPath); } function test_addAddressCannotBeZero() public { vm.expectRevert("Address cannot be 0"); - addresses.addAddress("DEV_MULTISIG", address(0), false); + addresses.addAddress("DEPLOYER_EOA", address(0), false); } function test_addAddressCannotBeZeroChainId() public { vm.expectRevert("ChainId cannot be 0"); - addresses.addAddress("DEV_MULTISIG", vm.addr(1), 0, false); + addresses.addAddress("DEPLOYER_EOA", vm.addr(1), 0, false); } function test_revertChangeAddressCannotBeZero() public { vm.expectRevert("Address cannot be 0"); - addresses.changeAddress("DEV_MULTISIG", address(0), false); + addresses.changeAddress("DEPLOYER_EOA", address(0), false); } function test_revertChangeAddresCannotBeZeroChainId() public { vm.expectRevert("ChainId cannot be 0"); - addresses.changeAddress("DEV_MULTISIG", vm.addr(1), 0, false); + addresses.changeAddress("DEPLOYER_EOA", vm.addr(1), 0, false); } function test_isContractFalse() public { - assertEq(addresses.isAddressContract("DEV_MULTISIG"), false); + assertEq(addresses.isAddressContract("DEPLOYER_EOA"), false); } function test_isContractTrue() public { @@ -296,8 +296,8 @@ contract TestAddresses is Test { } function addressIsNotPresentOnChain() public { - assertEq(addresses.isAddressSet("DEV_MULTISIG", 31337), true); - assertEq(addresses.isAddressSet("DEV_MULTISIG", 123), false); + assertEq(addresses.isAddressSet("DEPLOYER_EOA", 31337), true); + assertEq(addresses.isAddressSet("DEPLOYER_EOA", 123), false); } function test_checkAddressRevertIfNotContract() public { @@ -329,7 +329,7 @@ contract TestAddresses is Test { memory addressesPath = "./addresses/AddressesDuplicatedDifferentName.json"; vm.expectRevert( - "Address: 0x3dd46846eed8d147841ae162c8425c08bd8e1b41 already set on chain: 31337" + "Address: 0x9679e26bf0c470521de83ad77bb1bf1e7312f739 already set on chain: 31337" ); new Addresses(addressesPath); } diff --git a/test/BravoProposal.t.sol b/test/BravoProposal.t.sol new file mode 100644 index 00000000..52214c1e --- /dev/null +++ b/test/BravoProposal.t.sol @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.0; + +import {Test} from "@forge-std/Test.sol"; + +import {Addresses} from "@addresses/Addresses.sol"; +import {GovernorBravoProposal} from "@proposals/GovernorBravoProposal.sol"; +import {MockBravoProposal} from "@mocks/MockBravoProposal.sol"; + +contract BravoProposalIntegrationTest is Test { + Addresses public addresses; + GovernorBravoProposal public proposal; + + function setUp() public { + // Instantiate the Addresses contract + addresses = new Addresses("./addresses/Addresses.json"); + vm.makePersistent(address(addresses)); + + // Instantiate the BravoProposal contract + proposal = GovernorBravoProposal(new MockBravoProposal()); + + // Set the addresses contract + proposal.setAddresses(addresses); + + // Set the bravo address + proposal.setGovernor(addresses.getAddress("COMPOUND_GOVERNOR_BRAVO")); + } + + function test_setUp() public view { + assertEq( + proposal.name(), + string("ADJUST_WETH_IR_CURVE"), + "Wrong proposal name" + ); + assertEq( + proposal.description(), + string( + "Mock proposal that adjust IR Curve for Compound v3 WETH on Mainnet" + ), + "Wrong proposal description" + ); + assertEq( + address(proposal.governor()), + addresses.getAddress("COMPOUND_GOVERNOR_BRAVO"), + "Wrong governor address" + ); + } + + function test_build() public { + vm.expectRevert("No actions found"); + proposal.getProposalActions(); + + proposal.build(); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas + ) = proposal.getProposalActions(); + + address target = addresses.getAddress("COMPOUND_CONFIGURATOR"); + assertEq(targets.length, 2, "Wrong targets length"); + assertEq(targets[0], target, "Wrong target at index 0"); + assertEq(targets[1], target, "Wrong target at index 1"); + + uint256 expectedValue = 0; + assertEq(values.length, 2, "Wrong values length"); + assertEq(values[0], expectedValue, "Wrong value at index 0"); + assertEq(values[1], expectedValue, "Wrong value at index 1"); + + uint64 kink = 750000000000000000; + assertEq(calldatas.length, 2); + assertEq( + calldatas[0], + abi.encodeWithSignature( + "setBorrowKink(address,uint64)", + addresses.getAddress("COMPOUND_COMET"), + kink + ), + "Wrong calldata at index 0" + ); + + assertEq( + calldatas[1], + abi.encodeWithSignature( + "setSupplyKink(address,uint64)", + addresses.getAddress("COMPOUND_COMET"), + kink + ), + "Wrong calldata at index 1" + ); + } + + function test_simulate() public { + test_build(); + + proposal.simulate(); + + // check that proposal exists + assertTrue(proposal.checkOnChainCalldata()); + + proposal.validate(); + } + + function test_getCalldata() public { + test_build(); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas + ) = proposal.getProposalActions(); + + string[] memory signatures = new string[](targets.length); + + bytes memory expectedData = abi.encodeWithSignature( + "propose(address[],uint256[],string[],bytes[],string)", + targets, + values, + signatures, + calldatas, + proposal.description() + ); + + bytes memory data = proposal.getCalldata(); + + assertEq(data, expectedData, "Wrong propose calldata"); + } +} diff --git a/test/DuplicatedAction.t.sol b/test/DuplicatedAction.t.sol new file mode 100644 index 00000000..e3ca8c02 --- /dev/null +++ b/test/DuplicatedAction.t.sol @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.0; + +import {Test} from "@forge-std/Test.sol"; + +import {Addresses} from "@addresses/Addresses.sol"; +import {GovernorBravoProposal} from "@proposals/GovernorBravoProposal.sol"; +import {MockDuplicatedActionProposal} from "@mocks/MockDuplicatedActionProposal.sol"; + +contract DuplicatedActionProposalIntegrationTest is Test { + Addresses public addresses; + GovernorBravoProposal public proposal; + + function setUp() public { + // Instantiate the Addresses contract + addresses = new Addresses("./addresses/Addresses.json"); + vm.makePersistent(address(addresses)); + + // Instantiate the BravoProposal contract + proposal = GovernorBravoProposal(new MockDuplicatedActionProposal()); + + // Set the addresses contract + proposal.setAddresses(addresses); + + // Set the bravo address + proposal.setGovernor(addresses.getAddress("COMPOUND_GOVERNOR_BRAVO")); + } + + function test_build() public { + vm.expectRevert("No actions found"); + proposal.getProposalActions(); + + vm.expectRevert("Duplicated action found"); + proposal.build(); + } +} diff --git a/test/MultisigProposal.t.sol b/test/MultisigProposal.t.sol new file mode 100644 index 00000000..f6850a14 --- /dev/null +++ b/test/MultisigProposal.t.sol @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.0; + +import {Test} from "@forge-std/Test.sol"; + +import {Addresses} from "@addresses/Addresses.sol"; +import {MultisigProposal} from "@proposals/MultisigProposal.sol"; +import {MockMultisigProposal} from "@mocks/MockMultisigProposal.sol"; + +contract MultisigProposalIntegrationTest is Test { + Addresses public addresses; + MultisigProposal public proposal; + + struct Call3Value { + address target; + bool allowFailure; + uint256 value; + bytes callData; + } + + function setUp() public { + // Instantiate the Addresses contract + addresses = new Addresses("./addresses/Addresses.json"); + vm.makePersistent(address(addresses)); + + // Instantiate the MultisigProposal contract + proposal = MultisigProposal(new MockMultisigProposal()); + + // Set the addresses contract + proposal.setAddresses(addresses); + } + + function test_setUp() public view { + assertEq( + proposal.name(), + string("OPTMISM_MULTISIG_MOCK"), + "Wrong proposal name" + ); + assertEq( + proposal.description(), + string("Mock proposal that upgrade the L1 NFT Bridge"), + "Wrong proposal description" + ); + } + + function test_deploy() public { + vm.startPrank(addresses.getAddress("DEPLOYER_EOA")); + proposal.deploy(); + vm.stopPrank(); + + assertTrue( + addresses.isAddressSet("OPTIMISM_L1_NFT_BRIDGE_IMPLEMENTATION") + ); + } + + function test_build() public { + test_deploy(); + + vm.expectRevert("No actions found"); + proposal.getProposalActions(); + + proposal.build(); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas + ) = proposal.getProposalActions(); + + // check that the proposal targets are correct + assertEq(targets.length, 1, "Wrong targets length"); + assertEq( + targets[0], + addresses.getAddress("OPTIMISM_PROXY_ADMIN"), + "Wrong target at index 0" + ); + + // check that the proposal values are correct + assertEq(values.length, 1, "Wrong values length"); + assertEq(values[0], 0, "Wrong value at index 0"); + + // check that the proposal calldatas are correct + assertEq(calldatas.length, 1); + assertEq( + calldatas[0], + abi.encodeWithSignature( + "upgrade(address,address)", + addresses.getAddress("OPTIMISM_L1_NFT_BRIDGE_PROXY"), + addresses.getAddress("OPTIMISM_L1_NFT_BRIDGE_IMPLEMENTATION") + ), + "Wrong calldata at index 0" + ); + } + + function test_simulate() public { + test_build(); + + proposal.simulate(); + + proposal.validate(); + } + + function test_getCalldata() public { + test_build(); + + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas + ) = proposal.getProposalActions(); + + Call3Value[] memory calls = new Call3Value[](targets.length); + + for (uint256 i; i < calls.length; i++) { + calls[i] = Call3Value({ + target: targets[i], + allowFailure: false, + value: values[i], + callData: calldatas[i] + }); + } + + bytes memory expectedData = abi.encodeWithSignature( + "aggregate3Value((address,bool,uint256,bytes)[])", + calls + ); + + bytes memory data = proposal.getCalldata(); + + assertEq(data, expectedData, "Wrong aggregate calldata"); + } + + function test_checkOnChainCalldata() public { + vm.expectRevert("Not implemented"); + proposal.checkOnChainCalldata(); + } +}