feat(incentives): no reuse of incentive clones

boostxyz · Mar 15, 2024 · b496520 · b496520
1 parent 810f612
commit b496520
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 26 deletions.
diff --git a/src/BoostCore.sol b/src/BoostCore.sol
@@ -178,6 +178,13 @@ contract BoostCore is Ownable, ReentrancyGuard {
         incentives = new Incentive[](targets_.length);
         for (uint256 i = 0; i < targets_.length; i++) {
             // Deploy the clone, but don't initialize until it we've preflighted
+            _checkTarget(type(Incentive).interfaceId, targets_[i].instance);
+
+            // Ensure the target is a base implementation (incentive clones are not reusable)
+            if (!targets_[i].isBase) {
+                revert BoostError.InvalidInstance(type(Incentive).interfaceId, targets_[i].instance);
+            }
+
             incentives[i] = Incentive(_makeTarget(type(Incentive).interfaceId, targets_[i], false));
 
             bytes memory preflight = incentives[i].preflight(targets_[i].parameters);

diff --git a/src/actions/ContractAction.sol b/src/actions/ContractAction.sol
@@ -52,7 +52,7 @@ contract ContractAction is Action {
         return (success, returnData);
     }
 
-    function prepare(bytes calldata data_) public virtual view override returns (bytes memory bytes_) {
+    function prepare(bytes calldata data_) public view virtual override returns (bytes memory bytes_) {
         return _buildPayload(selector, data_);
     }
 

diff --git a/test/BoostCore.t.sol b/test/BoostCore.t.sol
@@ -245,7 +245,11 @@ contract BoostCoreTest is Test {
         return BoostLib.Target({
             isBase: true,
             instance: address(new ERC721MintAction()),
-            parameters: LibZip.cdCompress(abi.encode(ContractAction.InitPayload({chainId: block.chainid, target: target, selector: selector, value: value})))
+            parameters: LibZip.cdCompress(
+                abi.encode(
+                    ContractAction.InitPayload({chainId: block.chainid, target: target, selector: selector, value: value})
+                )
+                )
         });
     }
 

diff --git a/test/actions/ContractAction.t.sol b/test/actions/ContractAction.t.sol
@@ -95,6 +95,9 @@ contract ContractActionTest is Test {
     ////////////////////////////
 
     function testPrepare() public {
-        assertEq(action.prepare(abi.encode(address(0xdeadbeef), 100 ether)), abi.encodeWithSelector(target.mintPayable.selector, address(0xdeadbeef), 100 ether));
+        assertEq(
+            action.prepare(abi.encode(address(0xdeadbeef), 100 ether)),
+            abi.encodeWithSelector(target.mintPayable.selector, address(0xdeadbeef), 100 ether)
+        );
     }
-}
+}
diff --git a/test/budgets/VestingBudget.t.sol b/test/budgets/VestingBudget.t.sol
@@ -30,13 +30,15 @@ contract VestingBudgetTest is Test {
         vestingBudget = VestingBudget(payable(LibClone.clone(address(new VestingBudget()))));
         vestingBudget.initialize(
             LibZip.cdCompress(
-                abi.encode(VestingBudget.InitPayload({
-                    owner: address(this),
-                    authorized: new address[](0),
-                    start: uint64(block.timestamp),
-                    duration: uint64(1 days),
-                    cliff: 0
-                }))
+                abi.encode(
+                    VestingBudget.InitPayload({
+                        owner: address(this),
+                        authorized: new address[](0),
+                        start: uint64(block.timestamp),
+                        duration: uint64(1 days),
+                        cliff: 0
+                    })
+                )
             )
         );
     }
@@ -87,7 +89,15 @@ contract VestingBudgetTest is Test {
     function testInitialize() public {
         // Initializer can only be called on clones, not the base contract
         bytes memory data = LibZip.cdCompress(
-            abi.encode(VestingBudget.InitPayload({owner: address(this), authorized: new address[](0), start: 0, duration: 0, cliff: 0}))
+            abi.encode(
+                VestingBudget.InitPayload({
+                    owner: address(this),
+                    authorized: new address[](0),
+                    start: 0,
+                    duration: 0,
+                    cliff: 0
+                })
+            )
         );
         VestingBudget clone = VestingBudget(payable(LibClone.clone(address(vestingBudget))));
         clone.initialize(data);
@@ -208,9 +218,11 @@ contract VestingBudgetTest is Test {
         _vestAll();
 
         // Reclaim 99 tokens from the budget
-        assertTrue(vestingBudget.reclaim(
-            _makeFungibleTransfer(Budget.AssetType.ERC20, address(mockERC20), address(this), 99 ether)
-        ));
+        assertTrue(
+            vestingBudget.reclaim(
+                _makeFungibleTransfer(Budget.AssetType.ERC20, address(mockERC20), address(this), 99 ether)
+            )
+        );
 
         // Ensure the budget total is still 100
         assertEq(vestingBudget.total(address(mockERC20)), 100 ether);
@@ -438,7 +450,8 @@ contract VestingBudgetTest is Test {
         _vestAll();
 
         // Try to disburse 100 tokens from the budget as a non-owner
-        bytes memory data = _makeFungibleTransfer(Budget.AssetType.ERC20, address(mockERC20), address(0xdeadbeef), 100 ether);
+        bytes memory data =
+            _makeFungibleTransfer(Budget.AssetType.ERC20, address(mockERC20), address(0xdeadbeef), 100 ether);
         vm.prank(address(0xc0ffee));
         vm.expectRevert();
         vestingBudget.disburse(data);
@@ -571,9 +584,7 @@ contract VestingBudgetTest is Test {
         assertEq(vestingBudget.available(address(0)), 100 ether);
 
         // Disburse 25 ETH from the budget and ensure the budget has 75 ETH available
-        vestingBudget.disburse(
-            _makeFungibleTransfer(Budget.AssetType.ETH, address(0), address(0xdeadbeef), 25 ether)
-        );
+        vestingBudget.disburse(_makeFungibleTransfer(Budget.AssetType.ETH, address(0), address(0xdeadbeef), 25 ether));
         assertEq(vestingBudget.available(address(0)), 75 ether);
     }
 
@@ -752,7 +763,9 @@ contract VestingBudgetTest is Test {
 
     function _allocate(address asset, uint256 value) internal returns (bool) {
         if (asset == address(0)) {
-            return vestingBudget.allocate{value: value}(_makeFungibleTransfer(Budget.AssetType.ETH, asset, address(this), value));
+            return vestingBudget.allocate{value: value}(
+                _makeFungibleTransfer(Budget.AssetType.ETH, asset, address(this), value)
+            );
         } else {
             mockERC20.approve(address(vestingBudget), value);
             return vestingBudget.allocate(_makeFungibleTransfer(Budget.AssetType.ERC20, asset, address(this), value));

diff --git a/test/e2e/EndToEnd.t.sol b/test/e2e/EndToEnd.t.sol
@@ -285,12 +285,16 @@ contract EndToEnd is Test {
                                 registry.getIdentifier(BoostRegistry.RegistryType.ACTION, "ERC721MintAction")
                             )
                             ),
-                        parameters: LibZip.cdCompress(abi.encode(ContractAction.InitPayload({
-                            chainId: block.chainid,
-                            target: address(erc721),
-                            selector: MockERC721.mint.selector,
-                            value: erc721.mintPrice()
-                        })))
+                        parameters: LibZip.cdCompress(
+                            abi.encode(
+                                ContractAction.InitPayload({
+                                    chainId: block.chainid,
+                                    target: address(erc721),
+                                    selector: MockERC721.mint.selector,
+                                    value: erc721.mintPrice()
+                                })
+                            )
+                            )
                     }),
                     BoostLib.Target({
                         // "... and I don't have to specify a validator"