Skip to content

Commit

Permalink
feat: exclude fallible conversions from ToSql
Browse files Browse the repository at this point in the history
  • Loading branch information
CHr15F0x authored and Mirko-von-Leipzig committed Aug 10, 2023
1 parent 0199381 commit d62bcc5
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 13 deletions.
4 changes: 2 additions & 2 deletions crates/storage/src/connection/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ 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,
":event_count": &header.event_count,
":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")?;
Expand Down
6 changes: 3 additions & 3 deletions crates/storage/src/connection/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -172,8 +172,8 @@ pub(super) fn get_events<K: KeyFilter>(
// 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",
Expand Down
4 changes: 2 additions & 2 deletions crates/storage/src/connection/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()? {
Expand Down
6 changes: 6 additions & 0 deletions crates/storage/src/fake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ pub mod init {
})
.collect::<Vec<_>>();

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;

Expand Down
50 changes: 46 additions & 4 deletions crates/storage/src/params.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use anyhow::Result;
use pathfinder_common::trie::TrieNode;
use pathfinder_common::{
BlockHash, BlockNumber, BlockTimestamp, ByteCodeOffset, CallParam, CallResultValue, CasmHash,
Expand All @@ -16,6 +17,14 @@ pub trait ToSql {
fn to_sql(&self) -> ToSqlOutput<'_>;
}

pub trait TryIntoSql {
fn try_into_sql(&self) -> Result<ToSqlOutput<'_>>;
}

pub trait TryIntoSqlInt {
fn try_into_sql_int(&self) -> Result<i64>;
}

impl<Inner: ToSql> ToSql for Option<Inner> {
fn to_sql(&self) -> ToSqlOutput<'_> {
use rusqlite::types::Value;
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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<rusqlite::types::ToSqlOutput<'_>> {
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<i64> {
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<I: RowIndex>(&self, index: I) -> rusqlite::Result<$Type> {
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/src/prelude.rs
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 1 addition & 1 deletion crates/storage/src/schema/revision_0031.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d62bcc5

Please sign in to comment.