OG-588 Hardhat migration #683
Conversation
in most cases, hardhat is a replacement for ganache. fas NOTE: some tests STILL fail
| + const isHardHat = | ||
| + res && typeof res === "object" && res.error && res.error.message && !res.error.data; | ||
| + | ||
| + if (isHardHat) { | ||
| + if (res.error.message.includes('Transaction reverted without a reason string')) return 'revert' | ||
| + return res.error.message.match('VM Exception while processing transaction: reverted with reason string \'(.*)\'')[1] | ||
| + } |
There was a problem hiding this comment.
can you elaborate and write a very descriptive comment in this patchfile?
I was hoping we can get rid of patchfiles one day...
There was a problem hiding this comment.
On eth_call, ganache returns error object that has message, code, data fields. Truffle reads the revert reason from res.error.data in this case. But hardhat returns error object only with message and code, so to return the revert reason, I needed to parse res.error.message.
In general, when eth_call returns an error, the spec doesn't specify a data field containing the revert reason, so truffle is tailored specifically for ganache here
There was a problem hiding this comment.
can you please write this comment in the patch itself, maybe? it will be impossible to remember few months later
hardhat.config.ts
Outdated
| import '@nomiclabs/hardhat-web3' | ||
|
|
||
| module.exports = { | ||
| solidity: '0.7.3', |
There was a problem hiding this comment.
I know we don't use it yet to compile, but better to write here 0.8.7 anyways..
There was a problem hiding this comment.
Yes that was הכנה למזגן קצת
hardhat.config.ts
Outdated
| networks: { | ||
| hardhat: { chainId: 1337 }, | ||
| npmtest: { // used from "npm test". see package.json | ||
| verbose: process.env.VERBOSE, |
There was a problem hiding this comment.
verbose is not a hardhat parameter (I use .ts config, and it complains on unknown extra params)
There was a problem hiding this comment.
Weird, for me it doesn't complain. I can remove though
hardhat.config.ts
Outdated
| npmtest: { // used from "npm test". see package.json | ||
| verbose: process.env.VERBOSE, | ||
| url: 'http://127.0.0.1:8544', | ||
| network_id: '*' |
There was a problem hiding this comment.
network_id is also not defined (hh always reads from the provider the net/chain id)
There was a problem hiding this comment.
I know, forgot to remove after copy pasta from truffle.js
|
|
||
| misbehavingPaymaster = await TestPaymasterConfigurableMisbehavior.new() | ||
| await misbehavingPaymaster.setRevertPreRelayCallOnEvenBlocks(true) | ||
| await misbehavingPaymaster.setTrustedForwarder(forwarderInstance.address) | ||
| await misbehavingPaymaster.setRelayHub(relayHub.address) | ||
| await misbehavingPaymaster.deposit({ value: web3.utils.toWei('1', 'ether') }) | ||
|
|
||
| const relayProvider = await RelayProvider.newProvider({ | ||
| relayProvider = await RelayProvider.newProvider({ |
There was a problem hiding this comment.
to have access to relayProvider in the test, to stub validateRelayCall
| @@ -145,6 +144,7 @@ export class ServerTestEnvironment { | |||
| config: mergedConfig | |||
| }) | |||
| await this.relayClient.init() | |||
| this.gasLess = toChecksumAddress(this.relayClient.newAccount().address) | |||
There was a problem hiding this comment.
better move the toChecksumAddress INTO our newAccount method..
There was a problem hiding this comment.
You might be onto something 😆
| @@ -21,7 +21,7 @@ contract('AccountManager', function (accounts) { | |||
| const address = '0x982a8CbE734cb8c29A6a7E02a3B0e4512148F6F9' | |||
| const privateKey = '0xd353907ab062133759f149a3afcb951f0f746a65a60f351ba05a3ebf26b67f5c' | |||
| const config = configureGSN({ | |||
| methodSuffix: '', | |||
| methodSuffix: '_v4', | |||
There was a problem hiding this comment.
yes, but you changed the default in the provider to "_v4"
| @@ -61,7 +61,7 @@ export async function prepareTransaction (testRecipient: TestRecipientInstance, | |||
| relayData: { | |||
| pctRelayFee: '1', | |||
| baseRelayFee: '1', | |||
| gasPrice: '1', | |||
| gasPrice: '4494095', | |||
There was a problem hiding this comment.
why this number? I undetstand "500000" or "1", but this seems to be explicit number (or random...)
There was a problem hiding this comment.
That's just the baseFee hardhat had, but it can be 1e9 like the rest. That's the first test I changed so...
Truffle patch file note:
On eth_call, ganache returns error object that has message, code, data fields. Truffle reads the revert reason from res.error.data in this case. But hardhat returns error object only with message and code, so to return the revert reason, I needed to parse res.error.message.
In general, when eth_call returns an error, the spec doesn't specify a data field containing the revert reason, so truffle is tailored specifically for ganache here