Merge pull request #136 from PartyDAO/refactor/contribute-onlyDelegat…

…eCall refactor(crowdfund): make `contribute()` `onlyDelegateCall`
PartyDAO · Sep 22, 2022 · 62c3b1c · 62c3b1c
2 parents 25035f3 + 216ccd9
commit 62c3b1c
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 51 deletions.
diff --git a/contracts/crowdfund/AuctionCrowdfund.sol b/contracts/crowdfund/AuctionCrowdfund.sol
@@ -15,7 +15,7 @@ import "./Crowdfund.sol";
 
 /// @notice A crowdfund that can repeatedly bid on an auction for a specific NFT
 ///         (i.e. with a known token ID) until it wins.
-contract AuctionCrowdfund is Implementation, Crowdfund {
+contract AuctionCrowdfund is Crowdfund {
     using LibSafeERC721 for IERC721;
     using LibSafeCast for uint256;
     using LibRawResult for bytes;

diff --git a/contracts/crowdfund/BuyCrowdfundBase.sol b/contracts/crowdfund/BuyCrowdfundBase.sol
@@ -13,7 +13,7 @@ import "../gatekeepers/IGateKeeper.sol";
 import "./Crowdfund.sol";
 
 // Base for BuyCrowdfund and CollectionBuyCrowdfund
-abstract contract BuyCrowdfundBase is Implementation, Crowdfund {
+abstract contract BuyCrowdfundBase is Crowdfund {
     using LibSafeERC721 for IERC721;
     using LibSafeCast for uint256;
     using LibRawResult for bytes;

diff --git a/contracts/crowdfund/Crowdfund.sol b/contracts/crowdfund/Crowdfund.sol
@@ -15,7 +15,7 @@ import "./CrowdfundNFT.sol";
 // Base contract for AuctionCrowdfund/BuyCrowdfund.
 // Holds post-win/loss logic. E.g., burning contribution NFTs and creating a
 // party after winning.
-abstract contract Crowdfund is ERC721Receiver, CrowdfundNFT {
+abstract contract Crowdfund is Implementation, ERC721Receiver, CrowdfundNFT {
     using LibRawResult for bytes;
     using LibSafeCast for uint256;
     using LibAddress for address payable;
@@ -192,6 +192,7 @@ abstract contract Crowdfund is ERC721Receiver, CrowdfundNFT {
     function contribute(address delegate, bytes memory gateData)
         public
         payable
+        onlyDelegateCall
     {
         _contribute(
             msg.sender,

diff --git a/sol-tests/crowdfund/Crowdfund.t.sol b/sol-tests/crowdfund/Crowdfund.t.sol
@@ -81,24 +81,37 @@ contract CrowdfundTest is Test, TestUtils {
         }
     }
 
-    function _createCrowdfund(uint96 initialContribution)
+    function _createCrowdfund(
+        uint256 initialContribution,
+        address initialContributor,
+        address initialDelegate
+    )
         private
         returns (TestableCrowdfund cf)
     {
-        cf = new TestableCrowdfund{value: initialContribution }(
-            globals,
-            Crowdfund.CrowdfundOptions({
-                name: defaultName,
-                symbol: defaultSymbol,
-                splitRecipient: defaultSplitRecipient,
-                splitBps: defaultSplitBps,
-                initialContributor: address(this),
-                initialDelegate: defaultInitialDelegate,
-                gateKeeper: defaultGateKeeper,
-                gateKeeperId: defaultGateKeeperId,
-                governanceOpts: defaultGovernanceOpts
-            })
-        );
+        cf = TestableCrowdfund(payable(new Proxy{ value: initialContribution }(
+            Implementation(new TestableCrowdfund(globals)),
+            abi.encodeCall(TestableCrowdfund.initialize, (
+                Crowdfund.CrowdfundOptions({
+                    name: defaultName,
+                    symbol: defaultSymbol,
+                    splitRecipient: defaultSplitRecipient,
+                    splitBps: defaultSplitBps,
+                    initialContributor: initialContributor,
+                    initialDelegate: initialDelegate,
+                    gateKeeper: defaultGateKeeper,
+                    gateKeeperId: defaultGateKeeperId,
+                    governanceOpts: defaultGovernanceOpts
+                })
+            ))
+        )));
+    }
+
+    function _createCrowdfund(uint256 initialContribution)
+        private
+        returns (TestableCrowdfund cf)
+    {
+        return _createCrowdfund(initialContribution, address(this), defaultInitialDelegate);
     }
 
     function _createExpectedPartyOptions(TestableCrowdfund cf, uint256 finalPrice)
@@ -146,19 +159,10 @@ contract CrowdfundTest is Test, TestUtils {
         uint256 initialContribution = _randomRange(1, 1 ether);
         vm.deal(address(this), initialContribution);
         emit Contributed(initialContributor, initialContribution, initialDelegate, 0);
-        TestableCrowdfund cf = new TestableCrowdfund{value: initialContribution }(
-            globals,
-            Crowdfund.CrowdfundOptions({
-                name: defaultName,
-                symbol: defaultSymbol,
-                splitRecipient: defaultSplitRecipient,
-                splitBps: defaultSplitBps,
-                initialContributor: initialContributor,
-                initialDelegate: initialDelegate,
-                gateKeeper: defaultGateKeeper,
-                gateKeeperId: defaultGateKeeperId,
-                governanceOpts: defaultGovernanceOpts
-            })
+        TestableCrowdfund cf = _createCrowdfund(
+            initialContribution,
+            initialContributor,
+            initialDelegate
         );
         (
             uint256 ethContributed,
@@ -176,20 +180,7 @@ contract CrowdfundTest is Test, TestUtils {
 
     function test_creation_initialContribution_noValue() public {
         address initialContributor = _randomAddress();
-        TestableCrowdfund cf = new TestableCrowdfund(
-            globals,
-            Crowdfund.CrowdfundOptions({
-                name: defaultName,
-                symbol: defaultSymbol,
-                splitRecipient: defaultSplitRecipient,
-                splitBps: defaultSplitBps,
-                initialContributor: initialContributor,
-                initialDelegate: initialContributor,
-                gateKeeper: defaultGateKeeper,
-                gateKeeperId: defaultGateKeeperId,
-                governanceOpts: defaultGovernanceOpts
-            })
-        );
+        TestableCrowdfund cf = _createCrowdfund(0, initialContributor, initialContributor);
         (
             uint256 ethContributed,
             uint256 ethUsed,

diff --git a/sol-tests/crowdfund/TestableCrowdfund.sol b/sol-tests/crowdfund/TestableCrowdfund.sol
@@ -7,15 +7,15 @@ import "../../contracts/crowdfund/Crowdfund.sol";
 contract TestableCrowdfund is Crowdfund {
 
     uint256 public finalPrice;
-    CrowdfundLifecycle public lifeCycle = CrowdfundLifecycle.Active;
+    CrowdfundLifecycle public lifeCycle;
     FixedGovernanceOpts public govOpts;
 
-    constructor(IGlobals globals, CrowdfundOptions memory opts)
-        payable
-        Crowdfund(globals)
-    {
-        _initialize(opts);
+    constructor(IGlobals globals) Crowdfund(globals) {}
+
+    function initialize(CrowdfundOptions memory opts) external payable {
+        lifeCycle = CrowdfundLifecycle.Active;
         govOpts = opts.governanceOpts;
+        _initialize(opts);
     }
 
     function testSetFinalPrice(uint256 finalPrice_) external {