From 02bdfe4ce0eb763f08b18aa917851963750d9bbf Mon Sep 17 00:00:00 2001 From: Oleksandr Pravdyvyi Date: Wed, 22 Jan 2025 12:26:19 +0200 Subject: [PATCH] fix: refactor of sequencer to return errors immediately --- Cargo.lock | 1 + node_core/Cargo.toml | 3 ++ node_core/src/sequencer_client/json.rs | 8 +++++ node_core/src/sequencer_client/mod.rs | 21 +++++++++++-- node_rpc/src/process.rs | 3 +- node_rpc/src/types/err_rpc.rs | 4 +++ sequencer_core/src/lib.rs | 43 ++++++++++++++++++++------ sequencer_rpc/src/process.rs | 20 ++++++------ sequencer_rpc/src/types/err_rpc.rs | 10 ++++++ 9 files changed, 88 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 90a7994..00ae759 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2683,6 +2683,7 @@ dependencies = [ "rand 0.8.5", "reqwest 0.11.27", "risc0-zkvm", + "rpc_primitives", "secp256k1-zkp", "serde", "serde_json", diff --git a/node_core/Cargo.toml b/node_core/Cargo.toml index 61fcc28..fd6c25b 100644 --- a/node_core/Cargo.toml +++ b/node_core/Cargo.toml @@ -35,6 +35,9 @@ path = "../utxo" [dependencies.zkvm] path = "../zkvm" +[dependencies.rpc_primitives] +path = "../rpc_primitives" + [dependencies.secp256k1-zkp] workspace = true features = ["std", "rand-std", "rand", "serde", "global-context"] diff --git a/node_core/src/sequencer_client/json.rs b/node_core/src/sequencer_client/json.rs index 39105a8..e34e39d 100644 --- a/node_core/src/sequencer_client/json.rs +++ b/node_core/src/sequencer_client/json.rs @@ -1,3 +1,4 @@ +use rpc_primitives::errors::RpcError; use serde::{Deserialize, Serialize}; use storage::{block::Block, transaction::Transaction}; @@ -74,3 +75,10 @@ pub struct SequencerRpcResponse { pub result: serde_json::Value, pub id: u64, } + +#[derive(Debug, Clone, Deserialize)] +pub struct SequencerRpcError { + pub jsonrpc: String, + pub error: RpcError, + pub id: u64, +} diff --git a/node_core/src/sequencer_client/mod.rs b/node_core/src/sequencer_client/mod.rs index 50bab57..2ad1c53 100644 --- a/node_core/src/sequencer_client/mod.rs +++ b/node_core/src/sequencer_client/mod.rs @@ -3,7 +3,7 @@ use anyhow::Result; use json::{ GetBlockDataRequest, GetBlockDataResponse, GetGenesisIdRequest, GetGenesisIdResponse, RegisterAccountRequest, RegisterAccountResponse, SendTxRequest, SendTxResponse, - SequencerRpcRequest, SequencerRpcResponse, + SequencerRpcError, SequencerRpcRequest, SequencerRpcResponse, }; use k256::elliptic_curve::group::GroupEncoding; use reqwest::Client; @@ -26,6 +26,8 @@ pub enum SequencerClientError { HTTPError(reqwest::Error), #[error("Serde error")] SerdeError(serde_json::Error), + #[error("Internal error")] + InternalError(SequencerRpcError), } impl From for SequencerClientError { @@ -40,6 +42,12 @@ impl From for SequencerClientError { } } +impl From for SequencerClientError { + fn from(value: SequencerRpcError) -> Self { + SequencerClientError::InternalError(value) + } +} + impl SequencerClient { pub fn new(config: NodeConfig) -> Result { Ok(Self { @@ -62,9 +70,16 @@ impl SequencerClient { let call_res = call_builder.json(&request).send().await?; - let response = call_res.json::().await?; + let response_vall = call_res.json::().await?; - Ok(response.result) + if let Ok(response) = serde_json::from_value::(response_vall.clone()) + { + Ok(response.result) + } else { + let err_resp = serde_json::from_value::(response_vall)?; + + Err(err_resp.into()) + } } pub async fn get_block( diff --git a/node_rpc/src/process.rs b/node_rpc/src/process.rs index 1867ae8..ef0a99e 100644 --- a/node_rpc/src/process.rs +++ b/node_rpc/src/process.rs @@ -43,8 +43,7 @@ impl JsonHandler { let message_inner = self .process_request_internal(request) .await - .map_err(|e| e.0) - .map_err(rpc_error_responce_inverter); + .map_err(|e| e.0); Ok(Message::response(id, message_inner)) } else { Ok(Message::error(RpcError::parse_error( diff --git a/node_rpc/src/types/err_rpc.rs b/node_rpc/src/types/err_rpc.rs index 5f84e2b..c7ef25b 100644 --- a/node_rpc/src/types/err_rpc.rs +++ b/node_rpc/src/types/err_rpc.rs @@ -53,5 +53,9 @@ pub fn cast_seq_client_error_into_rpc_error(seq_cli_err: SequencerClientError) - match seq_cli_err { SequencerClientError::SerdeError(_) => RpcError::serialization_error(&error_string), SequencerClientError::HTTPError(_) => RpcError::new_internal_error(None, &error_string), + SequencerClientError::InternalError(err) => RpcError::new_internal_error( + err.error.data, + &serde_json::to_string(&err.error.error_struct).unwrap_or(String::default()), + ), } } diff --git a/sequencer_core/src/lib.rs b/sequencer_core/src/lib.rs index 04c1656..08972d8 100644 --- a/sequencer_core/src/lib.rs +++ b/sequencer_core/src/lib.rs @@ -57,9 +57,9 @@ impl SequencerCore { } } - fn execute_check_transaction_on_state( + pub fn transaction_pre_check( &mut self, - tx: TransactionMempool, + tx: &Transaction, ) -> Result<(), TransactionMalformationErrorKind> { let Transaction { hash, @@ -69,7 +69,7 @@ impl SequencerCore { ref utxo_commitments_created_hashes, ref nullifier_created_hashes, .. - } = tx.tx; + } = tx; //Sanity check match tx_kind { @@ -80,7 +80,7 @@ impl SequencerCore { //Public transactions can not make private operations. return Err( TransactionMalformationErrorKind::PublicTransactionChangedPrivateData { - tx: hash, + tx: *hash, }, ); } @@ -92,7 +92,7 @@ impl SequencerCore { //between public and private state. return Err( TransactionMalformationErrorKind::PrivateTransactionChangedPublicData { - tx: hash, + tx: *hash, }, ); } @@ -101,7 +101,7 @@ impl SequencerCore { }; //Tree checks - let tx_tree_check = self.store.pub_tx_store.get_tx(hash).is_some(); + let tx_tree_check = self.store.pub_tx_store.get_tx(*hash).is_some(); let nullifier_tree_check = nullifier_created_hashes .iter() .map(|nullifier_hash| { @@ -122,21 +122,46 @@ impl SequencerCore { .any(|check| check); if tx_tree_check { - return Err(TransactionMalformationErrorKind::TxHashAlreadyPresentInTree { tx: hash }); + return Err(TransactionMalformationErrorKind::TxHashAlreadyPresentInTree { tx: *hash }); } if nullifier_tree_check { return Err( - TransactionMalformationErrorKind::NullifierAlreadyPresentInTree { tx: hash }, + TransactionMalformationErrorKind::NullifierAlreadyPresentInTree { tx: *hash }, ); } if utxo_commitments_check { return Err( - TransactionMalformationErrorKind::UTXOCommitmentAlreadyPresentInTree { tx: hash }, + TransactionMalformationErrorKind::UTXOCommitmentAlreadyPresentInTree { tx: *hash }, ); } + Ok(()) + } + + pub fn push_tx_into_mempool_pre_check( + &mut self, + item: TransactionMempool, + ) -> Result<(), TransactionMalformationErrorKind> { + self.transaction_pre_check(&item.tx)?; + + self.mempool.push_item(item); + + Ok(()) + } + + fn execute_check_transaction_on_state( + &mut self, + tx: TransactionMempool, + ) -> Result<(), TransactionMalformationErrorKind> { + let Transaction { + hash, + ref utxo_commitments_created_hashes, + ref nullifier_created_hashes, + .. + } = tx.tx; + for utxo_comm in utxo_commitments_created_hashes { self.store .utxo_commitments_store diff --git a/sequencer_rpc/src/process.rs b/sequencer_rpc/src/process.rs index 7030913..098eb20 100644 --- a/sequencer_rpc/src/process.rs +++ b/sequencer_rpc/src/process.rs @@ -1,5 +1,7 @@ use actix_web::Error as HttpError; -use sequencer_core::sequecer_store::accounts_store::AccountPublicData; +use sequencer_core::{ + sequecer_store::accounts_store::AccountPublicData, TransactionMalformationErrorKind, +}; use serde_json::Value; use rpc_primitives::{ @@ -8,13 +10,10 @@ use rpc_primitives::{ parser::RpcRequest, }; -use crate::{ - rpc_error_responce_inverter, - types::rpc_structs::{ - GetBlockDataRequest, GetBlockDataResponse, GetGenesisIdRequest, GetGenesisIdResponse, - GetLastBlockRequest, GetLastBlockResponse, HelloRequest, HelloResponse, - RegisterAccountRequest, RegisterAccountResponse, SendTxRequest, SendTxResponse, - }, +use crate::types::rpc_structs::{ + GetBlockDataRequest, GetBlockDataResponse, GetGenesisIdRequest, GetGenesisIdResponse, + GetLastBlockRequest, GetLastBlockResponse, HelloRequest, HelloResponse, RegisterAccountRequest, + RegisterAccountResponse, SendTxRequest, SendTxResponse, }; use super::{respond, types::err_rpc::RpcErr, JsonHandler}; @@ -26,8 +25,7 @@ impl JsonHandler { let message_inner = self .process_request_internal(request) .await - .map_err(|e| e.0) - .map_err(rpc_error_responce_inverter); + .map_err(|e| e.0); Ok(Message::response(id, message_inner)) } else { Ok(Message::error(RpcError::parse_error( @@ -74,7 +72,7 @@ impl JsonHandler { { let mut state = self.sequencer_state.lock().await; - state.mempool.push_item(send_tx_req.transaction); + state.push_tx_into_mempool_pre_check(send_tx_req.transaction)?; } let helperstruct = SendTxResponse { diff --git a/sequencer_rpc/src/types/err_rpc.rs b/sequencer_rpc/src/types/err_rpc.rs index f5998d2..aef675c 100644 --- a/sequencer_rpc/src/types/err_rpc.rs +++ b/sequencer_rpc/src/types/err_rpc.rs @@ -1,6 +1,7 @@ use log::debug; use rpc_primitives::errors::{RpcError, RpcParseError}; +use sequencer_core::TransactionMalformationErrorKind; pub struct RpcErr(pub RpcError); @@ -40,6 +41,15 @@ impl RpcErrKind for RpcErrInternal { } } +impl RpcErrKind for TransactionMalformationErrorKind { + fn into_rpc_err(self) -> RpcError { + RpcError::new_internal_error( + Some(serde_json::to_value(self).unwrap()), + "transaction not accepted", + ) + } +} + #[allow(clippy::needless_pass_by_value)] pub fn from_rpc_err_into_anyhow_err(rpc_err: RpcError) -> anyhow::Error { debug!("Rpc error cast to anyhow error : err {rpc_err:?}");