Merge branch 'main' into audit

.gitmodules

@@ -19,3 +19,6 @@
 [submodule "lib/create3-factory"]
 	path = lib/create3-factory
 	url = https://github.com/zeframlou/create3-factory
+[submodule "lib/openzeppelin-foundry-upgrades"]
+	path = lib/openzeppelin-foundry-upgrades
+	url = https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades

README.md

@@ -10,7 +10,7 @@ These contracts are only a piece of a larger decentralized system that will allo
 
 - PTokens are tokenized versions of points
 - They will be redeemable for rewards after rewards have been distributed
-- They will map 1:1 with the points earned by user assets depsoited into the vault
+- They will map 1:1 with the points earned by user assets deposited into the vault
 - The vault has ownership and can grant minting/burning permissions to other contracts for future system expansion
 
 ### On The Off-Chain Dependencies

contracts/PointTokenVault.sol

@@ -26,7 +26,7 @@ contract PointTokenVault is UUPSUpgradeable, AccessControlUpgradeable, Multicall
     bytes32 public constant MERKLE_UPDATER_ROLE = keccak256("MERKLE_UPDATER_ROLE");
     bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE");
 
-    // Deposit asset balancess.
+    // Deposit asset balances.
     mapping(address => mapping(ERC20 => uint256)) public balances; // user => point-earning token => balance
 
     // Merkle root distribution.
@@ -40,7 +40,6 @@ contract PointTokenVault is UUPSUpgradeable, AccessControlUpgradeable, Multicall
     mapping(bytes32 => RedemptionParams) public redemptions; // pointsId => redemptionParams
 
     mapping(address => uint256) public caps; // asset => deposit cap
-    bool public isCapped;
 
     struct Claim {
         bytes32 pointsId;
@@ -55,24 +54,28 @@ contract PointTokenVault is UUPSUpgradeable, AccessControlUpgradeable, Multicall
         bool isMerkleBased;
     }
 
-    event Deposit(address indexed receiver, address indexed token, uint256 amount);
-    event Withdraw(address indexed user, address indexed token, uint256 amount);
+    event Deposit(address indexed depositor, address indexed receiver, address indexed token, uint256 amount);
+    event Withdraw(address indexed withdrawer, address indexed receiver, address indexed token, uint256 amount);
     event RootUpdated(bytes32 prevRoot, bytes32 newRoot);
     event PTokensClaimed(address indexed account, bytes32 indexed pointsId, uint256 amount);
     event RewardsClaimed(address indexed owner, address indexed receiver, bytes32 indexed pointsId, uint256 amount);
+    event RewardsConverted(address indexed owner, address indexed receiver, bytes32 indexed pointsId, uint256 amount);
     event RewardRedemptionSet(
         bytes32 indexed pointsId, ERC20 rewardToken, uint256 rewardsPerPToken, bool isMerkleBased
     );
     event PTokenDeployed(bytes32 indexed pointsId, address indexed pToken);
-    event CapSet(address indexed token, uint256 cap);
+    event CapSet(address indexed token, uint256 prevCap, uint256 cap);
 
     error ProofInvalidOrExpired();
     error ClaimTooLarge();
     error RewardsNotReleased();
+    error CantConvertMerkleRedemption();
     error PTokenAlreadyDeployed();
     error DepositExceedsCap();
     error PTokenNotDeployed();
+    error AmountTooSmall();
 
+    /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
         _disableInitializers();
     }
@@ -82,28 +85,31 @@ contract PointTokenVault is UUPSUpgradeable, AccessControlUpgradeable, Multicall
         __AccessControl_init();
         __Multicall_init();
         _grantRole(DEFAULT_ADMIN_ROLE, _admin);
-        isCapped = true;
     }
 
     // Rebasing and fee-on-transfer tokens must be wrapped before depositing.
     function deposit(ERC20 _token, uint256 _amount, address _receiver) public {
-        if (isCapped && (_amount + _token.balanceOf(address(this)) > caps[address(_token)])) {
-            revert DepositExceedsCap();
+        uint256 cap = caps[address(_token)];
+
+        if (cap != type(uint256).max) {
+            if (_amount + _token.balanceOf(address(this)) > cap) {
+                revert DepositExceedsCap();
+            }
         }
 
         _token.safeTransferFrom(msg.sender, address(this), _amount);
 
         balances[_receiver][_token] += _amount;
 
-        emit Deposit(_receiver, address(_token), _amount);
+        emit Deposit(msg.sender, _receiver, address(_token), _amount);
     }
 
     function withdraw(ERC20 _token, uint256 _amount, address _receiver) public {
         balances[msg.sender][_token] -= _amount;
 
         _token.safeTransfer(_receiver, _amount);
 
-        emit Withdraw(_receiver, address(_token), _amount);
+        emit Withdraw(msg.sender, _receiver, address(_token), _amount);
     }
 
     /// @notice Claims point tokens after verifying the merkle proof
@@ -156,14 +162,29 @@ contract PointTokenVault is UUPSUpgradeable, AccessControlUpgradeable, Multicall
     /// @notice Mints point tokens for rewards after redemption has been enabled
     function convertRewardsToPTokens(address _receiver, bytes32 _pointsId, uint256 _amountToConvert) public {
         RedemptionParams memory params = redemptions[_pointsId];
-        (ERC20 rewardToken, uint256 rewardsPerPToken) = (params.rewardToken, params.rewardsPerPToken);
+        (ERC20 rewardToken, uint256 rewardsPerPToken, bool isMerkleBased) =
+            (params.rewardToken, params.rewardsPerPToken, params.isMerkleBased);
 
         if (address(rewardToken) == address(0)) {
             revert RewardsNotReleased();
         }
 
+        if (isMerkleBased) {
+            revert CantConvertMerkleRedemption();
+        }
+
         rewardToken.safeTransferFrom(msg.sender, address(this), _amountToConvert);
-        pTokens[_pointsId].mint(_receiver, FixedPointMathLib.divWadDown(_amountToConvert, rewardsPerPToken)); // Round down for mint.
+
+        uint256 pTokensToMint = FixedPointMathLib.divWadDown(_amountToConvert, rewardsPerPToken); // Round down for mint.
+
+        // Dust guard.
+        if (pTokensToMint == 0) {
+            revert AmountTooSmall();
+        }
+
+        pTokens[_pointsId].mint(_receiver, pTokensToMint);
+
+        emit RewardsConverted(msg.sender, _receiver, _pointsId, _amountToConvert);
     }
 
     function deployPToken(bytes32 _pointsId) public {
@@ -214,12 +235,9 @@ contract PointTokenVault is UUPSUpgradeable, AccessControlUpgradeable, Multicall
     }
 
     function setCap(address _token, uint256 _cap) external onlyRole(OPERATOR_ROLE) {
+        uint256 prevCap = caps[_token];
         caps[_token] = _cap;
-        emit CapSet(_token, _cap);
-    }
-
-    function setIsCapped(bool _isCapped) external onlyRole(OPERATOR_ROLE) {
-        isCapped = _isCapped;
+        emit CapSet(_token, prevCap, _cap);
     }
 
     // Can be used to unlock reward token redemption (can also modify a live redemption, so use with care).

contracts/script/PointTokenVault.s.sol

@@ -10,6 +10,8 @@ import {ERC20} from "solmate/tokens/ERC20.sol";
 import {CREATE3} from "solmate/utils/CREATE3.sol";
 import {LibString} from "solady/utils/LibString.sol";
 
+import {Upgrades} from "openzeppelin-foundry-upgrades/Upgrades.sol";
+
 import {console} from "forge-std/console.sol";
 
 contract PointTokenVaultScripts is BatchScript {
@@ -55,15 +57,12 @@ contract PointTokenVaultScripts is BatchScript {
     }
 
     function run(string memory version) public returns (PointTokenVault) {
-        PointTokenVault pointTokenVaultImplementation = new PointTokenVault{salt: keccak256(abi.encode(version))}();
-
-        PointTokenVault pointTokenVault = PointTokenVault(payable(
-            address(
-                new ERC1967Proxy{salt: keccak256(abi.encode(version))}(
-                    address(pointTokenVaultImplementation),
-                    abi.encodeCall(PointTokenVault.initialize, (msg.sender)) // msg.sender is admin
+        PointTokenVault pointTokenVault = PointTokenVault(
+            payable(
+                Upgrades.deployUUPSProxy(
+                    "PointTokenVault.sol", abi.encodeCall(PointTokenVault.initialize, (msg.sender))
                 )
-            ))
+            )
         );
 
         return pointTokenVault;
@@ -87,11 +86,12 @@ contract PointTokenVaultScripts is BatchScript {
     function upgrade() public {
         vm.startBroadcast();
 
-        PointTokenVault currentPointTokenVault = PointTokenVault(payable(0xbff7Fb79efC49504afc97e74F83EE618768e63E9));
-
-        PointTokenVault PointTokenVaultImplementation = new PointTokenVault();
+        // address currentPointTokenVaultAddress = 0xbff7Fb79efC49504afc97e74F83EE618768e63E9;
 
-        currentPointTokenVault.upgradeToAndCall(address(PointTokenVaultImplementation), bytes(""));
+        // Once there is a v2, upgrade referencing v1 for automatic OZ safety checks
+        // Options memory opts;
+        // opts.referenceContract = "PointTokenVaultV1.sol";
+        // Upgrades.upgradeProxy(currentPointTokenVaultAddress, "PointTokenVaultV2.sol", "");
 
         vm.stopBroadcast();
     }
@@ -118,15 +118,30 @@ contract PointTokenVaultScripts is BatchScript {
         vm.stopBroadcast();
     }
 
+    // Useful for emergencies, where we need to override both the current and previous root at once
+    // For example, if minting for a specific pToken needs to be stopped, a root without any claim rights for the pToken would need to be pushed twice
+    function doublePushRoot(address pointTokenVaultAddress, bytes32 newRoot, address merkleUpdaterSafe) public {
+        uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY");
+        vm.startBroadcast(deployerPrivateKey);
+
+        bytes memory txn = abi.encodeWithSelector(PointTokenVault.updateRoot.selector, newRoot);
+        addToBatch(pointTokenVaultAddress, 0, txn);
+        addToBatch(pointTokenVaultAddress, 0, txn);
+
+        executeBatch(merkleUpdaterSafe, true);
+
+        vm.stopBroadcast();
+    }
+
     function setCap() public {
         uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY");
         vm.startBroadcast(deployerPrivateKey);
 
-        address pointTokenVault = 0xbff7Fb79efC49504afc97e74F83EE618768e63E9;
+        address pointTokenVaultAddress = 0xbff7Fb79efC49504afc97e74F83EE618768e63E9;
 
         bytes memory txn =
             abi.encodeWithSelector(PointTokenVault.setCap.selector, 0x791a051631c9c4cDf4E03Fb7Aec3163AE164A34B, 10e18);
-        addToBatch(pointTokenVault, 0, txn);
+        addToBatch(pointTokenVaultAddress, 0, txn);
 
         executeBatch(SEOPLIA_ADMIN_SAFE, true);
         vm.stopBroadcast();

contracts/test/PointTokenVault.t.sol

@@ -51,6 +51,8 @@ contract PointTokenVaultTest is Test {
         pointTokenVault.setCap(address(pointEarningToken), type(uint256).max);
     }
 
+    event Deposit(address indexed depositor, address indexed receiver, address indexed token, uint256 amount);
+
     function test_Deposit() public {
         pointEarningToken.mint(vitalik, 1.123e18);
 
@@ -65,13 +67,17 @@ contract PointTokenVaultTest is Test {
 
         // Can deposit for someone else
         vm.prank(vitalik);
+        vm.expectEmit(true, true, true, true);
+        emit Deposit(vitalik, toly, address(pointEarningToken), 0.623e18);
         pointTokenVault.deposit(pointEarningToken, 0.623e18, toly);
 
         assertEq(pointEarningToken.balanceOf(vitalik), 0);
         assertEq(pointTokenVault.balances(toly, pointEarningToken), 0.623e18);
         assertEq(pointTokenVault.balances(vitalik, pointEarningToken), 0.5e18);
     }
 
+    event Withdraw(address indexed withdrawer, address indexed receiver, address indexed token, uint256 amount);
+
     function test_Withdraw() public {
         pointEarningToken.mint(vitalik, 1.123e18);
 
@@ -87,6 +93,8 @@ contract PointTokenVaultTest is Test {
 
         // Can withdraw with a different receiver
         vm.prank(vitalik);
+        vm.expectEmit(true, true, true, true);
+        emit Withdraw(vitalik, toly, address(pointEarningToken), 0.5e18);
         pointTokenVault.withdraw(pointEarningToken, 0.5e18, toly);
 
         assertEq(pointEarningToken.balanceOf(vitalik), 0.623e18);
@@ -96,13 +104,17 @@ contract PointTokenVaultTest is Test {
         assertEq(pointTokenVault.balances(vitalik, pointEarningToken), 0);
     }
 
+    event CapSet(address indexed token, uint256 prevCap, uint256 cap);
+
     function test_DepositCaps() public {
         // Deploy a new mock token
         MockERC20 newMockToken = new MockERC20("New Test Token", "NTT", 18);
 
         // Set a cap for the new token
         uint256 capAmount = 1e18; // 1 token cap
         vm.prank(operator);
+        vm.expectEmit(true, true, true, true);
+        emit CapSet(address(newMockToken), 0, capAmount);
         pointTokenVault.setCap(address(newMockToken), capAmount);
 
         // Mint tokens to vitalik
@@ -123,9 +135,9 @@ contract PointTokenVaultTest is Test {
 
         assertEq(pointTokenVault.balances(vitalik, newMockToken), 1e18);
 
-        // Remove the cap
+        // Set deposit cap to max
         vm.prank(operator);
-        pointTokenVault.setIsCapped(false);
+        pointTokenVault.setCap(address(newMockToken), 2**256 - 1);
 
         // Approve and deposit more than the previous cap
         vm.startPrank(vitalik);
@@ -457,6 +469,8 @@ contract PointTokenVaultTest is Test {
         assertEq(pointTokenVault.pTokens(eigenPointsId).balanceOf(vitalik), 1e18);
     }
 
+    event RewardsConverted(address indexed owner, address indexed receiver, bytes32 indexed pointsId, uint256 amount);
+
     function test_MintPTokensForRewards() public {
         bytes32 root = 0x4e40a10ce33f33a4786960a8bb843fe0e170b651acd83da27abc97176c4bed3c;
 
@@ -491,6 +505,8 @@ contract PointTokenVaultTest is Test {
         vm.prank(vitalik);
         rewardToken.approve(address(pointTokenVault), 1e18);
         vm.prank(vitalik);
+        vm.expectEmit(true, true, true, true);
+        emit RewardsConverted(vitalik, vitalik, eigenPointsId, 1e18);
         pointTokenVault.convertRewardsToPTokens(vitalik, eigenPointsId, 1e18);
 
         assertEq(rewardToken.balanceOf(vitalik), 1e18);
@@ -504,6 +520,80 @@ contract PointTokenVaultTest is Test {
         assertEq(pointTokenVault.pTokens(eigenPointsId).balanceOf(vitalik), 0);
     }
 
+    function test_CantMintPTokensForRewardsMerkleBased() public {
+        bool IS_MERKLE_BASED = true;
+
+        bytes32 root = 0x409fd0e46d8453765fb513ae35a1899d667478c40233b67360023c86927eb802;
+
+        bytes32[] memory proof = new bytes32[](2);
+        proof[0] = 0x6d0fcb8de12b1f57f81e49fa18b641487b932cdba4f064409fde3b05d3824ca2;
+        proof[1] = 0xae126f1299213c869259b52ab24f7270f3cce1de54c187271c52373d8947c2fe;
+
+        vm.prank(merkleUpdater);
+        pointTokenVault.updateRoot(root);
+
+        vm.prank(vitalik);
+        pointTokenVault.claimPTokens(PointTokenVault.Claim(eigenPointsId, 1e18, 1e18, proof), vitalik);
+
+        rewardToken.mint(address(pointTokenVault), 3e18);
+
+        vm.prank(operator);
+        pointTokenVault.setRedemption(eigenPointsId, rewardToken, 2e18, IS_MERKLE_BASED);
+
+        bytes32[] memory redemptionProof = new bytes32[](1);
+        redemptionProof[0] = 0x4e40a10ce33f33a4786960a8bb843fe0e170b651acd83da27abc97176c4bed3c;
+        vm.prank(vitalik);
+        pointTokenVault.redeemRewards(PointTokenVault.Claim(eigenPointsId, 2e18, 2e18, redemptionProof), vitalik);
+
+        assertEq(rewardToken.balanceOf(vitalik), 2e18);
+        assertEq(pointTokenVault.pTokens(eigenPointsId).balanceOf(vitalik), 0);
+
+        // Can't mint ptokens if it's a merkle-based redemption
+        vm.prank(vitalik);
+        rewardToken.approve(address(pointTokenVault), 1e18);
+        vm.prank(vitalik);
+        vm.expectRevert(PointTokenVault.CantConvertMerkleRedemption.selector);
+        pointTokenVault.convertRewardsToPTokens(vitalik, eigenPointsId, 1e18);
+    }
+
+    function test_CantMintPTokensForRewardsAmountTooSmall() public {
+        bytes32 root = 0x4e40a10ce33f33a4786960a8bb843fe0e170b651acd83da27abc97176c4bed3c;
+
+        bytes32[] memory proof = new bytes32[](1);
+        proof[0] = 0x6d0fcb8de12b1f57f81e49fa18b641487b932cdba4f064409fde3b05d3824ca2;
+
+        vm.prank(merkleUpdater);
+        pointTokenVault.updateRoot(root);
+
+        vm.prank(vitalik);
+        pointTokenVault.claimPTokens(PointTokenVault.Claim(eigenPointsId, 1e18, 1e18, proof), vitalik);
+
+        rewardToken.mint(address(pointTokenVault), 3e18);
+
+        vm.prank(operator);
+        pointTokenVault.setRedemption(eigenPointsId, rewardToken, 2e18, false);
+
+        bytes32[] memory empty = new bytes32[](0);
+        vm.prank(vitalik);
+        pointTokenVault.redeemRewards(PointTokenVault.Claim(eigenPointsId, 2e18, 2e18, empty), vitalik);
+
+        assertEq(rewardToken.balanceOf(vitalik), 2e18);
+        assertEq(pointTokenVault.pTokens(eigenPointsId).balanceOf(vitalik), 0);
+
+        // Can't mint ptokens if the amount is too small
+        vm.prank(vitalik);
+        rewardToken.approve(address(pointTokenVault), 1);
+        vm.prank(vitalik);
+        vm.expectRevert(PointTokenVault.AmountTooSmall.selector);
+        pointTokenVault.convertRewardsToPTokens(vitalik, eigenPointsId, 1);
+
+        // Can mint anything above the absolute minimum
+        vm.prank(vitalik);
+        rewardToken.approve(address(pointTokenVault), 2);
+        vm.prank(vitalik);
+        pointTokenVault.convertRewardsToPTokens(vitalik, eigenPointsId, 2);
+    }
+
     function test_ReceiveETH() public payable {
         // Amount of ETH to send
         uint256 amountToSend = 1 ether;