test(consensus): declare HelloStarknet in the first proposal, assert transactions for reverts

crates/common/src/l2.rs

@@ -9,11 +9,13 @@ use crate::{
     BlockHeader,
     BlockNumber,
     BlockTimestamp,
+    CasmHash,
     EventCommitment,
     GasPrice,
     L1DataAvailabilityMode,
     ReceiptCommitment,
     SequencerAddress,
+    SierraHash,
     StarknetVersion,
     StateCommitment,
     StateDiffCommitment,
@@ -43,6 +45,7 @@ pub struct ConsensusFinalizedL2Block {
     pub state_update: StateUpdateData,
     pub transactions_and_receipts: Vec<(Transaction, Receipt)>,
     pub events: Vec<Vec<Event>>,
+    pub declared_classes: Vec<DeclaredClass>,
 }
 
 /// An L2 [BlockHeader] that is the result of executing a consensus proposal
@@ -75,6 +78,14 @@ pub struct ConsensusFinalizedBlockHeader {
     pub state_diff_length: u64,
 }
 
+#[derive(Clone, Debug)]
+pub struct DeclaredClass {
+    pub sierra_hash: SierraHash,
+    pub casm_hash_v2: CasmHash,
+    pub sierra_def: Vec<u8>,
+    pub casm_def: Vec<u8>,
+}
+
 impl From<L2Block> for L2BlockToCommit {
     fn from(block: L2Block) -> Self {
         L2BlockToCommit::FromFgw(block)

crates/common/src/lib.rs

@@ -35,7 +35,13 @@ pub mod trie;
 
 pub use header::{BlockHeader, BlockHeaderBuilder, L1DataAvailabilityMode, SignedBlockHeader};
 pub use l1::{L1BlockHash, L1BlockNumber, L1TransactionHash};
-pub use l2::{ConsensusFinalizedBlockHeader, ConsensusFinalizedL2Block, L2Block, L2BlockToCommit};
+pub use l2::{
+    ConsensusFinalizedBlockHeader,
+    ConsensusFinalizedL2Block,
+    DeclaredClass,
+    L2Block,
+    L2BlockToCommit,
+};
 pub use signature::BlockCommitmentSignature;
 pub use state_update::{FoundStorageValue, StateUpdate};
 

crates/pathfinder/src/consensus/inner/dummy_proposal.rs

@@ -97,7 +97,7 @@ pub(crate) fn create(
     let mut db_conn = main_storage.connection()?;
     let db_txn = db_conn.transaction()?;
 
-    if devnet::is_db_bootstrapped(&db_txn)? {
+    if devnet::devnet_genesis_exists(&db_txn)? {
         create_from_bootstrapped_devnet_db(
             &db_txn,
             height,
@@ -126,6 +126,7 @@ pub(crate) fn create(
 /// - Invokes the "increase_balance" and "get_balance" functions of random
 ///   deployed instances
 /// - Deploys more instances of the "Hello Starknet" contract randomly
+#[allow(clippy::too_many_arguments)]
 pub(crate) fn create_from_bootstrapped_devnet_db(
     db_txn: &pathfinder_storage::Transaction<'_>,
     height: u64,
@@ -168,43 +169,69 @@ pub(crate) fn create_from_bootstrapped_devnet_db(
     let mut batches = Vec::new();
     let mut next_txn_idx_start = 0;
 
-    // If there are no deployments in the DB yet, create one batch with a
-    // deployment
-    if deployed_in_db.is_empty() {
-        let first_batch = vec![account.hello_starknet_deploy()?];
+    // IMPORTANT
+    // Until ConcurrentStorageAdapter supports decided blocks we have to split
+    // declaring and deploying HelloStarknet into consecutive blocks. Otherwise the
+    // deployment will not succeed because the declaration may not be committed to
+    // storage yet, and we strictly do not want to mix successful and reverted
+    // transactions in the integration tests because this just simplifies
+    // correctness checks (either all successful or all reverted in case of a
+    // non-bootstrapped DB).
+    //
+    // Bootstrapped devnet DB already contains the genesis block, so the declaration
+    // of HelloStarknet falls into block number 1.
+    if height == 1 {
+        let first_batch = vec![account.hello_starknet_declare()?];
         next_txn_idx_start += first_batch.len();
         batches.push(first_batch);
-    }
-
-    let num_batches = rng.gen_range(1..=MAX_NUM_BATCHES);
+    } else {
+        // HelloStarknet need to be deployed at least once before we can invoke it, so
+        // if there are no deployments in the DB we just create a batch with the deploy
+        // transaction.
+        //
+        // IMPORTANT
+        // Until ConcurrentStorageAdapter supports decided blocks we have to split
+        // deploying HelloStarknet fir the first time and invoking it into consecutive
+        // blocks. Otherwise the first invokes will not succeed because the deployment
+        // may not be committed to storage yet, and we strictly do not want to mix
+        // successful and reverted transactions in the integration tests because
+        // this just simplifies correctness checks  (either all successful or all
+        // reverted in case of a non-bootstrapped DB).
+        if deployed_in_db.is_empty() {
+            // Declare goes into the first proposal, that's it
+            let first_batch = vec![account.hello_starknet_deploy()?];
+            next_txn_idx_start += first_batch.len();
+            batches.push(first_batch);
+        } else {
+            let num_batches = rng.gen_range(1..=MAX_NUM_BATCHES);
+
+            for _ in 0..num_batches {
+                let batch_tries = rng.gen_range(1..=MAX_BATCH_TRIES);
+                let mut batch = Vec::new();
+
+                for _ in 0..batch_tries {
+                    // Maybe deploy another instance
+                    if rng.gen() {
+                        batch.push(account.hello_starknet_deploy()?);
+                    }
 
-    for _ in 0..num_batches {
-        let batch_tries = rng.gen_range(1..=MAX_BATCH_TRIES);
-        let mut batch = Vec::new();
+                    // Invoke a random contract instance if there are any deployments in the DB
+                    if let Some(contract_address) = deployed_in_db.choose(&mut rng) {
+                        batch.push(account.hello_starknet_increase_balance(
+                            *contract_address,
+                            rng.gen_range(1..=1000),
+                        ));
+                        // This is a view function, but it still gives us a realistic transaction
+                        batch.push(account.hello_starknet_get_balance(*contract_address));
+                    }
 
-        for _ in 0..batch_tries {
-            // Maybe deploy a new instance
-            if rng.gen() {
-                batch.push(account.hello_starknet_deploy()?);
-            }
+                    next_txn_idx_start += batch.len();
+                }
 
-            // Invoke a random contract instance if there are any deployments in the DB
-            if let Some(contract_address) = deployed_in_db.choose(&mut rng) {
-                batch.push(
-                    account.hello_starknet_increase_balance(
-                        *contract_address,
-                        rng.gen_range(1..=1000),
-                    ),
-                );
-                // This is a view function, but it still gives us a realistic transaction
-                batch.push(account.hello_starknet_get_balance(*contract_address));
+                if !batch.is_empty() {
+                    batches.push(batch);
+                }
             }
-
-            next_txn_idx_start += batch.len();
-        }
-
-        if !batch.is_empty() {
-            batches.push(batch);
         }
     }
 

crates/pathfinder/src/devnet.rs

@@ -93,7 +93,7 @@ pub fn init_db(db_dir: &Path, proposer: Address) -> anyhow::Result<BootDb> {
     let db_txn = db_conn.transaction()?;
     let mut state_update = StateUpdateData::default();
 
-    let account = predeploy_contracts(&db_txn, &mut state_update)?;
+    let _account = predeploy_contracts(&db_txn, &mut state_update)?;
 
     let block_number = BlockNumber::GENESIS;
     let (storage_commitment, class_commitment) = update_starknet_state(
@@ -150,36 +150,19 @@ pub fn init_db(db_dir: &Path, proposer: Address) -> anyhow::Result<BootDb> {
         stopwatch.elapsed().as_millis(),
     );
 
-    let db_txn = db_conn.transaction()?;
-    declare(
-        storage.clone(),
-        db_txn,
-        &account,
-        fixtures::HELLO_CLASS,
-        proposer,
-    )?;
-    let db_txn = db_conn.transaction()?;
-    let latest_block_number = db_txn.block_number(BlockId::Latest)?.context("Empty DB")?;
-
     Ok(BootDb {
         db_file_path,
-        num_boot_blocks: latest_block_number.get() + 1,
+        num_boot_blocks: block_number.get() + 1,
     })
 }
 
-pub fn is_db_bootstrapped(db_txn: &pathfinder_storage::Transaction<'_>) -> anyhow::Result<bool> {
+/// Returns `Ok(true)` if the DB is bootstrapped and `Ok(false)` if it is not.
+pub fn devnet_genesis_exists(db_txn: &pathfinder_storage::Transaction<'_>) -> anyhow::Result<bool> {
     let Some(block_0_commitment) = db_txn.state_diff_commitment(BlockNumber::GENESIS)? else {
         return Ok(false);
     };
 
-    if block_0_commitment != fixtures::BLOCK_0_COMMITMENT {
-        return Ok(false);
-    }
-
-    let block_1_commitment = db_txn
-        .state_diff_commitment(BlockNumber::GENESIS + 1)?
-        .context("DB has only genesis block")?;
-    if block_1_commitment != fixtures::BLOCK_1_COMMITMENT {
+    if block_0_commitment != fixtures::GENESIS_COMMITMENT {
         return Ok(false);
     }
 
@@ -292,8 +275,14 @@ pub fn declare(
         state_update,
         transactions_and_receipts,
         events,
+        declared_classes,
     } = next_block;
 
+    assert_eq!(declared_classes.len(), 1);
+    let declared_class = &declared_classes[0];
+    assert_eq!(declared_class.sierra_hash, sierra_class_hash);
+    assert_eq!(declared_class.casm_hash_v2, casm_hash_v2);
+
     let next_header = header.compute_hash(latest_header.hash, state_commitment, compute_final_hash);
 
     db_txn.insert_block_header(&next_header)?;
@@ -437,10 +426,10 @@ pub mod tests {
         BlockHash,
         BlockHeader,
         BlockId,
-        BlockNumber,
         ClassHash,
         ConsensusFinalizedL2Block,
         ContractAddress,
+        DeclaredClass,
         StarknetVersion,
         StateCommitment,
     };
@@ -459,7 +448,6 @@ pub mod tests {
     fn init_declare_deploy_invoke_hello_abi() {
         // Block 0 - predeploys and initializes contracts, including the account we'll
         // use for testing
-        // Block 1 - declare the Hello Starknet contract class
         let proposer = Address(Felt::ONE);
         let db_dir = TempDir::new().unwrap();
         let BootDb { db_file_path, .. } = init_db(db_dir.path(), proposer).unwrap();
@@ -475,17 +463,10 @@ pub mod tests {
         let mut db_conn = storage.connection().unwrap();
         let db_txn = db_conn.transaction().unwrap();
 
-        let block_0_state_diff_commitment = db_txn
-            .state_diff_commitment(BlockNumber::GENESIS)
-            .unwrap()
-            .unwrap();
-        assert_eq!(block_0_state_diff_commitment, fixtures::BLOCK_0_COMMITMENT);
-
-        let block_1_header = db_txn.block_header(BlockId::Latest).unwrap().unwrap();
-
+        let block_0_header = db_txn.block_header(BlockId::Latest).unwrap().unwrap();
         assert_eq!(
-            block_1_header.state_diff_commitment,
-            fixtures::BLOCK_1_COMMITMENT
+            block_0_header.state_diff_commitment,
+            fixtures::GENESIS_COMMITMENT
         );
 
         let account = Account::from_storage(&db_txn).unwrap();
@@ -494,6 +475,30 @@ pub mod tests {
         let worker_pool: ValidatorWorkerPool =
             ExecutorWorkerPool::<ConcurrentStateReader>::new(1).get();
 
+        // Block 1 - declare the Hello Starknet contract class
+        let block_1_number = block_0_header.number + 1;
+        let (mut validator, _) = init_proposal_and_validator(
+            block_1_number,
+            0,
+            proposer,
+            Some(block_0_header.timestamp),
+            storage.clone(),
+            worker_pool.clone(),
+        )
+        .unwrap();
+        let declare = account.hello_starknet_declare().unwrap();
+        validator
+            .execute_batch::<ProdTransactionMapper>(
+                vec![declare],
+                pathfinder_compiler::ResourceLimits::for_test(),
+            )
+            .unwrap();
+        let block_1 = validator.consensus_finalize0().unwrap();
+
+        let db_txn = db_conn.transaction().unwrap();
+        let (block_1_header, _) =
+            insert_block(storage.clone(), db_txn, block_1, block_0_header.hash);
+
         // Block 2 - deploy a Hello Starknet contract instance via the UDC
         let block_2_number = block_1_header.number + 1;
         let (mut validator, _) = init_proposal_and_validator(
@@ -574,8 +579,28 @@ pub mod tests {
             state_update,
             transactions_and_receipts,
             events,
+            declared_classes,
         } = block;
 
+        declared_classes
+            .into_iter()
+            .try_for_each(
+                |DeclaredClass {
+                     sierra_hash,
+                     casm_hash_v2,
+                     sierra_def,
+                     casm_def,
+                 }| {
+                    db_txn.insert_sierra_class_definition(
+                        &sierra_hash,
+                        &sierra_def,
+                        &casm_def,
+                        &casm_hash_v2,
+                    )
+                },
+            )
+            .unwrap();
+
         let hello_contract_address =
             state_update
                 .contract_updates