Merge pull request #16 from liquity/dev-governance-refactor

Dev governance refactor

.gitignore

@@ -10,3 +10,8 @@ docs/
 
 # Dotenv file
 .env
+
+# Fuzzing
+crytic-export/
+echidna/
+medusa/

.gitmodules

@@ -7,3 +7,6 @@
 [submodule "lib/v4-core"]
 	path = lib/v4-core
 	url = https://github.com/Uniswap/v4-core
+[submodule "lib/chimera"]
+	path = lib/chimera
+	url = https://github.com/Recon-Fuzz/chimera

INTEGRATION.MD

@@ -0,0 +1,12 @@
+# Risks to integrators
+
+Somebody could claim on your behalf
+
+Votes not meeting the threshold may result in 0 rewards
+
+Claiming more than once will return 0
+
+## INVARIANT: You can only claim for previous epoch
+
+assert(votesSnapshot_.forEpoch == epoch() - 1); /// @audit INVARIANT: You can only claim for previous epoch
+/// All unclaimed rewards are always recycled

README.md

@@ -51,35 +51,33 @@ Claims for Initiatives which have met the minimum qualifying threshold, can be c
 in which they are awarded. Failure to do so will result in the unclaimed portion being reused in the following epoch.
 
 As Initiatives are assigned to arbitrary addresses, they can be used for any purpose, including EOAs, Multisigs, or smart contracts designed
-for targetted purposes. Smart contracts should be designed in a way that they can support BOLD and include any additional logic about
-how BOLD is to be used.
+for targetted purposes. Smart contracts should be designed in a way that they can support BOLD and include any additional logic about how BOLD is to be used.
+
+### Malicious Initiatives
+
+It's important to note that initiatives could be malicious, and the system does it's best effort to prevent any DOS to happen, however, a malicious initiative could drain all rewards if voted on.
 
 ## Voting
 
-Users with LQTY staked in Governance.sol, can allocate LQTY in the same epoch in which they were deposited. But the
-effective voting power at that point would be insignificant.
+Users with LQTY staked in Governance.sol, can allocate LQTY in the same epoch in which they were deposited. But the effective voting power at that point would be insignificant.
 
 Votes can take two forms, a vote for an Initiative or a veto vote. Initiatives which have received vetoes which are both:
 three times greater than the minimum qualifying threshold, and greater than the number of votes for will not be eligible for claims by being excluded from the vote count and maybe deregistered as an Initiative.
 
 Users may split their votes for and veto votes across any number of initiatives. But cannot vote for and veto vote the same Initiative.
 
-Each epoch is split into two parts, a six day period where both votes for and veto votes take place, and a final 24 hour period where votes
-can only be made as veto votes. This is designed to give a period where any detrimental distributions can be mitigated should there be
-sufficient will to do so by voters, but is not envisaged to be a regular occurance.
+Each epoch is split into two parts, a six day period where both votes for and veto votes take place, and a final 24 hour period where votes can only be made as veto votes. This is designed to give a period where any detrimental distributions can be mitigated should there be sufficient will to do so by voters, but is not envisaged to be a regular occurance.
 
 ## Snapshots
 
 Snapshots of results from the voting activity of an epoch takes place on an initiative by initiative basis in a permissionless manner.
-User interactions or direct calls following the closure of an epoch trigger the snapshot logic which makes a Claim available to a
-qualifying Initiative.
+User interactions or direct calls following the closure of an epoch trigger the snapshot logic which makes a Claim available to a qualifying Initiative.
 
 ## Bribing
 
 LQTY depositors can also receive bribes in the form of ERC20s in exchange for voting for a specified initiative.
 This is done externally to the Governance.sol logic and should be implemented at the initiative level.
-BaseInitiative.sol is a reference implementation which allows for bribes to be set and paid in BOLD + another token,
-all claims for bribes are made by directly interacting with the implemented BaseInitiative contract.
+BaseInitiative.sol is a reference implementation which allows for bribes to be set and paid in BOLD + another token, all claims for bribes are made by directly interacting with the implemented BaseInitiative contract.
 
 ## Example Initiatives
 
@@ -95,3 +93,18 @@ Claiming and depositing to gauges must be done manually after each epoch in whic
 ### Uniswap v4
 
 Simple hook for Uniswap v4 which implements a donate to a preconfigured pool. Allowing for adjustments to liquidity positions to make Claims which are smoothed over a vesting epoch.
+
+## Known Issues
+
+### Vetoed Initiatives and Initiatives that receive votes that are below the treshold cause a loss of emissions to the voted initiatives
+
+Because the system counts: valid_votes / total_votes
+By definition, initiatives that increase the  total_votes without receiving any rewards are stealing the rewards from other initiatives
+
+The rewards will be re-queued in the next epoch
+
+see: `test_voteVsVeto` as well as the miro and comments
+
+### User Votes, Initiative Votes and Global State Votes can desynchronize
+
+See `test_property_sum_of_lqty_global_user_matches_0`

echidna.yaml

@@ -0,0 +1,10 @@
+testMode: "assertion"
+prefix: "crytic_"
+coverage: true
+corpusDir: "echidna"
+balanceAddr: 0x1043561a8829300000
+balanceContract: 0x1043561a8829300000
+filterFunctions: []
+cryticArgs: ["--foundry-compile-all"]
+testMode: "exploration"
+testLimit: 500000000

medusa.json

@@ -0,0 +1,88 @@
+{
+    "fuzzing": {
+      "workers": 10,
+      "workerResetLimit": 50,
+      "timeout": 0,
+      "testLimit": 0,
+      "callSequenceLength": 100,
+      "corpusDirectory": "medusa",
+      "coverageEnabled": true,
+      "deploymentOrder": [
+        "CryticTester"
+      ],
+      "targetContracts": [
+        "CryticTester"
+      ],
+      "targetContractsBalances": [
+        "0x27b46536c66c8e3000000"
+      ],
+      "constructorArgs": {},
+      "deployerAddress": "0x30000",
+      "senderAddresses": [
+        "0x10000",
+        "0x20000",
+        "0x30000"
+      ],
+      "blockNumberDelayMax": 60480,
+      "blockTimestampDelayMax": 604800,
+      "blockGasLimit": 125000000,
+      "transactionGasLimit": 12500000,
+      "testing": {
+        "stopOnFailedTest": false,
+        "stopOnFailedContractMatching": false,
+        "stopOnNoTests": true,
+        "testAllContracts": false,
+        "traceAll": false,
+        "assertionTesting": {
+          "enabled": true,
+          "testViewMethods": true,
+          "panicCodeConfig": {
+            "failOnCompilerInsertedPanic": false,
+            "failOnAssertion": true,
+            "failOnArithmeticUnderflow": false,
+            "failOnDivideByZero": false,
+            "failOnEnumTypeConversionOutOfBounds": false,
+            "failOnIncorrectStorageAccess": false,
+            "failOnPopEmptyArray": false,
+            "failOnOutOfBoundsArrayAccess": false,
+            "failOnAllocateTooMuchMemory": false,
+            "failOnCallUninitializedVariable": false
+          }
+        },
+        "propertyTesting": {
+          "enabled": true,
+          "testPrefixes": [
+            "crytic_"
+          ]
+        },
+        "optimizationTesting": {
+          "enabled": false,
+          "testPrefixes": [
+            "optimize_"
+          ]
+        }
+      },
+      "chainConfig": {
+        "codeSizeCheckDisabled": true,
+        "cheatCodes": {
+          "cheatCodesEnabled": true,
+          "enableFFI": false
+        }
+      }
+    },
+    "compilation": {
+      "platform": "crytic-compile",
+      "platformConfig": {
+        "target": ".",
+        "solcVersion": "",
+        "exportDirectory": "",
+        "args": [
+          "--foundry-compile-all"
+        ]
+      }
+    },
+    "logging": {
+      "level": "info",
+      "logDirectory": ""
+    }
+  }

remappings.txt

@@ -1 +1,3 @@
 v4-core/=lib/v4-core/
+forge-std/=lib/forge-std/src/
+@chimera/=lib/chimera/src/

src/BribeInitiative.sol

@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.24;
 
-import {console} from "forge-std/console.sol";
-
 import {IERC20} from "openzeppelin-contracts/contracts/interfaces/IERC20.sol";
 import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
 
@@ -12,6 +10,10 @@ import {IBribeInitiative} from "./interfaces/IBribeInitiative.sol";
 
 import {DoubleLinkedList} from "./utils/DoubleLinkedList.sol";
 
+
+import {EncodingDecodingLib} from "src/utils/EncodingDecodingLib.sol";
+
+
 contract BribeInitiative is IInitiative, IBribeInitiative {
     using SafeERC20 for IERC20;
     using DoubleLinkedList for DoubleLinkedList.List;
@@ -56,18 +58,19 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
 
     /// @inheritdoc IBribeInitiative
     function depositBribe(uint128 _boldAmount, uint128 _bribeTokenAmount, uint16 _epoch) external {
-        bold.safeTransferFrom(msg.sender, address(this), _boldAmount);
-        bribeToken.safeTransferFrom(msg.sender, address(this), _bribeTokenAmount);
 
         uint16 epoch = governance.epoch();
-        require(_epoch > epoch, "BribeInitiative: only-future-epochs");
+        require(_epoch >= epoch, "BribeInitiative: only-future-epochs");
 
         Bribe memory bribe = bribeByEpoch[_epoch];
         bribe.boldAmount += _boldAmount;
         bribe.bribeTokenAmount += _bribeTokenAmount;
         bribeByEpoch[_epoch] = bribe;
 
         emit DepositBribe(msg.sender, _boldAmount, _bribeTokenAmount, _epoch);
+
+        bold.safeTransferFrom(msg.sender, address(this), _boldAmount);
+        bribeToken.safeTransferFrom(msg.sender, address(this), _bribeTokenAmount);
     }
 
     function _claimBribe(
@@ -76,7 +79,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
         uint16 _prevLQTYAllocationEpoch,
         uint16 _prevTotalLQTYAllocationEpoch
     ) internal returns (uint256 boldAmount, uint256 bribeTokenAmount) {
-        require(_epoch != governance.epoch(), "BribeInitiative: cannot-claim-for-current-epoch");
+        require(_epoch < governance.epoch(), "BribeInitiative: cannot-claim-for-current-epoch");
         require(!claimedBribeAtEpoch[_user][_epoch], "BribeInitiative: already-claimed");
 
         Bribe memory bribe = bribeByEpoch[_epoch];
@@ -164,8 +167,11 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
         emit ModifyLQTYAllocation(_user, _epoch, _lqty, _averageTimestamp);
     }
 
+    function _encodeLQTYAllocation(uint88 _lqty, uint32 _averageTimestamp) private pure returns (uint224) {
+        return EncodingDecodingLib.encodeLQTYAllocation(_lqty, _averageTimestamp);
+    }
     function _decodeLQTYAllocation(uint224 _value) private pure returns (uint88, uint32) {
-        return (uint88(_value >> 32), uint32(_value));
+        return EncodingDecodingLib.decodeLQTYAllocation(_value);
     }
 
     function _loadTotalLQTYAllocation(uint16 _epoch) private view returns (uint88, uint32) {

src/ForwardBribe.sol

@@ -23,7 +23,7 @@ contract ForwardBribe is BribeInitiative {
         uint boldAmount = bold.balanceOf(address(this));
         uint bribeTokenAmount = bribeToken.balanceOf(address(this));
 
-        if (boldAmount != 0) bold.safeTransfer(receiver, boldAmount);
-        if (bribeTokenAmount != 0) bribeToken.safeTransfer(receiver, bribeTokenAmount);
+        if (boldAmount != 0) bold.transfer(receiver, boldAmount);
+        if (bribeTokenAmount != 0) bribeToken.transfer(receiver, bribeTokenAmount);
     }
 }