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.

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

Gas limit tweaks and EMP bytecode optimization #1356

Merged
merged 3 commits into from
May 5, 2020

Conversation

nicholaspai
Copy link
Member

@nicholaspai nicholaspai commented May 4, 2020

This PR contains several features

  • Increases gas limit on CI local Ethereum networks to more realistic (but conservative) 9,000,000.
  • Reduces # of runs on optimizer in truffle-config to default of 200. Fewer runs = lower deployment cost and less deployed bytecode. Source: Removing Contract Size Limit ethereum/EIPs#1662 (comment)
  • Change bytecode to deployedBytecode in CalculateContractBytecode.js script.
  • Abstract deployment of EMP into a library, which EMPCreator will call via createExpiringMultiParty(params). H/t to @mrice32 for the idea. This separates EMP bytecode on-chain from the EMP-creator, which means that the EMP-creator is no longer the bytecode bottleneck for deploying the EMP set of contracts.

deployedBytecode Statistics:


This PR:

  • EMP Creator = 3041 bytes
  • EMP = 17170 bytes
  • EMPLib = 20974 bytes

Previous:

  • EMP Creator = 22983 bytes
  • EMP = 17170 bytes

Integrating this PR will give us the neccessary bytecode space to merge in both #1305 and #1334 . #1305 adds ~900 bytes and #1334 adds ~500 bytes. The bytecode limit is ~24,500 bytes, so we will have plenty of bytecode remaining.

Signed-off-by: Nick Pai <npai.nyc@gmail.com>
… to save bytecide, fix CalculateContractBytecode.js bug

Signed-off-by: Nick Pai <npai.nyc@gmail.com>
@nicholaspai nicholaspai added this to the Expiring Multiparty milestone May 4, 2020
Signed-off-by: Nick Pai <npai.nyc@gmail.com>
@@ -17,6 +18,10 @@ module.exports = async function(deployer, network, accounts) {
const finder = await Finder.deployed();
const tokenFactory = await TokenFactory.deployed();

// Deploy EMPLib and link to EMPCreator.
await deploy(deployer, network, ExpiringMultiPartyLib);
await deployer.link(ExpiringMultiPartyLib, ExpiringMultiPartyCreator);
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there another example in the repo of linking libraries with contracts before deploying? I couldn't find one so I did it explicitly here.

Copy link
Member

Choose a reason for hiding this comment

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

If you look at the TokenizedDerivativeUtils library from before we removed it from the repo, we did the same thing there.

@mrice32
Copy link
Member

mrice32 commented May 4, 2020

@nicholaspai Could you also include bytecode statistics for this new library? It'll probably be the largest IIUC.

@nicholaspai
Copy link
Member Author

@nicholaspai Could you also include bytecode statistics for this new library? It'll probably be the largest IIUC.

great point. Done. Interesting the amount of overhead a library/contract has.

Copy link
Member

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

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

This looks great -- to be honest, my biggest worry here is that this will make bytecode verification on Etherscan more difficult.

@nicholaspai
Copy link
Member Author

This looks great -- to be honest, my biggest worry here is that this will make bytecode verification on Etherscan more difficult.

Do libraries make it more difficult? The optimizer runs # shouldn't affect it.

@mrice32
Copy link
Member

mrice32 commented May 4, 2020

This looks great -- to be honest, my biggest worry here is that this will make bytecode verification on Etherscan more difficult.

Do libraries make it more difficult? The optimizer runs # shouldn't affect it.

They shouldn't, but you have to plug them in manually, making the verification process slightly more complicated, so you never know :).

@nicholaspai
Copy link
Member Author

This looks great -- to be honest, my biggest worry here is that this will make bytecode verification on Etherscan more difficult.

Do libraries make it more difficult? The optimizer runs # shouldn't affect it.

They shouldn't, but you have to plug them in manually, making the verification process slightly more complicated, so you never know :).

Ah, should we hold off then and do a testnet deployment + verification?

Copy link
Member

@chrismaree chrismaree left a comment

Choose a reason for hiding this comment

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

This is great! thanks for doing these optimizations. Glad we can fit all the required PRs now :)

@mrice32
Copy link
Member

mrice32 commented May 5, 2020

This looks great -- to be honest, my biggest worry here is that this will make bytecode verification on Etherscan more difficult.

Do libraries make it more difficult? The optimizer runs # shouldn't affect it.

They shouldn't, but you have to plug them in manually, making the verification process slightly more complicated, so you never know :).

Ah, should we hold off then and do a testnet deployment + verification?

Nah, I wouldn't worry about it. We'll cross that bridge when we come to it.

@nicholaspai nicholaspai merged commit 3eba0cf into master May 5, 2020
@nicholaspai nicholaspai deleted the npai/empdeployer-lib branch May 5, 2020 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants