Merge pull request #1293 from eqlabs/db_headers

refactor(storage): cleanup heades table

crates/common/src/header.rs

@@ -19,6 +19,8 @@ pub struct BlockHeader {
     pub state_commitment: StateCommitment,
     pub storage_commitment: StorageCommitment,
     pub transaction_commitment: TransactionCommitment,
+    pub transaction_count: usize,
+    pub event_count: usize,
 }
 
 pub struct BlockHeaderBuilder(BlockHeader);
@@ -112,6 +114,16 @@ impl BlockHeaderBuilder {
         self
     }
 
+    pub fn with_transaction_count(mut self, transaction_count: usize) -> Self {
+        self.0.transaction_count = transaction_count;
+        self
+    }
+
+    pub fn with_event_count(mut self, event_count: usize) -> Self {
+        self.0.event_count = event_count;
+        self
+    }
+
     pub fn finalize_with_hash(mut self, hash: BlockHash) -> BlockHeader {
         self.0.hash = hash;
         self.0

crates/gateway-types/src/reply.rs

@@ -290,6 +290,10 @@ pub mod transaction {
 
     impl<T> Dummy<T> for Receipt {
         fn dummy_with_rng<R: rand::Rng + ?Sized>(_: &T, rng: &mut R) -> Self {
+            let execution_status = Faker.fake_with_rng(rng);
+            let revert_error =
+                (execution_status == ExecutionStatus::Reverted).then(|| Faker.fake_with_rng(rng));
+
             // Those fields that were missing in very old receipts are always present
             Self {
                 actual_fee: Some(Faker.fake_with_rng(rng)),
@@ -299,8 +303,8 @@ pub mod transaction {
                 l2_to_l1_messages: Faker.fake_with_rng(rng),
                 transaction_hash: Faker.fake_with_rng(rng),
                 transaction_index: Faker.fake_with_rng(rng),
-                execution_status: Faker.fake_with_rng(rng),
-                revert_error: Faker.fake_with_rng(rng),
+                execution_status,
+                revert_error,
             }
         }
     }

crates/p2p_proto/proto/common.proto

@@ -118,6 +118,10 @@ message CommonTransactionReceiptProperties {
   // Optional
   MessageToL2 consumed_message = 6;
   ExecutionResources execution_resources = 7;
+  // FIXME: the following 2 fields are not yet in the spec
+  ExecutionStatus execution_status = 8;
+  // Empty if execution_status == SUCCEEDED
+  string revert_error = 9;
 }
 
 message ExecutionResources {
@@ -128,13 +132,22 @@ message ExecutionResources {
     uint64 output_builtin = 4;
     uint64 pedersen_builtin = 5;
     uint64 range_check_builtin = 6;
+    // FIXME: the following 3 fields are not yet in the spec
+    uint64 keccak_builtin = 7;
+    uint64 poseidon_builtin = 8;
+    uint64 segment_arena_builtin = 9;
   }
 
   BuiltinInstanceCounter builtin_instance_counter = 1;
   uint64 n_steps = 2;
   uint64 n_memory_holes = 3;
 }
 
+enum ExecutionStatus {
+  SUCCEEDED = 0;
+  REVERTED = 1;
+}
+
 message InvokeTransactionReceipt {
   CommonTransactionReceiptProperties common = 1;
 }

crates/p2p_proto/src/common.rs

@@ -218,6 +218,9 @@ pub mod execution_resources {
         pub output_builtin: u64,
         pub pedersen_builtin: u64,
         pub range_check_builtin: u64,
+        pub keccak_builtin: u64,
+        pub poseidon_builtin: u64,
+        pub segment_arena_builtin: u64,
     }
 }
 
@@ -232,6 +235,37 @@ pub struct CommonTransactionReceiptProperties {
     #[optional]
     pub consumed_message: Option<MessageToL2>,
     pub execution_resources: ExecutionResources,
+    pub execution_status: ExecutionStatus,
+    pub revert_error: String,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Dummy)]
+pub enum ExecutionStatus {
+    Succeeded,
+    Reverted,
+}
+
+impl TryFromProtobuf<i32> for ExecutionStatus {
+    fn try_from_protobuf(input: i32, field_name: &'static str) -> Result<Self, std::io::Error> {
+        let status = proto::common::ExecutionStatus::from_i32(input).ok_or(std::io::Error::new(
+            std::io::ErrorKind::InvalidData,
+            format!("Failed to convert protobuf output for {field_name}: {input} did not match any known execution status"),
+        ))?;
+
+        match status {
+            proto::common::ExecutionStatus::Succeeded => Ok(Self::Succeeded),
+            proto::common::ExecutionStatus::Reverted => Ok(Self::Reverted),
+        }
+    }
+}
+
+impl ToProtobuf<proto::common::ExecutionStatus> for ExecutionStatus {
+    fn to_protobuf(self) -> proto::common::ExecutionStatus {
+        match self {
+            ExecutionStatus::Succeeded => proto::common::ExecutionStatus::Succeeded,
+            ExecutionStatus::Reverted => proto::common::ExecutionStatus::Reverted,
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, ToProtobuf, TryFromProtobuf, Dummy)]

crates/pathfinder/src/p2p_network/client.rs

@@ -33,6 +33,8 @@ pub mod conv {
                 state_commitment: StateCommitment(header.state_commitment),
                 storage_commitment: StorageCommitment(header.storage_commitment),
                 transaction_commitment: TransactionCommitment(header.transaction_commitment),
+                transaction_count: header.transaction_count as usize,
+                event_count: header.event_count as usize,
             })
         }
     }
@@ -285,8 +287,8 @@ pub mod conv {
             use super::gw;
             use p2p_proto::common::{
                 DeclareTransactionReceipt, DeployAccountTransactionReceipt,
-                DeployTransactionReceipt, InvokeTransactionReceipt, L1HandlerTransactionReceipt,
-                Receipt,
+                DeployTransactionReceipt, ExecutionStatus, InvokeTransactionReceipt,
+                L1HandlerTransactionReceipt, Receipt,
             };
             use pathfinder_common::{
                 event::Event, ContractAddress, EntryPoint, EthereumAddress, EventData, EventKey,
@@ -329,10 +331,9 @@ pub mod conv {
                                         output_builtin: b.output_builtin,
                                         pedersen_builtin: b.pedersen_builtin,
                                         range_check_builtin: b.range_check_builtin,
-                                        // FIXME once p2p has these builtins.
-                                        keccak_builtin: Default::default(),
-                                        poseidon_builtin: Default::default(),
-                                        segment_arena_builtin: Default::default(),
+                                        keccak_builtin: b.keccak_builtin,
+                                        poseidon_builtin: b.poseidon_builtin,
+                                        segment_arena_builtin: b.segment_arena_builtin,
                                     }
                                 },
                                 n_steps: common.execution_resources.n_steps,
@@ -382,9 +383,12 @@ pub mod conv {
                                 common.transaction_index.into(),
                             )
                             .expect("u32::MAX is always smaller than i64::MAX"),
-                            // FIXME: once p2p supports reverted
-                            execution_status: Default::default(),
-                            revert_error: Default::default(),
+                            execution_status: match common.execution_status {
+                                ExecutionStatus::Succeeded => gw::ExecutionStatus::Succeeded,
+                                ExecutionStatus::Reverted => gw::ExecutionStatus::Reverted,
+                            },
+                            revert_error: (common.execution_status == ExecutionStatus::Reverted)
+                                .then_some(common.revert_error),
                         })
                     }
                 }

crates/pathfinder/src/p2p_network/sync_handlers.rs

@@ -252,8 +252,9 @@ mod conv {
             execution_resources::BuiltinInstanceCounter, invoke_transaction::EntryPoint,
             CommonTransactionReceiptProperties, DeclareTransaction, DeclareTransactionReceipt,
             DeployAccountTransaction, DeployAccountTransactionReceipt, DeployTransaction,
-            DeployTransactionReceipt, Event, ExecutionResources, InvokeTransaction,
-            InvokeTransactionReceipt, MessageToL1, MessageToL2, Receipt, Transaction,
+            DeployTransactionReceipt, Event, ExecutionResources, ExecutionStatus,
+            InvokeTransaction, InvokeTransactionReceipt, MessageToL1, MessageToL2, Receipt,
+            Transaction,
         };
         use pathfinder_common::{Fee, L1ToL2MessageNonce, TransactionNonce};
         use stark_hash::Felt;
@@ -304,11 +305,22 @@ mod conv {
                             output_builtin: b.output_builtin,
                             pedersen_builtin: b.pedersen_builtin,
                             range_check_builtin: b.range_check_builtin,
+                            keccak_builtin: b.keccak_builtin,
+                            poseidon_builtin: b.poseidon_builtin,
+                            segment_arena_builtin: b.segment_arena_builtin,
                         },
                         n_steps: x.n_steps,
                         n_memory_holes: x.n_memory_holes,
                     }
                 },
+                execution_status: match gw_r.execution_status {
+                    gw::ExecutionStatus::Succeeded => ExecutionStatus::Succeeded,
+                    gw::ExecutionStatus::Reverted => ExecutionStatus::Reverted,
+                },
+                revert_error: match gw_r.execution_status {
+                    gw::ExecutionStatus::Succeeded => Default::default(),
+                    gw::ExecutionStatus::Reverted => gw_r.revert_error.unwrap_or_default(),
+                },
             };
 
             let version = Felt::from_be_slice(gw_t.version().0.as_bytes())
@@ -773,7 +785,6 @@ mod tests {
             proptest! {
                 #[test]
                 fn forward((start, count, seed, num_blocks) in super::strategy::forward()) {
-                    // Initialize storage once for this proptest, greatly increases performance
                     let (storage, from_db) = storage_with_seed(seed, num_blocks);
 
                     let from_db = overlapping::forward(from_db, start, count).map(|(header, _, _)| header).collect::<Vec<_>>();
@@ -855,14 +866,10 @@ mod tests {
             }
 
             proptest! {
-                // FIXME: unignore once reverted is supported
                 #[test]
-                #[ignore]
                 fn forward((start, count, seed, num_blocks) in super::strategy::forward()) {
-                    // Initialize storage once for this proptest, greatly increases performance
-                    // static STORAGE: SeededStorage = OnceCell::new();
-                    // let (storage, from_db) = STORAGE.get_or_init(|| {storage_with_seed(seed)}).clone();
                     let (storage, from_db) = storage_with_seed(seed, num_blocks);
+
                     let start_hash = match from_db.get(usize::try_from(start).unwrap()).map(|x| x.0.hash) {
                         Some(h) => h,
                         None => {
@@ -893,13 +900,9 @@ mod tests {
                     prop_assert_eq!(from_p2p, from_db)
                 }
 
-                // FIXME: unignore once reverted is supported
                 #[test]
-                #[ignore]
                 fn backward((start, count, seed, num_blocks) in super::strategy::backward()) {
-                    // Initialize storage once for this proptest, greatly increases performance
                     let (storage, from_db) = storage_with_seed(seed, num_blocks);
-
                     let start_hash = match from_db.get(usize::try_from(start).unwrap()).map(|x| x.0.hash) {
                         Some(h) => h,
                         None => {

crates/pathfinder/src/state/sync.rs

@@ -658,6 +658,13 @@ async fn l2_update(
             "State root mismatch"
         );
 
+        let transaction_count = block.transactions.len();
+        let event_count = block
+            .transaction_receipts
+            .iter()
+            .map(|r| r.events.len())
+            .sum();
+
         // Update L2 database. These types shouldn't be options at this level,
         // but for now the unwraps are "safe" in that these should only ever be
         // None for pending queries to the sequencer, but we aren't using those here.
@@ -677,6 +684,8 @@ async fn l2_update(
             state_commitment,
             storage_commitment,
             transaction_commitment,
+            transaction_count,
+            event_count,
         };
 
         transaction

crates/rpc/src/cairo/ext_py.rs

@@ -344,7 +344,7 @@ pub enum CallFailure {
 /// Where should the call code get the used `BlockInfo::gas_price`
 #[derive(Debug)]
 pub enum GasPriceSource {
-    /// Use gasPrice recorded on the `starknet_blocks::gas_price`.
+    /// Use gasPrice recorded on the `headers::gas_price`.
     ///
     /// This is not implied by other arguments such as `at_block` because we might need to
     /// manufacture a block hash for some future use cases.