diff --git a/Cargo.lock b/Cargo.lock index 090eea0fcc..3b6f5c704a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6609,6 +6609,8 @@ dependencies = [ "serde_with", "sha3", "stark_hash", + "stark_poseidon", + "starknet-gateway-client", "starknet-gateway-types", "tempfile", "thiserror", diff --git a/crates/common/src/header.rs b/crates/common/src/header.rs index 7a28fbde0d..87dde49c25 100644 --- a/crates/common/src/header.rs +++ b/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 diff --git a/crates/gateway-types/src/reply.rs b/crates/gateway-types/src/reply.rs index 3b1498b1a2..6155ea6be1 100644 --- a/crates/gateway-types/src/reply.rs +++ b/crates/gateway-types/src/reply.rs @@ -290,6 +290,10 @@ pub mod transaction { impl Dummy for Receipt { fn dummy_with_rng(_: &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, } } } diff --git a/crates/p2p_proto/proto/common.proto b/crates/p2p_proto/proto/common.proto index 9cd4382041..63272fd5ed 100644 --- a/crates/p2p_proto/proto/common.proto +++ b/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,6 +132,10 @@ 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; @@ -135,6 +143,11 @@ message ExecutionResources { uint64 n_memory_holes = 3; } +enum ExecutionStatus { + SUCCEEDED = 0; + REVERTED = 1; +} + message InvokeTransactionReceipt { CommonTransactionReceiptProperties common = 1; } diff --git a/crates/p2p_proto/src/common.rs b/crates/p2p_proto/src/common.rs index 234d1875ef..13188f4a84 100644 --- a/crates/p2p_proto/src/common.rs +++ b/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, 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 for ExecutionStatus { + fn try_from_protobuf(input: i32, field_name: &'static str) -> Result { + 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 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)] diff --git a/crates/pathfinder/src/p2p_network/client.rs b/crates/pathfinder/src/p2p_network/client.rs index d4daabb4c3..0519aed3d1 100644 --- a/crates/pathfinder/src/p2p_network/client.rs +++ b/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), }) } } diff --git a/crates/pathfinder/src/p2p_network/sync_handlers.rs b/crates/pathfinder/src/p2p_network/sync_handlers.rs index 3acfd7ef1d..3c941a3c78 100644 --- a/crates/pathfinder/src/p2p_network/sync_handlers.rs +++ b/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::>(); @@ -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 => { diff --git a/crates/pathfinder/src/state/sync.rs b/crates/pathfinder/src/state/sync.rs index 2be536bd85..045c3bd835 100644 --- a/crates/pathfinder/src/state/sync.rs +++ b/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 diff --git a/crates/rpc/fixtures/mainnet.sqlite b/crates/rpc/fixtures/mainnet.sqlite index 8784173629..483cac8f9e 100644 Binary files a/crates/rpc/fixtures/mainnet.sqlite and b/crates/rpc/fixtures/mainnet.sqlite differ diff --git a/crates/rpc/src/cairo/ext_py.rs b/crates/rpc/src/cairo/ext_py.rs index 25fb64046c..88efdccaeb 100644 --- a/crates/rpc/src/cairo/ext_py.rs +++ b/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. diff --git a/crates/rpc/src/j2.rs b/crates/rpc/src/j2.rs new file mode 100644 index 0000000000..93ecd6982d --- /dev/null +++ b/crates/rpc/src/j2.rs @@ -0,0 +1,188 @@ +// #![allow(dead_code)] + +// use std::collections::HashMap; +// use std::marker::PhantomData; + +// use futures::Future; +// use serde::de::DeserializeOwned; +// use serde_json::Value; + +// use crate::context::RpcContext; + +// #[derive(Default)] +// struct MethodRouter { +// methods: HashMap<&'static str, Box>, +// } + +// impl MethodRouter { +// fn register( +// mut self, +// method_name: &'static str, +// method: impl IntoMethod + 'static, +// ) -> Self { +// self.methods.insert(method_name, method.into_method()); +// self +// } + +// async fn invoke( +// &self, +// method: &str, +// input: Value, +// state: RpcContext, +// ) -> Result { +// match self.methods.get(method) { +// Some(method) => method.invoke(input, state).await.map_err(Into::into), +// None => Err(crate::jsonrpc::RpcError::MethodNotFound { +// method: method.to_owned(), +// }), +// } +// } +// } + +// #[axum::async_trait] +// trait Method { +// async fn invoke( +// &self, +// input: Value, +// state: RpcContext, +// ) -> Result; +// } + +// trait IntoMethod { +// fn into_method(self) -> Box; +// } + +// /// impl for +// /// () -> T +// /// input only +// /// state only +// /// input and state + + + +// impl<'de, Func, Fut, Input> IntoMethod<((), Input)> for Func +// where +// Func: Fn(Input) -> Fut + Clone + 'static + std::marker::Sync + std::marker::Send, +// Fut: Future + std::marker::Send + 'static, +// Input: DeserializeOwned + std::marker::Sync + std::marker::Send + 'static, +// { +// fn into_method(self) -> Box { +// struct HandlerImpl +// where +// Func: Fn(Input) -> Fut + Clone, +// Fut: Future, +// { +// f: Func, +// _marker: PhantomData, +// } + +// #[axum::async_trait] +// impl<'de, Func, Fut, Input> Method for HandlerImpl +// where +// Func: Fn(Input) -> Fut + Clone + 'static + std::marker::Sync + std::marker::Send, +// Fut: Future + std::marker::Send, +// Input: DeserializeOwned + std::marker::Sync + std::marker::Send + 'static, +// { +// async fn invoke(&self, input: Value) -> u32 { +// let input: Input = serde_json::from_value(input).unwrap(); +// let f = self.f.clone(); +// async move { f(input).await }.await +// } +// } + +// Box::new(HandlerImpl { +// f: self, +// _marker: Default::default(), +// }) as Box +// } +// } + +// impl IntoMethod for Func +// where +// Func: Fn(u32) -> Fut + Clone + 'static + std::marker::Sync + std::marker::Send, +// Fut: Future + std::marker::Send + 'static, +// { +// fn into_method(self) -> Box { +// struct HandlerImpl +// where +// Func: Fn(u32) -> Fut + Clone, +// Fut: Future, +// { +// f: Func, +// } + +// #[derive(serde::Deserialize)] +// struct Input(u32); + +// #[axum::async_trait] +// impl Method for HandlerImpl +// where +// Func: Fn(u32) -> Fut + Clone + std::marker::Sync + std::marker::Send, +// Fut: Future + std::marker::Send, +// { +// async fn invoke(&self, input: Value) -> u32 { +// let input: Input = serde_json::from_value(input).unwrap(); +// let f = self.f.clone(); +// f(input.0).await +// } +// } + +// Box::new(HandlerImpl { f: self }) as Box +// } +// } + +// impl IntoMethod<()> for Func +// where +// Func: Fn() -> Value + Clone + 'static + std::marker::Sync, +// { +// fn into_method(self) -> Box { +// struct HandlerImpl u32 + Clone> { +// f: Func, +// } + +// #[axum::async_trait] +// impl u32 + Clone + std::marker::Sync> Method for HandlerImpl { +// async fn invoke(&self, _input: Value) -> u32 { +// let f = self.f.clone(); +// f() +// } +// } + +// Box::new(HandlerImpl { f: self }) as Box +// } +// } + +// #[tokio::test] +// async fn feature() { +// async fn echo(x: u32) -> u32 { +// x +// } + +// fn always_10() -> u32 { +// 10 +// } + +// async fn double(x: u32) -> u32 { +// x * 2 +// } + +// #[derive(serde::Deserialize)] +// struct In(u32); + +// async fn echo_serde(input: In) -> u32 { +// input.0 +// } + +// let router = MethodRouter::default() +// .register::("echo", echo) +// .register("always_10", always_10) +// .register::("double", double) +// .register("echo_serde", echo_serde); + +// let input = Value::Number(20.into()); + +// assert_eq!(router.invoke("echo_serde", input.clone()).await, 20); +// assert_eq!(router.invoke("echo", input.clone()).await, 20); +// assert_eq!(router.invoke("always_10", input.clone()).await, 10); +// assert_eq!(router.invoke("double", input).await, 20 * 2); +// } diff --git a/crates/rpc/src/v03/method.rs b/crates/rpc/src/v03/method.rs index cf3860cb44..2e063936f8 100644 --- a/crates/rpc/src/v03/method.rs +++ b/crates/rpc/src/v03/method.rs @@ -40,7 +40,7 @@ pub(crate) mod common { .ok_or_else(|| anyhow::anyhow!("Unsupported configuration"))?; // discussed during estimateFee work: when user is requesting using block_hash use the - // gasPrice from the starknet_blocks::gas_price column, otherwise (tags) get the latest + // gasPrice from the headers::gas_price column, otherwise (tags) get the latest // eth_gasPrice. // // the fact that [`base_block_and_pending_for_call`] transforms pending cases to use diff --git a/crates/rpc/src/v04/method.rs b/crates/rpc/src/v04/method.rs index a169073f68..df6e596900 100644 --- a/crates/rpc/src/v04/method.rs +++ b/crates/rpc/src/v04/method.rs @@ -44,7 +44,7 @@ pub(crate) mod common { .ok_or_else(|| anyhow::anyhow!("Unsupported configuration"))?; // discussed during estimateFee work: when user is requesting using block_hash use the - // gasPrice from the starknet_blocks::gas_price column, otherwise (tags) get the latest + // gasPrice from the headers::gas_price column, otherwise (tags) get the latest // eth_gasPrice. // // the fact that [`base_block_and_pending_for_call`] transforms pending cases to use diff --git a/crates/storage/Cargo.toml b/crates/storage/Cargo.toml index bd8e9e0959..21b53e65bf 100644 --- a/crates/storage/Cargo.toml +++ b/crates/storage/Cargo.toml @@ -31,6 +31,8 @@ serde_json = { workspace = true, features = [ serde_with = { workspace = true } sha3 = "0.10" stark_hash = { path = "../stark_hash" } +stark_poseidon = { path = "../stark_poseidon" } +starknet-gateway-client = { path = "../gateway-client" } starknet-gateway-types = { path = "../gateway-types" } thiserror = { workspace = true } tokio = { workspace = true } diff --git a/crates/storage/src/connection/block.rs b/crates/storage/src/connection/block.rs index b1ddbab017..4bd4402c21 100644 --- a/crates/storage/src/connection/block.rs +++ b/crates/storage/src/connection/block.rs @@ -1,5 +1,5 @@ use anyhow::Context; -use pathfinder_common::{BlockHash, BlockHeader, BlockNumber, StarknetVersion, StateCommitment}; +use pathfinder_common::{BlockHash, BlockHeader, BlockNumber, StarknetVersion}; use crate::{prelude::*, BlockId}; @@ -13,13 +13,13 @@ pub(super) fn insert_block_header( // Insert the header tx.inner().execute( - r"INSERT INTO starknet_blocks - ( number, hash, root, timestamp, gas_price, sequencer_address, version_id, transaction_commitment, event_commitment, class_commitment) - VALUES (:number, :hash, :root, :timestamp, :gas_price, :sequencer_address, :version_id, :transaction_commitment, :event_commitment, :class_commitment)", + r"INSERT INTO block_headers + ( number, hash, storage_commitment, timestamp, gas_price, sequencer_address, version_id, transaction_commitment, event_commitment, state_commitment, class_commitment, transaction_count, event_count) + VALUES (:number, :hash, :storage_commitment, :timestamp, :gas_price, :sequencer_address, :version_id, :transaction_commitment, :event_commitment, :state_commitment, :class_commitment, :transaction_count, :event_count)", named_params! { ":number": &header.number, ":hash": &header.hash, - ":root": &header.storage_commitment, + ":storage_commitment": &header.storage_commitment, ":timestamp": &header.timestamp, ":gas_price": &header.gas_price.to_be_bytes().as_slice(), ":sequencer_address": &header.sequencer_address, @@ -27,6 +27,9 @@ pub(super) fn insert_block_header( ":transaction_commitment": &header.transaction_commitment, ":event_commitment": &header.event_commitment, ":class_commitment": &header.class_commitment, + ":transaction_count": &header.transaction_count.try_into_sql_int()?, + ":event_count": &header.event_count.try_into_sql_int()?, + ":state_commitment": &header.state_commitment, }, ).context("Inserting block header")?; @@ -102,10 +105,10 @@ pub(super) fn purge_block(tx: &Transaction<'_>, block: BlockNumber) -> anyhow::R tx.inner() .execute( - "DELETE FROM starknet_blocks WHERE number = ?", + "DELETE FROM block_headers WHERE number = ?", params![&block], ) - .context("Deleting block from starknet_blocks table")?; + .context("Deleting block from block_headers table")?; Ok(()) } @@ -173,7 +176,7 @@ pub(super) fn block_header( block: BlockId, ) -> anyhow::Result> { // TODO: is LEFT JOIN reasonable? It's required because version ID can be null for non-existent versions. - const BASE_SQL: &str = "SELECT * FROM starknet_blocks LEFT JOIN starknet_versions ON starknet_blocks.version_id = starknet_versions.id"; + const BASE_SQL: &str = "SELECT * FROM block_headers LEFT JOIN starknet_versions ON block_headers.version_id = starknet_versions.id"; let sql = match block { BlockId::Latest => format!("{BASE_SQL} ORDER BY number DESC LIMIT 1"), BlockId::Number(_) => format!("{BASE_SQL} WHERE number = ?"), @@ -183,7 +186,7 @@ pub(super) fn block_header( let parse_row = |row: &rusqlite::Row<'_>| { let number = row.get_block_number("number")?; let hash = row.get_block_hash("hash")?; - let storage_commitment = row.get_storage_commitment("root")?; + let storage_commitment = row.get_storage_commitment("storage_commitment")?; let timestamp = row.get_timestamp("timestamp")?; let gas_price = row.get_gas_price("gas_price")?; let sequencer_address = row.get_sequencer_address("sequencer_address")?; @@ -191,10 +194,9 @@ pub(super) fn block_header( let event_commitment = row.get_event_commitment("event_commitment")?; let class_commitment = row.get_class_commitment("class_commitment")?; let starknet_version = row.get_starknet_version("version")?; - - // TODO: this really needs to get stored instead. - let state_commitment = StateCommitment::calculate(storage_commitment, class_commitment); - // TODO: test what happens when a field is null. + let event_count: usize = row.get("event_count")?; + let transaction_count: usize = row.get("transaction_count")?; + let state_commitment = row.get_state_commitment("state_commitment")?; let header = BlockHeader { hash, @@ -208,6 +210,8 @@ pub(super) fn block_header( storage_commitment, transaction_commitment, starknet_version, + transaction_count, + event_count, // TODO: store block hash in-line. // This gets filled in by a separate query, but really should get stored as a column in // order to support truncated history. @@ -234,7 +238,7 @@ pub(super) fn block_header( let parent_hash = tx .inner() .query_row( - "SELECT hash FROM starknet_blocks WHERE number = ?", + "SELECT hash FROM block_headers WHERE number = ?", params![&(header.number - 1)], |row| row.get_block_hash(0), ) @@ -262,8 +266,8 @@ pub(super) fn block_is_l1_accepted(tx: &Transaction<'_>, block: BlockId) -> anyh mod tests { use pathfinder_common::macro_prelude::*; use pathfinder_common::{ - BlockTimestamp, ClassCommitment, ClassHash, EventCommitment, GasPrice, StateUpdate, - TransactionCommitment, + BlockTimestamp, ClassCommitment, ClassHash, EventCommitment, GasPrice, StateCommitment, + StateUpdate, TransactionCommitment, }; use super::*; @@ -292,10 +296,11 @@ mod tests { starknet_version: StarknetVersion::default(), class_commitment, event_commitment: event_commitment_bytes!(b"event commitment genesis"), - // This needs to be calculated as this value is never actually stored directly. state_commitment: StateCommitment::calculate(storage_commitment, class_commitment), storage_commitment, transaction_commitment: transaction_commitment_bytes!(b"tx commitment genesis"), + transaction_count: 37, + event_count: 40, }; let header1 = genesis .child_builder() @@ -386,7 +391,7 @@ mod tests { // Overwrite the commitment fields to NULL. tx.inner().execute( - r"UPDATE starknet_blocks + r"UPDATE block_headers SET transaction_commitment=NULL, event_commitment=NULL, class_commitment=NULL, version_id=NULL WHERE number=?", params![&target.number], @@ -398,8 +403,6 @@ mod tests { expected.transaction_commitment = TransactionCommitment::ZERO; expected.event_commitment = EventCommitment::ZERO; expected.class_commitment = ClassCommitment::ZERO; - expected.state_commitment = - StateCommitment::calculate(expected.storage_commitment, expected.class_commitment); let result = tx.block_header(target.number.into()).unwrap().unwrap(); diff --git a/crates/storage/src/connection/event.rs b/crates/storage/src/connection/event.rs index ca32baf8b2..fab64b2475 100644 --- a/crates/storage/src/connection/event.rs +++ b/crates/storage/src/connection/event.rs @@ -81,7 +81,7 @@ pub(super) fn insert_events( stmt.execute(named_params![ ":block_number": &block_number, - ":idx": &idx, + ":idx": &idx.try_into_sql_int()?, ":transaction_hash": &transaction_hash, ":from_address": &event.from_address, ":keys": &keys, @@ -150,7 +150,7 @@ pub(super) fn get_events( let base_query = r#"SELECT block_number, - starknet_blocks.hash as block_hash, + block_headers.hash as block_hash, transaction_hash, starknet_transactions.idx as transaction_idx, from_address, @@ -158,7 +158,7 @@ pub(super) fn get_events( starknet_events.keys as keys FROM starknet_events INNER JOIN starknet_transactions ON (starknet_transactions.hash = starknet_events.transaction_hash) - INNER JOIN starknet_blocks ON (starknet_blocks.number = starknet_events.block_number)"#; + INNER JOIN block_headers ON (block_headers.number = starknet_events.block_number)"#; let (mut base_query, mut params) = event_query( base_query, @@ -172,8 +172,8 @@ pub(super) fn get_events( // We have to be able to decide if there are more events. We request one extra event // above the requested page size, so that we can decide. let limit = filter.page_size + 1; - params.push((":limit", limit.to_sql())); - params.push((":offset", filter.offset.to_sql())); + params.push((":limit", limit.try_into_sql()?)); + params.push((":offset", filter.offset.try_into_sql()?)); base_query.to_mut().push_str( " ORDER BY block_number, transaction_idx, starknet_events.idx LIMIT :limit OFFSET :offset", diff --git a/crates/storage/src/connection/state_update.rs b/crates/storage/src/connection/state_update.rs index 770bb8d3ff..1486b1a843 100644 --- a/crates/storage/src/connection/state_update.rs +++ b/crates/storage/src/connection/state_update.rs @@ -97,8 +97,8 @@ fn block_details( ) -> anyhow::Result> { use const_format::formatcp; - const PREFIX: &str = r"SELECT b1.number, b1.hash, b1.root, b1.class_commitment, b2.root, b2.class_commitment FROM starknet_blocks b1 - LEFT OUTER JOIN starknet_blocks b2 ON b2.number = b1.number - 1"; + const PREFIX: &str = r"SELECT b1.number, b1.hash, b1.storage_commitment, b1.class_commitment, b2.storage_commitment, b2.class_commitment FROM block_headers b1 + LEFT OUTER JOIN block_headers b2 ON b2.number = b1.number - 1"; const LATEST: &str = formatcp!("{PREFIX} ORDER BY b1.number DESC LIMIT 1"); const NUMBER: &str = formatcp!("{PREFIX} WHERE b1.number = ?"); diff --git a/crates/storage/src/connection/transaction.rs b/crates/storage/src/connection/transaction.rs index 6ba1f63bcc..41ea6cf863 100644 --- a/crates/storage/src/connection/transaction.rs +++ b/crates/storage/src/connection/transaction.rs @@ -43,7 +43,7 @@ pub(super) fn insert_transactions( VALUES (:hash, :idx, :block_hash, :tx, :receipt, :execution_status)", named_params![ ":hash": &transaction.hash(), - ":idx": &i, + ":idx": &i.try_into_sql_int()?, ":block_hash": &block_hash, ":tx": &tx_data, ":receipt": &serialized_receipt, @@ -131,7 +131,7 @@ pub(super) fn transaction_at_block( .context("Preparing statement")?; let mut rows = stmt - .query(params![&block_hash, &index]) + .query(params![&block_hash, &index.try_into_sql_int()?]) .context("Executing query")?; let row = match rows.next()? { @@ -156,7 +156,7 @@ pub(super) fn transaction_count(tx: &Transaction<'_>, block: BlockId) -> anyhow: .inner() .query_row( "SELECT COUNT(*) FROM starknet_transactions - JOIN starknet_blocks ON starknet_transactions.block_hash = starknet_blocks.hash + JOIN block_headers ON starknet_transactions.block_hash = block_headers.hash WHERE number = ?1", params![&number], |row| row.get(0), diff --git a/crates/storage/src/fake.rs b/crates/storage/src/fake.rs index 90c3d6e261..786ed07a11 100644 --- a/crates/storage/src/fake.rs +++ b/crates/storage/src/fake.rs @@ -121,6 +121,12 @@ pub mod init { }) .collect::>(); + header.transaction_count = transactions_and_receipts.len(); + header.event_count = transactions_and_receipts + .iter() + .map(|(_, r)| r.events.len()) + .sum(); + let block_hash = header.hash; let state_commitment = header.state_commitment; diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 8b2be27e81..5e863f8bdf 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -446,8 +446,7 @@ mod tests { std::fs::copy(&source_path, &db_path).unwrap(); - let mut database = rusqlite::Connection::open(db_path).unwrap(); - migrate_database(&mut database).unwrap(); + let database = rusqlite::Connection::open(db_path).unwrap(); let version = schema_version(&database).unwrap(); let expected = schema::migrations().len() + schema::BASE_SCHEMA_REVISION; diff --git a/crates/storage/src/params.rs b/crates/storage/src/params.rs index 68c31d643f..b6214a2bd8 100644 --- a/crates/storage/src/params.rs +++ b/crates/storage/src/params.rs @@ -1,3 +1,4 @@ +use anyhow::Result; use pathfinder_common::trie::TrieNode; use pathfinder_common::{ BlockHash, BlockNumber, BlockTimestamp, ByteCodeOffset, CallParam, CallResultValue, CasmHash, @@ -16,6 +17,14 @@ pub trait ToSql { fn to_sql(&self) -> ToSqlOutput<'_>; } +pub trait TryIntoSql { + fn try_into_sql(&self) -> Result>; +} + +pub trait TryIntoSqlInt { + fn try_into_sql_int(&self) -> Result; +} + impl ToSql for Option { fn to_sql(&self) -> ToSqlOutput<'_> { use rusqlite::types::Value; @@ -101,7 +110,6 @@ to_sql_compressed_felt!(ContractNonce, StorageValue, TransactionNonce); to_sql_int!(BlockNumber, BlockTimestamp); -// TODO: check if these can fail in rusqlite. to_sql_builtin!( String, &str, @@ -112,13 +120,15 @@ to_sql_builtin!( i32, i16, i8, - usize, - u64, u32, u16, u8 ); +try_into_sql!(usize, u64); + +try_into_sql_int!(usize, u64); + /// Extends [rusqlite::Row] to provide getters for our own foreign types. This is a work-around /// for the orphan rule -- our types live in a separate crate and can therefore not implement the /// rusqlite traits. @@ -417,6 +427,35 @@ macro_rules! to_sql_builtin { } } +macro_rules! try_into_sql { + ($target:ty) => { + impl TryIntoSql for $target { + fn try_into_sql(&self) -> anyhow::Result> { + use rusqlite::types::{ToSqlOutput, Value}; + Ok(ToSqlOutput::Owned(Value::Integer(i64::try_from(*self)?))) + } + } + }; + ($head:ty, $($rest:ty),+ $(,)?) => { + try_into_sql!($head); + try_into_sql!($($rest),+); + } +} + +macro_rules! try_into_sql_int { + ($target:ty) => { + impl TryIntoSqlInt for $target { + fn try_into_sql_int(&self) -> anyhow::Result { + Ok(i64::try_from(*self)?) + } + } + }; + ($head:ty, $($rest:ty),+ $(,)?) => { + try_into_sql_int!($head); + try_into_sql_int!($($rest),+); + } +} + macro_rules! row_felt_wrapper { ($fn_name:ident, $Type:ident) => { fn $fn_name(&self, index: I) -> rusqlite::Result<$Type> { @@ -426,7 +465,10 @@ macro_rules! row_felt_wrapper { }; } -use {row_felt_wrapper, to_sql_builtin, to_sql_compressed_felt, to_sql_felt, to_sql_int}; +use { + row_felt_wrapper, to_sql_builtin, to_sql_compressed_felt, to_sql_felt, to_sql_int, + try_into_sql, try_into_sql_int, +}; /// Used in combination with our own [ToSql] trait to provide functionality equivalent to /// [rusqlite::params!] for our own foreign types. diff --git a/crates/storage/src/prelude.rs b/crates/storage/src/prelude.rs index 4ede46443e..b8cfd783b0 100644 --- a/crates/storage/src/prelude.rs +++ b/crates/storage/src/prelude.rs @@ -1,5 +1,5 @@ //! Bundles commonly used items - meant for internal crate usage only. pub(crate) use crate::connection::Transaction; -pub(crate) use crate::params::{named_params, params, RowExt}; +pub(crate) use crate::params::{named_params, params, RowExt, TryIntoSql, TryIntoSqlInt}; pub(crate) use rusqlite::OptionalExtension; diff --git a/crates/storage/src/schema.rs b/crates/storage/src/schema.rs index 713e018f0c..2f35d784dd 100644 --- a/crates/storage/src/schema.rs +++ b/crates/storage/src/schema.rs @@ -8,6 +8,7 @@ mod revision_0035; mod revision_0036; mod revision_0037; mod revision_0038; +mod revision_0039; pub(crate) use base::base_schema; @@ -25,6 +26,7 @@ pub fn migrations() -> &'static [MigrationFn] { revision_0036::migrate, revision_0037::migrate, revision_0038::migrate, + revision_0039::migrate, ] } diff --git a/crates/storage/src/schema/revision_0031.rs b/crates/storage/src/schema/revision_0031.rs index e8de9f9d47..90f51b7d2c 100644 --- a/crates/storage/src/schema/revision_0031.rs +++ b/crates/storage/src/schema/revision_0031.rs @@ -25,7 +25,7 @@ pub(crate) fn migrate(tx: &rusqlite::Transaction<'_>) -> anyhow::Result<()> { let mut count = 0; let mut t = std::time::Instant::now(); while let Some(row) = rows.next().context("Reading next row")? { - let rowid: usize = row.get(0).context("Getting rowid")?; + let rowid: i64 = row.get(0).context("Getting rowid")?; let nonce: ContractNonce = row.get_contract_nonce(1).context("Getting nonce")?; write diff --git a/crates/storage/src/schema/revision_0039.rs b/crates/storage/src/schema/revision_0039.rs new file mode 100644 index 0000000000..ee15a3e395 --- /dev/null +++ b/crates/storage/src/schema/revision_0039.rs @@ -0,0 +1,109 @@ +use anyhow::Context; +use rusqlite::OptionalExtension; +use stark_hash::Felt; + +/// This migration renames the starknet_blocks to block_headers and adds the state_commitment, +/// transaction_count and event_count columns, and also renames the root column to storage_commitment. +pub(crate) fn migrate(tx: &rusqlite::Transaction<'_>) -> anyhow::Result<()> { + tracing::info!("Refactoring the block_headers table, this may take a while"); + + tx.execute("ALTER TABLE starknet_blocks RENAME TO block_headers", []) + .context("Renaming starknet_blocks table to block_headers")?; + + tx.execute( + "ALTER TABLE block_headers RENAME COLUMN root TO storage_commitment", + [], + ) + .context("Renaming block_headers.root column to block_headers.storage_commitment")?; + + tx.execute( + "ALTER TABLE block_headers ADD COLUMN state_commitment BLOB NOT NULL DEFAULT x'0000000000000000000000000000000000000000000000000000000000000000'", + [], + ) + .context("Adding state_commitment column")?; + + tx.execute( + "ALTER TABLE block_headers ADD COLUMN transaction_count INTEGER NOT NULL DEFAULT 0", + [], + ) + .context("Adding transaction_count column")?; + + tx.execute( + "ALTER TABLE block_headers ADD COLUMN event_count INTEGER NOT NULL DEFAULT 0", + [], + ) + .context("Adding event_count column")?; + + tx.execute( + r"UPDATE block_headers SET transaction_count = ( + SELECT COUNT(1) FROM starknet_transactions WHERE starknet_transactions.block_hash = block_headers.hash + )", + [], + ) + .context("Setting tx counts")?; + + tx.execute( + r"UPDATE block_headers SET event_count = ( + SELECT COUNT(1) FROM starknet_events WHERE starknet_events.block_number = block_headers.number + )", + [], + ) + .context("Setting event counts")?; + + tx.execute( + r"UPDATE block_headers SET state_commitment = storage_commitment WHERE class_commitment IS NULL OR class_commitment = x'0000000000000000000000000000000000000000000000000000000000000000'", + [], + ) + .context("Setting state_commitment = storage_commitment")?; + + let Some(start): Option = tx + .query_row( + "SELECT number FROM block_headers WHERE state_commitment = x'0000000000000000000000000000000000000000000000000000000000000000' ORDER BY number ASC LIMIT 1", + [], + |row| row.get(0), + ) + .optional() + .context("Counting rows")? else { + return Ok(()); + }; + + let mut reader = tx + .prepare( + "SELECT number, storage_commitment, class_commitment FROM block_headers WHERE number >= ?", + ) + .context("Preparing commitment reader statement")?; + let mut writer = tx + .prepare("UPDATE block_headers SET state_commitment = ? WHERE number = ?") + .context("Preparing commitment writer statement")?; + + let rows = reader + .query_map([start], |row| { + let number: u64 = row.get(0).unwrap(); + let storage: Vec = row.get(1).unwrap(); + let class: Vec = row.get(2).unwrap(); + + Ok((number, storage, class)) + }) + .context("Querying commitments")?; + + const GLOBAL_STATE_VERSION: Felt = pathfinder_common::felt_bytes!(b"STARKNET_STATE_V0"); + for row in rows { + let (number, storage, class) = row.context("Iterating over rows")?; + + let storage = Felt::from_be_slice(&storage).context("Parsing storage commitment bytes")?; + let class = Felt::from_be_slice(&class).context("Parsing class commitment bytes")?; + + let state_commitment: Felt = stark_poseidon::poseidon_hash_many(&[ + GLOBAL_STATE_VERSION.into(), + storage.into(), + class.into(), + ]) + .into(); + + writer + .execute(rusqlite::params![number, state_commitment.as_be_bytes()]) + .context("Updating state commitment")?; + } + + Ok(()) +} diff --git a/py/src/pathfinder_worker/call.py b/py/src/pathfinder_worker/call.py index e4bba1bc6e..10427734c4 100644 --- a/py/src/pathfinder_worker/call.py +++ b/py/src/pathfinder_worker/call.py @@ -111,7 +111,7 @@ # used from tests, and the query which asserts that the schema is of expected version. -EXPECTED_SCHEMA_REVISION = 38 +EXPECTED_SCHEMA_REVISION = 39 EXPECTED_CAIRO_VERSION = "0.12.1a0" # this is set by pathfinder automatically when #[cfg(debug_assertions)] @@ -608,7 +608,7 @@ def resolve_block( ) -> Tuple[BlockInfo, int, int]: """ forced_gas_price is the gas price we must use for this blockinfo, if None, - the one from starknet_blocks will be used. this allows the caller to select + the one from block_headers will be used. this allows the caller to select where the gas_price information is coming from, and for example, select different one for latest pointed out by hash or tag. """ @@ -617,11 +617,11 @@ def resolve_block( # it has been decided that the latest is whatever pathfinder knows to be latest synced block # regardless of it being the highest known (not yet synced) cursor = connection.execute( - "select number, timestamp, root, gas_price, sequencer_address, class_commitment, sn_ver.version from starknet_blocks left join starknet_versions sn_ver on (sn_ver.id = version_id) order by number desc limit 1" + "select number, timestamp, storage_commitment, gas_price, sequencer_address, class_commitment, sn_ver.version from block_headers left join starknet_versions sn_ver on (sn_ver.id = version_id) order by number desc limit 1" ) elif isinstance(at_block, int): cursor = connection.execute( - "select number, timestamp, root, gas_price, sequencer_address, class_commitment, sn_ver.version from starknet_blocks left join starknet_versions sn_ver on (sn_ver.id = version_id) where number = ?", + "select number, timestamp, storage_commitment, gas_price, sequencer_address, class_commitment, sn_ver.version from block_headers left join starknet_versions sn_ver on (sn_ver.id = version_id) where number = ?", [at_block], ) else: @@ -631,7 +631,7 @@ def resolve_block( at_block = b"\x00" * (32 - len(at_block)) + at_block cursor = connection.execute( - "select number, timestamp, root, gas_price, sequencer_address, class_commitment, sn_ver.version from starknet_blocks left join starknet_versions sn_ver on (sn_ver.id = version_id) where hash = ?", + "select number, timestamp, storage_commitment, gas_price, sequencer_address, class_commitment, sn_ver.version from block_headers left join starknet_versions sn_ver on (sn_ver.id = version_id) where hash = ?", [at_block], ) diff --git a/py/tests/pathfinder_worker/test_call.py b/py/tests/pathfinder_worker/test_call.py index 0f25b78353..f27040b38a 100644 --- a/py/tests/pathfinder_worker/test_call.py +++ b/py/tests/pathfinder_worker/test_call.py @@ -198,7 +198,7 @@ def inmemory_with_tables(): CREATE TABLE class_definitions ( hash BLOB PRIMARY KEY, definition BLOB, - block_number INTEGER REFERENCES starknet_blocks(number) NOT NULL + block_number INTEGER REFERENCES block_headers(number) NOT NULL ); -- This is missing the foreign key definition @@ -216,10 +216,10 @@ def inmemory_with_tables(): version TEXT NOT NULL UNIQUE ); - CREATE TABLE starknet_blocks ( + CREATE TABLE block_headers ( number INTEGER PRIMARY KEY, hash BLOB NOT NULL, - root BLOB NOT NULL, + storage_commitment BLOB NOT NULL, timestamp INTEGER NOT NULL, gas_price BLOB NOT NULL, sequencer_address BLOB NOT NULL, @@ -445,7 +445,7 @@ def populate_test_contract_with_132_on_3(con): # interestingly python sqlite does not accept X'0' here: cur.execute( - """insert into starknet_blocks (hash, number, timestamp, root, gas_price, sequencer_address, class_commitment) values (?, 1, 1, ?, ?, ?, ?)""", + """insert into block_headers (hash, number, timestamp, storage_commitment, gas_price, sequencer_address, class_commitment) values (?, 1, 1, ?, ?, ?, ?)""", [ b"some blockhash somewhere".rjust(32, b"\x00"), felt_to_bytes(state_root), @@ -615,7 +615,7 @@ def test_no_such_block(): common_command_data = f'"contract_address": "{contract_address}", "entry_point_selector": "{entry_point}", "calldata": ["0x84"], "gas_price": 0, "chain": "TESTNET", "pending_updates": {{}}, "pending_deployed": [], "pending_nonces": {{}}, "pending_timestamp": 0' - con.execute("delete from starknet_blocks") + con.execute("delete from block_headers") con.commit() output = default_132_on_3_scenario( @@ -824,7 +824,7 @@ def test_starknet_version_is_resolved(): ) version_id = cursor.lastrowid - con.execute("UPDATE starknet_blocks SET version_id = ?", [version_id]) + con.execute("UPDATE block_headers SET version_id = ?", [version_id]) (info, _root, _class_commitment) = resolve_block(con, "latest", 0) assert info.starknet_version == "0.9.1" @@ -1152,7 +1152,7 @@ def test_nonce_with_dummy(): ) cur.executemany( - "insert into starknet_blocks (hash, number, root, timestamp, gas_price, sequencer_address, version_id) values (?, ?, ?, ?, ?, ?, ?)", + "insert into block_headers (hash, number, storage_commitment, timestamp, gas_price, sequencer_address, version_id) values (?, ?, ?, ?, ?, ?, ?)", [ ( b"another block".rjust(32, b"\x00"), @@ -1452,7 +1452,7 @@ def setup_account_and_sierra_contract( # Block cur.execute( - """insert into starknet_blocks (hash, number, timestamp, root, gas_price, sequencer_address, class_commitment) values (?, 1, 1, ?, ?, ?, ?)""", + """insert into block_headers (hash, number, timestamp, storage_commitment, gas_price, sequencer_address, class_commitment) values (?, 1, 1, ?, ?, ?, ?)""", [ b"some blockhash somewhere".rjust(32, b"\x00"), felt_to_bytes(storage_root_node.hash()), @@ -1713,7 +1713,7 @@ def test_estimate_fee_for_deploy_newly_declared_account(): # Block cur.execute( - """insert into starknet_blocks (hash, number, timestamp, root, gas_price, sequencer_address, class_commitment) values (?, 1, 1, ?, ?, ?, ?)""", + """insert into block_headers (hash, number, timestamp, storage_commitment, gas_price, sequencer_address, class_commitment) values (?, 1, 1, ?, ?, ?, ?)""", [ b"some blockhash somewhere".rjust(32, b"\x00"), felt_to_bytes(0), @@ -2194,7 +2194,7 @@ def test_simulate_transaction_succeeds(): ) con.execute( - """insert into starknet_blocks (hash, number, timestamp, root, gas_price, sequencer_address) values (?, 1, 1, ?, ?, ?)""", + """insert into block_headers (hash, number, timestamp, storage_commitment, gas_price, sequencer_address) values (?, 1, 1, ?, ?, ?)""", [ b"some blockhash somewhere".rjust(32, b"\x00"), b"\x00" * 32, @@ -2324,7 +2324,7 @@ def deploy_contract(con, name, contract_address, class_hash): state_root = root_node.hash() cur.execute( - """insert into starknet_blocks (hash, number, timestamp, root, gas_price, sequencer_address, class_commitment) values (?, 1, 1, ?, ?, ?, ?)""", + """insert into block_headers (hash, number, timestamp, storage_commitment, gas_price, sequencer_address, class_commitment) values (?, 1, 1, ?, ?, ?, ?)""", [ b"some blockhash somewhere".rjust(32, b"\x00"), felt_to_bytes(state_root),