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

op-e2e: Add FPP test for empty blocks #5573

Merged
merged 2 commits into from
May 3, 2023
Merged

op-e2e: Add FPP test for empty blocks #5573

merged 2 commits into from
May 3, 2023

Conversation

Inphi
Copy link
Contributor

@Inphi Inphi commented May 2, 2023

Description

Add e2e test asserting that the fault proof program handles missing batches and empty blocks.

Metadata

@changeset-bot
Copy link

changeset-bot bot commented May 2, 2023

⚠️ No Changeset found

Latest commit: 574a340

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link

netlify bot commented May 2, 2023

Deploy Preview for opstack-docs canceled.

Name Link
🔨 Latest commit 574a340
🔍 Latest deploy log https://app.netlify.com/sites/opstack-docs/deploys/6451ae198f3ede00082d3333

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to play with the code directly to think through all the details here and wound up with the code below. The main change is that I've dropped the verifier node and monitored the safe head to know when empty blocks were inserted because the sequence window elapsed.

// TestVerifyL2OutputRoot_EmptyBlock asserts that the program can verify the output root of an empty block
// induced by missing batches.
// Setup is as follows:
// - create initial conditions and agreed l2 state
// - stop the batch submitter to induce empty blocks
// - wait for the seq window to expire so we can observe empty blocks
// - select an empty block as our claim
// - reboot the batch submitter
// - update the state root via a tx
// - run program
func testVerifyL2OutputRootEmptyBlock(t *testing.T, detached bool) {
	InitParallel(t)
	ctx := context.Background()

	cfg := DefaultSystemConfig(t)
	// We don't need a verifier - just the sequencer is enough
	delete(cfg.Nodes, "verifier")
	// Use a small sequencer window size to avoid test timeout while waiting for empty blocks
	// But not too small to ensure that the updated state after claim selection is published
	cfg.DeployConfig.SequencerWindowSize = 16

	sys, err := cfg.Start()
	require.Nil(t, err, "Error starting up system")
	defer sys.Close()

	log := testlog.Logger(t, log.LvlInfo)
	log.Info("genesis", "l2", sys.RollupConfig.Genesis.L2, "l1", sys.RollupConfig.Genesis.L1, "l2_time", sys.RollupConfig.Genesis.L2Time)

	l1Client := sys.Clients["l1"]
	l2Seq := sys.Clients["sequencer"]
	rollupRPCClient, err := rpc.DialContext(context.Background(), sys.RollupNodes["sequencer"].HTTPEndpoint())
	require.Nil(t, err)
	rollupClient := sources.NewRollupClient(client.NewBaseRPCClient(rollupRPCClient))

	verifierRPCClient, err := rpc.DialContext(context.Background(), sys.RollupNodes["sequencer"].HTTPEndpoint())
	require.Nil(t, err)
	verifierClient := sources.NewRollupClient(client.NewBaseRPCClient(verifierRPCClient))

	t.Log("Sending transactions to setup existing state, prior to challenged period")
	aliceKey := cfg.Secrets.Alice
	receipt := SendL2Tx(t, cfg, l2Seq, aliceKey, func(opts *TxOpts) {
		opts.ToAddr = &cfg.Secrets.Addresses().Bob
		opts.Value = big.NewInt(1_000)
	})

	require.NoError(t, waitForSafeHead(ctx, receipt.BlockNumber.Uint64(), rollupClient))

	t.Log("Capture current L2 head as agreed starting point")
	l2Head := receipt.BlockHash

	t.Log("=====Stopping batch submitter=====")
	err = sys.BatchSubmitter.Stop(ctx)
	require.NoError(t, err, "could not stop batch submitter")

	// Wait for the sequencer to catch up with the current L1 head so we know all submitted batches are processed
	t.Log("Wait for sequencer to catch up with last submitted batch")
	l1HeadNum, err := l1Client.BlockNumber(ctx)
	require.NoError(t, err)
	_, err = waitForL1OriginOnL2(l1HeadNum, l2Seq, 30*time.Second)
	require.NoError(t, err)

	// Get the current safe head now that the batcher is stopped
	safeBlock, err := l2Seq.BlockByNumber(ctx, big.NewInt(int64(rpc.SafeBlockNumber)))
	require.NoError(t, err)

	// Wait for safe head to start advancing again when the sequencing window elapses, for at least three blocks
	t.Log("Wait for safe head to advance after sequencing window elapses")
	require.NoError(t, waitForSafeHead(ctx, safeBlock.NumberU64()+3, rollupClient))

	// Use the 2nd empty block as our L2 claim block
	t.Log("Determine L2 claim")
	l2ClaimBlock, err := l2Seq.BlockByNumber(ctx, big.NewInt(int64(safeBlock.NumberU64()+2)))
	require.NoError(t, err, "get L2 claim block number")
	l2ClaimBlockNumber := l2ClaimBlock.Number().Uint64()
	l2Output, err := verifierClient.OutputAtBlock(ctx, l2ClaimBlockNumber)
	require.NoError(t, err, "could not get expected output")
	l2Claim := l2Output.OutputRoot

	t.Log("=====Restarting batch submitter=====")
	err = sys.BatchSubmitter.Start()
	require.NoError(t, err, "could not start batch submitter")

	t.Log("Add a transaction to the next batch after sequence of empty blocks")
	receipt = SendL2Tx(t, cfg, l2Seq, aliceKey, func(opts *TxOpts) {
		opts.ToAddr = &cfg.Secrets.Addresses().Bob
		opts.Value = big.NewInt(1_000)
		opts.Nonce = 1
	})
	require.NoError(t, waitForSafeHead(ctx, receipt.BlockNumber.Uint64(), rollupClient))

	t.Log("Determine L1 head that includes batch after sequence of empty blocks")
	l1HeadBlock, err := l1Client.BlockByNumber(ctx, nil)
	require.NoError(t, err, "get l1 head block")
	l1Head := l1HeadBlock.Hash()

	testFaultProofProgramScenario(t, ctx, sys, &FaultProofProgramTestScenario{
		L1Head:             l1Head,
		L2Head:             l2Head,
		L2Claim:            common.Hash(l2Claim),
		L2ClaimBlockNumber: l2ClaimBlockNumber,
	})
}

I had to increase the timeout in the waitForSafeHead as well because it takes more than 30 seconds for the sequence window to elapse. Given the timeout shouldn't be reached just setting a fixed wait period that's well after what we expect to wait is fine IMO.

op-e2e/system_fpp_test.go Show resolved Hide resolved
op-e2e/system_fpp_test.go Outdated Show resolved Hide resolved
op-e2e/system_fpp_test.go Outdated Show resolved Hide resolved
op-e2e/system_fpp_test.go Show resolved Hide resolved
@ajsutton
Copy link
Contributor

ajsutton commented May 2, 2023

btw the code I posted is just a "this may be helpful" not a "this is definitely the right way to do it" thing.

@Inphi
Copy link
Contributor Author

Inphi commented May 2, 2023

For some reason, waiting for a given safe head (via optimism_syncStatus) returns before that the block is fully imported by the verifier. Not sure what's going on there, but I've replaced it to poll for the block number directly instead. As long as the verifier is not peered with the sequencer, all blocks retrieved from it are safe.

@Inphi
Copy link
Contributor Author

Inphi commented May 2, 2023

I had to play with the code directly to think through all the details here and wound up with the code below. The main change is that I've dropped the verifier node and monitored the safe head to know when empty blocks were inserted because the sequence window elapsed.

This actually works better. Thanks for code suggestion. As for the 30s timeout, it's unavoidable due to seq window waits. Not terribly ideal for local execution. But CI uses a larger timeout so it's fine.

@Inphi Inphi force-pushed the inphi/fpp-empty branch 2 times, most recently from b520441 to 85ccb78 Compare May 2, 2023 16:07
@Inphi Inphi marked this pull request as ready for review May 2, 2023 18:03
@Inphi Inphi requested a review from a team as a code owner May 2, 2023 18:03
@Inphi Inphi requested a review from tynes May 2, 2023 18:03
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just need to pass through detached for the existing test as well I think.

op-e2e/system_fpp_test.go Show resolved Hide resolved
Asserting that the FPP will generate empty blocks when there are missing
batches
@mergify
Copy link
Contributor

mergify bot commented May 3, 2023

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented May 3, 2023

This PR is next in line to be merged, and will be merged as soon as checks pass.

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented May 3, 2023

This PR is next in line to be merged, and will be merged as soon as checks pass.

@OptimismBot OptimismBot merged commit 0706f72 into develop May 3, 2023
@OptimismBot OptimismBot deleted the inphi/fpp-empty branch May 3, 2023 00:58
@mergify mergify bot removed the on-merge-train label May 3, 2023
}

func TestVerifyL2OutputRootEmptyBlockDetached(t *testing.T) {
testVerifyL2OutputRootEmptyBlock(t, true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Successfully merging this pull request may close these issues.

None yet

4 participants