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

Мigrate to truffle #146

Merged
merged 113 commits into from
Nov 30, 2020
Merged

Мigrate to truffle #146

merged 113 commits into from
Nov 30, 2020

Conversation

elenadimitrova
Copy link
Contributor

@elenadimitrova elenadimitrova commented Jul 29, 2020

Here we migrate the solution from etherlime to the truffle framework. Truffle is used for compilation (using docker), testing, deployment and its extensions framework additionally implements code coverage, gas cost testing and etherscan verification.

Notable changes include

  • Using bn.js consistently for comparing and working with big numbers
  • Network ganache is now named development, note that configuration file has changed from ganache.json to development.json
  • The deployer role is now minimal and it's no longer required to deploy new instances of a contract or create references to existing contracts. e.g. deployer.deploy(GuardianStorage) becomes GuardianStorage.new() and deployer.wrapDeployedContract(ENS, newConfig.ENS.ensRegistry) becomes ENS.at(newConfig.ENS.ensRegistry).
  • verboseWaitForTransaction is no longer required as the transaction receipt is returned directly for each contract call.
  • .contractAddress becomes .address
  • Contracts loading from outside the build folder is done via @truffle\contract until Allow artifacts loading from outside contracts and test folders trufflesuite/truffle#3436 is resolved

Plugins used

These reports differ to the benchmark script results in that gas consumption per method is given which doesn't take into consideration chained calls though Proxy and/or Relayer contracts (meta transactions). We could accept these instances as relayed transactions take on average the same amount of extra gas ~45,000 while the Proxy calls are relevant only for the BaseWallet calls which are minimal.

blockers

nice-to-haves

@elenadimitrova elenadimitrova self-assigned this Jul 29, 2020
@elenadimitrova elenadimitrova force-pushed the experiment/migrate-to-truffle branch 2 times, most recently from 5031307 to 26d129c Compare October 20, 2020 06:59
@elenadimitrova elenadimitrova force-pushed the experiment/migrate-to-truffle branch 3 times, most recently from 1bd3946 to d481b6d Compare October 22, 2020 14:54
@elenadimitrova elenadimitrova force-pushed the experiment/migrate-to-truffle branch 6 times, most recently from 381180b to 54d619e Compare November 4, 2020 12:58
@elenadimitrova elenadimitrova force-pushed the experiment/migrate-to-truffle branch 5 times, most recently from d55613c to 0bc3141 Compare November 9, 2020 12:22
@elenadimitrova elenadimitrova marked this pull request as ready for review November 9, 2020 12:22
@elenadimitrova elenadimitrova force-pushed the experiment/migrate-to-truffle branch 7 times, most recently from 9f7f4f4 to 6720304 Compare November 11, 2020 12:29
const VersionManagerWrapper = await VersionManager.new(config.modules.VersionManager);

const ModuleRegistryWrapper = await ModuleRegistry.new(config.contracts.ModuleRegistry);
const MultiSigWrapper = await MultiSig.new(config.contracts.MultiSigWallet);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be ContractWrapper = await Contract.at(config...) instead of using ContractWrapper = await Contract.new(config...)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks, updated.

@elenadimitrova elenadimitrova force-pushed the experiment/migrate-to-truffle branch 3 times, most recently from f0f7c30 to 64d92fb Compare November 28, 2020 14:55
const { pkey, infuraKey } = manager;
return (pkey, infuraKey);
function getKeys() {
// NOTE: While https://github.com/trufflesuite/truffle/issues/1054 is implemented we are using a temporary fix
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI we used to have a custom AWSWalletProvider to solve that issue https://github.com/argentlabs/solidity/blob/dev/truffle-awswallet-provider/index.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a card to investigate that if the truffle fix is delayed

@elenadimitrova elenadimitrova merged commit 9f1f8ed into develop Nov 30, 2020
@elenadimitrova elenadimitrova deleted the experiment/migrate-to-truffle branch November 30, 2020 13:03
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.

None yet

4 participants