feat[contracts]: slightly better account funding for hardhat accounts#1056
feat[contracts]: slightly better account funding for hardhat accounts#1056smartcontracts wants to merge 25 commits intoregenesis/0.4.0from
Conversation
* add ovmCALLVALUE context * add ovmBALANCE * test success and revert cases * test empty contract case * chore: lint
* fix ovmDELEGATECALL type, update tests * add ovmSELFBALANCE * fix ovmDELEGATECALL jumping to CALL * chore: lint
…as lower than the ovmCALLVALUE
🦋 Changeset detectedLatest commit: 985cefc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
Codecov Report
@@ Coverage Diff @@
## feat/OP759/value-calls #1056 +/- ##
=======================================================
Coverage 85.95% 85.95%
=======================================================
Files 52 52
Lines 1972 1972
Branches 308 308
=======================================================
Hits 1695 1695
Misses 277 277 Continue to review full report at Codecov.
|
| const depositAmount = balance.div(2) // Deposit half of the wallet's balance into L2. | ||
| await Proxy__OVM_L1ETHGateway.connect(wallet).deposit(8_000_000, '0x', { | ||
| value: depositAmount, | ||
| gasLimit: 2_000_000, // Idk, gas estimation was broken and this fixes it. |
There was a problem hiding this comment.
Do you think this a problem with the contract being non deterministic in its gas usage or a hardhat thing?
There was a problem hiding this comment.
It seemed to be a hardhat thing that had to do with doing the transactions in parallel. Not exactly sure of the root cause but didn't seem important.
There was a problem hiding this comment.
I have also seen the enqueue gas burn mess with gas estimates before, I don't think this is a new issue.
| hre, | ||
| 'Proxy__OVM_L1ETHGateway', | ||
| { | ||
| iface: 'OVM_L1ETHGateway', |
There was a problem hiding this comment.
Just a note that the order of merging of this against #988 will matter here.
ben-chain
left a comment
There was a problem hiding this comment.
LGTM, commented one way this might potentially be sped up but looks good. Will approve once you respond and/or update.
| ) | ||
| const balance = await wallet.getBalance() | ||
| const depositAmount = balance.div(2) // Deposit half of the wallet's balance into L2. | ||
| await Proxy__OVM_L1ETHGateway.connect(wallet).deposit(8_000_000, '0x', { |
There was a problem hiding this comment.
To make this go faster, I think we can consider removing this await entirely. Becuase the deposits do not matter until they are synced by l2geth, which has not even started yet when this process is going, it feels to me like there will not be a race condition.
There was a problem hiding this comment.
I originally did it because I was having some issues with hardhat not processing things properly in parallel. I can try removing the sleep but didn't really seem like we really needed to save the extra 4 seconds.
ff0f828 to
057de77
Compare
f4ca0c3 to
3dc67db
Compare
|
replaced with #1065 due to some crazy merge conflicts |
Description
Slightly improves the funding mechanism by (1) making it parallel and (2) making it always fund the default hardhat accounts instead of funding whichever accounts are present in the config.