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

bug: pending txs should be stored in graceful shutdown #373

Open
1 task done
apoorvsadana opened this issue Nov 3, 2024 · 2 comments · May be fixed by #372
Open
1 task done

bug: pending txs should be stored in graceful shutdown #373

apoorvsadana opened this issue Nov 3, 2024 · 2 comments · May be fixed by #372

Comments

@apoorvsadana
Copy link
Contributor

Is there an existing issue?

  • I have searched the existing issues

Description of bug

If some transactions have been processed due to ticks but haven't been included in a block because the block hasn't sealed, then if you close the sequencer and restart, the txs will be lost.

Steps to reproduce

To reproduce

  1. Start a madara sequencer with 2s tick time and 1hr block time
  2. Send a basic txn and wait for receipt => works
  3. Restart madara (before block is created so anytime before 1hr works)
  4. Get receipt again => fails

This can break tooling like explorers or other apps that index pending blocks. Maybe not a good design but we should still try to track processed txs across restarts.

@cchudant
Copy link
Member

cchudant commented Nov 4, 2024

Ah, it was a known bug but I was about to fix it when implemented saving mempool to db. When block prod is enabled it always recreates a pending block on startup, it can't restart exec from a current pending block.
I don't see how this would break explorers and other apps, i would guess this is something that be handled by their reorg handling.
But yea we need to fix that, what would you prefer between:

  • On restart, continue the pending block, unchanged
  • On restart, recreate a pending block and execute all of the transactions from the old pending block in the new pending block.

The second option would allow us to update the gas prices on restart (which would be useful after stopping for long), and would also allow us to change protocol version & other stuff across the restart if the configuration change, so that it's taken into account for this pending block and not the next one
However, any change in the header (like gas prices) could in theory make the transactions revert after restart, where we would have accepted them earlier.

For the first option, I believe we also have to reexecute the transactions because we can't supply the old state update & execution state to blockifier easily - the difference would only be that we don't recreate the block header

What do you think? @apoorvsadana

@apoorvsadana
Copy link
Contributor Author

Thanks for this detailed explanation like always!

I don't see how this would break explorers and other apps, i would guess this is something that be handled by their reorg handling.

If the explorer has reorg handling, it mostly won't break but I can see issues happening if users can't trust pending receipts. For v0, block times will mostly be large because of proving costs but the reason that's fine is that we've pending receipts so you still get soft confirmations and good UX. For example, Pragma is considering doing hyperlane bridging with pending txs, if on restart the txn is lost, it can be a issue. cc @EvolveArt @akhercha.

However, any change in the header (like gas prices) could in theory make the transactions revert after restart, where we would have accepted them earlier.

You're right, I think we can't afford this for now because our block times are large so people will rely on pending receipts. So they need to be pretty strong confirmations. if there's a chance that it can be reverted, it will be difficult to create a good UX. So I would suggest we do option (1) if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants