add e2e test follows_most_recent_chain

[jonastheis]

No description provided.

[frisitano]

@jonastheis is this still relevant?

[frisitano]

closing as stale

[jonastheis]

I do believe this is still relevant and we should add an e2e test that asserts our fork-choice rule

[frisitano]

We have the following. Can you review if we need to add some more please?

rollup-node/crates/node/tests/sync.rs

Lines 541 to 599 in f0c0fe7

	#[allow(clippy::large_stack_frames)]
	#[tokio::test]
	async fn test_chain_orchestrator_reorg_with_gap_above_head() -> eyre::Result<()> {
	test_chain_orchestrator_fork_choice(100, Some(95), 20, |e| {
	if let ChainOrchestratorEvent::ChainReorged(chain_import) = e {
	// Assert that the chain import is as expected.
	assert_eq!(chain_import.chain.len(), 21);
	true
	} else {
	false
	}
	})
	.await
	}
	#[allow(clippy::large_stack_frames)]
	#[tokio::test]
	async fn test_chain_orchestrator_reorg_with_gap_below_head() -> eyre::Result<()> {
	test_chain_orchestrator_fork_choice(100, Some(50), 20, |e| {
	if let ChainOrchestratorEvent::ChainReorged(chain_import) = e {
	// Assert that the chain import is as expected.
	assert_eq!(chain_import.chain.len(), 21);
	true
	} else {
	false
	}
	})
	.await
	}
	#[allow(clippy::large_stack_frames)]
	#[tokio::test]
	async fn test_chain_orchestrator_extension_with_gap() -> eyre::Result<()> {
	test_chain_orchestrator_fork_choice(100, None, 20, |e| {
	if let ChainOrchestratorEvent::ChainExtended(chain_import) = e {
	// Assert that the chain import is as expected.
	assert_eq!(chain_import.chain.len(), 21);
	true
	} else {
	false
	}
	})
	.await
	}
	#[allow(clippy::large_stack_frames)]
	#[tokio::test]
	async fn test_chain_orchestrator_extension_no_gap() -> eyre::Result<()> {
	test_chain_orchestrator_fork_choice(100, None, 0, |e| {
	if let ChainOrchestratorEvent::ChainExtended(chain_import) = e {
	// Assert that the chain import is as expected.
	assert_eq!(chain_import.chain.len(), 1);
	true
	} else {
	false
	}
	})
	.await
	}

[jonastheis]

I think one case that is missing: the block of the new canonical chain at fork height is older than the block in the new fork at the same height. however, the canonical chain is still the former (and has newer timestamps in the future).

[frisitano]

I've tried to illustrate my interpretation of this case in the diagram below. Are you saying that for block n+1 the fork has a newer block, but for block n+2 the canonical chain has a newer block? block number described by n and slot and time by t.

graph LR
    subgraph Canonical Chain
        A["Block n-2<br/>t<sub>n-2</sub>"]
        B["Block n-1<br/>t<sub>n-1</sub>"]
        C["Block n<br/>t<sub>n</sub>"]
        D["Block n+1<br/>t<sub>n+1</sub>"]
        E["Block n+2<br/>t<sub>n+4</sub>"]
        A --> B --> C --> D --> E
    end

    subgraph Fork
        F["Block n+1<br/>t<sub>n+2</sub>"]
        G["Block n+2<br/>t<sub>n+3</sub>"]
        C --> F --> G
    end

Loading

[jonastheis]

Yeah exactly. It was one of the cases that I encountered when switching between l2geth sequencer and l2reth sequencer but could happen in practice as well

[frisitano]

Yeah exactly. It was one of the cases that I encountered when switching between l2geth sequencer and l2reth sequencer but could happen in practice as well

Got it, so the expected behaviour is that initially the follower would follow the fork at block n+1 and then after it received block n+2 it would follow the canonical chain?

[jonastheis]

Yep

[frisitano]

Yep

I'm just trying to understand how this could occur in practice?