Revert "Feature #909 proxy delegatecall (#1152)" (#1241)

This reverts commit 78afc93.
celo-org · Oct 8, 2019 · d05e4c5 · d05e4c5
1 parent 9a50adf
commit d05e4c5
Show file tree

Hide file tree

Showing 6 changed files with 1 addition and 72 deletions.
diff --git a/packages/protocol/contracts/common/MultiSig.sol b/packages/protocol/contracts/common/MultiSig.sol
@@ -2,7 +2,6 @@ pragma solidity ^0.5.3;
 /* solhint-disable no-inline-assembly, avoid-low-level-calls, func-name-mixedcase, func-order */
 
 import "./Initializable.sol";
-import "./libraries/AddressesHelper.sol";
 
 
 /// @title Multisignature wallet - Allows multiple parties to agree on transactions before
@@ -262,10 +261,6 @@ contract MultiSig is Initializable {
     returns (bool)
   {
     bool result;
-
-    if (dataLength > 0)
-      require(AddressesHelper.isContract(destination), "Invalid contract address");
-
     /* solhint-disable max-line-length */
     assembly {
       let x := mload(0x40)   // "Allocate" memory for output (0x40 is where "free memory" pointer is stored by convention)

diff --git a/packages/protocol/contracts/common/Proxy.sol b/packages/protocol/contracts/common/Proxy.sol
@@ -1,7 +1,6 @@
 pragma solidity ^0.5.3;
 /* solhint-disable no-inline-assembly, no-complex-fallback, avoid-low-level-calls */
 
-import "./libraries/AddressesHelper.sol";
 
 /**
  * @title A Proxy utilizing the Unstructured Storage pattern.
@@ -33,15 +32,8 @@ contract Proxy {
   function () external payable {
     bytes32 implementationPosition = IMPLEMENTATION_POSITION;
 
-    address implementationAddress;
-
-    assembly {
-      implementationAddress := sload(implementationPosition)
-    }
-
-    require(AddressesHelper.isContract(implementationAddress), "Invalid contract address");
-
     assembly {
+      let implementationAddress := sload(implementationPosition)
       let newCallDataPosition := mload(0x40)
       mstore(0x40, add(newCallDataPosition, calldatasize))
 
@@ -122,8 +114,6 @@ contract Proxy {
   function _setImplementation(address implementation) public onlyOwner {
     bytes32 implementationPosition = IMPLEMENTATION_POSITION;
 
-    require(AddressesHelper.isContract(implementation), "Invalid contract address");
-
     assembly {
       sstore(implementationPosition, implementation)
     }

diff --git a/packages/protocol/contracts/common/libraries/AddressesHelper.sol b/packages/protocol/contracts/common/libraries/AddressesHelper.sol
diff --git a/packages/protocol/contracts/governance/Proposals.sol b/packages/protocol/contracts/governance/Proposals.sol
@@ -4,7 +4,6 @@ import "openzeppelin-solidity/contracts/math/SafeMath.sol";
 import "solidity-bytes-utils/contracts/BytesLib.sol";
 
 import "../common/FixidityLib.sol";
-import "../common/libraries/AddressesHelper.sol";
 
 /**
  * @title A library operating on Celo Governance proposals.
@@ -325,10 +324,6 @@ library Proposals {
     returns (bool)
   {
     bool result;
-
-    if (dataLength > 0)
-      require(AddressesHelper.isContract(destination), "Invalid contract address");
-
     /* solhint-disable no-inline-assembly */
     assembly {
       /* solhint-disable max-line-length */

diff --git a/packages/protocol/test/common/proxy.ts b/packages/protocol/test/common/proxy.ts
@@ -116,13 +116,6 @@ contract('Proxy', (accounts: string[]) => {
       assert.equal(events[0].event, 'ImplementationSet')
     })
 
-    it('should not allow to call a non contract address', async () =>
-      assertRevert(
-        proxy._setAndInitializeImplementation(accounts[1], initializeData(42), {
-          from: accounts[1],
-        })
-      ))
-
     it('should not allow a non-owner to set an implementation', async () =>
       assertRevert(
         proxy._setAndInitializeImplementation(hasInitializer.address, initializeData(42), {

diff --git a/packages/protocol/test/governance/governance.ts b/packages/protocol/test/governance/governance.ts
@@ -1625,29 +1625,6 @@ contract('Governance', (accounts: string[]) => {
           await assertRevert(governance.execute(proposalId, index))
         })
       })
-
-      describe('when the proposal cannot execute because it is not a contract address', () => {
-        beforeEach(async () => {
-          await governance.propose(
-            [transactionSuccess1.value],
-            [accounts[1]],
-            transactionSuccess1.data,
-            [transactionSuccess1.data.length],
-            // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails
-            { value: minDeposit }
-          )
-          await timeTravel(dequeueFrequency, web3)
-          await governance.approve(proposalId, index)
-          await timeTravel(approvalStageDuration, web3)
-          await mockLockedGold.setWeight(account, weight)
-          await governance.vote(proposalId, index, value)
-          await timeTravel(referendumStageDuration, web3)
-        })
-
-        it('should revert', async () => {
-          await assertRevert(governance.execute(proposalId, index))
-        })
-      })
     })
 
     describe('when executing a proposal with two transactions', () => {