Escape Hatch #243
Escape Hatch #243
Conversation
…instead sequence based on hotshot liveness at l1 block height.
…cape hatch test to e2e test.
…overeign sequencer is enabled.
zacshowa
requested review from
ImJeremyHe,
alysiahuggins,
Sneh1999,
sveitser and
jbearer
as code owners
…e no longer an issue.
…o-espresso-integration into escape-hatch-simple
cmd/replay/main.go
Outdated
| // The following call will be used to check the liveness based on the l1 block height. | ||
| // isHotShotLive := wavmio.IsHotShotLive(message.Message.Header.BlockNumber) | ||
| // | ||
| // The following line should be changed to if validatingAgainstEspresso && isHotShotLive { |
There was a problem hiding this comment.
As discussed, we dont need this because we will not be creating espresso transactions if the escape hatch is enabled
There was a problem hiding this comment.
I can remove this comment then!
execution/gethexec/sequencer.go
Outdated
| err error | ||
| ) | ||
|
|
||
| if l1Reader != nil { |
There was a problem hiding this comment.
we should add a comment here describing why we are creating a light client reader for other readers
| // Initialize shouldSequenceWithEspresso to false and if we have a light client reader then give it a value based on hotshot liveness | ||
| // This is a side effect of the sequencer having the capability to run without an L1 reader. For the Espresso integration this is a necessary component of the sequencer. | ||
| // However, many tests use the case of having a nil l1 reader | ||
| if s.lightClientReader != nil { |
There was a problem hiding this comment.
I am thinking this might cause some issues - so lets say I have espresso enabled but I run the sequencer without the lightClientReader , I will never create espresso transactions then. It could just be a honest configuration issue but I might not realize it. Let me know your thoughts on the same!
There was a problem hiding this comment.
You're right that this would be an issue if someone were to attempt to set up a sequencer node without enabling the parent chain reader, but in that case it should act as a normal nitro sequencer.
I think it's a fine bound to add to our sequencer that we need an l1 reader for it to function in espresso mode properly. Fundamentally we need to read from the light client to decide how to sequence with the escape hatch. Doing it this way allows normal nitro sequencers with this code base, but also adds compatibility with espresso. I think its fine to expect node operators to configure their sequencer appropriately when they want to use espresso.
There was a problem hiding this comment.
As discussed, it might be best to just use the sovereign sequencer flag and if it is turned on, return an error if a sequencer's lightClientReader is nil
…we can't construct a sequencer with a nil lightClientReader
Closes #221
This PR:
Adds a check in the sequencer that takes the l1 block height for transactions that are about to be sequencedand determines if hotshot is live based on that height. If hot shot is live we designate these transactions as espresso messages and attempt to sequence them with the sovereign sequencer. otherwise we designate them as normal Arbitrum messages and sequence them based on normal Arbitrum behavior.
In the future the
shouldSequenceWithEspressowill also be gated by thechainConfig.EnableEspressoflag to enable correct migration.This PR may break the espresso finality node test as we have no clear way to disable the sovereign sequencer until we add the chain config checks. I will fix this in the PR related to adding chain config checks.
This PR does not:
Add any checks related to the chain config.
Key places to review:
sequencer.goinexecution/gethexecand the E2E test for correctness.Run the E2E test with
go test -run ^TestEspressoE2E$ github.com/offchainlabs/nitro/system_tests -vThe E2E test passes locally.