Bitcoin e2e test framework

[jfldde]

Description

• Introduces new bitcoin e2e test framework.
• Handles spawning and the lifecycle of bitcoind nodes and the sequencer/prover/full node as required by the test cases.
• Supports running multiple Bitcoin nodes in a cluster, enabling network-related tests
• All citrea nodes are optional and can be configured by the TestCaseConfig, i.e.
• Each Citrea nodes can be optionally included in tests through the TestCaseConfig

const config = TestCaseConfig {
    with_sequencer: true,
    with_full_node: false,
    with_prover: true,
    ..Default::default()
}

As seen in the prover_test.rs file

• All nodes and components are fully configurable via the TestCase trait, with sane defaults provided for most test scenarios

Adds three basic tests to showcase its functionalities :

• BasicSequencerTest : Validates basic sequencer functionality.
• BasicProverTest : Tests the prover flow, up to blob inscription transactions.
• BasicSyncTest : Verifies synchronization between multiple Bitcoin nodes.

Linked Issues

• Fixes E2E testing framework #954

[rakanalh]

Quick glance, i like where this is going. I believe this could replace the way we write E2E in general not only for the bitcoin DA.

1. I would like to see this under bin/citrea/tests and not bin/citrea/tests/bitcoin-e2e
2. Maybe add an option to use MockDA as well as Bitcoin DA (configurable).
3. Port existing tests (apart from the already impemented basic ones) from sequencer / prover / full node to show case the completeness of the framework.
4. wait_for_* methods are either implemented in their respective structs (Sequencer: wait_for_l2_block as an example) of in the test_helpers but not in both for consistency.

[jfldde]

Great feedback @rakanalh. I'll get onto it.

[jfldde]

Regarding point 4, you're talking about wait_for_height implementation I reckon since the Node trait wait_for_ready is used for waiting for the processes to be reachable by their respective clients. There's also a wait_for_sync which wait for multiple bitcoin nodes to reach the same height. There's room for cleanup though.

[eyusufatik]

I must say I'm impressed. This looks very good and useful.

I have a few general questsions:

1. do you run the system bitcoin? everybody might have different versions. a good idea is to bundle our stuff with some kind of docker image etc.
2. Is the data dirs randomly generated? Do I get a fresh bitcoin node/sequencer/full node etc. all the time? Can we also have persisting data dirs too? Some tests in current e2e suite is about closing down and reopening the nodes.
3. I noticed we spawn /target/debug/citrea instead of running the nodes as a tokio task. I aggee this waw is better. However does this mean before running these tests, do i need to build the project, or is there a way to bundle these two actions together?

I agree with @rakanalh we should port our current e2e tests here. However, we can do it in different PRs, step by step. Or we'll have to wait on the "porting" pr to get merged. This would slow down other prs.

[eyusufatik]

One more feature we'd need is to be able to pass different genesises, not just bitcoin-regtest.

[jfldde]

1. do you run the system bitcoin? everybody might have different versions. a good idea is to bundle our stuff with some kind of docker image etc.
2. Is the data dirs randomly generated? Do I get a fresh bitcoin node/sequencer/full node etc. all the time? Can we also have persisting data dirs too? Some tests in current e2e suite is about closing down and reopening the nodes.
3. I noticed we spawn /target/debug/citrea instead of running the nodes as a tokio task. I aggee this waw is better. However does this mean before running these tests, do i need to build the project, or is there a way to bundle these two actions together?

1 - Good point, I'll do this.
2 - They are randomly generated for a single test case. We can already shutdown and restart a node from within a same test case. We can add helper restart method on nodes when we tackle these tests. Dirs are persisted for now but I'll add some dir cleanup on exit that will be active by default.
3 - For now, binaries need to be accesible. It either resolves to CITREA env or debug build binary. If none of them are available, test won't be able to run. This need to be handled for CI. Ideally, we would bundle everything using docker and take care of point 1 at the same time

[eyusufatik]

renaming node to bitcoinnode might be a good idea as well. I was confused for some time

[kpp]

I am impressed. Good job!

I would say we should merge this asap and open other followup PRs to port e2e test using this framework. It's clear that we can't predict all our needs right now, so those PRs will extend API to the desired level.

[jfldde]

I'll add a HOLD MERGE tag until I resolved point 1 raised above by @eyusufatik regarding bitcoin binary.
I'll also need to adapt CI a bit to pre-build citrea binary

[codecov]

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.7%. Comparing base (b889300) to head (aaf2a3a).
Report is 11 commits behind head on nightly.

Files with missing lines	Patch %	Lines
crates/bitcoin-da/src/service.rs	0.0%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
crates/sequencer/src/config.rs	100.0% <100.0%> (ø)
...vereign-sdk/full-node/sov-stf-runner/src/config.rs	92.8% <100.0%> (ø)
...full-node/sov-stf-runner/src/prover_service/mod.rs	69.2% <ø> (ø)
crates/bitcoin-da/src/service.rs	43.4% <0.0%> (-0.2%)	⬇️

... and 43 files with indirect coverage changes

[ercecan]

renaming node to bitcoinnode might be a good idea as well. I was confused for some time

seconded

[jfldde]

Should already be renamed as of now, anything you still find confusing @ercecan ?

[ercecan]

Should already be renamed as of now, anything you still find confusing @ercecan ?

Nope, my bad

[ercecan]

Really cool

[jfldde]

Lots of files have been touched as a side effect but should have limited impact.

• Add Serialize on all configs in order to write the toml files.
• Change TestClient and make_test_client to return the error instead of panicking when it cannots connect to the endpoints.
• Use DEFAULT_PROOF_WAIT_DURATION directly in wait_for_prover_l1_height as default value