Make KycOnboarding compatible with V1 bank (#551)

* Make KycOnboarding compatible with V1 bank * Lint fix * Fix slither
openlawteam · Aug 5, 2022 · be171ad · be171ad
1 parent a4c79f9
commit be171ad
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 49 deletions.
diff --git a/contracts/adapters/KycOnboarding.sol b/contracts/adapters/KycOnboarding.sol
@@ -9,7 +9,6 @@ import "../utils/Signatures.sol";
 import "../helpers/WETH.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import "./modifiers/Reimbursable.sol";
 
 /**
 MIT License
@@ -35,12 +34,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 SOFTWARE.
  */
 
-contract KycOnboardingContract is
-    AdapterGuard,
-    MemberGuard,
-    Signatures,
-    Reimbursable
-{
+contract KycOnboardingContract is AdapterGuard, MemberGuard, Signatures {
     using Address for address payable;
     using SafeERC20 for IERC20;
 
@@ -266,7 +260,7 @@ contract KycOnboardingContract is
         uint256 amount,
         uint256 memberNonce,
         bytes memory signature
-    ) internal reimbursable(dao) {
+    ) internal {
         require(
             !isActiveMember(dao, dao.getCurrentDelegateKey(kycedMember)) ||
                 _daoCanTopUp(dao, tokenAddr),
@@ -306,19 +300,38 @@ contract KycOnboardingContract is
                 // hence even if we change that, it belongs to the DAO,
                 // so it is fine to send eth to it.
                 // slither-disable-next-line arbitrary-send
-                bank.addToBalance{value: details.amount}(
-                    dao,
-                    DaoHelper.GUILD,
-                    DaoHelper.ETH_TOKEN,
-                    details.amount
-                );
+                try
+                    bank.addToBalance{value: details.amount}(
+                        dao,
+                        DaoHelper.GUILD,
+                        DaoHelper.ETH_TOKEN,
+                        details.amount
+                    )
+                {} catch {
+                    require(bank.dao() == dao, "bank not configured");
+                    bank.addToBalance{value: details.amount}(
+                        DaoHelper.GUILD,
+                        DaoHelper.ETH_TOKEN,
+                        details.amount
+                    );
+                }
             } else {
-                bank.addToBalance(
-                    dao,
-                    DaoHelper.GUILD,
-                    tokenAddr,
-                    details.amount
-                );
+                try
+                    bank.addToBalance{value: details.amount}(
+                        dao,
+                        DaoHelper.GUILD,
+                        tokenAddr,
+                        details.amount
+                    )
+                {} catch {
+                    require(bank.dao() == dao, "invalid dao");
+                    bank.addToBalance{value: details.amount}(
+                        DaoHelper.GUILD,
+                        tokenAddr,
+                        details.amount
+                    );
+                }
+
                 IERC20 erc20 = IERC20(tokenAddr);
                 erc20.safeTransferFrom(
                     msg.sender,
@@ -348,12 +361,21 @@ contract KycOnboardingContract is
             }
         }
 
-        bank.addToBalance(
-            dao,
-            kycedMember,
-            DaoHelper.UNITS,
-            details.unitsRequested
-        );
+        try
+            bank.addToBalance(
+                dao,
+                kycedMember,
+                DaoHelper.UNITS,
+                details.unitsRequested
+            )
+        {} catch {
+            require(bank.dao() == dao, "invalid dao");
+            bank.addToBalance(
+                kycedMember,
+                DaoHelper.UNITS,
+                details.unitsRequested
+            );
+        }
 
         if (amount > details.amount && tokenAddr == DaoHelper.ETH_TOKEN) {
             payable(msg.sender).sendValue(msg.value - details.amount);

diff --git a/contracts/extensions/bank/Bank.sol b/contracts/extensions/bank/Bank.sol
@@ -349,6 +349,14 @@ contract BankExtension is IExtension, ERC165 {
         return internalTokens.length;
     }
 
+    function addToBalance(
+        address,
+        address,
+        uint256
+    ) external payable {
+        revert("not implemented");
+    }
+
     /**
      * @notice Adds to a member's balance of a given token
      * @param member The member whose balance will be updated
@@ -391,6 +399,23 @@ contract BankExtension is IExtension, ERC165 {
         _createNewAmountCheckpoint(DaoHelper.TOTAL, token, newTotalAmount);
     }
 
+    function subtractFromBalance(
+        address,
+        address,
+        uint256
+    ) external pure {
+        revert("not implemented");
+    }
+
+    function internalTransfer(
+        address,
+        address,
+        address,
+        uint256
+    ) external pure {
+        revert("not implemented");
+    }
+
     /**
      * @notice Make an internal token transfer
      * @param from The member who is sending tokens
@@ -491,23 +516,6 @@ contract BankExtension is IExtension, ERC165 {
     {
         return
             super.supportsInterface(interfaceId) ||
-            this.subtractFromBalance.selector == interfaceId ||
-            this.addToBalance.selector == interfaceId ||
-            this.getPriorAmount.selector == interfaceId ||
-            this.balanceOf.selector == interfaceId ||
-            this.internalTransfer.selector == interfaceId ||
-            this.nbInternalTokens.selector == interfaceId ||
-            this.getInternalToken.selector == interfaceId ||
-            this.getTokens.selector == interfaceId ||
-            this.nbTokens.selector == interfaceId ||
-            this.getToken.selector == interfaceId ||
-            this.updateToken.selector == interfaceId ||
-            this.registerPotentialNewInternalToken.selector == interfaceId ||
-            this.registerPotentialNewToken.selector == interfaceId ||
-            this.setMaxExternalTokens.selector == interfaceId ||
-            this.isTokenAllowed.selector == interfaceId ||
-            this.isInternalToken.selector == interfaceId ||
-            this.withdraw.selector == interfaceId ||
             this.withdrawTo.selector == interfaceId;
     }
 

diff --git a/test/extensions/bank.test.js b/test/extensions/bank.test.js
@@ -156,21 +156,31 @@ describe("Extension - Bank", () => {
     it("should not be possible to call addToBalance without the ADD_TO_BALANCE permission", async () => {
       const extension = this.extensions.bankExt;
       await expect(
-        extension.addToBalance(this.dao.address, daoOwner, ETH_TOKEN, 1)
+        extension["addToBalance(address,address,address,uint256)"](
+          this.dao.address,
+          daoOwner,
+          ETH_TOKEN,
+          1
+        )
       ).to.be.revertedWith("accessDenied");
     });
 
     it("should not be possible to call subtractFromBalance without the SUB_FROM_BALANCE permission", async () => {
       const extension = this.extensions.bankExt;
       await expect(
-        extension.subtractFromBalance(this.dao.address, daoOwner, ETH_TOKEN, 1)
+        extension["subtractFromBalance(address,address,address,uint256)"](
+          this.dao.address,
+          daoOwner,
+          ETH_TOKEN,
+          1
+        )
       ).to.be.revertedWith("accessDenied");
     });
 
     it("should not be possible to call internalTransfer without the INTERNAL_TRANSFER permission", async () => {
       const extension = this.extensions.bankExt;
       await expect(
-        extension.internalTransfer(
+        extension["internalTransfer(address,address,address,address,uint256)"](
           this.dao.address,
           daoOwner,
           creator,

diff --git a/utils/hardhat-test-util.js b/utils/hardhat-test-util.js
@@ -339,9 +339,15 @@ module.exports = (() => {
         from: owner,
       });
 
-      await extensions.bankExt.addToBalance(dao.address, newMember, UNITS, 1, {
-        from: owner,
-      });
+      await extensions.bankExt["addToBalance(address,address,address,uint256)"](
+        dao.address,
+        newMember,
+        UNITS,
+        1,
+        {
+          from: owner,
+        }
+      );
     }
 
     await dao.finalizeDao({ from: owner });