Merge pull request #17 from liquity/dev

Audit Fixes - System Changes - Code Refactoring - Mega Merge

.gitignore

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

.gitmodules

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

Invariants.MD

@@ -0,0 +1,10 @@
+Snapshot Solvency
+
+            uint256 claim = _votesForInitiativeSnapshot.votes * boldAccrued / _votesSnapshot.votes;
+For each initiative this is what the value is
+If the initiative is "Claimable" this is what it receives
+The call never reverts
+The sum of claims is less than the boldAccrued
+
+Veto consistency
+

README.md

@@ -44,48 +44,42 @@ In order to unstake and withdraw LQTY, a User must first deallocate a sufficient
 Initiative can be added permissionlessly, requiring the payment of a 100 BOLD fee, and in the following epoch become active
 for voting. During each snapshot, Initiatives which received as sufficient number of Votes that their incentive payout equals
 at least 500 BOLD, will be eligible to Claim ("minimum qualifying threshold"). Initiatives failing to meet the minimum qualifying threshold will not qualify to claim for that epoch.
-Initiatives failing to meet the minimum qualifying threshold for a claim during four consecutive epochs may be deregistered permissionlessly, requiring
-reregistration to become eligible for voting again.
+Initiatives failing to meet the minimum qualifying threshold for a claim during four consecutive epochs may be deregistered permissionlessly, requiring reregistration to become eligible for voting again.
 
-Claims for Initiatives which have met the minimum qualifying threshold, can be claimed permissionlessly, but must be claimed by the end of the epoch
-in which they are awarded. Failure to do so will result in the unclaimed portion being reused in the following epoch.
+Claims for Initiatives which have met the minimum qualifying threshold, can be claimed permissionlessly, but must be claimed by the end of the epoch 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.
+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.
+
+### 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
 
 To facilitate the development of liquidity for BOLD and other relevant tokens after the launch of Liquity v2, initial example initiatives will be added.
-They will be available from the first epoch in which claims are available (epoch 1), added in the construtor. Following epoch 1, these examples have
-no further special status and can be removed by LQTY voters
+They will be available from the first epoch in which claims are available (epoch 1), added in the construtor. Following epoch 1, these examples have no further special status and can be removed by LQTY voters
 
 ### Curve v2
 
@@ -95,3 +89,40 @@ 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`
+
+## Testing
+
+To run foundry, just 
+```
+forge test
+```
+
+
+Please note the `TrophiesToFoundry`, which are repros of broken invariants, left failing on purpose
+
+### Invariant Testing
+
+We had a few issues with Medusa due to the use of `vm.warp`, we recommend using Echidna
+
+Run echidna with:
+
+```
+echidna . --contract CryticTester --config echidna.yaml
+```
+
+You can also run Echidna on Recon by simply pasting the URL of the Repo / Branch

ToFix.MD

@@ -0,0 +1,31 @@
+- Add properties check to ensure that the math is sound <- HUGE, let's add it now
+
+A vote is: User TS * Votes
+So an allocation should use that
+We need to remove the data from the valid allocation
+And not from a random one
+
+I think the best test is to simply store the contribution done
+And see whether removing it is idempotent
+
+We would need a ton of work to make it even better
+
+
+Specifically, if a user removes their votes, we need to see that reflect correctly
+Because that's key
+
+- From there, try fixing with a reset on deposit and withdraw
+
+- Add a test that checks every: initiative, user allocation, ensure they are zero after a deposit and a withdrawal
+- Add a test that checks every: X, ensure they use the correct TS
+
+- From there, reason around the deeper rounding errors
+
+
+
+Optimizations
+Put the data in the storage
+Remove all castings that are not safe
+Invariant test it
+
+--

echidna.yaml

@@ -0,0 +1,10 @@
+testMode: "property"
+prefix: "optimize_"
+coverage: true
+corpusDir: "echidna"
+balanceAddr: 0x1043561a8829300000
+balanceContract: 0x1043561a8829300000
+filterFunctions: []
+cryticArgs: ["--foundry-compile-all"]
+
+shrinkLimit: 100000

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,4 @@
 v4-core/=lib/v4-core/
+forge-std/=lib/forge-std/src/
+@chimera/=lib/chimera/src/
+openzeppelin/=lib/openzeppelin-contracts/

script/DeploySepolia.s.sol

@@ -92,10 +92,12 @@ contract DeploySepoliaScript is Script, Deployers {
                 votingThresholdFactor: VOTING_THRESHOLD_FACTOR,
                 minClaim: MIN_CLAIM,
                 minAccrual: MIN_ACCRUAL,
-                epochStart: uint32(block.timestamp),
+                epochStart: uint32(block.timestamp - VESTING_EPOCH_START),
+                /// @audit Ensures that `initialInitiatives` can be voted on
                 epochDuration: EPOCH_DURATION,
                 epochVotingCutoff: EPOCH_VOTING_CUTOFF
             }),
+            deployer,
             initialInitiatives
         );
         assert(governance == uniV4Donations.governance());