Audit 2/scope

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

@@ -88,6 +88,87 @@ contract VotingEscrow is
 
     bool private _lockNFTSet;
 
+    /*//////////////////////////////////////////////////////////////
+                              Added: V2
+    //////////////////////////////////////////////////////////////*/
+
+    /// @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
+    );
+
+    /// @notice Emitted when the migrator is added, activating the migration
+    event MigrationEnabled(address migrator);
+
+    error MigrationAlreadySet();
+    error MigrationNotActive();
+
+    /// @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;
+        IERC20(token).approve(migrator, totalLocked);
+        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 (votingPower(_tokenId) == 0) revert CannotExit();
+
+        // the user should be approved
+        address owner = IERC721EMB(lockNFT).ownerOf(_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 +489,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/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();

test/escrow/escrow/Lock.t.sol

@@ -5,8 +5,12 @@ import {EscrowBase} from "./EscrowBase.sol";
 import {console2 as console} from "forge-std/console2.sol";
 import {IDAO} from "@aragon/osx/core/dao/IDAO.sol";
 import {DAO} from "@aragon/osx/core/dao/DAO.sol";
+import {DaoUnauthorized} from "@aragon/osx/core/utils/auth.sol";
 import {Multisig, MultisigSetup} from "@aragon/multisig/MultisigSetup.sol";
 
+import {IERC721EnumerableUpgradeable as IERC721Enumerable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/IERC721EnumerableUpgradeable.sol";
+import {IERC721MetadataUpgradeable as IERC721Metadata} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/IERC721MetadataUpgradeable.sol";
+
 import {ProxyLib} from "@libs/ProxyLib.sol";
 
 import {IEscrowCurveTokenStorage} from "@escrow-interfaces/IEscrowCurveIncreasing.sol";
@@ -31,12 +35,15 @@ contract TestLockMintBurn is EscrowBase, IEscrowCurveTokenStorage, IGaugeVote {
         assertEq(_nftLock.name(), _name);
         assertEq(_nftLock.symbol(), _symbol);
         assertEq(_nftLock.escrow(), _escrow);
+        assertEq(_nftLock.baseTokenURI(), "");
         assertEq(address(_nftLock.dao()), _dao);
     }
 
     function testSupportsInterface() public view {
         assertTrue(nftLock.supportsInterface(type(ILock).interfaceId));
         assertFalse(nftLock.supportsInterface(0xffffffff));
+        assertTrue(nftLock.supportsInterface(type(IERC721Metadata).interfaceId));
+        assertTrue(nftLock.supportsInterface(type(IERC721Enumerable).interfaceId));
     }
 
     function testFuzz_OnlyEscrowCanMint(address _notEscrow) public {
@@ -95,6 +102,54 @@ contract TestLockMintBurn is EscrowBase, IEscrowCurveTokenStorage, IGaugeVote {
         vm.expectRevert("revert");
         newLock.mint(address(reentrant), 1);
     }
+
+    function testSetNFTMetadata() public {
+        vm.prank(address(escrow));
+        nftLock.mint(address(123), 1);
+
+        assertEq(nftLock.tokenURI(1), "");
+
+        vm.prank(address(this));
+        nftLock.setBaseURI("https://example.com/");
+        assertEq(nftLock.baseTokenURI(), "https://example.com/");
+        assertEq(nftLock.tokenURI(1), "https://example.com/1");
+
+        vm.prank(address(this));
+        nftLock.setTokenURI(1, "?tokenId=1");
+        assertEq(nftLock.tokenURI(1), "https://example.com/?tokenId=1");
+
+        vm.prank(address(escrow));
+        nftLock.mint(address(123), 2);
+
+        assertEq(nftLock.tokenURI(2), "https://example.com/2");
+    }
+
+    function testOnlyOwnerCanSetNFTMetadata(address _notEscrow) public {
+        vm.assume(_notEscrow != address(this));
+
+        bytes memory data = abi.encodeWithSelector(
+            DaoUnauthorized.selector,
+            address(dao),
+            address(nftLock),
+            address(_notEscrow),
+            nftLock.LOCK_ADMIN_ROLE()
+        );
+
+        vm.prank(address(escrow));
+        nftLock.mint(address(123), 1);
+
+        vm.prank(_notEscrow);
+        vm.expectRevert(data);
+        nftLock.setBaseURI("https://example.com/");
+
+        assertEq(nftLock.baseTokenURI(), "");
+        assertEq(nftLock.tokenURI(1), "");
+
+        vm.prank(_notEscrow);
+        vm.expectRevert(data);
+        nftLock.setTokenURI(1, "?tokenId=1");
+        assertEq(nftLock.tokenURI(1), "");
+    }
 }
 
 contract NFTReentrant {

test/voting/GaugeVote.t.sol

@@ -77,15 +77,39 @@ contract TestGaugeVote is GaugeVotingBase {
         vm.expectRevert(VotingInactive.selector);
         voter.vote(0, votes);
 
-        // try to reset
-        vm.expectRevert(VotingInactive.selector);
-        voter.reset(0);
-
         // vote multiple
         vm.expectRevert(VotingInactive.selector);
         voter.voteMultiple(ids, votes);
     }
 
+    function testFuzz_canResetInDistributionPeriod() public {
+        // create the vote
+        votes.push(GaugeVote(1, gauge));
+
+        // vote
+        vm.startPrank(owner);
+        {
+            voter.vote(tokenId, votes);
+        }
+        vm.stopPrank();
+
+        // check the vote
+        assertEq(voter.isVoting(tokenId), true);
+
+        // warp to the next distribution period
+        vm.warp(block.timestamp + 1 weeks);
+        vm.assume(!voter.votingActive());
+
+        // try to reset
+        vm.startPrank(owner);
+        {
+            voter.reset(tokenId);
+        }
+        vm.stopPrank();
+
+        assertEq(voter.isVoting(tokenId), false);
+    }
+
     // can't vote if you don't own the token
     function testCannotVoteIfYouDontOwnTheToken() public {
         // try to vote as this address (not the holder)