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

fix: use dedicated gas limit for each tx in inspect_batch #764

Merged
merged 5 commits into from
Dec 4, 2024

Conversation

elfedy
Copy link
Contributor

@elfedy elfedy commented Dec 3, 2024

What 💻

Use dedicated gas limit for each tx in inspect_batch.

Why ✋

After marking factory dependencies and obtaining the amount of pubdata to be published to the L1, the bootloader checks If there will be enough gas to pay for it. If there's not, it will nearCallPanic().
This combined with a very big gasPerPubdata default and the fact that we shared the gas limit among all batches made the script run revert on cases with large factory deps. We need to use a full gas-limit for each tx on the batch.

Closes #650

Evidence 📷

Previously failing can_deploy_factory_deps tests now pass.

Notes 📝

Something related to this problem is the very high gasPerPubdata that the local vm has by default and the fact that when forking an url, the fair_l2_gas_price and l1_gas_price values from the fork that would lead to real-world gasPerPubdata values are not copied correctly to the local environment. This is a problem to be addressed on a future PR.

@elfedy elfedy requested a review from a team as a code owner December 3, 2024 17:51
Karrq
Karrq previously approved these changes Dec 3, 2024
crates/zksync/core/src/vm/inspect.rs Outdated Show resolved Hide resolved
@elfedy elfedy requested a review from Karrq December 4, 2024 11:49
@elfedy elfedy merged commit da9586f into main Dec 4, 2024
12 checks passed
@elfedy elfedy deleted the elfedy-factory-deps-batch branch December 4, 2024 13:45
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.

Fix factory dependencies batching
2 participants