From 3cf71c0b59b05d7d119bc9f045d1f0ad086e7942 Mon Sep 17 00:00:00 2001 From: jb caron Date: Tue, 23 Jan 2024 12:40:53 +0100 Subject: [PATCH 1/6] =?UTF-8?q?refactor:=20=F0=9F=94=A5=20remove=20debug?= =?UTF-8?q?=20println!?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/client/deoxys/src/convert.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/client/deoxys/src/convert.rs b/crates/client/deoxys/src/convert.rs index 84e9e0c9b1..b5086e6492 100644 --- a/crates/client/deoxys/src/convert.rs +++ b/crates/client/deoxys/src/convert.rs @@ -141,14 +141,12 @@ fn l1_handler_transaction(tx: &p::L1HandlerTransaction) -> mp_transactions::Hand /// Converts a starknet version string to a felt value. /// If the string contains more than 31 bytes, the function panics. fn starknet_version(version: &Option) -> Felt252Wrapper { - let ret = match version { + match version { Some(version) => { Felt252Wrapper::try_from(version.as_bytes()).expect("Failed to convert version to felt: string is too long") } None => Felt252Wrapper::ZERO, - }; - println!("Starknet version: {}", ret.from_utf8().unwrap()); - ret + } } fn fee(felt: starknet_ff::FieldElement) -> u128 { From a77caa9dd58d8b09890fea74b9e460c68ffd7f0d Mon Sep 17 00:00:00 2001 From: jb caron Date: Tue, 23 Jan 2024 13:26:31 +0100 Subject: [PATCH 2/6] fix: :bug: change method to find transaction change the tx search in the extrinsic block to a search directly in the starknet block using the transaction hash cache to retrieve the index --- crates/client/rpc/src/lib.rs | 104 ++++++++++++++--------------------- 1 file changed, 40 insertions(+), 64 deletions(-) diff --git a/crates/client/rpc/src/lib.rs b/crates/client/rpc/src/lib.rs index 0501cf5b05..60deff0a12 100644 --- a/crates/client/rpc/src/lib.rs +++ b/crates/client/rpc/src/lib.rs @@ -1397,19 +1397,10 @@ where let starknet_block: mp_block::Block = get_block_by_block_hash(self.client.as_ref(), substrate_block_hash).unwrap_or_default(); let block_header = starknet_block.header(); - let block_hash = block_header.hash::().into(); + let block_hash: Felt252Wrapper = block_header.hash::().into(); let block_number = block_header.block_number; - let block_extrinsics = self - .client - .block_body(substrate_block_hash) - .map_err(|e| { - error!("Failed to get block body. Substrate block hash: {substrate_block_hash}, error: {e}"); - StarknetRpcApiError::InternalServerError - })? - .ok_or(StarknetRpcApiError::BlockNotFound)?; - - let chain_id = self.chain_id()?.0.into(); + let chain_id = self.chain_id()?.0; let starknet_version = starknet_block.header().protocol_version; @@ -1419,31 +1410,33 @@ where StarknetRpcApiError::InternalServerError })?; - let (tx_index, transaction) = self - .client - .runtime_api() - .get_index_and_tx_for_tx_hash(substrate_block_hash, block_extrinsics, chain_id, transaction_hash.into()) - .map_err(|e| { - error!( - "Failed to get index for transaction hash. Substrate block hash: {substrate_block_hash}, \ - transaction hash: {transaction_hash}, error: {e}" - ); - StarknetRpcApiError::InternalServerError - })? - .expect("the transaction should be present in the substrate extrinsics"); // not reachable + let block_txs_hashes: Vec<_> = if let Some(tx_hashes) = self.get_cached_transaction_hashes(block_hash.into()) { + tx_hashes + .into_iter() + .map(|h| { + h256_to_felt(h) + .map_err(|e| { + CallError::Failed(anyhow::anyhow!( + "The hash cached for block with hash {block_hash:?} is an invalid felt: '{h}'. The \ + caching db has probably been tempered" + )) + }) + .unwrap() + }) + .collect() + } else { + starknet_block + .transactions_hashes::(chain_id.into(), Some(starknet_block.header().block_number)) + .map(FieldElement::from) + .collect() + }; - let events = self - .client - .runtime_api() - .get_events_for_tx_by_index(substrate_block_hash, tx_index) - .map_err(|e| { - error!( - "Failed to get events for transaction index. Substrate block hash: {substrate_block_hash}, \ - transaction idx: {tx_index}, error: {e}" - ); - StarknetRpcApiError::InternalServerError - })? - .expect("the transaction should be present in the substrate extrinsics"); // not reachable + let (tx_index, _) = + block_txs_hashes.into_iter().enumerate().find(|(_, hash)| hash == &transaction_hash.into()).unwrap().into(); + + let transaction = starknet_block.transactions().get(tx_index).unwrap(); + + let events = vec![]; let execution_result = { let revert_error = self @@ -1476,24 +1469,7 @@ where } } - let events_converted: Vec = - events.clone().into_iter().map(event_conversion).collect(); - - let actual_fee = if fee_disabled { - FieldElement::ZERO - } else { - // Event { - // from_address: fee_token_address, - // keys: [selector("Transfer")], - // data: [ - // send_from_address, // account_contract_address - // send_to_address, // to (sequencer address) - // expected_fee_value_low, // transfer amount (fee) - // expected_fee_value_high, - // ]}, - // fee transfer must be the last event, except enabled disable-transaction-fee feature - events_converted.last().unwrap().data[2] - }; + let actual_fee = FieldElement::ZERO; let actual_status = if starknet_block.header().block_number <= mc_deoxys::l1::ETHEREUM_STATE_UPDATE.lock().unwrap().block_number.0 @@ -1524,13 +1500,13 @@ where // TODO: use actual execution ressources let receipt = match transaction { mp_transactions::Transaction::Declare(_) => TransactionReceipt::Declare(DeclareTransactionReceipt { - transaction_hash, + transaction_hash: transaction_hash.into(), actual_fee, finality_status: actual_status, - block_hash, + block_hash: block_hash.into(), block_number, messages_sent: messages.into_iter().map(message_conversion).collect(), - events: events_converted, + events, execution_result, execution_resources: ExecutionResources { steps: 0, @@ -1549,10 +1525,10 @@ where transaction_hash, actual_fee, finality_status: actual_status, - block_hash, + block_hash: block_hash.into(), block_number, messages_sent: messages.into_iter().map(message_conversion).collect(), - events: events_converted, + events, contract_address: tx.get_account_address(), execution_result, execution_resources: ExecutionResources { @@ -1572,10 +1548,10 @@ where transaction_hash, actual_fee, finality_status: actual_status, - block_hash, + block_hash: block_hash.into(), block_number, messages_sent: Default::default(), - events: events_converted, + events, contract_address: tx.get_account_address(), execution_result, execution_resources: ExecutionResources { @@ -1594,10 +1570,10 @@ where transaction_hash, actual_fee, finality_status: actual_status, - block_hash, + block_hash: block_hash.into(), block_number, messages_sent: messages.into_iter().map(message_conversion).collect(), - events: events_converted, + events, execution_result, execution_resources: ExecutionResources { steps: 0, @@ -1616,10 +1592,10 @@ where transaction_hash, actual_fee, finality_status: actual_status, - block_hash, + block_hash: block_hash.into(), block_number, messages_sent: messages.into_iter().map(message_conversion).collect(), - events: events_converted, + events, execution_result, execution_resources: ExecutionResources { steps: 0, From 4c0434293ec2e9d5fcb3a2f4f286438965237f43 Mon Sep 17 00:00:00 2001 From: jb caron Date: Sat, 27 Jan 2024 16:07:41 +0100 Subject: [PATCH 3/6] fix: :bug: add events in block storage Added the OrderedEvents data structure to store an event related to a tx index of the block. Added events in the get_transaction_receipt method. #31 --- crates/client/deoxys/src/convert.rs | 10 +++++- crates/client/rpc/src/lib.rs | 8 ++++- crates/pallets/starknet/src/lib.rs | 3 ++ crates/primitives/block/src/lib.rs | 20 ++++++++--- crates/primitives/block/src/ordered_events.rs | 33 +++++++++++++++++++ 5 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 crates/primitives/block/src/ordered_events.rs diff --git a/crates/client/deoxys/src/convert.rs b/crates/client/deoxys/src/convert.rs index b5086e6492..91994d833b 100644 --- a/crates/client/deoxys/src/convert.rs +++ b/crates/client/deoxys/src/convert.rs @@ -30,7 +30,15 @@ pub fn block(block: &p::Block) -> mp_block::Block { extra_data: block.block_hash.map(|h| sp_core::U256::from_big_endian(&h.to_bytes_be())), }; - mp_block::Block::new(header, transactions) + let ordered_events: Vec = block + .transaction_receipts + .iter() + .enumerate() + .filter(|(_, r)| r.events.len() > 0) + .map(|(i, r)| mp_block::OrderedEvents::new(i as u128, r.events.iter().map(event).collect())) + .collect(); + + mp_block::Block::new(header, transactions, ordered_events) } fn transactions(txs: &[p::TransactionType]) -> Vec { diff --git a/crates/client/rpc/src/lib.rs b/crates/client/rpc/src/lib.rs index 60deff0a12..1342a62bce 100644 --- a/crates/client/rpc/src/lib.rs +++ b/crates/client/rpc/src/lib.rs @@ -1436,7 +1436,13 @@ where let transaction = starknet_block.transactions().get(tx_index).unwrap(); - let events = vec![]; + let events = starknet_block + .events() + .into_iter() + .filter(|event| event.index == tx_index as u128) + .flat_map(|event| event.events.clone()) + .map(event_conversion) + .collect(); let execution_result = { let revert_error = self diff --git a/crates/pallets/starknet/src/lib.rs b/crates/pallets/starknet/src/lib.rs index ed2bfa796f..702be3fd95 100644 --- a/crates/pallets/starknet/src/lib.rs +++ b/crates/pallets/starknet/src/lib.rs @@ -1030,6 +1030,8 @@ impl Pallet { let extra_data = None; let l1_gas_price = T::L1GasPrice::get(); + let ordered_events = vec![]; + let block = StarknetBlock::new( StarknetHeader::new( parent_block_hash.into(), @@ -1046,6 +1048,7 @@ impl Pallet { extra_data, ), transactions, + ordered_events, ); // Save the block number <> hash mapping. let blockhash = block.header().hash::(); diff --git a/crates/primitives/block/src/lib.rs b/crates/primitives/block/src/lib.rs index a3f8836d55..942efe7f32 100644 --- a/crates/primitives/block/src/lib.rs +++ b/crates/primitives/block/src/lib.rs @@ -4,20 +4,25 @@ #[doc(hidden)] pub extern crate alloc; +use alloc::vec::Vec; + mod header; +mod ordered_events; pub mod state_update; -use alloc::vec::Vec; - pub use header::*; use mp_felt::Felt252Wrapper; use mp_hashers::HasherT; use mp_transactions::compute_hash::ComputeTransactionHash; use mp_transactions::Transaction; +pub use ordered_events::*; /// Block Transactions pub type BlockTransactions = Vec; +/// Block Events +pub type BlockEvents = Vec; + /// Block tag. /// /// A tag specifying a dynamic reference to a block. @@ -50,6 +55,8 @@ pub struct Block { header: Header, /// The block transactions. transactions: BlockTransactions, + /// The block events. + events: BlockEvents, } impl Block { @@ -59,8 +66,8 @@ impl Block { /// /// * `header` - The block header. /// * `transactions` - The block transactions. - pub fn new(header: Header, transactions: BlockTransactions) -> Self { - Self { header, transactions } + pub fn new(header: Header, transactions: BlockTransactions, events: BlockEvents) -> Self { + Self { header, transactions, events } } /// Return a reference to the block header @@ -73,6 +80,11 @@ impl Block { &self.transactions } + // Return a reference to all events + pub fn events(&self) -> &BlockEvents { + &self.events + } + /// Returns an iterator that iterates over all transaction hashes. /// /// Those transactions are computed using the given `chain_id`. diff --git a/crates/primitives/block/src/ordered_events.rs b/crates/primitives/block/src/ordered_events.rs new file mode 100644 index 0000000000..0c8f43b70c --- /dev/null +++ b/crates/primitives/block/src/ordered_events.rs @@ -0,0 +1,33 @@ +#![cfg_attr(not(feature = "std"), no_std)] +use starknet_api::transaction::Event; + +#[doc(hidden)] +pub extern crate alloc; + +use alloc::vec::Vec; + +#[derive(Clone, Debug, PartialEq, Eq, Default)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "parity-scale-codec", derive(parity_scale_codec::Encode, parity_scale_codec::Decode))] +/// Starknet OrderEvents definition +pub struct OrderedEvents { + /// the index of the transaction in the block + pub index: u128, + /// The events of the transaction + pub events: Vec, +} + +impl OrderedEvents { + /// Creates a new OrderedEvents + pub fn new(index: u128, events: Vec) -> Self { + Self { index, events } + } + + pub fn index(&self) -> u128 { + self.index + } + + pub fn events(&self) -> &Vec { + &self.events + } +} From d38a549322d4539ad83581b1790a9a4cc6de021f Mon Sep 17 00:00:00 2001 From: jb caron Date: Sat, 27 Jan 2024 16:45:53 +0100 Subject: [PATCH 4/6] refactor: :recycle: remove wait_for_tx_inclusion don't wait the tx inclusion in the block and just return TxnHashNotFound --- crates/client/rpc/src/lib.rs | 52 +++++++----------------------------- 1 file changed, 9 insertions(+), 43 deletions(-) diff --git a/crates/client/rpc/src/lib.rs b/crates/client/rpc/src/lib.rs index 1342a62bce..69ef1c4016 100644 --- a/crates/client/rpc/src/lib.rs +++ b/crates/client/rpc/src/lib.rs @@ -1350,49 +1350,15 @@ where &self, transaction_hash: FieldElement, ) -> RpcResult { - async fn wait_for_tx_inclusion( - madara_backend: Arc>, - transaction_hash: FieldElement, - ) -> Result<::Hash, StarknetRpcApiError> { - let substrate_block_hash; - - loop { - let substrate_block_hash_from_db = madara_backend - .mapping() - .block_hash_from_transaction_hash(H256::from(transaction_hash.to_bytes_be())) - .map_err(|e| { - error!("Failed to interact with db backend error: {e}"); - StarknetRpcApiError::InternalServerError - })?; - - match substrate_block_hash_from_db { - Some(block_hash) => { - substrate_block_hash = block_hash; - break; - } - None => { - // TODO: hardcoded to match the blocktime; make it dynamic - tokio::time::sleep(std::time::Duration::from_millis(6000)).await; - continue; - } - }; - } - - Ok(substrate_block_hash) - } - - let substrate_block_hash = match tokio::time::timeout( - std::time::Duration::from_millis(60000), - wait_for_tx_inclusion(self.backend.clone(), transaction_hash), - ) - .await - { - Err(_) => { - error!("did not receive tx hash within 1 minute"); - return Err(StarknetRpcApiError::TxnHashNotFound.into()); - } - Ok(res) => res, - }?; + let substrate_block_hash = self + .backend + .mapping() + .block_hash_from_transaction_hash(H256::from(transaction_hash.to_bytes_be())) + .map_err(|e| { + error!("Failed to interact with db backend error: {e}"); + StarknetRpcApiError::InternalServerError + })? + .ok_or(StarknetRpcApiError::TxnHashNotFound)?; let starknet_block: mp_block::Block = get_block_by_block_hash(self.client.as_ref(), substrate_block_hash).unwrap_or_default(); From 2477017b5d0eef23ae1a296b47f527f03366eac8 Mon Sep 17 00:00:00 2001 From: jb caron Date: Sun, 28 Jan 2024 17:56:31 +0100 Subject: [PATCH 5/6] style: :rotating_light: fmt formate comments --- crates/client/deoxys/src/state_updates.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/client/deoxys/src/state_updates.rs b/crates/client/deoxys/src/state_updates.rs index 3c728b3817..b760f56cab 100644 --- a/crates/client/deoxys/src/state_updates.rs +++ b/crates/client/deoxys/src/state_updates.rs @@ -285,10 +285,11 @@ impl Decode for StateUpdateWrapper { // // for contract in &state_update.state_diff.deployed_contracts { // // let contract_address = -// ContractAddress(PatriciaKey(contract.address.try_into().map_err(|_| // BuildCommitmentStateDiffError::ConversionError)? -// )); let class_hash = // ClassHash(contract.class_hash.try_into().map_err(|_| -// // BuildCommitmentStateDiffError::ConversionError)?); let compiled_class_hash: -// // CompiledClassHash = calculate_compiled_class_hash(&class_hash); +// ContractAddress(PatriciaKey(contract.address.try_into().map_err(|_| // +// BuildCommitmentStateDiffError::ConversionError)? )); let class_hash = // +// ClassHash(contract.class_hash.try_into().map_err(|_| // +// BuildCommitmentStateDiffError::ConversionError)? ); let compiled_class_hash: // +// CompiledClassHash = calculate_compiled_class_hash(&class_hash); // // commitment_state_diff.address_to_class_hash.insert(contract_address, class_hash); // // commitment_state_diff.class_hash_to_compiled_class_hash.insert(class_hash, From 11bca07a36412734bbc1be489855a9f53804e725 Mon Sep 17 00:00:00 2001 From: jb caron Date: Sun, 28 Jan 2024 18:00:06 +0100 Subject: [PATCH 6/6] docs: update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c68450e71..9907168f8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Next release +- feat: store events in block, return events in call get_transaction_receipt - feat: types in `mp-transactions` impl a method to get their version - feat: make L1 gas price a `const` of the `RuntimeConfig` - fix: broken class hashes and contracts in genesis