fix: add_rollup test #19438
fix: add_rollup test #19438
Conversation
spalladino
left a comment
spalladino
left a comment
There was a problem hiding this comment.
Looks good! Just a note on awaiting txs and a possible refactor. Feel free to ignore if you think it's not worth it!
| `Minting ${mintsNeeded} times (${mintAmount} per mint) to fund new FeeJuicePortal at ${newFeeJuicePortalAddress}`, | ||
| ); | ||
| for (let i = 0; i < mintsNeeded; i++) { | ||
| await feeAssetHandler.write.mint([newFeeJuicePortalAddress.toString() as Hex]); |
There was a problem hiding this comment.
Heads up this won't wait for the tx to be mined. It should be ok since the deployL1Contract call below will wait until its own tx is mined, which means all older txs will also be awaited to be mined.
Alternatively, you can clean up much of the code here by using ethereum/src/contracts/fee_asset_handler.ts which abstracts a few things. I don't think it handles this loop, but we could just add it there and hide this complexity.
There was a problem hiding this comment.
Thanks for the great suggestions! I've changed it to use the contract helper (also updated it so the apis are consistent with other contracts). And realized I could call setMintAmount so the code is cleaner now!
LeilaWang
force-pushed
the
lw/fix_add_rollup
branch
from
f393c78 to
ad5b30f
Compare
spalladino
left a comment
spalladino
left a comment
There was a problem hiding this comment.
Looks great, thanks for the changes. Out of curiosity, what changed recently that caused this to break?
|
We changed the out hash to be inserted when an epoch is proven. So for this test, I had to create the prover in order to submit the epoch proof and update the out hash. And the code path for rewards is triggered, so now we need to fund it 🥲 Previously, the out hash was inserted when a checkpoint is proposed. And we used the cheatcode to simply update the proven checkpoint number for it to be able to be consumed. |
Fix the
add_rollupe2e test by funding the new rollup's fee juice portal.