Skip to content

Add Buidler plugin / Finalize API #421

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

Merged
merged 21 commits into from
Nov 30, 2019
Merged

Add Buidler plugin / Finalize API #421

merged 21 commits into from
Nov 30, 2019

Conversation

cgewecke
Copy link
Member

@cgewecke cgewecke commented Oct 12, 2019

Continuation of #372. This is the root PR for phase II of the 0.7.x improvements.

Overview

  • Create buidler coverage plugin
  • Adapt integration test suite for buidler
  • Make plugin helpers/utilities +/- platform neutral.
  • E2E test vs MolochVentures/moloch (possibly others?)
  • Write Buidler and API documentation

Tasks

⚠️ Actively edited ⚠️

@cgewecke cgewecke mentioned this pull request Oct 12, 2019
13 tasks
@codecov-io
Copy link

codecov-io commented Oct 14, 2019

Codecov Report

Merging #421 into beta will increase coverage by 0.16%.
The diff coverage is 99.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##             beta     #421      +/-   ##
==========================================
+ Coverage   99.06%   99.22%   +0.16%     
==========================================
  Files          14       20       +6     
  Lines         639      779     +140     
==========================================
+ Hits          633      773     +140     
  Misses          6        6
Impacted Files Coverage Δ
plugins/resources/truffle.ui.js 100% <ø> (ø)
plugins/bin.js 100% <ø> (ø)
lib/validator.js 100% <ø> (ø) ⬆️
lib/ui.js 100% <ø> (ø) ⬆️
plugins/truffle.plugin.js 100% <100%> (ø)
utils.js 100% <100%> (ø)
plugins/resources/buidler.utils.js 100% <100%> (ø)
api.js 100% <100%> (ø)
lib/instrumenter.js 100% <100%> (ø) ⬆️
plugins/resources/truffle.utils.js 100% <100%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a43ee65...28ab3d2. Read the comment docs.

@cgewecke cgewecke marked this pull request as ready for review November 30, 2019 19:02
@cgewecke cgewecke merged commit 212c88f into beta Nov 30, 2019
@roderik
Copy link

roderik commented Dec 9, 2019

I was trying this plugin for buidler and we are using both the typescript stuff, and I need to pass a ganache setting. There is currently no d.ts file to extend the BuidlerConfig class.

@cgewecke
Copy link
Member Author

cgewecke commented Dec 9, 2019

@roderik Hi.

Could you elaborate on this problem a bit - you're configuring .solcover.js providerOptions and seeing a TS error?

@roderik
Copy link

roderik commented Dec 9, 2019

If you look at all the plugins, they all have a Typescript section in the docs: https://buidler.dev/plugins/nomiclabs-buidler-ethers.html#typescript-support

This makes it so that you can add plugin fields to the BuidlerConfig

const config: BuidlerConfig = {
  defaultNetwork: 'buidlerevm',
  solc: solcconfig,
  typechain: {
    target: 'ethers'
  },
  networks: {
    buidlerevm: {
      accounts: waffleDefaultAccounts.map(acc => ({
        balance: acc.balance,
        privateKey: acc.secretKey
      }))
    }
  },
  analytics: {
    enabled: false
  }
};

Without these, i cannot add the solidity-coverage configuration to the config. (for some reason configuring it via a separate config file did not pass my testrpc options)

@cgewecke
Copy link
Member Author

cgewecke commented Dec 9, 2019

@roderik Ah, this plugin is not designed the same way - it loads the options itself from .solcover.js only.

for some reason configuring it via a separate config file did not pass my testrpc options

That's a serious problem - if you share the solcover.js config I will try to reproduce and fix.

@cgewecke
Copy link
Member Author

cgewecke commented Dec 9, 2019

@roderik

There's also a working 'complex project' example at sc-forks/moloch that might be helpful for comparison purposes....

Their buidler config defines a coverage network:

coverage: {
   url: 'http://localhost:8555'
}

Their .solcover.js includes provider settings like mnemonic

module.exports = {
  client: require('ganache-cli'),
  providerOptions: {
    mnemonic: "fetch local valve black attend double eye excite planet primary install allow"
  },
  skipFiles: [
    'Migrations.sol',
    'Token.sol',
    'oz',
    'gnosis-safe'
  ]
}

And lastly, the command is launched:

npx buidler coverage --network coverage

@cgewecke
Copy link
Member Author

@roderik If you need any help or even if you get this working, please just lmk

You're one of the first users and an experienced veteran - it would be great if you told me everything that's wrong with it :)

@roderik
Copy link

roderik commented Dec 10, 2019

Will retry today, it has something to do with Waffle / buidlerevm / test-rpc

I put the waffle addresses into buidlerem like this

    buidlerevm: {
      accounts: waffleDefaultAccounts.map(acc => ({
        balance: acc.balance,
        privateKey: acc.secretKey
      }))
    }

and that works perfectly. When I switch to coverage, test-rpc starts with different addresses, and they have no ETH. So I tried to set the gasprice at 0 via the .solcover.js file, but that did not stick.

Then i was reading the code and i had the impression (could have misread) that putting a solcoverjs key in the builder config would also be picked up as config. There i have the accounts as well and i was thinking to push the addresses into test-rpc.

But then there was no typescript definition so all hell broke loose :)

Will start again and try to make it work. Will share the result

@cgewecke
Copy link
Member Author

Ah @roderik ganache is currently a requirement because of the way the coverage tool collects data - it launches a server instance and listens to its EVM. You just need to require it to the solcoverjs client option.

Am talking to NomicLabs about how to integrate BuidlerEVM - it's new. Looks amazing actually.

I think you can do something very similar to the Moloch config in the comment above, but adapt the private key / balance array to the format described in the ganache-core options.

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