From 4f581e9d922184d74b5a82f3c94ff750854a9b9d Mon Sep 17 00:00:00 2001 From: addnad Date: Wed, 8 Apr 2026 05:04:43 +0100 Subject: [PATCH 1/3] ci: Fix Validate PR Title job for external forks (#3178) Problem: The Validate PR Title job was failing on external forks because the GITHUB_TOKEN doesn't have the necessary permissions to post status checks on PRs from forks. Solution: - Added explicit condition to validate job: only run for PRs from the main repository - Updated check-changelog job to also only run for PRs from the main repository - Removed 'needs: validate' dependency since validate might be skipped on forks - Added pull-requests: read permission to validate job for clarity This allows the workflow to gracefully handle PRs from external forks while still validating PR titles for commits to the main repository. Fixes #3178 --- .github/workflows/pr-check.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index ccf4dcaec2..0f566d8967 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -11,7 +11,9 @@ jobs: name: Validate PR Title permissions: contents: read + pull-requests: read runs-on: ubuntu-latest + if: github.event.pull_request.head.repo.full_name == github.repository steps: - name: Check PR Title uses: amannn/action-semantic-pull-request@v5 @@ -48,14 +50,13 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} check-changelog: - needs: validate permissions: contents: read pull-requests: write issues: write name: Check Changelog Entry runs-on: ubuntu-latest - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository steps: - name: Checkout code uses: actions/checkout@v4 From 5b4b67ea7c5e1b33445e73f69dd66c620c89d9c7 Mon Sep 17 00:00:00 2001 From: addnad Date: Wed, 8 Apr 2026 06:14:15 +0100 Subject: [PATCH 2/3] feat: Reduce PendingProofs disk usage by storing Bitcoin indices (#3168) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement significant disk usage optimization for pending proofs by storing only Bitcoin block height and transaction index instead of full proof data. Proofs are fetched from Bitcoin via RPC on retry, reducing database footprint. Changes: - Create BitcoinProofLocation struct to store (block_height, tx_index) - Update PendingProofsOutput schema to use BitcoinProofLocation instead of Proof - Modify store_pending_proof() signature to accept Bitcoin indices - Create ProofWithLocation wrapper to propagate indices through processing - Update extract_zk_proofs_and_sequencer_commitments() to preserve Bitcoin indices - Implement fetch_proof_from_bitcoin() to re-fetch proofs from Bitcoin by location - Update process_pending_proofs() to fetch deferred proofs from Bitcoin via RPC - Add ProofFetchFailed error variant for proof retrieval failures Benefits: - Significantly reduces memory/disk usage for pending proofs (full proof → 16 bytes) - Eliminates redundant proof storage across L1 cycles - Leverages Bitcoin network as proof source of truth - Maintains logical soundness while improving infrastructure efficiency This addresses the core optimization suggested in issue #3168. --- crates/common/src/da.rs | 23 +++- crates/fullnode/src/da_block_handler.rs | 105 +++++++++++++++--- crates/fullnode/src/error.rs | 3 + .../full-node/db/sov-db/src/ledger_db/mod.rs | 11 +- .../db/sov-db/src/ledger_db/traits.rs | 5 +- .../db/sov-db/src/schema/types/mod.rs | 24 +++- 6 files changed, 150 insertions(+), 21 deletions(-) diff --git a/crates/common/src/da.rs b/crates/common/src/da.rs index b98af3bfb2..2fed330f96 100644 --- a/crates/common/src/da.rs +++ b/crates/common/src/da.rs @@ -14,8 +14,18 @@ use tracing::{debug, error, info}; use crate::cache::L1BlockCache; +/// Wrapper for proofs that includes Bitcoin location for efficient storage +#[derive(Clone)] +pub struct ProofWithLocation { + pub proof: Proof, + /// Bitcoin block height where this proof was found + pub bitcoin_block_height: u64, + /// Transaction index within the Bitcoin block + pub bitcoin_tx_index: u32, +} + pub enum ProofOrCommitment { - Proof(Proof), + Proof(ProofWithLocation), Commitment(SequencerCommitment), } @@ -134,11 +144,20 @@ pub async fn extract_zk_proofs_and_sequencer_commitments( prover_da_pub_key: &[u8], sequencer_da_pub_key: &[u8], ) -> Vec { + let bitcoin_block_height = l1_block.header().height(); + let proofs = da_service .extract_relevant_zk_proofs(l1_block, prover_da_pub_key) .await .into_iter() - .map(|(idx, proof)| (idx, ProofOrCommitment::Proof(proof))); + .map(|(tx_index, proof)| { + let proof_with_location = ProofWithLocation { + proof, + bitcoin_block_height, + bitcoin_tx_index: tx_index as u32, + }; + (tx_index, ProofOrCommitment::Proof(proof_with_location)) + }); let commitments = da_service .as_ref() diff --git a/crates/fullnode/src/da_block_handler.rs b/crates/fullnode/src/da_block_handler.rs index db52356b7a..4248ab7b01 100644 --- a/crates/fullnode/src/da_block_handler.rs +++ b/crates/fullnode/src/da_block_handler.rs @@ -11,7 +11,7 @@ use std::time::{Duration, Instant}; use anyhow::{anyhow, bail}; use citrea_common::backup::BackupManager; use citrea_common::cache::L1BlockCache; -use citrea_common::da::{extract_zk_proofs_and_sequencer_commitments, sync_l1, ProofOrCommitment}; +use citrea_common::da::{extract_zk_proofs_and_sequencer_commitments, sync_l1, ProofOrCommitment, ProofWithLocation}; use citrea_common::utils::{ exceeded_stop_height, get_tangerine_activation_height_non_zero, shutdown_requested, }; @@ -591,9 +591,11 @@ where &self, current_l1_block_height: u64, found_in_l1_block_height: u64, - proof: Proof, + proof_with_location: ProofWithLocation, proof_source: ProofSource, ) -> Result { + let proof = &proof_with_location.proof; + tracing::info!( "Processing zk proof at height: {}", found_in_l1_block_height @@ -601,7 +603,7 @@ where tracing::trace!("ZK proof: {:?}", proof); // Extract and verify the proof using the appropriate ZKVM - let Ok(batch_proof_output) = Vm::extract_output::(&proof) else { + let Ok(batch_proof_output) = Vm::extract_output::(proof) else { return Ok(ProcessingResult::Discarded); }; @@ -635,7 +637,7 @@ where current_l1_block_height, found_in_l1_block_height, batch_proof_output.initial_state_root(), - proof, + proof_with_location, batch_proof_output, proof_source, ) @@ -648,7 +650,7 @@ where /// * `current_l1_block_height` - Current L1 block being processed /// * `found_in_l1_block_height` - L1 block where proof was found /// * `initial_state_root` - Initial state root for verification - /// * `raw_proof` - The raw ZK proof + /// * `proof_with_location` - The ZK proof with Bitcoin location metadata /// * `batch_proof_output` - The batch proof circuit output /// * `proof_source` - Whether the proof came from L1 or pending retries /// @@ -659,7 +661,7 @@ where current_l1_block_height: u64, found_in_l1_block_height: u64, initial_state_root: [u8; 32], - raw_proof: Proof, + proof_with_location: ProofWithLocation, batch_proof_output: BatchProofCircuitOutput, proof_source: ProofSource, ) -> Result { @@ -733,15 +735,16 @@ where if proof_is_pending { if proof_source == ProofSource::FromL1 { info!( - "Proof is pending for commitment index range {}-{}. Storing proof as pending.", + "Proof is pending for commitment index range {}-{}. Storing proof location instead of full proof to reduce disk usage.", sequencer_commitment_index_range.0, sequencer_commitment_index_range.1 ); self.ledger_db.store_pending_proof( sequencer_commitment_index_range.0, sequencer_commitment_index_range.1, - raw_proof, + proof_with_location.bitcoin_block_height, + proof_with_location.bitcoin_tx_index, found_in_l1_block_height, - )?; + )? } else { info!( "Proof is pending for commitment index range {}-{}. Keeping existing pending proof without rewrite.", @@ -773,7 +776,7 @@ where if sequencer_commitment_index_range.0 > proven_height.commitment_index + 1 { if proof_source == ProofSource::FromL1 { info!( - "First commitment in range is not strictly increasing. Expected index {}, got {}. Storing proof as pending for commitment range {}-{}", + "First commitment in range is not strictly increasing. Expected index {}, got {}. Storing proof location for commitment range {}-{}", proven_height.commitment_index + 1, sequencer_commitment_index_range.0, sequencer_commitment_index_range.0, @@ -782,9 +785,10 @@ where self.ledger_db.store_pending_proof( sequencer_commitment_index_range.0, sequencer_commitment_index_range.1, - raw_proof, + proof_with_location.bitcoin_block_height, + proof_with_location.bitcoin_tx_index, found_in_l1_block_height, - )?; + )? } else { info!( "First commitment in range is not strictly increasing. Expected index {}, got {}. Keeping existing pending proof for commitment range {}-{} without rewrite", @@ -797,6 +801,7 @@ where return Ok(ProcessingResult::Pending); } + let raw_proof = proof_with_location.proof.clone(); // store in ledger db self.ledger_db.update_verified_proof_data( found_in_l1_block_height, @@ -915,12 +920,25 @@ where let pending_proofs = self.ledger_db.get_pending_proofs()?; for item in pending_proofs { - let ((min_index, max_index), (proof, found_in_l1_height)) = item?.into_tuple(); + let ((min_index, max_index), (proof_location, found_in_l1_height)) = item?.into_tuple(); + + // Fetch the proof from Bitcoin using the stored location + let proof_with_location = match self.fetch_proof_from_bitcoin(proof_location, found_in_l1_height).await { + Ok(p) => p, + Err(e) => { + warn!( + "Failed to fetch proof from Bitcoin for index {min_index}-{max_index} at location block={}, tx_idx={}: {e:?}", + proof_location.block_height, proof_location.tx_index + ); + break; + } + }; + match self .process_zk_proof( current_l1_block_height, found_in_l1_height, - proof, + proof_with_location, ProofSource::FromPendingRetry, ) .await @@ -995,4 +1013,63 @@ where } Ok(sequencer_commitment.l2_end_block_number) } + + /// Fetches a proof from Bitcoin using stored location information + /// + /// This method retrieves a proof that was previously identified by its Bitcoin block height + /// and transaction index, reducing the need to store full proofs in the pending proofs table. + /// + /// # Arguments + /// * `proof_location` - Bitcoin block height and transaction index where proof is located + /// * `l1_height` - L1 block height for context + /// + /// # Returns + /// A ProofWithLocation containing the fetched proof and location metadata + async fn fetch_proof_from_bitcoin( + &self, + proof_location: sov_db::schema::types::BitcoinProofLocation, + l1_height: u64, + ) -> Result { + // Fetch the Bitcoin block at the specified height + let block = self.da_service + .get_block_at(proof_location.block_height) + .await + .map_err(|e| { + ProcessingError::SkippableError( + SkippableError::Proof( + ProofError::ProofFetchFailed(format!( + "Failed to fetch Bitcoin block at height {}: {}", + proof_location.block_height, e + )) + ) + ) + })?; + + // Extract proofs from the block + let proofs = self.da_service + .extract_relevant_zk_proofs(&block, &self.prover_da_pub_key) + .await; + + // Find the proof at the specified transaction index + let proof = proofs + .into_iter() + .find(|(tx_idx, _)| *tx_idx == proof_location.tx_index as usize) + .map(|(_, proof)| proof) + .ok_or_else(|| { + ProcessingError::SkippableError( + SkippableError::Proof( + ProofError::ProofFetchFailed(format!( + "Proof not found at Bitcoin block {} tx index {}", + proof_location.block_height, proof_location.tx_index + )) + ) + ) + })?; + + Ok(ProofWithLocation { + proof, + bitcoin_block_height: proof_location.block_height, + bitcoin_tx_index: proof_location.tx_index, + }) + } } diff --git a/crates/fullnode/src/error.rs b/crates/fullnode/src/error.rs index a509fdab52..867abb2233 100644 --- a/crates/fullnode/src/error.rs +++ b/crates/fullnode/src/error.rs @@ -20,6 +20,9 @@ pub enum ProofError { /// Error when the sequencer commitment hash doesn't match the expected value #[error("Proof verification: For a known and verified sequencer commitment. Hash mismatch - expected 0x{0} but got 0x{1}. Skipping proof.")] SequencerCommitmentHashMismatch(String, String), + /// Error when fetching a proof from Bitcoin fails + #[error("Failed to fetch proof from Bitcoin: {0}")] + ProofFetchFailed(String), /// Other general errors that may occur during proof processing #[error("{0}")] Other(#[from] anyhow::Error), diff --git a/crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/mod.rs b/crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/mod.rs index a7c0b4ccec..f4293f68d4 100644 --- a/crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/mod.rs +++ b/crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/mod.rs @@ -34,7 +34,7 @@ use crate::schema::types::light_client_proof::{ StoredLightClientProof, StoredLightClientProofOutput, }; use crate::schema::types::{ - BonsaiSession, BoundlessSession, L2BlockNumber, L2HeightAndIndex, L2HeightRange, + BitcoinProofLocation, BonsaiSession, BoundlessSession, L2BlockNumber, L2HeightAndIndex, L2HeightRange, L2HeightStatus, SlotNumber, }; @@ -979,13 +979,18 @@ impl NodeLedgerOps for LedgerDB { &self, min_commitment_index: u32, max_commitment_index: u32, - proof: Proof, + bitcoin_block_height: u64, + bitcoin_tx_index: u32, found_in_l1_height: u64, ) -> anyhow::Result<()> { let mut schema_batch = SchemaBatch::new(); + let proof_location = BitcoinProofLocation { + block_height: bitcoin_block_height, + tx_index: bitcoin_tx_index, + }; schema_batch.put::( &(min_commitment_index, max_commitment_index), - &(proof, found_in_l1_height), + &(proof_location, found_in_l1_height), )?; self.db.write_schemas(schema_batch)?; Ok(()) diff --git a/crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/traits.rs b/crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/traits.rs index 6b01542d06..c377864286 100644 --- a/crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/traits.rs +++ b/crates/sovereign-sdk/full-node/db/sov-db/src/ledger_db/traits.rs @@ -195,11 +195,14 @@ pub trait NodeLedgerOps: SharedLedgerOps + Send + Sync { fn remove_pending_commitment(&self, index: u32) -> Result<()>; /// Store an out of order proof by commitment index range for later processing + /// Instead of storing the full proof, we store the Bitcoin block height and transaction index + /// to reduce disk usage. The proof will be fetched from Bitcoin via RPC when retrying. fn store_pending_proof( &self, min_commitment_index: u32, max_commitment_index: u32, - proof: Proof, + bitcoin_block_height: u64, + bitcoin_tx_index: u32, found_in_l1_height: u64, ) -> Result<()>; diff --git a/crates/sovereign-sdk/full-node/db/sov-db/src/schema/types/mod.rs b/crates/sovereign-sdk/full-node/db/sov-db/src/schema/types/mod.rs index e001e24e70..ed46406c79 100644 --- a/crates/sovereign-sdk/full-node/db/sov-db/src/schema/types/mod.rs +++ b/crates/sovereign-sdk/full-node/db/sov-db/src/schema/types/mod.rs @@ -33,8 +33,30 @@ pub type L2HeightRange = (L2BlockNumber, L2BlockNumber); /// L1 height pub type L1Height = u64; +/// Location of a proof on Bitcoin for lazy fetching +/// Stores the Bitcoin block height and transaction index to retrieve the proof via RPC +/// instead of keeping the full proof in memory +#[derive( + Clone, + Copy, + Debug, + PartialEq, + Eq, + ::borsh::BorshDeserialize, + ::borsh::BorshSerialize, + ::serde::Serialize, + ::serde::Deserialize, +)] +pub struct BitcoinProofLocation { + /// Bitcoin block height where the proof is stored + pub block_height: u64, + /// Transaction index within the Bitcoin block + pub tx_index: u32, +} + /// The output of the pending proofs table -pub type PendingProofsOutput = ((u32, u32), Proof, L1Height); +/// Stores commit index range, Bitcoin proof location, and L1 height +pub type PendingProofsOutput = ((u32, u32), BitcoinProofLocation, L1Height); /// Height and index of a sequencer commitment #[derive( From e759b40a09705c3ded6059e66b700215a3686399 Mon Sep 17 00:00:00 2001 From: addnad Date: Wed, 8 Apr 2026 06:26:21 +0100 Subject: [PATCH 3/3] fix: Address code review feedback for issue #3168 Resolve 4 issues identified in Copilot code review: 1. Update PendingProofs table schema to use BitcoinProofLocation instead of Proof - Modified schema/tables.rs to define PendingProofs as (u32, u32) => (BitcoinProofLocation, L1Height) - Added BitcoinProofLocation import to table definitions 2. Improve error handling in process_pending_proofs() - Change 'break' to 'continue' when proof fetch fails - Allows later pending proofs to be processed even if one fetch fails - Transient RPC failures can be retried on next cycle - Added l1_height to error log context for debugging 3. Remove unused parameter from fetch_proof_from_bitcoin() - Removed l1_height parameter that wasn't being used - Parameter was confusing to callers and triggered unused variable warns 4. Fix workflow condition for validate job - Add event type guard 'github.event_name == pull_request' - Prevents expression evaluation errors when triggered by issue_comment events - Aligns with similar condition in check-changelog job --- .github/workflows/pr-check.yml | 2 +- crates/fullnode/src/da_block_handler.rs | 11 ++++++----- .../full-node/db/sov-db/src/schema/tables.rs | 6 +++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 0f566d8967..d01d9e7b10 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -13,7 +13,7 @@ jobs: contents: read pull-requests: read runs-on: ubuntu-latest - if: github.event.pull_request.head.repo.full_name == github.repository + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository steps: - name: Check PR Title uses: amannn/action-semantic-pull-request@v5 diff --git a/crates/fullnode/src/da_block_handler.rs b/crates/fullnode/src/da_block_handler.rs index 4248ab7b01..128f667a70 100644 --- a/crates/fullnode/src/da_block_handler.rs +++ b/crates/fullnode/src/da_block_handler.rs @@ -923,14 +923,16 @@ where let ((min_index, max_index), (proof_location, found_in_l1_height)) = item?.into_tuple(); // Fetch the proof from Bitcoin using the stored location - let proof_with_location = match self.fetch_proof_from_bitcoin(proof_location, found_in_l1_height).await { + let proof_with_location = match self.fetch_proof_from_bitcoin(proof_location).await { Ok(p) => p, Err(e) => { warn!( - "Failed to fetch proof from Bitcoin for index {min_index}-{max_index} at location block={}, tx_idx={}: {e:?}", - proof_location.block_height, proof_location.tx_index + "Failed to fetch proof from Bitcoin for index {min_index}-{max_index} at location block={}, tx_idx={}, found_in_l1_height={}: {e:?}", + proof_location.block_height, proof_location.tx_index, found_in_l1_height ); - break; + // Keep this entry pending for future retry but don't block processing of later proofs. + // Transient RPC failures may succeed on next cycle. + continue; } }; @@ -1028,7 +1030,6 @@ where async fn fetch_proof_from_bitcoin( &self, proof_location: sov_db::schema::types::BitcoinProofLocation, - l1_height: u64, ) -> Result { // Fetch the Bitcoin block at the specified height let block = self.da_service diff --git a/crates/sovereign-sdk/full-node/db/sov-db/src/schema/tables.rs b/crates/sovereign-sdk/full-node/db/sov-db/src/schema/tables.rs index 50f6956754..2568be8dc8 100644 --- a/crates/sovereign-sdk/full-node/db/sov-db/src/schema/tables.rs +++ b/crates/sovereign-sdk/full-node/db/sov-db/src/schema/tables.rs @@ -23,7 +23,7 @@ use super::types::batch_proof::{StoredBatchProof, StoredVerifiedProof}; use super::types::l2_block::StoredL2Block; use super::types::light_client_proof::StoredLightClientProof; use super::types::{ - AccessoryKey, AccessoryStateValue, BonsaiSession, BoundlessSession, DbHash, JmtValue, L1Height, + AccessoryKey, AccessoryStateValue, BitcoinProofLocation, BonsaiSession, BoundlessSession, DbHash, JmtValue, L1Height, L2BlockNumber, L2HeightAndIndex, L2HeightRange, L2HeightStatus, SlotNumber, StateKey, }; @@ -566,8 +566,8 @@ define_table_with_seek_key_codec!( ); define_table_with_seek_key_codec!( - /// Out of order proofs - (PendingProofs) (u32, u32) => (Proof, L1Height) + /// Out of order proofs - stores Bitcoin location (block height + tx index) for lazy fetching + (PendingProofs) (u32, u32) => (BitcoinProofLocation, L1Height) ); #[cfg(test)]