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: Spawn NodeExecutor before applying replay transactions #544

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

dutterbutter
Copy link
Collaborator

What 💻

Why ✋

  • When making use of replay_tx we were sending a command over a channel and then waiting for a response but the NodeExecutor had not been started yet, so it never read that command or sent back a reply which caused the node to hang.

Evidence 📷

Include screenshots, screen recordings, or console output here demonstrating that your changes work as intended

./target/release/anvil-zksync --reset-cache=true --cache=none replay_tx --fork-url mainnet 0x4f8a6c93fee26a3dd01ee0c92f0adeb78960ba36b20b24de61392983234e60dd
20:40:41  INFO 
20:40:41  INFO Validating 0x6dbfcea28c98a8ee26a3688436c29be3579f70adb90ee801eaee976e7b364996
20:40:41  INFO Executing 0x6dbfcea28c98a8ee26a3688436c29be3579f70adb90ee801eaee976e7b364996
20:40:44  INFO 
20:40:44  INFO ✅  [SUCCESS] Hash: 0x6dbfcea28c98a8ee26a3688436c29be3579f70adb90ee801eaee976e7b364996
20:40:44  INFO Initiator: 0x29fe51e7faeca871ce07a790bac48bbc1c9868fb
20:40:44  INFO Payer: 0x29fe51e7faeca871ce07a790bac48bbc1c9868fb
20:40:44  INFO Gas Limit: 3_000_000 | Used: 273_310 | Refunded: 2_726_690
20:40:44  INFO Paid: 0.0000123673 ETH (273310 gas * 0.04525000 gwei)
20:40:44  INFO Refunded: 0.0001233827 ETH
20:40:44  INFO 
20:40:44  INFO 
20:40:44  INFO Validating 0x4f8a6c93fee26a3dd01ee0c92f0adeb78960ba36b20b24de61392983234e60dd
20:40:44  INFO Executing 0x4f8a6c93fee26a3dd01ee0c92f0adeb78960ba36b20b24de61392983234e60dd
20:40:46  INFO 
20:40:46  INFO ✅  [SUCCESS] Hash: 0x4f8a6c93fee26a3dd01ee0c92f0adeb78960ba36b20b24de61392983234e60dd
20:40:46  INFO Initiator: 0x03ac0b1b952c643d66a4dc1fbc75118109cc074c
20:40:46  INFO Payer: 0x03ac0b1b952c643d66a4dc1fbc75118109cc074c
20:40:46  INFO Gas Limit: 1_000_000 | Used: 313_047 | Refunded: 686_953
20:40:46  INFO Paid: 0.0000141654 ETH (313047 gas * 0.04525000 gwei)
20:40:46  INFO Refunded: 0.0000310846 ETH

@dutterbutter dutterbutter requested a review from a team as a code owner January 16, 2025 20:49
@dutterbutter dutterbutter requested a review from itegulov January 16, 2025 20:59
@dutterbutter
Copy link
Collaborator Author

dutterbutter commented Jan 16, 2025

@itegulov we should probably cut another release once this is merged.

Any ideas how we can add an e2e test for replay_tx? I suppose its the same situation with e2e testing fork.

Copy link
Contributor

@itegulov itegulov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah testing-wise same situation I am afraid.

On the general main flow I think we should consolidate all commands to reuse common parts (maybe return a list of futures they require to run). Maybe even something akin to node framework from zksync-era that was done by @popzxc

@dutterbutter dutterbutter merged commit 3afd716 into main Jan 17, 2025
14 checks passed
@dutterbutter dutterbutter deleted the db/fix-replay-transactions branch January 17, 2025 13:36
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.

[BUG] Replay transactions broken
2 participants