Skip to content

Commit

Permalink
Use SHORT_PREFIX env in CI (#1206)
Browse files Browse the repository at this point in the history
* Use SHORT_PREFIX env to reduce prefix length.

Reduce nonce finding difficulty, speeds up bitcoin_e2e tests and addresses related flakiness.

* Use SHORT_PREFIX for test and coverage only

* Revet short backtraces

* Revert redundant to_vec

* Revert import in bitcoin-da

* add command explaining prefix stuff

* Merge comments

---------

Co-authored-by: eyusufatik <[email protected]>
  • Loading branch information
jfldde and eyusufatik authored Sep 20, 2024
1 parent effa602 commit 7a077f0
Show file tree
Hide file tree
Showing 12 changed files with 56 additions and 22 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ jobs:
env:
RUST_BACKTRACE: 1
USE_DOCKER: "true"
SHORT_PREFIX: 1
- name: Upload coverage
uses: codecov/codecov-action@v4
with:
Expand Down Expand Up @@ -385,10 +386,10 @@ jobs:
run: make test
env:
RUST_BACKTRACE: 1
REPR_GUEST_BUILD: 1
BONSAI_API_URL: ${{ secrets.BONSAI_API_URL }} # TODO: remove this once we don't use the client on tests
BONSAI_API_KEY: ${{ secrets.BONSAI_API_KEY }} # TODO: remove this once we don't use the client on tests
USE_DOCKER: "true"
SHORT_PREFIX: 1

system-contracts:
strategy:
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ coverage-html: ## Coverage in HTML format
docs: ## Generates documentation locally
cargo doc --open

set-git-hook:
set-git-hook:
git config core.hooksPath .githooks

# Downloads and unpacks Ethereum Foundation tests in the `$(EF_TESTS_DIR)` directory.
Expand Down
2 changes: 1 addition & 1 deletion bin/citrea/tests/bitcoin_e2e/framework.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl TestFramework {

for node in self.get_nodes_as_log_provider() {
println!("{} logs (last 25 lines):", node.kind());
if let Err(e) = tail_file(&node.log_path(), 25) {
if let Err(e) = tail_file(&node.log_path(), 300) {
eprint!("{e}");
}
}
Expand Down
2 changes: 1 addition & 1 deletion bin/citrea/tests/bitcoin_e2e/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl Prover {
})
}

pub async fn wait_for_l1_height(&self, height: u64, timeout: Option<Duration>) {
pub async fn wait_for_l1_height(&self, height: u64, timeout: Option<Duration>) -> Result<()> {
wait_for_prover_l1_height(&self.client, height, timeout).await
}
}
Expand Down
6 changes: 3 additions & 3 deletions bin/citrea/tests/bitcoin_e2e/tests/prover_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl TestCase for BasicProverTest {

da.generate(FINALITY_DEPTH, None).await?;
let finalized_height = da.get_finalized_height().await?;
prover.wait_for_l1_height(finalized_height, None).await;
prover.wait_for_l1_height(finalized_height, None).await?;

da.generate(FINALITY_DEPTH, None).await?;
let proofs = full_node
Expand Down Expand Up @@ -215,7 +215,7 @@ impl TestCase for SkipPreprovenCommitmentsTest {
let finalized_height = da.get_finalized_height().await?;
prover
.wait_for_l1_height(finalized_height, Some(Duration::from_secs(300)))
.await;
.await?;

da.generate(FINALITY_DEPTH, None).await?;
let proofs = full_node
Expand Down Expand Up @@ -284,7 +284,7 @@ impl TestCase for SkipPreprovenCommitmentsTest {

prover
.wait_for_l1_height(finalized_height, Some(Duration::from_secs(300)))
.await;
.await?;

da.generate(FINALITY_DEPTH, None).await?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl TestCase for LedgerGetCommitmentsProverTest {
let finalized_height = da.get_finalized_height().await?;

// wait here until we see from prover's rpc that it finished proving
prover.wait_for_l1_height(finalized_height, None).await;
prover.wait_for_l1_height(finalized_height, None).await?;

let commitments = prover
.client
Expand Down
8 changes: 6 additions & 2 deletions bin/citrea/tests/e2e/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ async fn test_all_flow() {
wait_for_l1_block(&da_service, 3, None).await;

// wait here until we see from prover's rpc that it finished proving
wait_for_prover_l1_height(&prover_node_test_client, 4, None).await;
wait_for_prover_l1_height(&prover_node_test_client, 4, None)
.await
.unwrap();

let commitments = prover_node_test_client
.ledger_get_sequencer_commitments_on_slot_by_number(3)
Expand Down Expand Up @@ -285,7 +287,9 @@ async fn test_all_flow() {
wait_for_l1_block(&da_service, 5, None).await;

// wait here until we see from prover's rpc that it finished proving
wait_for_prover_l1_height(&prover_node_test_client, 5, None).await;
wait_for_prover_l1_height(&prover_node_test_client, 5, None)
.await
.unwrap();

let commitments = prover_node_test_client
.ledger_get_sequencer_commitments_on_slot_by_number(5)
Expand Down
4 changes: 3 additions & 1 deletion bin/citrea/tests/e2e/proving.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ async fn full_node_verify_proof_and_store() {
wait_for_l2_block(&full_node_test_client, 5, None).await;

// wait here until we see from prover's rpc that it finished proving
wait_for_prover_l1_height(&prover_node_test_client, 4, None).await;
wait_for_prover_l1_height(&prover_node_test_client, 4, None)
.await
.unwrap();

let commitments = prover_node_test_client
.ledger_get_sequencer_commitments_on_slot_by_number(3)
Expand Down
4 changes: 2 additions & 2 deletions bin/citrea/tests/e2e/reopen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ async fn test_reopen_prover() -> Result<(), anyhow::Error> {
wait_for_l1_block(&da_service, 4, None).await;

// wait here until we see from prover's rpc that it finished proving
wait_for_prover_l1_height(&prover_node_test_client, 5, None).await;
wait_for_prover_l1_height(&prover_node_test_client, 5, None).await?;
// Contains the proof
wait_for_l1_block(&da_service, 5, None).await;

Expand Down Expand Up @@ -505,7 +505,7 @@ async fn test_reopen_prover() -> Result<(), anyhow::Error> {
// Commitment is sent
wait_for_l1_block(&da_service, 8, None).await;
// wait here until we see from prover's rpc that it finished proving
wait_for_prover_l1_height(&prover_node_test_client, 9, None).await;
wait_for_prover_l1_height(&prover_node_test_client, 9, None).await?;

// Should now have 8 blocks = 2 commitments of blocks 1-4 and 5-8
// there is an extra soft confirmation due to the prover publishing a proof. This causes
Expand Down
12 changes: 8 additions & 4 deletions bin/citrea/tests/e2e/syncing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ async fn test_prover_sync_with_commitments() -> Result<(), anyhow::Error> {
seq_test_client.send_publish_batch_request().await;

// wait here until we see from prover's rpc that it finished proving
wait_for_prover_l1_height(&prover_node_test_client, 3, None).await;
wait_for_prover_l1_height(&prover_node_test_client, 3, None).await?;

// Submit an L2 block to prevent sequencer from falling behind.
seq_test_client.send_publish_batch_request().await;
Expand All @@ -364,7 +364,7 @@ async fn test_prover_sync_with_commitments() -> Result<(), anyhow::Error> {
wait_for_l1_block(&da_service, 4, None).await;

// wait here until we see from prover's rpc that it finished proving
wait_for_prover_l1_height(&prover_node_test_client, 4, None).await;
wait_for_prover_l1_height(&prover_node_test_client, 4, None).await?;

// Should now have 8 blocks = 2 commitments of blocks 1-4 and 5-9
// there is an extra soft confirmation due to the prover publishing a proof. This causes
Expand Down Expand Up @@ -491,7 +491,9 @@ async fn test_full_node_sync_status() {
for _ in 0..19 {
da_service.publish_test_block().await.unwrap();
}
wait_for_prover_l1_height(&full_node_test_client, 1, Some(Duration::from_secs(60))).await;
wait_for_prover_l1_height(&full_node_test_client, 1, Some(Duration::from_secs(60)))
.await
.unwrap();
let l1_status = full_node_test_client.citrea_sync_status().await.l1_status;
match l1_status {
LayerStatus::Syncing(syncing) => {
Expand All @@ -500,7 +502,9 @@ async fn test_full_node_sync_status() {
}
_ => panic!("Expected syncing status"),
}
wait_for_prover_l1_height(&full_node_test_client, 20, Some(Duration::from_secs(60))).await;
wait_for_prover_l1_height(&full_node_test_client, 20, Some(Duration::from_secs(60)))
.await
.unwrap();
let l1_status = full_node_test_client.citrea_sync_status().await.l1_status;
match l1_status {
LayerStatus::Synced(synced_up_to) => assert_eq!(synced_up_to, 20),
Expand Down
8 changes: 5 additions & 3 deletions bin/citrea/tests/test_helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::net::SocketAddr;
use std::path::{Path, PathBuf};
use std::time::{Duration, SystemTime};

use anyhow::bail;
use borsh::BorshDeserialize;
use citrea::{CitreaRollupBlueprint, MockDemoRollup};
use citrea_primitives::TEST_PRIVATE_KEY;
Expand Down Expand Up @@ -225,9 +226,9 @@ pub async fn wait_for_prover_l1_height(
prover_client: &TestClient,
num: u64,
timeout: Option<Duration>,
) {
) -> anyhow::Result<()> {
let start = SystemTime::now();
let timeout = timeout.unwrap_or(Duration::from_secs(DEFAULT_PROOF_WAIT_DURATION)); // Default 300 seconds timeout
let timeout = timeout.unwrap_or(Duration::from_secs(DEFAULT_PROOF_WAIT_DURATION)); // Default 600 seconds timeout
loop {
debug!("Waiting for prover height {}", num);
let latest_block = prover_client.ledger_get_last_scanned_l1_height().await;
Expand All @@ -237,11 +238,12 @@ pub async fn wait_for_prover_l1_height(

let now = SystemTime::now();
if start + timeout <= now {
panic!("Timeout. Latest prover L1 height is {}", latest_block);
bail!("Timeout. Latest prover L1 height is {}", latest_block);
}

sleep(Duration::from_secs(1)).await;
}
Ok(())
}

#[instrument(level = "debug", skip(da_service))]
Expand Down
25 changes: 23 additions & 2 deletions crates/primitives/src/constants.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,29 @@
// Generates a prefix for REVEAL_BATCH_PROOF_PREFIX and REVEAL_LIGHT_CLIENT_PREFIX constants based on compile-time environment.
// Returns a single-byte prefix [1] if SHORT_PREFIX env var is set, otherwise defaults to [1, 1].
// This greatly reduces the time required to find a nonce when generating batch proving txs and LightClientTxs
// We have to make these prefixes constants due to zk proving.
// But two bytes takes too long to generate nonce, making tests very flaky and slow.
// So in CI we define an env var SHORT_PREFIX to use less bytes.
// This doesn't change any method ids, just the prefixes.
const fn get_reveal_batch_proof_prefix() -> &'static [u8] {
match option_env!("SHORT_PREFIX") {
Some(_) => &[1],
None => &[1, 1],
}
}

const fn get_reveal_light_client_prefix() -> &'static [u8] {
match option_env!("SHORT_PREFIX") {
Some(_) => &[2],
None => &[2, 2],
}
}

/// Prefix for the reveal transaction ids - batch proof namespace.
pub const REVEAL_BATCH_PROOF_PREFIX: &[u8] = [1, 1].as_slice();
pub const REVEAL_BATCH_PROOF_PREFIX: &[u8] = get_reveal_batch_proof_prefix();

/// Prefix for the reveal transaction ids - light client namespace.
pub const REVEAL_LIGHT_CLIENT_PREFIX: &[u8] = [2, 2].as_slice();
pub const REVEAL_LIGHT_CLIENT_PREFIX: &[u8] = get_reveal_light_client_prefix();

pub const TEST_PRIVATE_KEY: &str =
"1212121212121212121212121212121212121212121212121212121212121212";
Expand Down

0 comments on commit 7a077f0

Please sign in to comment.