Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Feature #909 proxy delegatecall" #1241

Merged
merged 1 commit into from
Oct 8, 2019
Merged

Revert "Feature #909 proxy delegatecall" #1241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions packages/protocol/contracts/common/MultiSig.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 1 addition & 11 deletions packages/protocol/contracts/common/Proxy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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)
}
Expand Down
21 changes: 0 additions & 21 deletions packages/protocol/contracts/common/libraries/AddressesHelper.sol
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions packages/protocol/contracts/governance/Proposals.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 */
Expand Down
7 changes: 0 additions & 7 deletions packages/protocol/test/common/proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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), {
Expand Down
23 changes: 0 additions & 23 deletions packages/protocol/test/governance/governance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down