From 5f03912b5eac9a8036579f0a185399534004f79d Mon Sep 17 00:00:00 2001 From: Shahak Shama Date: Tue, 17 Oct 2023 18:03:18 +0300 Subject: [PATCH] feat(JSON-RPC): get_transaction_receipt supports pending --- .../V0_4_0/starknet_api_openrpc.json | 47 +++-------- crates/papyrus_rpc/src/v0_4_0/api/api_impl.rs | 48 +++++++++-- crates/papyrus_rpc/src/v0_4_0/api/mod.rs | 6 +- crates/papyrus_rpc/src/v0_4_0/api/test.rs | 79 ++++++++++++++++--- crates/papyrus_rpc/src/v0_4_0/transaction.rs | 29 +++++++ .../src/reader/objects/test_utils.rs | 49 ++++++++++++ .../src/reader/objects/transaction.rs | 10 +++ crates/test_utils/src/lib.rs | 4 + 8 files changed, 210 insertions(+), 62 deletions(-) diff --git a/crates/papyrus_rpc/resources/V0_4_0/starknet_api_openrpc.json b/crates/papyrus_rpc/resources/V0_4_0/starknet_api_openrpc.json index b9b5b27384..62d24e8433 100644 --- a/crates/papyrus_rpc/resources/V0_4_0/starknet_api_openrpc.json +++ b/crates/papyrus_rpc/resources/V0_4_0/starknet_api_openrpc.json @@ -2341,9 +2341,8 @@ } ] }, - "PENDING_COMMON_RECEIPT_PROPERTIES": { - "title": "Pending common receipt properties", - "description": "Common properties for a pending transaction receipt", + "PENDING_TXN_RECEIPT": { + "title": "Pending Transaction Receipt", "type": "object", "properties": { "transaction_hash": { @@ -2392,6 +2391,11 @@ "execution_status": { "title": "Execution status", "$ref": "#/components/schemas/TXN_EXECUTION_STATUS" + }, + "contract_address": { + "title": "Contract address", + "description": "The address of the deployed contract", + "$ref": "#/components/schemas/FELT" } }, "required": [ @@ -2402,41 +2406,8 @@ "events", "finality_status", "execution_status" - ] - }, - "PENDING_DEPLOY_TXN_RECEIPT": { - "title": "Pending deploy Transaction Receipt", - "allOf": [ - { - "title": "Common receipt properties", - "$ref": "#/components/schemas/PENDING_COMMON_RECEIPT_PROPERTIES" - }, - { - "type": "object", - "title": "Contract address", - "properties": { - "contract_address": { - "title": "Contract address", - "description": "The address of the deployed contract", - "$ref": "#/components/schemas/FELT" - } - } - } - ] - }, - "PENDING_TXN_RECEIPT": { - "title": "Pending Transaction Receipt", - "oneOf": [ - { - "title": "Pending deploy transaction receipt", - "$ref": "#/components/schemas/PENDING_DEPLOY_TXN_RECEIPT" - }, - { - "title": "Pending common receipt properties", - "$comment": "Used for pending invoke and declare transaction receipts", - "$ref": "#/components/schemas/PENDING_COMMON_RECEIPT_PROPERTIES" - } - ] + ], + "additionalProperties": false }, "MSG_TO_L1": { "title": "Message to L1", diff --git a/crates/papyrus_rpc/src/v0_4_0/api/api_impl.rs b/crates/papyrus_rpc/src/v0_4_0/api/api_impl.rs index 3eb76a5379..9121b251e4 100644 --- a/crates/papyrus_rpc/src/v0_4_0/api/api_impl.rs +++ b/crates/papyrus_rpc/src/v0_4_0/api/api_impl.rs @@ -51,6 +51,9 @@ use super::super::broadcasted_transaction::{ use super::super::state::{AcceptedStateUpdate, PendingStateUpdate, StateUpdate}; use super::super::transaction::{ Event, + GeneralTransactionReceipt, + PendingTransactionFinalityStatus, + PendingTransactionReceipt, Transaction, TransactionOutput, TransactionReceipt, @@ -359,16 +362,45 @@ impl JsonRpcV0_4Server for JsonRpcServerV0_4Impl { } #[instrument(skip(self), level = "debug", err, ret)] - fn get_transaction_receipt( + async fn get_transaction_receipt( &self, transaction_hash: TransactionHash, - ) -> RpcResult { + ) -> RpcResult { let txn = self.storage_reader.begin_ro_txn().map_err(internal_server_error)?; - let transaction_index = txn - .get_transaction_idx_by_hash(&transaction_hash) - .map_err(internal_server_error)? - .ok_or_else(|| ErrorObjectOwned::from(TRANSACTION_HASH_NOT_FOUND))?; + let Some(transaction_index) = + txn.get_transaction_idx_by_hash(&transaction_hash).map_err(internal_server_error)? + else { + // The transaction is not in any non-pending block. Search for it in the pending block + // and if it's not found, return error. + + // TODO(shahak): Consider cloning the transactions and the receipts in order to free + // the lock sooner (Check which is better). + let pending_block = &self.pending_data.read().await.block; + + let client_transaction_receipt = pending_block + .transaction_receipts + .iter() + .find(|receipt| receipt.transaction_hash == transaction_hash) + .ok_or_else(|| ErrorObjectOwned::from(TRANSACTION_HASH_NOT_FOUND))? + .clone(); + let client_transaction = &pending_block + .transactions + .iter() + .find(|transaction| transaction.transaction_hash() == transaction_hash) + .ok_or_else(|| ErrorObjectOwned::from(TRANSACTION_HASH_NOT_FOUND))?; + let starknet_api_output = + client_transaction_receipt.into_starknet_api_transaction_output(client_transaction); + let output = Into::::into(starknet_api_output); + return Ok(GeneralTransactionReceipt::PendingTransactionReceipt( + PendingTransactionReceipt { + // ACCEPTED_ON_L2 is the only finality status of a pending transaction. + finality_status: PendingTransactionFinalityStatus::AcceptedOnL2, + transaction_hash, + output, + }, + )); + }; let block_number = transaction_index.0; let status = get_block_status(&txn, block_number)?; @@ -396,13 +428,13 @@ impl JsonRpcV0_4Server for JsonRpcServerV0_4Impl { let output = TransactionOutput::from_thin_transaction_output(thin_tx_output, events); - Ok(TransactionReceipt { + Ok(GeneralTransactionReceipt::TransactionReceipt(TransactionReceipt { finality_status: status.into(), transaction_hash, block_hash, block_number, output, - }) + })) } #[instrument(skip(self), level = "debug", err, ret)] diff --git a/crates/papyrus_rpc/src/v0_4_0/api/mod.rs b/crates/papyrus_rpc/src/v0_4_0/api/mod.rs index a1d4230d82..ea91b55dd0 100644 --- a/crates/papyrus_rpc/src/v0_4_0/api/mod.rs +++ b/crates/papyrus_rpc/src/v0_4_0/api/mod.rs @@ -40,10 +40,10 @@ use super::transaction::{ DeployAccountTransaction, DeployAccountTransactionV1, Event, + GeneralTransactionReceipt, InvokeTransaction, InvokeTransactionV0, InvokeTransactionV1, - TransactionReceipt, TransactionWithHash, }; use super::write_api_result::{AddDeclareOkResult, AddDeployAccountOkResult, AddInvokeOkResult}; @@ -109,10 +109,10 @@ pub trait JsonRpc { /// Gets the transaction receipt by the transaction hash. #[method(name = "getTransactionReceipt")] - fn get_transaction_receipt( + async fn get_transaction_receipt( &self, transaction_hash: TransactionHash, - ) -> RpcResult; + ) -> RpcResult; /// Gets the contract class definition associated with the given hash. #[method(name = "getClass")] diff --git a/crates/papyrus_rpc/src/v0_4_0/api/test.rs b/crates/papyrus_rpc/src/v0_4_0/api/test.rs index fb9ecde406..16403e9a23 100644 --- a/crates/papyrus_rpc/src/v0_4_0/api/test.rs +++ b/crates/papyrus_rpc/src/v0_4_0/api/test.rs @@ -21,7 +21,8 @@ use papyrus_storage::state::StateStorageWriter; use papyrus_storage::test_utils::get_test_storage; use papyrus_storage::StorageScope; use pretty_assertions::assert_eq; -use rand::random; +use rand::{random, RngCore}; +use rand_chacha::ChaCha8Rng; use reqwest::StatusCode; use serde::{Deserialize, Serialize}; use starknet_api::block::{BlockHash, BlockHeader, BlockNumber, BlockStatus}; @@ -50,7 +51,10 @@ use starknet_client::reader::objects::state::{ StateDiff as ClientStateDiff, StorageEntry as ClientStorageEntry, }; -use starknet_client::reader::objects::transaction::Transaction as ClientTransaction; +use starknet_client::reader::objects::transaction::{ + Transaction as ClientTransaction, + TransactionReceipt as ClientTransactionReceipt, +}; use starknet_client::starknet_error::{KnownStarknetErrorCode, StarknetError, StarknetErrorCode}; use starknet_client::writer::objects::response::{ DeclareResponse, @@ -93,7 +97,10 @@ use super::super::state::{ use super::super::transaction::{ DeployAccountTransaction, Event, + GeneralTransactionReceipt, InvokeTransactionV1, + PendingTransactionFinalityStatus, + PendingTransactionReceipt, TransactionFinalityStatus, TransactionOutput, TransactionReceipt, @@ -673,8 +680,10 @@ async fn get_class() { #[tokio::test] async fn get_transaction_receipt() { let method_name = "starknet_V0_4_getTransactionReceipt"; - let (module, mut storage_writer) = - get_test_rpc_server_and_storage_writer::(); + let pending_data = get_test_pending_data(); + let (module, mut storage_writer) = get_test_rpc_server_and_storage_writer_from_params::< + JsonRpcServerV0_4Impl, + >(None, None, Some(pending_data.clone()), None, None); let block = get_test_block(1, None, None, None); storage_writer .begin_rw_txn() @@ -731,6 +740,53 @@ async fn get_transaction_receipt() { assert_eq!(res.finality_status, TransactionFinalityStatus::AcceptedOnL1); assert_eq!(res.output.execution_status(), &TransactionExecutionStatus::Succeeded); + // Ask for a pending transaction. + let mut rng = get_rng(); + let pending_transaction_hash = TransactionHash(StarkHash::from(rng.next_u64())); + let (mut client_transaction, _) = generate_client_transaction_and_rpc_transaction(&mut rng); + let mut client_transaction_receipt = ClientTransactionReceipt::get_test_instance(&mut rng); + *client_transaction.transaction_hash_mut() = pending_transaction_hash; + client_transaction_receipt.transaction_hash = pending_transaction_hash; + + { + let pending_block = &mut pending_data.write().await.block; + pending_block.transactions.push(client_transaction.clone()); + pending_block.transaction_receipts.push(client_transaction_receipt.clone()); + } + + let starknet_api_output = client_transaction_receipt + .clone() + .into_starknet_api_transaction_output(&client_transaction); + let output = Into::::into(starknet_api_output); + let expected_result = + GeneralTransactionReceipt::PendingTransactionReceipt(PendingTransactionReceipt { + finality_status: PendingTransactionFinalityStatus::AcceptedOnL2, + transaction_hash: pending_transaction_hash, + output, + }); + let (json_response, result) = raw_call::<_, TransactionHash, PendingTransactionReceipt>( + &module, + method_name, + &Some(pending_transaction_hash), + ) + .await; + // See above for explanation why we compare the json strings. + assert_eq!( + serde_json::to_string(&result.clone().unwrap()).unwrap(), + serde_json::to_string(&expected_result).unwrap(), + ); + // Validating schema again since pending has a different schema + assert!(validate_schema( + &get_starknet_spec_api_schema_for_method_results( + &[( + SpecFile::StarknetApiOpenrpc, + &[method_name_to_spec_method_name(method_name).as_str()] + )], + &VERSION_0_4, + ), + &json_response["result"], + )); + // Ask for an invalid transaction. call_api_then_assert_and_validate_schema_for_err::<_, TransactionHash, TransactionReceipt>( &module, @@ -1190,12 +1246,13 @@ async fn get_storage_at() { assert_matches!(err, Error::Call(err) if err == BLOCK_NOT_FOUND.into()); } -fn generate_client_transaction_and_rpc_transaction() -> (ClientTransaction, TransactionWithHash) { - let mut rng = get_rng(); +fn generate_client_transaction_and_rpc_transaction( + rng: &mut ChaCha8Rng, +) -> (ClientTransaction, TransactionWithHash) { // TODO(shahak): Remove retry once v3 transactions are supported and the impl of TryInto will // become impl of Into. loop { - let client_transaction = ClientTransaction::get_test_instance(&mut rng); + let client_transaction = ClientTransaction::get_test_instance(rng); let Ok(starknet_api_transaction): Result = client_transaction.clone().try_into() else { @@ -1246,16 +1303,12 @@ async fn get_transaction_by_hash() { // Ask for a transaction in the pending block. let (client_transaction, expected_transaction_with_hash) = - generate_client_transaction_and_rpc_transaction(); + generate_client_transaction_and_rpc_transaction(&mut get_rng()); pending_data.write().await.block.transactions.push(client_transaction.clone()); let res = module .call::<_, TransactionWithHash>(method_name, (client_transaction.transaction_hash(), 0)) .await .unwrap(); - println!( - "{:?}, {:?}", - expected_transaction_with_hash.transaction_hash, expected_transaction.transaction_hash - ); assert_eq!(res, expected_transaction_with_hash); // Ask for an invalid transaction. @@ -1338,7 +1391,7 @@ async fn get_transaction_by_block_id_and_index() { // Get transaction of pending block. let (client_transaction, expected_transaction_with_hash) = - generate_client_transaction_and_rpc_transaction(); + generate_client_transaction_and_rpc_transaction(&mut get_rng()); pending_data.write().await.block.transactions.push(client_transaction); let res = module .call::<_, TransactionWithHash>(method_name, (BlockId::Tag(Tag::Pending), 0)) diff --git a/crates/papyrus_rpc/src/v0_4_0/transaction.rs b/crates/papyrus_rpc/src/v0_4_0/transaction.rs index 90bb2e69ba..8c11445ea4 100644 --- a/crates/papyrus_rpc/src/v0_4_0/transaction.rs +++ b/crates/papyrus_rpc/src/v0_4_0/transaction.rs @@ -367,6 +367,16 @@ pub enum TransactionFinalityStatus { AcceptedOnL1, } +/// Transaction Finality status on starknet for transactions in the pending block. +#[derive( + Debug, Copy, Clone, Eq, PartialEq, Hash, Deserialize, Serialize, PartialOrd, Ord, Default, +)] +pub enum PendingTransactionFinalityStatus { + #[serde(rename = "ACCEPTED_ON_L2")] + #[default] + AcceptedOnL2, +} + impl From for TransactionFinalityStatus { fn from(status: BlockStatus) -> Self { match status { @@ -381,6 +391,13 @@ impl From for TransactionFinalityStatus { } } +#[derive(Debug, Clone, Eq, PartialEq, Hash, Deserialize, Serialize, PartialOrd, Ord)] +#[serde(untagged)] +pub enum GeneralTransactionReceipt { + TransactionReceipt(TransactionReceipt), + PendingTransactionReceipt(PendingTransactionReceipt), +} + #[derive(Debug, Clone, Eq, PartialEq, Hash, Deserialize, Serialize, PartialOrd, Ord)] pub struct TransactionReceipt { pub finality_status: TransactionFinalityStatus, @@ -391,8 +408,20 @@ pub struct TransactionReceipt { pub output: TransactionOutput, } +#[derive(Debug, Clone, Eq, PartialEq, Hash, Deserialize, Serialize, PartialOrd, Ord)] +pub struct PendingTransactionReceipt { + pub finality_status: PendingTransactionFinalityStatus, + pub transaction_hash: TransactionHash, + #[serde(flatten)] + pub output: TransactionOutput, +} + #[derive(Debug, Clone, Eq, PartialEq, Hash, Deserialize, Serialize, PartialOrd, Ord)] #[serde(tag = "type")] +// Applying deny_unknown_fields on the inner type instead of on PendingTransactionReceipt because +// of a bug that makes deny_unknown_fields not work well with flatten: +// https://github.com/serde-rs/serde/issues/1358 +#[serde(deny_unknown_fields)] pub enum TransactionOutput { #[serde(rename = "DECLARE")] Declare(DeclareTransactionOutput), diff --git a/crates/starknet_client/src/reader/objects/test_utils.rs b/crates/starknet_client/src/reader/objects/test_utils.rs index 56dc066d04..16b9a1ac90 100644 --- a/crates/starknet_client/src/reader/objects/test_utils.rs +++ b/crates/starknet_client/src/reader/objects/test_utils.rs @@ -1,32 +1,48 @@ +use std::collections::HashMap; + use starknet_api::core::{ ClassHash, CompiledClassHash, ContractAddress, EntryPointSelector, + EthAddress, Nonce, }; use starknet_api::data_availability::DataAvailabilityMode; +use starknet_api::hash::StarkHash; use starknet_api::transaction::{ AccountDeploymentData, Calldata, ContractAddressSalt, + Event, Fee, + L1ToL2Payload, + L2ToL1Payload, PaymasterData, ResourceBoundsMapping, Tip, + TransactionExecutionStatus, TransactionHash, + TransactionOffsetInBlock, TransactionSignature, TransactionVersion, }; use test_utils::{auto_impl_get_test_instance, get_number_of_variants, GetTestInstance}; use crate::reader::objects::transaction::{ + BuiltinInstanceCounter, DeployTransaction, + EmptyBuiltinInstanceCounter, + ExecutionResources, IntermediateDeclareTransaction, IntermediateDeployAccountTransaction, IntermediateInvokeTransaction, L1HandlerTransaction, + L1ToL2Message, + L1ToL2Nonce, + L2ToL1Message, Transaction, + TransactionReceipt, }; auto_impl_get_test_instance! { @@ -101,4 +117,37 @@ auto_impl_get_test_instance! { pub entry_point_selector: EntryPointSelector, pub calldata: Calldata, } + pub struct TransactionReceipt { + pub transaction_index: TransactionOffsetInBlock, + pub transaction_hash: TransactionHash, + pub l1_to_l2_consumed_message: L1ToL2Message, + pub l2_to_l1_messages: Vec, + pub events: Vec, + pub execution_resources: ExecutionResources, + pub actual_fee: Fee, + pub execution_status: TransactionExecutionStatus, + } + pub struct L1ToL2Message { + pub from_address: EthAddress, + pub to_address: ContractAddress, + pub selector: EntryPointSelector, + pub payload: L1ToL2Payload, + pub nonce: L1ToL2Nonce, + } + pub struct L1ToL2Nonce(pub StarkHash); + pub struct ExecutionResources { + pub n_steps: u64, + pub builtin_instance_counter: BuiltinInstanceCounter, + pub n_memory_holes: u64, + } + pub enum BuiltinInstanceCounter { + NonEmpty(HashMap) = 0, + Empty(EmptyBuiltinInstanceCounter) = 1, + } + pub struct EmptyBuiltinInstanceCounter {} + pub struct L2ToL1Message { + pub from_address: ContractAddress, + pub to_address: EthAddress, + pub payload: L2ToL1Payload, + } } diff --git a/crates/starknet_client/src/reader/objects/transaction.rs b/crates/starknet_client/src/reader/objects/transaction.rs index e35022ef7a..e54c29ec74 100644 --- a/crates/starknet_client/src/reader/objects/transaction.rs +++ b/crates/starknet_client/src/reader/objects/transaction.rs @@ -92,6 +92,16 @@ impl Transaction { } } + pub fn transaction_hash_mut(&mut self) -> &mut TransactionHash { + match self { + Transaction::Declare(tx) => &mut tx.transaction_hash, + Transaction::Deploy(tx) => &mut tx.transaction_hash, + Transaction::DeployAccount(tx) => &mut tx.transaction_hash, + Transaction::Invoke(tx) => &mut tx.transaction_hash, + Transaction::L1Handler(tx) => &mut tx.transaction_hash, + } + } + pub fn transaction_type(&self) -> TransactionType { match self { Transaction::Declare(_) => TransactionType::Declare, diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs index 90c9908a4a..dccb10e047 100644 --- a/crates/test_utils/src/lib.rs +++ b/crates/test_utils/src/lib.rs @@ -503,6 +503,10 @@ auto_impl_get_test_instance! { pub function_idx: FunctionIndex, pub selector: EntryPointSelector, } + pub struct Event { + pub from_address: ContractAddress, + pub content: EventContent, + } pub struct FunctionIndex(pub usize); pub struct EntryPointOffset(pub usize); pub struct EntryPointSelector(pub StarkHash);