When progressInterop() returns a DecisionRewind, this indicates that the state of the Supernode needs to be rewound due to an invalid L1 head. In this case, applyRewindPlan() is called to perform this rewind. In addition to rewinding the VerifiedDB, LogsDB and DenyList, applyRewindPlan() also calls RewindEngine() on the ChainContainer:
|
if plan.ResetAllChainsTo != nil { |
|
if err := chain.RewindEngine(i.ctx, *plan.ResetAllChainsTo, eth.BlockRef{}); err != nil { |
|
i.log.Error("failed to reset chain engine on rewind", "chain", chainID, "err", err) |
|
recordErr(fmt.Errorf("chain %s: reset chain engine on rewind: %w", chainID, err)) |
|
} |
|
} |
This call rewinds the executing engine via the EngineController to the previous timestamp. However, this is not actually necessary in this case, since the goal is to rewind the state of the Supernode to align with the current state of the chains, not the other way around. In fact, the DecisionRewind case indicates that the chains have suffered or are suffering a reorg, and it might be preferable to just keep re-trying the cross-validation loop until they have settled into a consistent state (and the Supernode has been rewound far back enough to be consistent with the current chain state).
Additionally, rewinding the engine in the DecisionRewind case brings up the following issues:
- If
applyRewindPlan() rewinds the engine but still fails because of an error when rewinding the LogsDB, the rewind will remain as a pending transition and re-attempted on the next iteration. In this case, the engine will be rewound again to the same block, repeating the procedure which involves two forkchoice updates (one to a synthetic block, and another back to the actual target block). Additionally, if there is the possibility that the chain has advanced beyond that block, or suffered a reorg to a different fork, it will be brought back unnecessarily.
- As can be seen above,
RewindEngine() is not actually called when plan.ResetAllChainsTo == nil. This happens in the case when the rewind would completely clear the VerifiedDB and LogsDB (and rewind back to before the activation timestamp), and is because if plan.ResetAllChainsTo == nil we lack information of what timestamp to rewind the engine back to. However, this is inconsistent: if we want to rewind the engine on DecisionRewind, it would be expected to also happen in this case. Avoiding rewinding the engine entirely would avoid this inconsistency.
When
progressInterop()returns aDecisionRewind, this indicates that the state of the Supernode needs to be rewound due to an invalid L1 head. In this case,applyRewindPlan()is called to perform this rewind. In addition to rewinding theVerifiedDB,LogsDBandDenyList,applyRewindPlan()also callsRewindEngine()on theChainContainer:_audits_Ethereum-optimism_optimism_interopv2/op-supernode/supernode/activity/interop/interop.go
Lines 560 to 565 in bd29b7d
This call rewinds the executing engine via the
EngineControllerto the previous timestamp. However, this is not actually necessary in this case, since the goal is to rewind the state of the Supernode to align with the current state of the chains, not the other way around. In fact, theDecisionRewindcase indicates that the chains have suffered or are suffering a reorg, and it might be preferable to just keep re-trying the cross-validation loop until they have settled into a consistent state (and the Supernode has been rewound far back enough to be consistent with the current chain state).Additionally, rewinding the engine in the
DecisionRewindcase brings up the following issues:applyRewindPlan()rewinds the engine but still fails because of an error when rewinding theLogsDB, the rewind will remain as a pending transition and re-attempted on the next iteration. In this case, the engine will be rewound again to the same block, repeating the procedure which involves two forkchoice updates (one to a synthetic block, and another back to the actual target block). Additionally, if there is the possibility that the chain has advanced beyond that block, or suffered a reorg to a different fork, it will be brought back unnecessarily.RewindEngine()is not actually called whenplan.ResetAllChainsTo == nil. This happens in the case when the rewind would completely clear theVerifiedDBandLogsDB(and rewind back to before the activation timestamp), and is because ifplan.ResetAllChainsTo == nilwe lack information of what timestamp to rewind the engine back to. However, this is inconsistent: if we want to rewind the engine onDecisionRewind, it would be expected to also happen in this case. Avoiding rewinding the engine entirely would avoid this inconsistency.