Audit 2/scope

audits/AUDIT_2.md

@@ -0,0 +1,18 @@
+# Notes for the auditors for the second audit
+
+The changes in the second audit are a relatively small set of contract changes. Overall there are 2 things we are looking to address:
+
+1. Faciliate the migration from old to new staking contract, to reset the state.
+
+For DAOs wishing to start new governance 'seasons', there is a requirement for voting power to be reset. The changes to the Voting Escrow contract are intended to reflect this:
+
+- A new set of contracts will be deployed
+- Direct migration will be enabled between the old and new contracts
+- Existing stakers should be able to migrate in a single transaction, skipping the exit queue mechanics
+- Existing stakers can migrate even if the destination staking contract is locked for new stakers, this gives existing stakers a window to migrate and gain voting power ahead of new entrants.
+
+2. Add small improvements, these are detailed in the following contracts
+
+- a: lock, add NFT metadata URI
+- b: voting contract: permit resets outside of voting windows so users can unstake more easily
+- c: move from a quadratic -> linear voting curve

src/escrow/increasing/Lock.sol

@@ -2,14 +2,23 @@
 pragma solidity ^0.8.17;
 
 import {ILock} from "@escrow-interfaces/ILock.sol";
+import {ERC721Upgradeable as ERC721} from "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
 import {ERC721EnumerableUpgradeable as ERC721Enumerable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
+import {ERC721URIStorageUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721URIStorageUpgradeable.sol";
 import {ReentrancyGuardUpgradeable as ReentrancyGuard} from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
 import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import {DaoAuthorizableUpgradeable as DaoAuthorizable} from "@aragon/osx/core/plugin/dao-authorizable/DaoAuthorizableUpgradeable.sol";
 import {IDAO} from "@aragon/osx/core/dao/IDAO.sol";
 
 /// @title NFT representation of an escrow locking mechanism
-contract Lock is ILock, ERC721Enumerable, UUPSUpgradeable, DaoAuthorizable, ReentrancyGuard {
+contract Lock is
+    ILock,
+    ERC721Enumerable,
+    UUPSUpgradeable,
+    DaoAuthorizable,
+    ReentrancyGuard,
+    ERC721URIStorageUpgradeable
+{
     /// @dev enables transfers without whitelisting
     address public constant WHITELIST_ANY_ADDRESS =
         address(uint160(uint256(keccak256("WHITELIST_ANY_ADDRESS"))));
@@ -23,6 +32,8 @@ contract Lock is ILock, ERC721Enumerable, UUPSUpgradeable, DaoAuthorizable, Reen
     /// @notice Whitelisted contracts that are allowed to transfer
     mapping(address => bool) public whitelisted;
 
+    string public baseTokenURI;
+
     /*//////////////////////////////////////////////////////////////
                               Modifiers
     //////////////////////////////////////////////////////////////*/
@@ -38,7 +49,7 @@ contract Lock is ILock, ERC721Enumerable, UUPSUpgradeable, DaoAuthorizable, Reen
 
     function supportsInterface(
         bytes4 _interfaceId
-    ) public view override(ERC721Enumerable) returns (bool) {
+    ) public view override(ERC721Enumerable, ERC721URIStorageUpgradeable) returns (bool) {
         return super.supportsInterface(_interfaceId) || _interfaceId == type(ILock).interfaceId;
     }
 
@@ -83,6 +94,16 @@ contract Lock is ILock, ERC721Enumerable, UUPSUpgradeable, DaoAuthorizable, Reen
         emit WhitelistSet(WHITELIST_ANY_ADDRESS, true);
     }
 
+    /// @notice Override the beforeTokenTransfer as required by ERC721Enumerable
+    function _beforeTokenTransfer(
+        address _from,
+        address _to,
+        uint256 _tokenId,
+        uint256 batchSize
+    ) internal override(ERC721, ERC721Enumerable) {
+        super._beforeTokenTransfer(_from, _to, _tokenId, batchSize);
+    }
+
     /// @dev Override the transfer to check if the recipient is whitelisted
     /// This avoids needing to check for mint/burn but is less idomatic than beforeTokenTransfer
     function _transfer(address _from, address _to, uint256 _tokenId) internal override {
@@ -110,6 +131,30 @@ contract Lock is ILock, ERC721Enumerable, UUPSUpgradeable, DaoAuthorizable, Reen
         _burn(_tokenId);
     }
 
+    function _burn(uint256 _tokenId) internal override(ERC721, ERC721URIStorageUpgradeable) {
+        super._burn(_tokenId);
+    }
+
+    function tokenURI(
+        uint256 _tokenId
+    ) public view override(ERC721, ERC721URIStorageUpgradeable) returns (string memory) {
+        return super.tokenURI(_tokenId);
+    }
+
+    function _baseURI() internal view override returns (string memory) {
+        return baseTokenURI;
+    }
+
+    function setBaseURI(string memory _baseTokenURI) external auth(LOCK_ADMIN_ROLE) {
+        baseTokenURI = _baseTokenURI;
+
+        emit BaseURISet(baseTokenURI);
+    }
+
+    function setTokenURI(uint256 _tokenId, string memory _tokenURI) external auth(LOCK_ADMIN_ROLE) {
+        _setTokenURI(_tokenId, _tokenURI);
+    }
+
     /*//////////////////////////////////////////////////////////////
                               UUPS Upgrade
     //////////////////////////////////////////////////////////////*/
@@ -123,5 +168,5 @@ contract Lock is ILock, ERC721Enumerable, UUPSUpgradeable, DaoAuthorizable, Reen
     /// @notice Internal method authorizing the upgrade of the contract via the [upgradeability mechanism for UUPS proxies](https://docs.openzeppelin.com/contracts/4.x/api/proxy#UUPSUpgradeable) (see [ERC-1822](https://eips.ethereum.org/EIPS/eip-1822)).
     function _authorizeUpgrade(address) internal virtual override auth(LOCK_ADMIN_ROLE) {}
 
-    uint256[48] private __gap;
+    uint256[47] private __gap;
 }

src/escrow/increasing/VotingEscrowIncreasing.sol

@@ -13,6 +13,7 @@ import {IClock} from "@clock/IClock.sol";
 import {IEscrowCurveIncreasing as IEscrowCurve} from "./interfaces/IEscrowCurveIncreasing.sol";
 import {IExitQueue} from "./interfaces/IExitQueue.sol";
 import {IVotingEscrowIncreasing as IVotingEscrow} from "./interfaces/IVotingEscrowIncreasing.sol";
+import {IMigrateable} from "./interfaces/IMigrateable.sol";
 
 // libraries
 import {SafeERC20Upgradeable as SafeERC20} from "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
@@ -29,7 +30,8 @@ contract VotingEscrow is
     ReentrancyGuard,
     Pausable,
     DaoAuthorizable,
-    UUPSUpgradeable
+    UUPSUpgradeable,
+    IMigrateable
 {
     using SafeERC20 for IERC20;
     using SafeCast for uint256;
@@ -88,6 +90,76 @@ contract VotingEscrow is
 
     bool private _lockNFTSet;
 
+    /*//////////////////////////////////////////////////////////////
+                              Added: V2
+    //////////////////////////////////////////////////////////////*/
+
+    /// @notice The destination staking contract can add this to allow another address to call
+    /// the migrate function on it
+    bytes32 public constant MIGRATOR_ROLE = keccak256("MIGRATOR");
+
+    /// @notice destination migration contract
+    address public migrator;
+
+    /// @notice The Escrow Admin can enable migrations by setting a destination migration contract.
+    /// @dev This function also approves all tokens in this contract to be transferred to the new migrator.
+    /// @param _migrator The address of the destination migration contract
+    function enableMigration(address _migrator) external auth(ESCROW_ADMIN_ROLE) {
+        if (migrator != address(0)) revert MigrationAlreadySet();
+        migrator = _migrator;
+        // we approve max in the event that new deposits happen
+        IERC20(token).approve(migrator, type(uint256).max);
+        emit MigrationEnabled(_migrator);
+    }
+
+    /// @notice Defined on the staking contract being exited from - burn the tokenId and mint a new one.
+    /// @dev Skips withdrawal queue logic and vote resets
+    /// @param _tokenId veNFT to migrate from
+    /// @return newTokenId veNFT created during the migrationg
+    function migrateFrom(uint256 _tokenId) external returns (uint256 newTokenId) {
+        // check the migration contract is set and the tokenid is active
+        if (migrator == address(0)) revert MigrationNotActive();
+        if (!IERC721EMB(lockNFT).isApprovedOrOwner(_msgSender(), _tokenId)) revert NotOwner();
+        if (votingPower(_tokenId) == 0) revert CannotExit();
+
+        // the user should be approved
+        address owner = IERC721EMB(lockNFT).ownerOf(_tokenId);
+
+        // reset votes from voting contract
+        if (isVoting(_tokenId)) {
+            ISimpleGaugeVoter(voter).reset(_tokenId);
+        }
+
+        LockedBalance memory oldLocked = _locked[_tokenId];
+        uint256 value = oldLocked.amount;
+
+        // burn the current veNFT and write a zero checkpoint.
+        _locked[_tokenId] = LockedBalance(0, 0);
+        totalLocked -= value;
+        _checkpointClear(_tokenId);
+        IERC721EMB(lockNFT).burn(_tokenId);
+
+        // createLockFor on the new contract for the owner of the veNFT
+        newTokenId = VotingEscrow(migrator).migrateTo(value, owner);
+
+        // emit the migrated event
+        emit Migrated(owner, _tokenId, newTokenId, value);
+
+        return newTokenId;
+    }
+
+    /// @notice Defined on the destination staking contract. Creates a new veNFT with the old params
+    /// @dev Skips validations like pause, allowing migration ahead of general release.
+    /// @param _value The amount of underlying token to be migrated.
+    /// @param _for The original owner of the lock
+    /// @return newTokenId the veNFT on the destination staking contract
+    function migrateTo(
+        uint256 _value,
+        address _for
+    ) external nonReentrant auth(MIGRATOR_ROLE) returns (uint256 newTokenId) {
+        return _createLockFor(_value, _for);
+    }
+
     /*//////////////////////////////////////////////////////////////
                               Initialization
     //////////////////////////////////////////////////////////////*/
@@ -408,5 +480,6 @@ contract VotingEscrow is
     function _authorizeUpgrade(address) internal virtual override auth(ESCROW_ADMIN_ROLE) {}
 
     /// @dev Reserved storage space to allow for layout changes in the future.
-    uint256[39] private __gap;
+    /// @dev V2: -1 slot for migrator contract
+    uint256[38] private __gap;
 }

src/escrow/increasing/interfaces/ILock.sol

@@ -1,6 +1,16 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
+/*///////////////////////////////////////////////////////////////
+                        METADATA
+//////////////////////////////////////////////////////////////*/
+
+interface IMetadataEvents {
+    /// @notice Event emmited when the base metadata URI is updated
+    /// @param uri New base URI
+    event BaseURISet(string uri);
+}
+
 /*///////////////////////////////////////////////////////////////
                         WHITELIST
 //////////////////////////////////////////////////////////////*/
@@ -21,7 +31,9 @@ interface IWhitelist is IWhitelistEvents, IWhitelistErrors {
     function whitelisted(address addr) external view returns (bool);
 }
 
-interface ILock is IWhitelist {
+interface IMetadata is IMetadataEvents {}
+
+interface ILock is IWhitelist, IMetadata {
     error OnlyEscrow();
 
     /// @notice Address of the escrow contract that holds underyling assets

src/escrow/increasing/interfaces/IMigrateable.sol

@@ -0,0 +1,34 @@
+// SPDX License Identifier: AGPL-3.0 or later
+
+pragma solidity ^0.8.17;
+
+interface IMigrateableFrom {
+    function migrator() external view returns (address);
+    function enableMigration(address _migrator) external;
+    function migrateFrom(uint256 _tokenId) external returns (uint256 newTokenId);
+}
+
+interface IMigrateableTo {
+    function migrateTo(uint256 _value, address _for) external returns (uint256 newTokenId);
+}
+
+interface IMigrateableEventsAndErrors {
+    /// @notice Emitted when the migrator is added, activating the migration
+    event MigrationEnabled(address migrator);
+
+    /// @notice Emitted when the user migrates to the new destination contract
+    /// @param owner The owner of the veNFT at the time of the migration
+    /// @param oldTokenId TokenId burned in the old staking contract
+    /// @param newTokenId TokenId minted in the new staking contract
+    /// @param amount The locked amount migrated between contracts
+    event Migrated(
+        address indexed owner,
+        uint256 indexed oldTokenId,
+        uint256 indexed newTokenId,
+        uint256 amount
+    );
+    error MigrationAlreadySet();
+    error MigrationNotActive();
+}
+
+interface IMigrateable is IMigrateableFrom, IMigrateableTo, IMigrateableEventsAndErrors {}

src/libs/CurveConstantLib.sol

@@ -3,19 +3,14 @@ pragma solidity ^0.8.0;
 
 /// @title CurveConstantLib
 /// @notice Precomputed coefficients for escrow curve
-/// This curve implementation is a quadratic curve of the form y = (1/7)t^2 + (2/7)t + 1
-/// Which is a transformation of the quadratic curve y = (x^2 + 6)/7
-/// That starts with 1 unit of voting in period 1, and max 6 in period 6.
-/// To use this in zero indexed time, with a per-second rate of increase,
-/// we transform this to the polynomial y = (1/7)t^2 + (2/7)t + 1
-/// where t = timestamp / 2_weeks (2 weeks is one period)
-/// Below are the shared coefficients for the linear and quadratic terms
+/// That starts with 1 unit of voting in period 1, and max 6 in period 6, increasing linearly
 library CurveConstantLib {
     int256 internal constant SHARED_CONSTANT_COEFFICIENT = 1e18;
-    /// @dev 2 / (7 * 2_weeks) - expressed in fixed point
-    int256 internal constant SHARED_LINEAR_COEFFICIENT = 236205593348;
-    /// @dev 1 / (7 * (2_weeks)^2) - expressed in fixed point
-    int256 internal constant SHARED_QUADRATIC_COEFFICIENT = 97637;
+    /// @dev rate of increase per second expressed in fixed point
+    int256 internal constant SHARED_LINEAR_COEFFICIENT = 826719576719;
+
+    /// @dev linear curve with zero quadratic term
+    int256 internal constant SHARED_QUADRATIC_COEFFICIENT = 0;
 
     /// @dev the maxiumum number of epochs the cure can keep increasing
     uint256 internal constant MAX_EPOCHS = 5;

src/voting/SimpleGaugeVoter.sol

@@ -81,7 +81,7 @@ contract SimpleGaugeVoter is
     }
 
     /*///////////////////////////////////////////////////////////////
-                               Voting 
+                               Voting
     //////////////////////////////////////////////////////////////*/
 
     /// @notice extrememly simple for loop. We don't need reentrancy checks in this implementation
@@ -181,7 +181,7 @@ contract SimpleGaugeVoter is
         voteData.lastVoted = block.timestamp;
     }
 
-    function reset(uint256 _tokenId) external nonReentrant whenNotPaused whenVotingActive {
+    function reset(uint256 _tokenId) external nonReentrant whenNotPaused {
         if (!IVotingEscrow(escrow).isApprovedOrOwner(msg.sender, _tokenId))
             revert NotApprovedOrOwner();
         if (!isVoting(_tokenId)) revert NotCurrentlyVoting();