From e7b727c0bac035fc9b93ff283a3a4253d4939766 Mon Sep 17 00:00:00 2001 From: Sergio Chouhy Date: Mon, 14 Jul 2025 15:45:20 -0300 Subject: [PATCH] make sequencer validate transaction signatures before adding transactions to blocks --- common/src/block.rs | 6 +- common/src/merkle_tree_public/merkle_tree.rs | 11 ++- .../src/merkle_tree_public/tree_leav_item.rs | 6 +- common/src/rpc_primitives/requests.rs | 3 +- common/src/transaction.rs | 7 +- node_core/src/chain_storage/block_store.rs | 4 +- node_core/src/chain_storage/mod.rs | 28 ++++--- node_rpc/src/process.rs | 12 ++- sequencer_core/src/lib.rs | 79 +++++++++++-------- sequencer_core/src/transaction_mempool.rs | 55 +++++++------ sequencer_rpc/src/process.rs | 5 +- 11 files changed, 118 insertions(+), 98 deletions(-) diff --git a/common/src/block.rs b/common/src/block.rs index ae8765e..4553245 100644 --- a/common/src/block.rs +++ b/common/src/block.rs @@ -1,7 +1,7 @@ use rs_merkle::Hasher; use serde::{Deserialize, Serialize}; -use crate::{merkle_tree_public::hasher::OwnHasher, transaction::TransactionBody}; +use crate::{merkle_tree_public::hasher::OwnHasher, transaction::SignedTransaction}; pub type BlockHash = [u8; 32]; pub type Data = Vec; @@ -13,7 +13,7 @@ pub struct Block { pub prev_block_id: BlockId, pub prev_block_hash: BlockHash, pub hash: BlockHash, - pub transactions: Vec, + pub transactions: Vec, pub data: Data, } @@ -22,7 +22,7 @@ pub struct HashableBlockData { pub block_id: BlockId, pub prev_block_id: BlockId, pub prev_block_hash: BlockHash, - pub transactions: Vec, + pub transactions: Vec, pub data: Data, } diff --git a/common/src/merkle_tree_public/merkle_tree.rs b/common/src/merkle_tree_public/merkle_tree.rs index d8229af..8886745 100644 --- a/common/src/merkle_tree_public/merkle_tree.rs +++ b/common/src/merkle_tree_public/merkle_tree.rs @@ -7,7 +7,10 @@ use serde::{ Deserialize, Deserializer, Serialize, }; -use crate::{transaction::TransactionBody, utxo_commitment::UTXOCommitment}; +use crate::{ + transaction::{AuthenticatedTransaction, SignedTransaction, TransactionBody}, + utxo_commitment::UTXOCommitment, +}; use super::{hasher::OwnHasher, tree_leav_item::TreeLeavItem, TreeHashType}; @@ -81,7 +84,7 @@ impl<'de, Leav: TreeLeavItem + Clone + Deserialize<'de>> serde::Deserialize<'de> } } -pub type PublicTransactionMerkleTree = HashStorageMerkleTree; +pub type PublicTransactionMerkleTree = HashStorageMerkleTree; pub type UTXOCommitmentsMerkleTree = HashStorageMerkleTree; @@ -139,7 +142,7 @@ impl HashStorageMerkleTree { } } - pub fn add_tx(&mut self, tx: Leav) { + pub fn add_tx(&mut self, tx: &Leav) { let last = self.leaves.len(); self.leaves.insert(last, tx.clone()); @@ -266,7 +269,7 @@ mod tests { let mut tree = HashStorageMerkleTree::new(vec![tx1.clone()]); - tree.add_tx(tx2.clone()); + tree.add_tx(&tx2); assert_eq!(tree.leaves.len(), 2); assert_eq!(tree.get_tx(tx2.hash()), Some(&tx2)); } diff --git a/common/src/merkle_tree_public/tree_leav_item.rs b/common/src/merkle_tree_public/tree_leav_item.rs index 52bb8ef..5599172 100644 --- a/common/src/merkle_tree_public/tree_leav_item.rs +++ b/common/src/merkle_tree_public/tree_leav_item.rs @@ -1,4 +1,4 @@ -use crate::{transaction::TransactionBody, utxo_commitment::UTXOCommitment}; +use crate::{transaction::SignedTransaction, utxo_commitment::UTXOCommitment}; use super::TreeHashType; @@ -6,9 +6,9 @@ pub trait TreeLeavItem { fn hash(&self) -> TreeHashType; } -impl TreeLeavItem for TransactionBody { +impl TreeLeavItem for SignedTransaction { fn hash(&self) -> TreeHashType { - self.hash() + self.body.hash() } } diff --git a/common/src/rpc_primitives/requests.rs b/common/src/rpc_primitives/requests.rs index 7508ce9..95b004f 100644 --- a/common/src/rpc_primitives/requests.rs +++ b/common/src/rpc_primitives/requests.rs @@ -1,5 +1,6 @@ use crate::block::Block; use crate::parse_request; +use crate::transaction::SignedTransaction; use crate::transaction::TransactionBody; use super::errors::RpcParseError; @@ -20,7 +21,7 @@ pub struct RegisterAccountRequest { #[derive(Serialize, Deserialize, Debug)] pub struct SendTxRequest { - pub transaction: TransactionBody, + pub transaction: SignedTransaction, ///UTXO Commitment Root, Pub Tx Root pub tx_roots: [[u8; 32]; 2], } diff --git a/common/src/transaction.rs b/common/src/transaction.rs index 6e6f651..1544d67 100644 --- a/common/src/transaction.rs +++ b/common/src/transaction.rs @@ -217,16 +217,16 @@ impl TransactionBody { } } -type SignaturePrivateKey = Scalar; +pub type SignaturePrivateKey = Scalar; -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] struct TransactionSignature; type TransactionHash = TreeHashType; /// A transaction with a signature. /// Meant to be sent through the network to the sequencer -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] pub struct SignedTransaction { pub body: TransactionBody, signature: TransactionSignature, @@ -253,6 +253,7 @@ impl SignedTransaction { /// A transaction with a valid signature over the hash of its body. /// Can only be constructed from an `SignedTransaction` /// if the signature is valid +#[derive(Clone)] pub struct AuthenticatedTransaction { hash: TransactionHash, signed_tx: SignedTransaction, diff --git a/node_core/src/chain_storage/block_store.rs b/node_core/src/chain_storage/block_store.rs index ba5b24b..c6bc386 100644 --- a/node_core/src/chain_storage/block_store.rs +++ b/node_core/src/chain_storage/block_store.rs @@ -6,7 +6,7 @@ use anyhow::{anyhow, Result}; use common::block::Block; use common::merkle_tree_public::merkle_tree::HashStorageMerkleTree; use common::nullifier::UTXONullifier; -use common::transaction::TransactionBody; +use common::transaction::{AuthenticatedTransaction, SignedTransaction, TransactionBody}; use common::utxo_commitment::UTXOCommitment; use log::error; use storage::sc_db_utils::{DataBlob, DataBlobChangeVariant}; @@ -87,7 +87,7 @@ impl NodeBlockStore { )?) } - pub fn get_snapshot_transaction(&self) -> Result> { + pub fn get_snapshot_transaction(&self) -> Result> { Ok(serde_json::from_slice( &self.dbio.get_snapshot_transaction()?, )?) diff --git a/node_core/src/chain_storage/mod.rs b/node_core/src/chain_storage/mod.rs index c875bbd..04ba03a 100644 --- a/node_core/src/chain_storage/mod.rs +++ b/node_core/src/chain_storage/mod.rs @@ -127,8 +127,8 @@ impl NodeChainStore { let block_id = block.block_id; for tx in &block.transactions { - if !tx.execution_input.is_empty() { - let public_action = serde_json::from_slice::(&tx.execution_input); + if !tx.body.execution_input.is_empty() { + let public_action = serde_json::from_slice::(&tx.body.execution_input); if let Ok(public_action) = public_action { match public_action { @@ -162,24 +162,25 @@ impl NodeChainStore { } self.utxo_commitments_store.add_tx_multiple( - tx.utxo_commitments_created_hashes + tx.body + .utxo_commitments_created_hashes .clone() .into_iter() .map(|hash| UTXOCommitment { hash }) .collect(), ); - for nullifier in tx.nullifier_created_hashes.iter() { + for nullifier in tx.body.nullifier_created_hashes.iter() { self.nullifier_store.insert(UTXONullifier { utxo_hash: *nullifier, }); } - if !tx.encoded_data.is_empty() { + if !tx.body.encoded_data.is_empty() { let ephemeral_public_key_sender = - serde_json::from_slice::(&tx.ephemeral_pub_key)?; + serde_json::from_slice::(&tx.body.ephemeral_pub_key)?; - for (ciphertext, nonce, tag) in tx.encoded_data.clone() { + for (ciphertext, nonce, tag) in tx.body.encoded_data.clone() { let slice = nonce.as_slice(); let nonce = accounts::key_management::constants_types::Nonce::clone_from_slice(slice); @@ -204,7 +205,7 @@ impl NodeChainStore { } } - self.pub_tx_store.add_tx(tx.clone()); + self.pub_tx_store.add_tx(tx); } self.block_store.put_block_at_id(block)?; @@ -278,7 +279,7 @@ mod tests { use accounts::account_core::Account; use common::block::{Block, Data}; use common::merkle_tree_public::TreeHashType; - use common::transaction::{TransactionBody, TxKind}; + use common::transaction::{SignaturePrivateKey, SignedTransaction, TransactionBody, TxKind}; use secp256k1_zkp::Tweak; use std::path::PathBuf; use tempfile::tempdir; @@ -300,10 +301,10 @@ mod tests { nullifier_created_hashes: Vec<[u8; 32]>, utxo_commitments_spent_hashes: Vec<[u8; 32]>, utxo_commitments_created_hashes: Vec<[u8; 32]>, - ) -> TransactionBody { + ) -> SignedTransaction { let mut rng = rand::thread_rng(); - TransactionBody { + let body = TransactionBody { tx_kind: TxKind::Private, execution_input: vec![], execution_output: vec![], @@ -318,7 +319,8 @@ mod tests { secret_r: [0; 32], sc_addr: "sc_addr".to_string(), state_changes: (serde_json::Value::Null, 0), - } + }; + SignedTransaction::from_transaction_body(body, SignaturePrivateKey::ONE) } fn create_sample_block(block_id: u64, prev_block_id: u64) -> Block { @@ -414,7 +416,7 @@ mod tests { store .utxo_commitments_store .add_tx_multiple(vec![UTXOCommitment { hash: [3u8; 32] }]); - store.pub_tx_store.add_tx(create_dummy_transaction( + store.pub_tx_store.add_tx(&create_dummy_transaction( [12; 32], vec![[9; 32]], vec![[7; 32]], diff --git a/node_rpc/src/process.rs b/node_rpc/src/process.rs index a188ec8..149121a 100644 --- a/node_rpc/src/process.rs +++ b/node_rpc/src/process.rs @@ -312,42 +312,46 @@ impl JsonHandler { ShowTransactionResponse { hash: req.tx_hash, - tx_kind: tx.tx_kind, + tx_kind: tx.body.tx_kind, public_input: if let Ok(action) = - serde_json::from_slice::(&tx.execution_input) + serde_json::from_slice::(&tx.body.execution_input) { action.into_hexed_print() } else { "".to_string() }, public_output: if let Ok(action) = - serde_json::from_slice::(&tx.execution_output) + serde_json::from_slice::(&tx.body.execution_output) { action.into_hexed_print() } else { "".to_string() }, utxo_commitments_created_hashes: tx + .body .utxo_commitments_created_hashes .iter() .map(|val| hex::encode(val.clone())) .collect::>(), utxo_commitments_spent_hashes: tx + .body .utxo_commitments_spent_hashes .iter() .map(|val| hex::encode(val.clone())) .collect::>(), utxo_nullifiers_created_hashes: tx + .body .nullifier_created_hashes .iter() .map(|val| hex::encode(val.clone())) .collect::>(), encoded_data: tx + .body .encoded_data .iter() .map(|val| (hex::encode(val.0.clone()), hex::encode(val.1.clone()))) .collect::>(), - ephemeral_pub_key: hex::encode(tx.ephemeral_pub_key.clone()), + ephemeral_pub_key: hex::encode(tx.body.ephemeral_pub_key.clone()), } } }; diff --git a/sequencer_core/src/lib.rs b/sequencer_core/src/lib.rs index bec16b0..d480670 100644 --- a/sequencer_core/src/lib.rs +++ b/sequencer_core/src/lib.rs @@ -5,14 +5,14 @@ use common::{ block::{Block, HashableBlockData}, merkle_tree_public::TreeHashType, nullifier::UTXONullifier, - transaction::{TransactionBody, TxKind}, + transaction::{AuthenticatedTransaction, SignedTransaction, TransactionBody, TxKind}, utxo_commitment::UTXOCommitment, }; use config::SequencerConfig; use mempool::MemPool; use sequencer_store::{accounts_store::AccountPublicData, SequecerChainStore}; use serde::{Deserialize, Serialize}; -use transaction_mempool::TransactionMempool; +use transaction_mempool::MempoolTransaction; pub mod config; pub mod sequencer_store; @@ -20,7 +20,7 @@ pub mod transaction_mempool; pub struct SequencerCore { pub store: SequecerChainStore, - pub mempool: MemPool, + pub mempool: MemPool, pub sequencer_config: SequencerConfig, pub chain_height: u64, } @@ -35,6 +35,7 @@ pub enum TransactionMalformationErrorKind { MempoolFullForRound { tx: TreeHashType }, ChainStateFurtherThanTransactionState { tx: TreeHashType }, FailedToInsert { tx: TreeHashType, details: String }, + InvalidSignature, } impl Display for TransactionMalformationErrorKind { @@ -53,7 +54,7 @@ impl SequencerCore { config.genesis_id, config.is_genesis_random, ), - mempool: MemPool::::default(), + mempool: MemPool::::default(), chain_height: config.genesis_id, sequencer_config: config, } @@ -71,9 +72,12 @@ impl SequencerCore { pub fn transaction_pre_check( &mut self, - tx: &TransactionBody, + tx: SignedTransaction, tx_roots: [[u8; 32]; 2], - ) -> Result<(), TransactionMalformationErrorKind> { + ) -> Result { + let tx = tx + .into_authenticated() + .map_err(|_| TransactionMalformationErrorKind::InvalidSignature)?; let TransactionBody { tx_kind, ref execution_input, @@ -81,10 +85,10 @@ impl SequencerCore { ref utxo_commitments_created_hashes, ref nullifier_created_hashes, .. - } = tx; - let tx_hash = tx.hash(); + } = tx.body(); let mempool_size = self.mempool.len(); + let tx_hash = *tx.hash(); if mempool_size >= self.sequencer_config.max_num_tx_in_block { return Err(TransactionMalformationErrorKind::MempoolFullForRound { tx: tx_hash }); @@ -151,53 +155,53 @@ impl SequencerCore { if tx_tree_check { return Err( - TransactionMalformationErrorKind::TxHashAlreadyPresentInTree { tx: tx.hash() }, + TransactionMalformationErrorKind::TxHashAlreadyPresentInTree { tx: *tx.hash() }, ); } if nullifier_tree_check { return Err( - TransactionMalformationErrorKind::NullifierAlreadyPresentInTree { tx: tx.hash() }, + TransactionMalformationErrorKind::NullifierAlreadyPresentInTree { tx: *tx.hash() }, ); } if utxo_commitments_check { return Err( TransactionMalformationErrorKind::UTXOCommitmentAlreadyPresentInTree { - tx: tx.hash(), + tx: *tx.hash(), }, ); } - Ok(()) + Ok(tx) } pub fn push_tx_into_mempool_pre_check( &mut self, - item: TransactionMempool, + item: SignedTransaction, tx_roots: [[u8; 32]; 2], ) -> Result<(), TransactionMalformationErrorKind> { - self.transaction_pre_check(&item.tx, tx_roots)?; + let item = self.transaction_pre_check(item, tx_roots)?; - self.mempool.push_item(item); + self.mempool.push_item(item.into()); Ok(()) } fn execute_check_transaction_on_state( &mut self, - tx: TransactionMempool, + tx: &MempoolTransaction, ) -> Result<(), TransactionMalformationErrorKind> { let TransactionBody { ref utxo_commitments_created_hashes, ref nullifier_created_hashes, .. - } = tx.tx; + } = tx.tx.body(); for utxo_comm in utxo_commitments_created_hashes { self.store .utxo_commitments_store - .add_tx(UTXOCommitment { hash: *utxo_comm }); + .add_tx(&UTXOCommitment { hash: *utxo_comm }); } for nullifier in nullifier_created_hashes.iter() { @@ -206,7 +210,7 @@ impl SequencerCore { }); } - self.store.pub_tx_store.add_tx(tx.tx); + self.store.pub_tx_store.add_tx(tx.tx.as_signed()); Ok(()) } @@ -227,7 +231,7 @@ impl SequencerCore { .pop_size(self.sequencer_config.max_num_tx_in_block); for tx in &transactions { - self.execute_check_transaction_on_state(tx.clone())?; + self.execute_check_transaction_on_state(&tx)?; } let prev_block_hash = self @@ -239,7 +243,10 @@ impl SequencerCore { let hashable_data = HashableBlockData { block_id: new_block_height, prev_block_id: self.chain_height, - transactions: transactions.into_iter().map(|tx_mem| tx_mem.tx).collect(), + transactions: transactions + .into_iter() + .map(|tx_mem| tx_mem.tx.as_signed().clone()) + .collect(), data: vec![], prev_block_hash, }; @@ -259,10 +266,10 @@ mod tests { use super::*; use std::path::PathBuf; - use common::transaction::{TransactionBody, TxKind}; + use common::transaction::{SignaturePrivateKey, SignedTransaction, TransactionBody, TxKind}; use rand::Rng; use secp256k1_zkp::Tweak; - use transaction_mempool::TransactionMempool; + use transaction_mempool::MempoolTransaction; fn setup_sequencer_config() -> SequencerConfig { let mut rng = rand::thread_rng(); @@ -285,10 +292,10 @@ mod tests { nullifier_created_hashes: Vec<[u8; 32]>, utxo_commitments_spent_hashes: Vec<[u8; 32]>, utxo_commitments_created_hashes: Vec<[u8; 32]>, - ) -> TransactionBody { + ) -> SignedTransaction { let mut rng = rand::thread_rng(); - TransactionBody { + let body = TransactionBody { tx_kind: TxKind::Private, execution_input: vec![], execution_output: vec![], @@ -303,12 +310,15 @@ mod tests { secret_r: [0; 32], sc_addr: "sc_addr".to_string(), state_changes: (serde_json::Value::Null, 0), - } + }; + SignedTransaction::from_transaction_body(body, SignaturePrivateKey::ONE) } fn common_setup(sequencer: &mut SequencerCore) { let tx = create_dummy_transaction(vec![[9; 32]], vec![[7; 32]], vec![[8; 32]]); - let tx_mempool = TransactionMempool { tx }; + let tx_mempool = MempoolTransaction { + tx: tx.into_authenticated().unwrap(), + }; sequencer.mempool.push_item(tx_mempool); sequencer.produce_new_block_with_mempool_transactions(); @@ -344,7 +354,7 @@ mod tests { let tx = create_dummy_transaction(vec![[91; 32]], vec![[71; 32]], vec![[81; 32]]); let tx_roots = sequencer.get_tree_roots(); - let result = sequencer.transaction_pre_check(&tx, tx_roots); + let result = sequencer.transaction_pre_check(tx, tx_roots); assert!(result.is_ok()); } @@ -363,10 +373,12 @@ mod tests { let tx_roots = sequencer.get_tree_roots(); // Fill the mempool - let dummy_tx = TransactionMempool { tx: tx.clone() }; + let dummy_tx = MempoolTransaction { + tx: tx.clone().into_authenticated().unwrap(), + }; sequencer.mempool.push_item(dummy_tx); - let result = sequencer.transaction_pre_check(&tx, tx_roots); + let result = sequencer.transaction_pre_check(tx, tx_roots); assert!(matches!( result, @@ -383,9 +395,8 @@ mod tests { let tx = create_dummy_transaction(vec![[93; 32]], vec![[73; 32]], vec![[83; 32]]); let tx_roots = sequencer.get_tree_roots(); - let tx_mempool = TransactionMempool { tx }; - let result = sequencer.push_tx_into_mempool_pre_check(tx_mempool.clone(), tx_roots); + let result = sequencer.push_tx_into_mempool_pre_check(tx, tx_roots); assert!(result.is_ok()); assert_eq!(sequencer.mempool.len(), 1); } @@ -396,7 +407,9 @@ mod tests { let mut sequencer = SequencerCore::start_from_config(config); let tx = create_dummy_transaction(vec![[94; 32]], vec![[7; 32]], vec![[8; 32]]); - let tx_mempool = TransactionMempool { tx }; + let tx_mempool = MempoolTransaction { + tx: tx.into_authenticated().unwrap(), + }; sequencer.mempool.push_item(tx_mempool); let block_id = sequencer.produce_new_block_with_mempool_transactions(); diff --git a/sequencer_core/src/transaction_mempool.rs b/sequencer_core/src/transaction_mempool.rs index d6d81f2..cb40ad5 100644 --- a/sequencer_core/src/transaction_mempool.rs +++ b/sequencer_core/src/transaction_mempool.rs @@ -1,43 +1,42 @@ -use common::{merkle_tree_public::TreeHashType, transaction::TransactionBody}; +use common::{merkle_tree_public::TreeHashType, transaction::AuthenticatedTransaction}; use mempool::mempoolitem::MemPoolItem; use serde::{Deserialize, Serialize}; -#[derive(Debug, Clone)] -pub struct TransactionMempool { - pub tx: TransactionBody, +pub struct MempoolTransaction { + pub tx: AuthenticatedTransaction, } -impl From for TransactionMempool { - fn from(value: TransactionBody) -> Self { +impl From for MempoolTransaction { + fn from(value: AuthenticatedTransaction) -> Self { Self { tx: value } } } -impl Serialize for TransactionMempool { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - self.tx.serialize(serializer) - } -} +// impl Serialize for TransactionMempool { +// fn serialize(&self, serializer: S) -> Result +// where +// S: serde::Serializer, +// { +// self.tx.serialize(serializer) +// } +// } +// +// impl<'de> Deserialize<'de> for TransactionMempool { +// fn deserialize(deserializer: D) -> Result +// where +// D: serde::Deserializer<'de>, +// { +// match TransactionBody::deserialize(deserializer) { +// Ok(tx) => Ok(TransactionMempool { tx }), +// Err(err) => Err(err), +// } +// } +// } -impl<'de> Deserialize<'de> for TransactionMempool { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - match TransactionBody::deserialize(deserializer) { - Ok(tx) => Ok(TransactionMempool { tx }), - Err(err) => Err(err), - } - } -} - -impl MemPoolItem for TransactionMempool { +impl MemPoolItem for MempoolTransaction { type Identifier = TreeHashType; fn identifier(&self) -> Self::Identifier { - self.tx.hash() + *self.tx.hash() } } diff --git a/sequencer_rpc/src/process.rs b/sequencer_rpc/src/process.rs index 5bac835..102da72 100644 --- a/sequencer_rpc/src/process.rs +++ b/sequencer_rpc/src/process.rs @@ -81,10 +81,7 @@ impl JsonHandler { { let mut state = self.sequencer_state.lock().await; - state.push_tx_into_mempool_pre_check( - send_tx_req.transaction.into(), - send_tx_req.tx_roots, - )?; + state.push_tx_into_mempool_pre_check(send_tx_req.transaction, send_tx_req.tx_roots)?; } let helperstruct = SendTxResponse {