Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Use hardcoded deployedBytecode for local EVM execution #2198

Merged
merged 4 commits into from
Sep 26, 2019
Merged

Conversation

xianny
Copy link
Contributor

@xianny xianny commented Sep 24, 2019

Description

Resolves #2192

Try to use hardcoded deployedBytecode wherever possible for local EVM execution.

This means that we are able to instantiate wrappers and call pure functions completely locally.

E.g. in a unit testing scenario, we should now be able to do this:

const address = constants.NULL_ADDRESS;
const supportedProvider = someProvider;
const devUtils = new DevUtils(address, supportedProvider);
const assetData = devUtils.encodeERC20AssetData.callAsync(zrxTokenAddress);

Without this PR, one would have to supply a valid address with a DevUtils contract deployed, in order to use any of the pure functions therein. You could still do this locally by deploying an instance of the contract on Ganache, but would introduce dependencies on Ganache, @0x/migrations, blah blah.

Note that the deployedBytecode is now hardcoded within the contract wrappers. If a developer is making changes to the contract, they would need to regenerate the contract wrapper using the abi-gen utility to have the changes reflected.

Testing instructions

Types of changes

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@xianny xianny requested a review from feuGeneA as a code owner September 24, 2019 18:13
@buildsize
Copy link

buildsize bot commented Sep 24, 2019

File name Previous Size New Size Change
init.py 7.04 KB 7.04 KB 0 bytes (0%)
abi_gen_dummy.ts 164.71 KB 175.19 KB 10.48 KB (6%)
lib_dummy.ts 4.84 KB 5.24 KB 416 bytes (8%)
test_lib_dummy.ts 13.44 KB 14.02 KB 597 bytes (4%)

@xianny xianny requested a review from hysz September 24, 2019 18:13
@xianny xianny force-pushed the 2192-fix-eth-call branch 3 times, most recently from fe2bd6b to d19f277 Compare September 24, 2019 23:21
@xianny xianny requested a review from abandeali1 as a code owner September 24, 2019 23:21
@coveralls
Copy link

coveralls commented Sep 25, 2019

Coverage Status

Coverage remained the same at 72.355% when pulling 8e07395 on 2192-fix-eth-call into a347c1e on 3.0.

provider,
txDefaults,
{},
StaticCallProxyContract.deployedBytecode,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to specify this line here? It would default to this value, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope it would default to IAssetProxyContract.deployedBytecode, which was causing it to fail before!

@@ -28,6 +28,7 @@ import * as ethers from 'ethers';
// tslint:disable:no-parameter-reassignment
// tslint:disable-next-line:class-name
export class CoordinatorContract extends BaseContract {
public static deployedBytecode = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why blank?

@@ -46,6 +46,7 @@ export interface CoordinatorRegistryCoordinatorEndpointSetEventArgs extends Deco
// tslint:disable:no-parameter-reassignment
// tslint:disable-next-line:class-name
export class CoordinatorRegistryContract extends BaseContract {
public static deployedBytecode = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why blank?

@@ -28,6 +28,7 @@ import * as ethers from 'ethers';
// tslint:disable:no-parameter-reassignment
// tslint:disable-next-line:class-name
export class DutchAuctionContract extends BaseContract {
public static deployedBytecode = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why blank?

@@ -28,6 +28,7 @@ import * as ethers from 'ethers';
// tslint:disable:no-parameter-reassignment
// tslint:disable-next-line:class-name
export class IAssetProxyContract extends BaseContract {
public static deployedBytecode = '0x';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this sometimes generate as ''' and sometimes as '0x'?

@@ -28,6 +28,7 @@ import * as ethers from 'ethers';
// tslint:disable:no-parameter-reassignment
// tslint:disable-next-line:class-name
export class IValidatorContract extends BaseContract {
public static deployedBytecode = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why blank?

@@ -28,6 +28,7 @@ import * as ethers from 'ethers';
// tslint:disable:no-parameter-reassignment
// tslint:disable-next-line:class-name
export class IWalletContract extends BaseContract {
public static deployedBytecode = '0x';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this sometimes generate as ''' and sometimes as '0x'?

@@ -28,6 +28,7 @@ import * as ethers from 'ethers';
// tslint:disable:no-parameter-reassignment
// tslint:disable-next-line:class-name
export class OrderValidatorContract extends BaseContract {
public static deployedBytecode = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why blank?

packages/abi-gen/src/index.ts Show resolved Hide resolved
) {
assert.isString('contractName', contractName);
assert.isETHAddressHex('address', address);
if (deployedBytecode !== undefined) {
assert.isHexString('deployedBytecode', deployedBytecode);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are tests running? Wouldn't this fail tests of the generated wrappers that have their deployedBytecode initialized to ''?

@xianny
Copy link
Contributor Author

xianny commented Sep 25, 2019

Updated to not allow bytecode to be initialised as empty string or 0x

@xianny
Copy link
Contributor Author

xianny commented Sep 26, 2019

Updates:
If there's no valid deployedBytecode, then it will be generated like this:

public static deployedBytecode: string | undefined;

otherwise it will be

public static deployedBytecode = '0x123456....';

Warnings look like this now:
Screen Shot 2019-09-26 at 8 19 35 AM

@xianny xianny merged commit cb20f03 into 3.0 Sep 26, 2019
@xianny xianny deleted the 2192-fix-eth-call branch September 26, 2019 15:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants