From e6e2bf3bbfdc23907972af442c94ab993845fbc4 Mon Sep 17 00:00:00 2001 From: Mirko von Leipzig Date: Wed, 16 Mar 2022 10:14:56 +0200 Subject: [PATCH 1/3] fix(storage): transaction fails after reorg Inserting a transaction after a reorg failed due to it already existing. Also standardized upsert behaviour as REPLACE. --- crates/pathfinder/src/cairo/ext_py.rs | 2 +- crates/pathfinder/src/rpc.rs | 12 ++++++------ crates/pathfinder/src/state.rs | 2 +- crates/pathfinder/src/state/sync.rs | 6 +++--- crates/pathfinder/src/storage/contract.rs | 12 ++++++------ crates/pathfinder/src/storage/ethereum.rs | 14 +++++--------- crates/pathfinder/src/storage/state.rs | 17 ++++++++--------- 7 files changed, 30 insertions(+), 35 deletions(-) diff --git a/crates/pathfinder/src/cairo/ext_py.rs b/crates/pathfinder/src/cairo/ext_py.rs index 5df3fa8a9e..4b5af987a0 100644 --- a/crates/pathfinder/src/cairo/ext_py.rs +++ b/crates/pathfinder/src/cairo/ext_py.rs @@ -256,7 +256,7 @@ mod tests { crate::storage::ContractCodeTable::insert(tx, hash, &abi, &bytecode, &contract_definition) .unwrap(); - crate::storage::ContractsTable::insert(tx, crate::core::ContractAddress(address), hash) + crate::storage::ContractsTable::upsert(tx, crate::core::ContractAddress(address), hash) .unwrap(); // this will create the table, not created by migration diff --git a/crates/pathfinder/src/rpc.rs b/crates/pathfinder/src/rpc.rs index 045b3bd4de..8ce795185a 100644 --- a/crates/pathfinder/src/rpc.rs +++ b/crates/pathfinder/src/rpc.rs @@ -362,8 +362,8 @@ mod tests { ContractCodeTable::insert_compressed(&db_txn, &contract0_code).unwrap(); ContractCodeTable::insert_compressed(&db_txn, &contract1_code).unwrap(); - ContractsTable::insert(&db_txn, contract0_addr, contract0_hash).unwrap(); - ContractsTable::insert(&db_txn, contract1_addr, contract1_hash).unwrap(); + ContractsTable::upsert(&db_txn, contract0_addr, contract0_hash).unwrap(); + ContractsTable::upsert(&db_txn, contract1_addr, contract1_hash).unwrap(); let mut global_tree = GlobalStateTree::load(&db_txn, GlobalRoot(StarkHash::ZERO)).unwrap(); let contract_state_hash = @@ -479,19 +479,19 @@ mod tests { let transaction_data0 = [(txn0, receipt0)]; let transaction_data1 = [(txn1, receipt1), (txn2, receipt2)]; let transaction_data2 = [(txn3, receipt3), (txn4, receipt4), (txn5, receipt5)]; - StarknetTransactionsTable::insert_block_transactions( + StarknetTransactionsTable::upsert( &db_txn, genesis_hash, &transaction_data0, ) .unwrap(); - StarknetTransactionsTable::insert_block_transactions( + StarknetTransactionsTable::upsert( &db_txn, block1_hash, &transaction_data1, ) .unwrap(); - StarknetTransactionsTable::insert_block_transactions( + StarknetTransactionsTable::upsert( &db_txn, latest_hash, &transaction_data2, @@ -1575,7 +1575,7 @@ mod tests { .context("Deploy testing contract") .unwrap(); - crate::storage::ContractsTable::insert( + crate::storage::ContractsTable::upsert( &tx, crate::core::ContractAddress(address), hash, diff --git a/crates/pathfinder/src/state.rs b/crates/pathfinder/src/state.rs index ef82e1186f..1b2b8f3da7 100644 --- a/crates/pathfinder/src/state.rs +++ b/crates/pathfinder/src/state.rs @@ -72,7 +72,7 @@ pub(crate) fn update_contract_state( .context("Contract hash is missing from contracts table")?; let contract_state_hash = calculate_contract_state_hash(contract_hash, new_contract_root); - ContractsStateTable::insert(db, contract_state_hash, contract_hash, new_contract_root) + ContractsStateTable::upsert(db, contract_state_hash, contract_hash, new_contract_root) .context("Insert constract state hash into contracts state table")?; Ok(contract_state_hash) diff --git a/crates/pathfinder/src/state/sync.rs b/crates/pathfinder/src/state/sync.rs index 250ab9066c..404bb27dee 100644 --- a/crates/pathfinder/src/state/sync.rs +++ b/crates/pathfinder/src/state/sync.rs @@ -455,7 +455,7 @@ async fn l2_update( .into_iter() .zip(block.transaction_receipts.into_iter()) .collect::>(); - StarknetTransactionsTable::insert_block_transactions( + StarknetTransactionsTable::upsert( &transaction, starknet_block.hash, &transaction_data, @@ -554,8 +554,8 @@ fn deploy_contract( global_tree .set(contract.address, state_hash) .context("Adding deployed contract to global state tree")?; - ContractsStateTable::insert(transaction, state_hash, contract.hash, contract_root) + ContractsStateTable::upsert(transaction, state_hash, contract.hash, contract_root) .context("Insert constract state hash into contracts state table")?; - ContractsTable::insert(transaction, contract.address, contract.hash) + ContractsTable::upsert(transaction, contract.address, contract.hash) .context("Inserting contract hash into contracts table") } diff --git a/crates/pathfinder/src/storage/contract.rs b/crates/pathfinder/src/storage/contract.rs index 5919979386..fa3fd2e597 100644 --- a/crates/pathfinder/src/storage/contract.rs +++ b/crates/pathfinder/src/storage/contract.rs @@ -135,17 +135,17 @@ impl ContractCodeTable { pub struct ContractsTable {} impl ContractsTable { - /// Insert a contract into the table, does nothing if the contract already exists. + /// Insert a contract into the table, overwrites the data if it already exists. /// /// Note that [hash](ContractHash) must reference a contract stored in [ContractCodeTable]. - pub fn insert( + pub fn upsert( transaction: &Transaction, address: ContractAddress, hash: ContractHash, ) -> anyhow::Result<()> { // A contract may be deployed multiple times due to L2 reorgs, so we ignore all after the first. transaction.execute( - r"INSERT OR IGNORE INTO contracts (address, hash) VALUES (:address, :hash)", + r"INSERT OR REPLACE INTO contracts (address, hash) VALUES (:address, :hash)", named_params! { ":address": &address.0.to_be_bytes()[..], ":hash": &hash.0.to_be_bytes()[..], @@ -201,7 +201,7 @@ mod tests { let address = ContractAddress(StarkHash::from_hex_str("abc").unwrap()); let hash = ContractHash(StarkHash::from_hex_str("123").unwrap()); - ContractsTable::insert(&transaction, address, hash).unwrap_err(); + ContractsTable::upsert(&transaction, address, hash).unwrap_err(); } #[test] @@ -215,7 +215,7 @@ mod tests { let definition = vec![9, 13, 25]; ContractCodeTable::insert(&transaction, hash, &[][..], &[][..], &definition[..]).unwrap(); - ContractsTable::insert(&transaction, address, hash).unwrap(); + ContractsTable::upsert(&transaction, address, hash).unwrap(); let result = ContractsTable::get_hash(&transaction, address).unwrap(); @@ -241,7 +241,7 @@ mod tests { ContractCodeTable::insert(&transaction, hash, &abi[..], &code[..], &definition[..]) .unwrap(); - ContractsTable::insert(&transaction, address, hash).unwrap(); + ContractsTable::upsert(&transaction, address, hash).unwrap(); let result = ContractCodeTable::get_code(&transaction, address).unwrap(); diff --git a/crates/pathfinder/src/storage/ethereum.rs b/crates/pathfinder/src/storage/ethereum.rs index f07395a16a..e670298964 100644 --- a/crates/pathfinder/src/storage/ethereum.rs +++ b/crates/pathfinder/src/storage/ethereum.rs @@ -15,16 +15,14 @@ pub struct EthereumBlocksTable {} impl EthereumBlocksTable { /// Inserts a new Ethereum block with the given [hash](EthereumBlockHash) and [number](EthereumBlockNumber). /// - /// Does nothing if the hash already exists. + /// overwrites the data if the hash already exists. pub fn insert( transaction: &Transaction, hash: EthereumBlockHash, number: EthereumBlockNumber, ) -> anyhow::Result<()> { transaction.execute( - r"INSERT INTO ethereum_blocks ( hash, number) - VALUES (:hash, :number) - ON CONFLICT DO NOTHING", + "INSERT OR REPLACE INTO ethereum_blocks (hash, number) VALUES (:hash, :number)", named_params! { ":hash": hash.0.as_bytes(), ":number": number.0, @@ -46,20 +44,18 @@ pub struct EthereumTransactionsTable {} impl EthereumTransactionsTable { /// Insert a new Ethereum transaction. /// - /// Does nothing if the ethereum hash already exists. + /// Overwrites the data if the ethereum hash already exists. /// /// Note that [block_hash](EthereumBlockHash) must reference an /// Ethereum block stored in [EthereumBlocksTable]. - pub fn insert( + pub fn upsert( transaction: &Transaction, block_hash: EthereumBlockHash, tx_hash: EthereumTransactionHash, tx_index: EthereumTransactionIndex, ) -> anyhow::Result<()> { transaction.execute( - r"INSERT INTO ethereum_transactions ( hash, idx, block_hash) - VALUES (:hash, :idx, :block_hash) - ON CONFLICT DO NOTHING", + "INSERT OR REPLACE INTO ethereum_transactions (hash, idx, block_hash) VALUES (:hash, :idx, :block_hash)", named_params! { ":hash": tx_hash.0.as_bytes(), ":idx": tx_index.0, diff --git a/crates/pathfinder/src/storage/state.rs b/crates/pathfinder/src/storage/state.rs index 5cb6e8ed60..11148baa9f 100644 --- a/crates/pathfinder/src/storage/state.rs +++ b/crates/pathfinder/src/storage/state.rs @@ -403,7 +403,9 @@ impl From for StarknetBlocksBlockId { pub struct StarknetTransactionsTable {} impl StarknetTransactionsTable { /// Inserts a Starknet block's transactions and transaction receipts into the [StarknetTransactionsTable]. - pub fn insert_block_transactions( + /// + /// overwrites existing data if the transaction hash already exists. + pub fn upsert( connection: &Connection, block: StarknetBlockHash, transaction_data: &[(transaction::Transaction, transaction::Receipt)], @@ -427,8 +429,7 @@ impl StarknetTransactionsTable { .compress(&receipt) .context("Compress Starknet transaction receipt")?; - connection.execute(r"INSERT INTO starknet_transactions ( hash, idx, block_hash, tx, receipt) - VALUES (:hash, :idx, :block_hash, :tx, :receipt)", + connection.execute(r"INSERT OR REPLACE INTO starknet_transactions (hash, idx, block_hash, tx, receipt) VALUES (:hash, :idx, :block_hash, :tx, :receipt)", named_params![ ":hash": &transaction.transaction_hash.0.as_be_bytes()[..], ":idx": i, @@ -667,17 +668,15 @@ pub struct StarknetBlock { pub struct ContractsStateTable {} impl ContractsStateTable { - /// Insert a state hash into the table. Does nothing if the state hash already exists. - pub fn insert( + /// Insert a state hash into the table, overwrites the data if the hash already exists. + pub fn upsert( transaction: &Transaction, state_hash: ContractStateHash, hash: ContractHash, root: ContractRoot, ) -> anyhow::Result<()> { transaction.execute( - r"INSERT INTO contract_states ( state_hash, hash, root) - VALUES (:state_hash, :hash, :root) - ON CONFLICT DO NOTHING", + "INSERT OR IGNORE INTO contract_states (state_hash, hash, root) VALUES (:state_hash, :hash, :root)", named_params! { ":state_hash": &state_hash.0.to_be_bytes()[..], ":hash": &hash.0.to_be_bytes()[..], @@ -738,7 +737,7 @@ mod tests { let hash = ContractHash(StarkHash::from_hex_str("123").unwrap()); let root = ContractRoot(StarkHash::from_hex_str("def").unwrap()); - ContractsStateTable::insert(&transaction, state_hash, hash, root).unwrap(); + ContractsStateTable::upsert(&transaction, state_hash, hash, root).unwrap(); let result = ContractsStateTable::get_root(&transaction, state_hash).unwrap(); From 193d117655ec9ad408bf43ccde91fcdaf2f55862 Mon Sep 17 00:00:00 2001 From: Mirko von Leipzig Date: Wed, 16 Mar 2022 10:44:06 +0200 Subject: [PATCH 2/3] fix(sync): L2 reorg tail is off-by-one --- crates/pathfinder/src/state/sync/l2.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pathfinder/src/state/sync/l2.rs b/crates/pathfinder/src/state/sync/l2.rs index a0c817721e..0b8594ab9c 100644 --- a/crates/pathfinder/src/state/sync/l2.rs +++ b/crates/pathfinder/src/state/sync/l2.rs @@ -234,7 +234,7 @@ async fn reorg( }; let reorg_tail = new_head - .map(|x| x.0) + .map(|x| x.0 + 1) .unwrap_or(StarknetBlockNumber::GENESIS); tx_event From 2c9af51b1bdc46f5c39d8a03bf690fde2f522655 Mon Sep 17 00:00:00 2001 From: Mirko von Leipzig Date: Wed, 16 Mar 2022 11:02:38 +0200 Subject: [PATCH 3/3] fix: rustfmt --- crates/pathfinder/src/rpc.rs | 21 +++------------------ crates/pathfinder/src/state/sync.rs | 8 ++------ 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/crates/pathfinder/src/rpc.rs b/crates/pathfinder/src/rpc.rs index 8ce795185a..a89a96c317 100644 --- a/crates/pathfinder/src/rpc.rs +++ b/crates/pathfinder/src/rpc.rs @@ -479,24 +479,9 @@ mod tests { let transaction_data0 = [(txn0, receipt0)]; let transaction_data1 = [(txn1, receipt1), (txn2, receipt2)]; let transaction_data2 = [(txn3, receipt3), (txn4, receipt4), (txn5, receipt5)]; - StarknetTransactionsTable::upsert( - &db_txn, - genesis_hash, - &transaction_data0, - ) - .unwrap(); - StarknetTransactionsTable::upsert( - &db_txn, - block1_hash, - &transaction_data1, - ) - .unwrap(); - StarknetTransactionsTable::upsert( - &db_txn, - latest_hash, - &transaction_data2, - ) - .unwrap(); + StarknetTransactionsTable::upsert(&db_txn, genesis_hash, &transaction_data0).unwrap(); + StarknetTransactionsTable::upsert(&db_txn, block1_hash, &transaction_data1).unwrap(); + StarknetTransactionsTable::upsert(&db_txn, latest_hash, &transaction_data2).unwrap(); db_txn.commit().unwrap(); storage diff --git a/crates/pathfinder/src/state/sync.rs b/crates/pathfinder/src/state/sync.rs index 404bb27dee..1c15e099d5 100644 --- a/crates/pathfinder/src/state/sync.rs +++ b/crates/pathfinder/src/state/sync.rs @@ -455,12 +455,8 @@ async fn l2_update( .into_iter() .zip(block.transaction_receipts.into_iter()) .collect::>(); - StarknetTransactionsTable::upsert( - &transaction, - starknet_block.hash, - &transaction_data, - ) - .context("Insert transaction data into database")?; + StarknetTransactionsTable::upsert(&transaction, starknet_block.hash, &transaction_data) + .context("Insert transaction data into database")?; // Track combined L1 and L2 state. let l1_l2_head = RefsTable::get_l1_l2_head(&transaction).context("Query L1-L2 head")?;