feat(conductor, proto)!: celestia base heights in commitment state #1121
Conversation
SuperFluffy
left a comment
There was a problem hiding this comment.
Looks good. This is not strictly necessary, but I think I would like to set the first Celestia heright that is fetched on startup to celestia_base_height.saturating_sub(celestia_variance).
Also, given that this is a breaking change anyway, can you also bump the celestia base height (and variance) to uint32/u64? It turns out that when interacting with most of their RPCs u64 is the expected width, and not u32.
@SuperFluffy This is intentionally not the behavior here. The base is the lowest height to look, the variance defines how far above you can go. Long term we need better logic in conductor on setting the base height. Right now this is acceptable because we never post out of order. |
|
|
||
| use self::state::StateSender; | ||
|
|
||
| type CelestiaHeight = u64; |
There was a problem hiding this comment.
note: doing this because it makes the update enums much more clear or what is being passed in.
| // The lowest block number of celestia chain to be searched for rollup blocks given current state | ||
| uint64 base_celestia_height = 3; |
There was a problem hiding this comment.
is this equal to the celestia height the firm block was in? it seems so from the code but want to double check
There was a problem hiding this comment.
Right now yes, long term not necessarily. This is ok right now because sequencer relayer posts such that astria blocks are in order in celestia blocks and we have a single relayer. Long term need to add more complex logic into conductor with timestamps and acceptable time smear.
* main: fix(charts): conductor configmap fix (#1146) feat(sequencer): add `allowed_fee_asset_ids` abci query and `sequencer_client` support (#1127) chore(conductor): release conductor 0.17 (#1139) feat: use macro to declare metric constants (#1129) refactor(merkle): remove source of panics in audit API (#1137) feat(conductor): skip outdated block metadata (#1120) refactor(sequencer): remove mint module (#1134) feat(bridge-withdrawer): add justfile (#1135) chore(chart): change evm back to latest on dev (#1132) feat(conductor, proto)!: celestia base heights in commitment state (#1121)
Summary
Updates the execution api to have celestia base height as a part of commitment instead of genesis, requiring rollups to track the base celestia height as it increases. This enables conductor on resync to move much faster.
astriaorg/astria-geth#22 contains an implementation which works with this PR.
Background
Conductor resync needs to grab all data from genesis for the rollup currently, even if much of that data is previously executed.
Changes
Testing
Manually tested running linked PR in dev cluster, updated blackbox tests.
Breaking Changelist