From 11b86e38bd1a15bb6c3c42b1f8300974de327761 Mon Sep 17 00:00:00 2001 From: OmriEshhar1 <45786542+OmriEshhar1@users.noreply.github.com> Date: Sun, 29 Oct 2023 16:41:37 +0200 Subject: [PATCH] feat: support pending block for get_block_w_full_transactions (#1304) --- .../V0_4_0/starknet_api_openrpc.json | 116 ++++++++++-------- crates/papyrus_rpc/src/v0_4_0/api/api_impl.rs | 63 +++++++--- crates/papyrus_rpc/src/v0_4_0/api/mod.rs | 2 +- crates/papyrus_rpc/src/v0_4_0/api/test.rs | 87 +++++++++++-- 4 files changed, 193 insertions(+), 75 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 c4f33fbfe8..fd5408fbfb 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 @@ -1409,67 +1409,83 @@ "PENDING_BLOCK_WITH_TX_HASHES": { "title": "Pending block with transaction hashes", "description": "The dynamic block being constructed by the sequencer. Note that this object will be deprecated upon decentralization.", - "allOf": [ - { - "title": "Block body with transactions hashes", - "$ref": "#/components/schemas/BLOCK_BODY_WITH_TX_HASHES" + "properties": { + "timestamp": { + "title": "Timestamp", + "description": "The time in which the block was created, encoded in Unix time", + "type": "integer", + "minimum": 0 }, - { - "title": "Pending block header", - "type": "object", - "properties": { - "timestamp": { - "title": "Timestamp", - "description": "The time in which the block was created, encoded in Unix time", - "type": "integer", - "minimum": 0 - }, - "sequencer_address": { - "title": "Sequencer address", - "description": "The StarkNet identity of the sequencer submitting this block", - "$ref": "#/components/schemas/FELT" - }, - "parent_hash": { - "title": "Parent hash", - "description": "The hash of this block's parent", - "$ref": "#/components/schemas/BLOCK_HASH" - } + "sequencer_address": { + "title": "Sequencer address", + "description": "The StarkNet identity of the sequencer submitting this block", + "$ref": "#/components/schemas/FELT" + }, + "parent_hash": { + "title": "Parent hash", + "description": "The hash of this block's parent", + "$ref": "#/components/schemas/BLOCK_HASH" + }, + "transactions": { + "title": "Transaction", + "description": "The hashes of the transactions included in this block", + "type": "array", + "items": { + "description": "The hash of a single transaction", + "$ref": "#/components/schemas/TXN_HASH" } } - ], + }, "additionalProperties": false }, "PENDING_BLOCK_WITH_TXS": { "title": "Pending block with transactions", "description": "The dynamic block being constructed by the sequencer. Note that this object will be deprecated upon decentralization.", - "allOf": [ - { - "title": "Block body with transactions", - "$ref": "#/components/schemas/BLOCK_BODY_WITH_TXS" + "properties": { + "timestamp": { + "title": "Timestamp", + "description": "The time in which the block was created, encoded in Unix time", + "type": "integer", + "minimum": 0 }, - { - "type": "object", - "title": "Block Info", - "properties": { - "timestamp": { - "title": "Timestamp", - "description": "The time in which the block was created, encoded in Unix time", - "type": "integer", - "minimum": 0 - }, - "sequencer_address": { - "title": "Sequencer address", - "description": "The StarkNet identity of the sequencer submitting this block", - "$ref": "#/components/schemas/FELT" - }, - "parent_hash": { - "title": "Parent hash", - "description": "The hash of this block's parent", - "$ref": "#/components/schemas/BLOCK_HASH" - } + "sequencer_address": { + "title": "Sequencer address", + "description": "The StarkNet identity of the sequencer submitting this block", + "$ref": "#/components/schemas/FELT" + }, + "parent_hash": { + "title": "Parent hash", + "description": "The hash of this block's parent", + "$ref": "#/components/schemas/BLOCK_HASH" + }, + "transactions": { + "title": "Transactions", + "description": "The transactions in this block", + "type": "array", + "items": { + "title": "transactions in block", + "type": "object", + "allOf": [ + { + "title": "transaction", + "$ref": "#/components/schemas/TXN" + }, + { + "type": "object", + "properties": { + "transaction_hash": { + "title": "transaction hash", + "$ref": "#/components/schemas/TXN_HASH" + } + }, + "required": [ + "transaction_hash" + ] + } + ] } } - ], + }, "additionalProperties": false }, "DEPLOYED_CONTRACT_ITEM": { 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 3015c0a55c..2cdc8e19cc 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 @@ -157,7 +157,7 @@ impl JsonRpcV0_4Server for JsonRpcServerV0_4Impl { #[instrument(skip(self), level = "debug", err, ret)] async fn get_block_w_transaction_hashes(&self, block_id: BlockId) -> RpcResult { - if block_id == BlockId::Tag(Tag::Pending) { + if let BlockId::Tag(Tag::Pending) = block_id { let block = self.pending_data.read().await.block.clone(); let pending_block_header = PendingBlockHeader { parent_hash: block.parent_block_hash, @@ -170,29 +170,58 @@ impl JsonRpcV0_4Server for JsonRpcServerV0_4Impl { .iter() .map(|transaction| transaction.transaction_hash()) .collect(); - Ok(Block { + return Ok(Block { status: None, header, transactions: Transactions::Hashes(transaction_hashes), - }) - } else { - let txn = self.storage_reader.begin_ro_txn().map_err(internal_server_error)?; - let block_number = get_block_number(&txn, block_id)?; - let status = get_block_status(&txn, block_number)?; - let header = - GeneralBlockHeader::BlockHeader(get_block_header_by_number(&txn, block_number)?); - let transaction_hashes = get_block_tx_hashes_by_number(&txn, block_number)?; - - Ok(Block { - status: Some(status), - header, - transactions: Transactions::Hashes(transaction_hashes), - }) + }); } + + let txn = self.storage_reader.begin_ro_txn().map_err(internal_server_error)?; + let block_number = get_block_number(&txn, block_id)?; + let status = get_block_status(&txn, block_number)?; + let header = + GeneralBlockHeader::BlockHeader(get_block_header_by_number(&txn, block_number)?); + let transaction_hashes = get_block_tx_hashes_by_number(&txn, block_number)?; + + Ok(Block { + status: Some(status), + header, + transactions: Transactions::Hashes(transaction_hashes), + }) } #[instrument(skip(self), level = "debug", err, ret)] - fn get_block_w_full_transactions(&self, block_id: BlockId) -> RpcResult { + async fn get_block_w_full_transactions(&self, block_id: BlockId) -> RpcResult { + if let BlockId::Tag(Tag::Pending) = block_id { + let block = self.pending_data.read().await.block.clone(); + let pending_block_header = PendingBlockHeader { + parent_hash: block.parent_block_hash, + sequencer_address: block.sequencer_address, + timestamp: block.timestamp, + }; + let header = GeneralBlockHeader::PendingBlockHeader(pending_block_header); + let client_transactions = block.transactions; + let transactions = client_transactions + .iter() + .map(|client_transaction| { + let starknet_api_transaction: StarknetApiTransaction = + client_transaction.clone().try_into().map_err(internal_server_error)?; + Ok(TransactionWithHash { + transaction: starknet_api_transaction + .try_into() + .map_err(internal_server_error)?, + transaction_hash: client_transaction.transaction_hash(), + }) + }) + .collect::, ErrorObjectOwned>>()?; + return Ok(Block { + status: None, + header, + transactions: Transactions::Full(transactions), + }); + } + let txn = self.storage_reader.begin_ro_txn().map_err(internal_server_error)?; let block_number = get_block_number(&txn, block_id)?; let status = get_block_status(&txn, block_number)?; 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 4b9a38c7d8..e46e30506b 100644 --- a/crates/papyrus_rpc/src/v0_4_0/api/mod.rs +++ b/crates/papyrus_rpc/src/v0_4_0/api/mod.rs @@ -78,7 +78,7 @@ pub trait JsonRpc { /// Gets block information with full transactions given a block identifier. #[method(name = "getBlockWithTxs")] - fn get_block_w_full_transactions(&self, block_id: BlockId) -> RpcResult; + async fn get_block_w_full_transactions(&self, block_id: BlockId) -> RpcResult; /// Gets the value of the storage at the given address, key, and block. #[method(name = "getStorageAt")] 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 7483cd57d6..071a033262 100644 --- a/crates/papyrus_rpc/src/v0_4_0/api/test.rs +++ b/crates/papyrus_rpc/src/v0_4_0/api/test.rs @@ -25,7 +25,7 @@ use rand::{random, RngCore}; use rand_chacha::ChaCha8Rng; use reqwest::StatusCode; use serde::{Deserialize, Serialize}; -use starknet_api::block::{BlockHash, BlockHeader, BlockNumber, BlockStatus}; +use starknet_api::block::{BlockHash, BlockHeader, BlockNumber, BlockStatus, BlockTimestamp}; use starknet_api::core::{ClassHash, ContractAddress, Nonce, PatriciaKey}; use starknet_api::deprecated_contract_class::{ ContractClassAbiEntry, @@ -69,6 +69,7 @@ use starknet_client::writer::objects::transaction::{ use starknet_client::writer::{MockStarknetWriter, WriterClientError, WriterClientResult}; use starknet_client::ClientError; use test_utils::{ + auto_impl_get_test_instance, get_rng, get_test_block, get_test_body, @@ -78,7 +79,7 @@ use test_utils::{ }; use super::super::api::EventsChunk; -use super::super::block::{Block, GeneralBlockHeader}; +use super::super::block::{Block, GeneralBlockHeader, PendingBlockHeader}; use super::super::broadcasted_transaction::BroadcastedDeclareTransaction; use super::super::deprecated_contract_class::ContractClass as DeprecatedContractClass; use super::super::error::{ @@ -359,8 +360,10 @@ async fn get_block_transaction_count() { async fn get_block_w_full_transactions() { // TODO(omri): Add test for pending block. let method_name = "starknet_V0_4_getBlockWithTxs"; - 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 @@ -449,13 +452,44 @@ async fn get_block_w_full_transactions() { .await .unwrap_err(); assert_matches!(err, Error::Call(err) if err == BLOCK_NOT_FOUND.into()); + + // Get pending block. + let transaction_count = 3; + let mut rng = get_rng(); + let (client_transaction, rpc_transaction) = + generate_client_transaction_and_rpc_transaction(&mut rng); + let expected_pending_block = Block { + header: GeneralBlockHeader::PendingBlockHeader(PendingBlockHeader::get_test_instance( + &mut rng, + )), + status: None, + transactions: Transactions::Full(vec![rpc_transaction; transaction_count]), + }; + pending_data + .write() + .await + .block + .transactions + .extend(iter::repeat(client_transaction).take(transaction_count)); + // Using call_api_then_assert_and_validate_schema_for_result in order to validate the schema for + // pending block. + call_api_then_assert_and_validate_schema_for_result::<_, BlockId, Block>( + &module, + method_name, + &Some(BlockId::Tag(Tag::Pending)), + &VERSION_0_4, + &expected_pending_block, + ) + .await; } #[tokio::test] async fn get_block_w_transaction_hashes() { let method_name = "starknet_V0_4_getBlockWithTxHashes"; - 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 @@ -540,6 +574,37 @@ async fn get_block_w_transaction_hashes() { .await .unwrap_err(); assert_matches!(err, Error::Call(err) if err == BLOCK_NOT_FOUND.into()); + + // Get pending block. + let transaction_count = 3; + let mut rng = get_rng(); + let (client_transaction, _) = generate_client_transaction_and_rpc_transaction(&mut rng); + let expected_pending_block = Block { + header: GeneralBlockHeader::PendingBlockHeader(PendingBlockHeader::get_test_instance( + &mut rng, + )), + status: None, + transactions: Transactions::Hashes(vec![ + client_transaction.transaction_hash(); + transaction_count + ]), + }; + pending_data + .write() + .await + .block + .transactions + .extend(iter::repeat(client_transaction).take(transaction_count)); + // Using call_api_then_assert_and_validate_schema_for_result in order to validate the schema for + // pending block. + call_api_then_assert_and_validate_schema_for_result::<_, BlockId, Block>( + &module, + method_name, + &Some(BlockId::Tag(Tag::Pending)), + &VERSION_0_4, + &expected_pending_block, + ) + .await; } #[tokio::test] @@ -750,7 +815,7 @@ async fn get_transaction_receipt() { assert_eq!(res.finality_status, TransactionFinalityStatus::AcceptedOnL1); assert_eq!(res.output.execution_status(), &TransactionExecutionStatus::Succeeded); - // Add a pneding transaction and ask for its receipt. + // Add a pending transaction and ask for its receipt. let mut rng = get_rng(); let (client_transaction, client_transaction_receipt, expected_receipt) = generate_client_transaction_client_receipt_and_rpc_receipt(&mut rng); @@ -2538,3 +2603,11 @@ fn spec_api_methods_coverage() { .collect::>(); assert!(method_names_in_spec.eq(&implemented_method_names)); } + +auto_impl_get_test_instance! { + pub struct PendingBlockHeader { + pub parent_hash: BlockHash, + pub sequencer_address: ContractAddress, + pub timestamp: BlockTimestamp, + } +}