search

OpenZeppelin / openzeppelin-sdk

OpenZeppelin SDK repository for CLI and upgrades.js. No longer actively developed.
MIT License
431 stars 201 forks source link

Cannot initialize contracts with structs #1523

Open mdnorman opened 4 years ago

mdnorman commented 4 years ago

SimpleProject.createProxy fails when initArgs contains a hash in order to initialize a struct.

For example, my Box contract expects a BoxInfo struct in initializeBox.

However, if I attempt to execute the initializeBox as an initMethod during createProxy, it fails during eth_estimateGas:

deploy.ts output:

Proxy Admin Account: 0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1
Owner Account: 0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0
(node:30570) UnhandledPromiseRejectionWarning: Error: Error: Returned error: VM Exception while processing transaction: revert
    at Object.<anonymous> (.../tutorials/blockchain/tutorial-openzeppelin-contract/node_modules/@openzeppelin/upgrades/lib/utils/Transactions.js:129:27)
    at Generator.throw (<anonymous>)
    at rejected (.../tutorials/blockchain/tutorial-openzeppelin-contract/node_modules/@openzeppelin/upgrades/lib/utils/Transactions.js:11:65)
(node:30570) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:30570) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

ganache-cli output:

eth_accounts
net_version
eth_estimateGas
eth_getBlockByNumber
web3_clientVersion
eth_accounts
eth_gasPrice
eth_sendTransaction

  Transaction: 0xf0b82d013bfa394656010dd33e17f5546b7efa7b3df208cb813e3675797088f1
  Contract created: 0x254dffcd3277c0b1660f6d42efbb754edababc2b
  Gas usage: 611505
  Block Number: 5
  Block Time: Tue Apr 07 2020 09:19:02 GMT-0500 (Central Daylight Time)

eth_getTransactionReceipt
eth_getCode
net_version
eth_estimateGas
eth_estimateGas
eth_estimateGas

If I set gas to 5000000, it fails with a revert:

deploy.ts output:

Proxy Admin Account: 0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1
Owner Account: 0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0
(node:30608) UnhandledPromiseRejectionWarning: Error: Returned error: VM Exception while processing transaction: revert
...

I am able to create the proxy, and then call initializeBox later via web3 with my deploy script, so web3 can handle the conversion of hashes to structs properly. This appears to be specifically an issue with the createProxy call, or one of its underlying eth libraries.

This is tangentially related to https://github.com/OpenZeppelin/openzeppelin-sdk/issues/1373. However, @ts-ignore doesn't solve the problem because it's not only a typing issue.

abcoathup commented 4 years ago

Hi @mdnorman! Apologies for the delay, I am still catching up after vacation over Easter.

I’m sorry that you had this issue.

When I attempted to reproduce, I received a different error (my JavaScript syntax may be incorrect):

Box.sol

// contracts/Box.sol
pragma solidity ^0.5.0;
pragma experimental ABIEncoderV2;

contract Box {
    struct Info {
        string message;
    }

    Info private value;

    function store(Info memory newValue) public {
        value = newValue;
    }

    function retrieve() public view returns (string memory) {
        return value.message;
    }
}

deploy.js

const { ZWeb3, Contracts, SimpleProject } = require('@openzeppelin/upgrades');

async function main() {
  // Initialize a web3 provider
  ZWeb3.initialize("http://localhost:8545");

  // Load the contract
  const Box = Contracts.getFromLocal('Box');

  // Instantiate a project
  const myProject = new SimpleProject('MyProject', {
    from: await ZWeb3.defaultAccount()
  });

  const info = {value: 'hello'};

  // Create a proxy for the contract
  const proxy = await myProject.createProxy(Box, { initMethod: 'store', initArgs: [info] });
}

main();

deploy programmatically

$ node deploy.js
(node:13059) UnhandledPromiseRejectionWarning: Error: Error: Returned error: VM Exception while processing transaction: revert
    at Object.<anonymous> (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/utils/Transactions.js:129:27)
    at Generator.throw (<anonymous>)
    at rejected (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/utils/Transactions.js:11:65)
(node:13059) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:13059) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I also get an issue when using the CLI to deploy:

oz deploy --verbose

$ npx oz deploy --verbose
[2020-04-15T06:16:42.582Z@SolidityProjectCompiler.js#call] <started> Nothing to compile, all contracts are up to date.
[2020-04-15T06:16:42.585Z@ProjectFile.js#write] <started> Updated /home/abcoathup/projects/forum/issue1523/.openzeppelin/project.json
? Choose the kind of deployment upgradeable
? Pick a network development
? Pick a contract to deploy Box
[2020-04-15T06:16:45.731Z@NetworkController.js#uploadContract] <started> Validating and deploying contract Box
[2020-04-15T06:16:45.731Z@BaseSimpleProject.js#setImplementation] <started> Deploying logic contract for Box
[2020-04-15T06:16:45.828Z@NetworkController.js#uploadContract] <succeeded> Contract Box deployed
[2020-04-15T06:16:45.830Z@NetworkController.js#push] <started> All implementations have been deployed
[2020-04-15T06:16:45.831Z@NetworkFile.js#write] <started> Created .openzeppelin/dev-1586931398623.json
? Call a function to initialize the instance after creating it? Yes
? Select which function store(newValue: (string))
? newValue: (string): hello
[2020-04-15T06:16:51.834Z@ProxyAdmin.js#deploy] <started> Setting everything up to create contract instances
[2020-04-15T06:16:51.911Z@ProxyAdmin.js#deploy] <succeeded> Setting everything up to create contract instances
[2020-04-15T06:16:51.914Z@NetworkFile.js#write] <started> Updated /home/abcoathup/projects/forum/issue1523/.openzeppelin/dev-1586931398623.json
[2020-04-15T06:16:51.915Z@errors.js#call] <started> Error: types/value length mismatch (coderType="tuple", value=["hello"], version=4.0.46)
    at Object.throwError (/home/abcoathup/projects/forum/issue1523/node_modules/ethers/errors.js:76:17)
    at pack (/home/abcoathup/projects/forum/issue1523/node_modules/ethers/utils/abi-coder.js:638:16)
    at CoderTuple.encode (/home/abcoathup/projects/forum/issue1523/node_modules/ethers/utils/abi-coder.js:804:16)
    at /home/abcoathup/projects/forum/issue1523/node_modules/ethers/utils/abi-coder.js:645:59
    at Array.forEach (<anonymous>)
    at pack (/home/abcoathup/projects/forum/issue1523/node_modules/ethers/utils/abi-coder.js:644:12)
    at CoderTuple.encode (/home/abcoathup/projects/forum/issue1523/node_modules/ethers/utils/abi-coder.js:804:16)
    at AbiCoder.encode (/home/abcoathup/projects/forum/issue1523/node_modules/ethers/utils/abi-coder.js:941:77)
    at encodeParams (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/helpers/encodeCall.js:7:40)
    at Object.encodeCall [as default] (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/helpers/encodeCall.js:11:31)
    at Object.buildCallData (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/utils/ABIs.js:23:42)
    at ProxyAdminProject._getAndLogInitCallData (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/project/BaseSimpleProject.js:200:61)
    at ProxyAdminProject.<anonymous> (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/project/BaseSimpleProject.js:87:39)
    at Generator.next (<anonymous>)
    at fulfilled (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/project/BaseSimpleProject.js:5:58)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Thanks so much for reporting it! The project owner will review and triage this issue during the next week.

mdnorman commented 4 years ago

From your example, one issue is that the hash you're passing in is using the key value instead of the struct key of message.

ie

const info = {value: 'hello'};

should be

const info = {message: 'hello'};

Regardless, it looks like you're getting the same error in your deploy.js example that I did.

I'd given up on oz deploy for structs because I assumed the command line wouldn't parse the structure properly.

abcoathup commented 4 years ago

Hi @mdnorman,

Good spot. Updating my deploy.js example gives the following:

$ node deploy.js
(node:25160) UnhandledPromiseRejectionWarning: Error: Error: Returned error: VM Exception while processing transaction: revert
    at Object.<anonymous> (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/utils/Transactions.js:129:27)
    at Generator.throw (<anonymous>)
    at rejected (/home/abcoathup/projects/forum/issue1523/node_modules/@openzeppelin/upgrades/lib/utils/Transactions.js:11:65)
(node:25160) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:25160) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
KimiWu123 commented 4 years ago

Hi @abcoathup , Will struct be supported in the next release? Another question, when the next version will be released? We'd like to use 0.6.0. Thanks.

abcoathup commented 4 years ago

Hi @KimiWu123,

Please see the roadmap for what is coming up next: https://github.com/OpenZeppelin/openzeppelin-sdk/issues/1528

Included in this is a plan to move Proxies to OpenZeppelin Contracts (which the latest release uses Solidity 0.6). If you are developing upgradeable contracts OpenZeppelin Contracts Ethereum Package v3.0 has been released (which uses Solidity 0.6)

frangio commented 4 years ago

Supporting structs is unfortunately not a priority for us at the moment.