diff --git a/applications/tari_app_grpc/proto/types.proto b/applications/tari_app_grpc/proto/types.proto index 74ca15d641..87b26f012d 100644 --- a/applications/tari_app_grpc/proto/types.proto +++ b/applications/tari_app_grpc/proto/types.proto @@ -169,6 +169,8 @@ message TransactionInput { ComSignature script_signature = 7; // The offset public key, K_O bytes sender_offset_public_key = 8; + // The hash of the output this input is spending + bytes output_hash = 9; } // Output for a transaction, defining the new ownership of coins that are being transferred. The commitment is a diff --git a/applications/tari_app_grpc/src/conversions/aggregate_body.rs b/applications/tari_app_grpc/src/conversions/aggregate_body.rs index d894a5c6c8..eb5b22b27f 100644 --- a/applications/tari_app_grpc/src/conversions/aggregate_body.rs +++ b/applications/tari_app_grpc/src/conversions/aggregate_body.rs @@ -27,14 +27,16 @@ use tari_utilities::convert::try_convert_all; use crate::tari_rpc as grpc; -impl From for grpc::AggregateBody { - fn from(source: AggregateBody) -> Self { - Self { +impl TryFrom for grpc::AggregateBody { + type Error = String; + + fn try_from(source: AggregateBody) -> Result { + Ok(Self { inputs: source .inputs() .iter() - .map(|input| grpc::TransactionInput::from(input.clone())) - .collect(), + .map(|input| grpc::TransactionInput::try_from(input.clone())) + .collect::, _>>()?, outputs: source .outputs() .iter() @@ -45,7 +47,7 @@ impl From for grpc::AggregateBody { .iter() .map(|kernel| grpc::TransactionKernel::from(kernel.clone())) .collect(), - } + }) } } diff --git a/applications/tari_app_grpc/src/conversions/block.rs b/applications/tari_app_grpc/src/conversions/block.rs index 231748eec0..292d7c1361 100644 --- a/applications/tari_app_grpc/src/conversions/block.rs +++ b/applications/tari_app_grpc/src/conversions/block.rs @@ -26,12 +26,14 @@ use tari_core::blocks::Block; use crate::tari_rpc as grpc; -impl From for grpc::Block { - fn from(block: Block) -> Self { - Self { - body: Some(block.body.into()), +impl TryFrom for grpc::Block { + type Error = String; + + fn try_from(block: Block) -> Result { + Ok(Self { + body: Some(block.body.try_into()?), header: Some(block.header.into()), - } + }) } } diff --git a/applications/tari_app_grpc/src/conversions/historical_block.rs b/applications/tari_app_grpc/src/conversions/historical_block.rs index 4163e94eeb..e391eb75bd 100644 --- a/applications/tari_app_grpc/src/conversions/historical_block.rs +++ b/applications/tari_app_grpc/src/conversions/historical_block.rs @@ -20,7 +20,7 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; use tari_core::{blocks::HistoricalBlock, chain_storage::ChainStorageError}; @@ -32,7 +32,11 @@ impl TryFrom for grpc::HistoricalBlock { fn try_from(hb: HistoricalBlock) -> Result { Ok(Self { confirmations: hb.confirmations, - block: Some(hb.try_into_block()?.into()), + block: Some( + hb.try_into_block()? + .try_into() + .map_err(ChainStorageError::ConversionError)?, + ), }) } } diff --git a/applications/tari_app_grpc/src/conversions/new_block_template.rs b/applications/tari_app_grpc/src/conversions/new_block_template.rs index 05efd2fee7..dd8ab8cc41 100644 --- a/applications/tari_app_grpc/src/conversions/new_block_template.rs +++ b/applications/tari_app_grpc/src/conversions/new_block_template.rs @@ -31,8 +31,10 @@ use tari_utilities::ByteArray; use crate::tari_rpc as grpc; -impl From for grpc::NewBlockTemplate { - fn from(block: NewBlockTemplate) -> Self { +impl TryFrom for grpc::NewBlockTemplate { + type Error = String; + + fn try_from(block: NewBlockTemplate) -> Result { let header = grpc::NewBlockHeaderTemplate { version: block.header.version as u32, height: block.header.height, @@ -44,14 +46,14 @@ impl From for grpc::NewBlockTemplate { pow_data: block.header.pow.pow_data, }), }; - Self { + Ok(Self { body: Some(grpc::AggregateBody { inputs: block .body .inputs() .iter() - .map(|input| grpc::TransactionInput::from(input.clone())) - .collect(), + .map(|input| grpc::TransactionInput::try_from(input.clone())) + .collect::, _>>()?, outputs: block .body .outputs() @@ -66,7 +68,7 @@ impl From for grpc::NewBlockTemplate { .collect(), }), header: Some(header), - } + }) } } impl TryFrom for NewBlockTemplate { diff --git a/applications/tari_app_grpc/src/conversions/transaction.rs b/applications/tari_app_grpc/src/conversions/transaction.rs index b3c4261ef0..7f4159718a 100644 --- a/applications/tari_app_grpc/src/conversions/transaction.rs +++ b/applications/tari_app_grpc/src/conversions/transaction.rs @@ -29,13 +29,15 @@ use tari_utilities::ByteArray; use crate::tari_rpc as grpc; -impl From for grpc::Transaction { - fn from(source: Transaction) -> Self { - Self { +impl TryFrom for grpc::Transaction { + type Error = String; + + fn try_from(source: Transaction) -> Result { + Ok(Self { offset: Vec::from(source.offset.as_bytes()), - body: Some(source.body.into()), + body: Some(source.body.try_into()?), script_offset: Vec::from(source.script_offset.as_bytes()), - } + }) } } diff --git a/applications/tari_app_grpc/src/conversions/transaction_input.rs b/applications/tari_app_grpc/src/conversions/transaction_input.rs index 6f2e8a0152..6ddcf9bdff 100644 --- a/applications/tari_app_grpc/src/conversions/transaction_input.rs +++ b/applications/tari_app_grpc/src/conversions/transaction_input.rs @@ -26,7 +26,7 @@ use tari_common_types::types::{Commitment, PublicKey}; use tari_core::transactions::transaction::TransactionInput; use tari_crypto::{ script::{ExecutionStack, TariScript}, - tari_utilities::{ByteArray, Hashable}, + tari_utilities::ByteArray, }; use crate::tari_rpc as grpc; @@ -35,51 +35,95 @@ impl TryFrom for TransactionInput { type Error = String; fn try_from(input: grpc::TransactionInput) -> Result { - let features = input - .features - .map(TryInto::try_into) - .ok_or_else(|| "transaction output features not provided".to_string())??; - - let commitment = Commitment::from_bytes(&input.commitment) - .map_err(|err| format!("Could not convert input commitment:{}", err))?; - let script_signature = input .script_signature .ok_or_else(|| "script_signature not provided".to_string())? .try_into() .map_err(|_| "script_signature could not be converted".to_string())?; - let sender_offset_public_key = - PublicKey::from_bytes(input.sender_offset_public_key.as_bytes()).map_err(|err| format!("{:?}", err))?; - let script = TariScript::from_bytes(input.script.as_slice()).map_err(|err| format!("{:?}", err))?; - let input_data = ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?; + // Check if the received Transaction input is in compact form or not + if !input.commitment.is_empty() { + let commitment = Commitment::from_bytes(&input.commitment).map_err(|e| e.to_string())?; + let features = input + .features + .map(TryInto::try_into) + .ok_or_else(|| "transaction output features not provided".to_string())??; - Ok(Self { - features, - commitment, - script, - input_data, - script_signature, - sender_offset_public_key, - }) + let sender_offset_public_key = + PublicKey::from_bytes(input.sender_offset_public_key.as_bytes()).map_err(|err| format!("{:?}", err))?; + + Ok(TransactionInput::new_with_output_data( + features, + commitment, + TariScript::from_bytes(input.script.as_slice()).map_err(|err| format!("{:?}", err))?, + ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?, + script_signature, + sender_offset_public_key, + )) + } else { + if input.output_hash.is_empty() { + return Err("Compact Transaction Input does not contain `output_hash`".to_string()); + } + Ok(TransactionInput::new_with_output_hash( + input.output_hash, + ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?, + script_signature, + )) + } } } -impl From for grpc::TransactionInput { - fn from(input: TransactionInput) -> Self { - let hash = input.hash(); - Self { - features: Some(input.features.into()), - commitment: Vec::from(input.commitment.as_bytes()), - hash, - script: input.script.as_bytes(), - input_data: input.input_data.as_bytes(), - script_signature: Some(grpc::ComSignature { - public_nonce_commitment: Vec::from(input.script_signature.public_nonce().as_bytes()), - signature_u: Vec::from(input.script_signature.u().as_bytes()), - signature_v: Vec::from(input.script_signature.v().as_bytes()), - }), - sender_offset_public_key: input.sender_offset_public_key.as_bytes().to_vec(), +impl TryFrom for grpc::TransactionInput { + type Error = String; + + fn try_from(input: TransactionInput) -> Result { + let script_signature = Some(grpc::ComSignature { + public_nonce_commitment: Vec::from(input.script_signature.public_nonce().as_bytes()), + signature_u: Vec::from(input.script_signature.u().as_bytes()), + signature_v: Vec::from(input.script_signature.v().as_bytes()), + }); + if input.is_compact() { + let output_hash = input.output_hash(); + Ok(Self { + features: None, + commitment: Vec::new(), + hash: Vec::new(), + script: Vec::new(), + input_data: Vec::new(), + script_signature, + sender_offset_public_key: Vec::new(), + output_hash, + }) + } else { + let features = input + .features() + .map_err(|_| "Non-compact Transaction input should contain features".to_string())?; + + Ok(Self { + features: Some(features.clone().into()), + commitment: input + .commitment() + .map_err(|_| "Non-compact Transaction input should contain commitment".to_string())? + .clone() + .as_bytes() + .to_vec(), + hash: input + .canonical_hash() + .map_err(|_| "Non-compact Transaction input should be able to be hashed".to_string())?, + + script: input + .script() + .map_err(|_| "Non-compact Transaction input should contain script".to_string())? + .as_bytes(), + input_data: input.input_data.as_bytes(), + script_signature, + sender_offset_public_key: input + .sender_offset_public_key() + .map_err(|_| "Non-compact Transaction input should contain sender_offset_public_key".to_string())? + .as_bytes() + .to_vec(), + output_hash: Vec::new(), + }) } } } diff --git a/applications/tari_base_node/src/grpc/base_node_grpc_server.rs b/applications/tari_base_node/src/grpc/base_node_grpc_server.rs index c6c1f8237a..89368c7e78 100644 --- a/applications/tari_base_node/src/grpc/base_node_grpc_server.rs +++ b/applications/tari_base_node/src/grpc/base_node_grpc_server.rs @@ -234,9 +234,26 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { Ok(data) => data, }; for transaction in transactions.unconfirmed_pool { + let transaction = match tari_rpc::Transaction::try_from(transaction) { + Ok(t) => t, + Err(e) => { + warn!( + target: LOG_TARGET, + "Error sending converting transaction for GRPC: {}", e + ); + match tx.send(Err(Status::internal("Error converting transaction"))).await { + Ok(_) => (), + Err(send_err) => { + warn!(target: LOG_TARGET, "Error sending error to GRPC client: {}", send_err) + }, + } + return; + }, + }; + match tx .send(Ok(tari_rpc::GetMempoolTransactionsResponse { - transaction: Some(transaction.into()), + transaction: Some(transaction), })) .await { @@ -638,7 +655,7 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { total_fees: new_template.total_fees.into(), algo: Some(tari_rpc::PowAlgo { pow_algo: pow }), }), - new_block_template: Some(new_template.into()), + new_block_template: Some(new_template.try_into().map_err(Status::internal)?), initial_sync_achieved: (*status_watch.borrow()).bootstrapped, }; @@ -677,7 +694,7 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { // construct response let block_hash = new_block.hash(); let mining_hash = new_block.header.merged_mining_hash(); - let block: Option = Some(new_block.into()); + let block: Option = Some(new_block.try_into().map_err(Status::internal)?); let response = tari_rpc::GetNewBlockResult { block_hash, diff --git a/applications/tari_console_wallet/src/automation/commands.rs b/applications/tari_console_wallet/src/automation/commands.rs index 863c6b2dbc..19742cb1d1 100644 --- a/applications/tari_console_wallet/src/automation/commands.rs +++ b/applications/tari_console_wallet/src/automation/commands.rs @@ -53,6 +53,7 @@ use tari_crypto::{ }; use tari_utilities::hex::Hex; use tari_wallet::{ + error::WalletError, output_manager_service::handle::OutputManagerHandle, transaction_service::handle::{TransactionEvent, TransactionServiceHandle}, WalletSqlite, @@ -941,7 +942,10 @@ fn write_utxos_to_csv_file(utxos: Vec, file_path: String) -> Re i + 1, utxo.value.0, utxo.spending_key.to_hex(), - utxo.as_transaction_input(&factory)?.commitment.to_hex(), + utxo.as_transaction_input(&factory)? + .commitment() + .map_err(|e| CommandError::WalletError(WalletError::TransactionError(e)))? + .to_hex(), utxo.features.flags, utxo.features.maturity, utxo.script.to_hex(), diff --git a/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs b/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs index 1afc38d7b8..70ab54c629 100644 --- a/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs +++ b/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs @@ -190,7 +190,7 @@ impl wallet_server::Wallet for WalletGrpcServer { match response { Ok(resp) => Ok(Response::new(GetCoinbaseResponse { - transaction: Some(resp.into()), + transaction: Some(resp.try_into().map_err(Status::internal)?), })), Err(err) => Err(Status::unknown(err.to_string())), } diff --git a/applications/tari_console_wallet/src/ui/state/app_state.rs b/applications/tari_console_wallet/src/ui/state/app_state.rs index d9a6a7c8df..16aca48ebe 100644 --- a/applications/tari_console_wallet/src/ui/state/app_state.rs +++ b/applications/tari_console_wallet/src/ui/state/app_state.rs @@ -969,8 +969,10 @@ pub struct CompletedTransactionInfo { fn first_unique_id(tx: &CompletedTransaction) -> String { let body = tx.transaction.body(); for input in body.inputs() { - if let Some(ref unique_id) = input.features.unique_id { - return unique_id.to_hex(); + if let Ok(features) = input.features() { + if let Some(ref unique_id) = features.unique_id { + return unique_id.to_hex(); + } } } for output in body.outputs() { diff --git a/applications/tari_merge_mining_proxy/src/common/merge_mining.rs b/applications/tari_merge_mining_proxy/src/common/merge_mining.rs index 7045074afb..0b13b1a1f1 100644 --- a/applications/tari_merge_mining_proxy/src/common/merge_mining.rs +++ b/applications/tari_merge_mining_proxy/src/common/merge_mining.rs @@ -20,7 +20,7 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; use tari_app_grpc::tari_rpc as grpc; use tari_core::{ @@ -42,5 +42,5 @@ pub fn add_coinbase( .map_err(MmProxyError::MissingDataError)?; block_template.body.add_output(output); block_template.body.add_kernel(kernel); - Ok(block_template.into()) + block_template.try_into().map_err(MmProxyError::ConversionError) } diff --git a/applications/tari_merge_mining_proxy/src/error.rs b/applications/tari_merge_mining_proxy/src/error.rs index 4a6f062077..7e50f6be55 100644 --- a/applications/tari_merge_mining_proxy/src/error.rs +++ b/applications/tari_merge_mining_proxy/src/error.rs @@ -80,6 +80,8 @@ pub enum MmProxyError { InvalidHeaderValue(#[from] InvalidHeaderValue), #[error("Block was lost due to a failed precondition, and should be retried")] FailedPreconditionBlockLostRetry, + #[error("Could not convert data:{0}")] + ConversionError(String), #[error("No reachable servers in configuration")] ServersUnavailable, } diff --git a/applications/tari_stratum_transcoder/src/error.rs b/applications/tari_stratum_transcoder/src/error.rs index ea9870b0cf..998ba9a4ef 100644 --- a/applications/tari_stratum_transcoder/src/error.rs +++ b/applications/tari_stratum_transcoder/src/error.rs @@ -66,6 +66,8 @@ pub enum StratumTranscoderProxyError { CoinbaseBuilderError(#[from] CoinbaseBuildError), #[error("Unexpected Tari base node response: {0}")] UnexpectedTariBaseNodeResponse(String), + #[error("Could not convert data:{0}")] + ConversionError(String), } impl From for StratumTranscoderProxyError { diff --git a/applications/tari_stratum_transcoder/src/proxy.rs b/applications/tari_stratum_transcoder/src/proxy.rs index d0b4eec220..b0eaf682a3 100644 --- a/applications/tari_stratum_transcoder/src/proxy.rs +++ b/applications/tari_stratum_transcoder/src/proxy.rs @@ -21,7 +21,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use std::{ - convert::TryFrom, + convert::{TryFrom, TryInto}, future::Future, net::SocketAddr, pin::Pin, @@ -302,7 +302,8 @@ impl InnerService { match block { Ok(block) => { let mut client = self.base_node_client.clone(); - let grpc_block: tari_app_grpc::tari_rpc::Block = block.into(); + let grpc_block: tari_app_grpc::tari_rpc::Block = + block.try_into().map_err(StratumTranscoderProxyError::ConversionError)?; match client.submit_block(grpc_block).await { Ok(_) => { json_response = proxy::json_response( diff --git a/base_layer/core/src/base_node/proto/response.rs b/base_layer/core/src/base_node/proto/response.rs index 918f16fd57..23f700d9c1 100644 --- a/base_layer/core/src/base_node/proto/response.rs +++ b/base_layer/core/src/base_node/proto/response.rs @@ -67,7 +67,13 @@ impl TryFrom for ProtoNodeCommsResponse { use ci::NodeCommsResponse::*; match response { HistoricalBlocks(historical_blocks) => { - let historical_blocks = historical_blocks.into_iter().map(Into::into).collect(); + let historical_blocks = historical_blocks + .into_iter() + .map(TryInto::try_into) + .collect::, _>>()? + .into_iter() + .map(Into::into) + .collect(); Ok(ProtoNodeCommsResponse::HistoricalBlocks(historical_blocks)) }, // This would only occur if a programming error sent out the unsupported response @@ -98,11 +104,13 @@ impl TryInto> for base_node_proto::BlockHeaderResponse { } } -impl From> for base_node_proto::HistoricalBlockResponse { - fn from(v: Option) -> Self { - Self { - block: v.map(Into::into), - } +impl TryFrom> for base_node_proto::HistoricalBlockResponse { + type Error = String; + + fn try_from(v: Option) -> Result { + Ok(Self { + block: v.map(TryInto::try_into).transpose()?, + }) } } diff --git a/base_layer/core/src/base_node/proto/rpc.rs b/base_layer/core/src/base_node/proto/rpc.rs index 9659a99a19..fc1165ac3e 100644 --- a/base_layer/core/src/base_node/proto/rpc.rs +++ b/base_layer/core/src/base_node/proto/rpc.rs @@ -20,16 +20,20 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +use std::convert::{TryFrom, TryInto}; + use tari_utilities::Hashable; use crate::{blocks::Block, chain_storage::PrunedOutput, proto::base_node as proto}; -impl From for proto::BlockBodyResponse { - fn from(block: Block) -> Self { - Self { +impl TryFrom for proto::BlockBodyResponse { + type Error = String; + + fn try_from(block: Block) -> Result { + Ok(Self { hash: block.hash(), - body: Some(block.body.into()), - } + body: Some(block.body.try_into()?), + }) } } diff --git a/base_layer/core/src/base_node/service/service.rs b/base_layer/core/src/base_node/service/service.rs index 91baa0bd5f..830068f633 100644 --- a/base_layer/core/src/base_node/service/service.rs +++ b/base_layer/core/src/base_node/service/service.rs @@ -503,7 +503,7 @@ async fn handle_outbound_request( let request_key = generate_request_key(&mut OsRng); let service_request = proto::BaseNodeServiceRequest { request_key, - request: Some(request.try_into().map_err(CommsInterfaceError::ApiError)?), + request: Some(request.try_into().map_err(CommsInterfaceError::InternalError)?), }; let mut send_msg_params = SendMessageParams::new(); diff --git a/base_layer/core/src/base_node/sync/rpc/service.rs b/base_layer/core/src/base_node/sync/rpc/service.rs index 0b9c71c75e..9c2d33f1a0 100644 --- a/base_layer/core/src/base_node/sync/rpc/service.rs +++ b/base_layer/core/src/base_node/sync/rpc/service.rs @@ -22,6 +22,7 @@ use std::{ cmp, + convert::TryFrom, sync::{Arc, Weak}, }; @@ -178,9 +179,17 @@ impl BaseNodeSyncService for BaseNodeSyncRpcServ Ok(blocks) => { let blocks = blocks .into_iter() - .map(|hb| hb.try_into_block().map_err(RpcStatus::log_internal_error(LOG_TARGET))) + .map(|hb| { + match hb.try_into_block().map_err(RpcStatus::log_internal_error(LOG_TARGET)) { + Ok(b) => Ok(b.to_compact()), + Err(e) => Err(e), + } + }) .map(|block| match block { - Ok(b) => Ok(proto::base_node::BlockBodyResponse::from(b)), + Ok(b) => proto::base_node::BlockBodyResponse::try_from(b).map_err(|e| { + log::error!(target: LOG_TARGET, "Internal error: {}", e); + RpcStatus::general_default() + }), Err(err) => Err(err), }); diff --git a/base_layer/core/src/blocks/block.rs b/base_layer/core/src/blocks/block.rs index 30f606d07a..808a56a3be 100644 --- a/base_layer/core/src/blocks/block.rs +++ b/base_layer/core/src/blocks/block.rs @@ -134,6 +134,14 @@ impl Block { let (i, o, k) = self.body.dissolve(); (self.header, i, o, k) } + + /// Return a cloned version of this block with the TransactionInputs in their compact form + pub fn to_compact(&self) -> Self { + Self { + header: self.header.clone(), + body: self.body.to_compact(), + } + } } impl Display for Block { diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index ad9eb4dd0d..925513dde4 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -78,7 +78,7 @@ use crate::{ common::rolling_vec::RollingVec, consensus::{chain_strength_comparer::ChainStrengthComparer, ConsensusConstants, ConsensusManager}, proof_of_work::{monero_rx::MoneroPowData, PowAlgorithm, TargetDifficultyWindow}, - transactions::transaction::TransactionKernel, + transactions::transaction::{TransactionInput, TransactionKernel}, validation::{ helpers::calc_median_timestamp, DifficultyCalculator, @@ -1146,7 +1146,7 @@ pub fn calculate_mmr_roots(db: &T, block: &Block) -> Resul } for input in body.inputs().iter() { - input_mmr.push(input.hash())?; + input_mmr.push(input.canonical_hash()?)?; // Search the DB for the output leaf index so that it can be marked as spent/deleted. // If the output hash is not found, check the current output_mmr. This allows zero-conf transactions @@ -1366,7 +1366,36 @@ fn fetch_block(db: &T, height: u64) -> Result o, + Ok(None) => { + return Err(ChainStorageError::InvalidBlock( + "An Input in a block doesn't contain a matching spending output".to_string(), + )) + }, + Err(e) => return Err(e), + }; + + match utxo_mined_info.output { + PrunedOutput::Pruned { .. } => Ok(compact_input), + PrunedOutput::NotPruned { output } => { + compact_input.add_output_data( + output.features, + output.commitment, + output.script, + output.sender_offset_public_key, + ); + Ok(compact_input) + }, + } + }) + .collect::, _>>()?; + let mut unpruned = vec![]; let mut pruned = vec![]; for output in outputs { diff --git a/base_layer/core/src/chain_storage/error.rs b/base_layer/core/src/chain_storage/error.rs index 96c207fc5c..db73f7ea54 100644 --- a/base_layer/core/src/chain_storage/error.rs +++ b/base_layer/core/src/chain_storage/error.rs @@ -26,7 +26,13 @@ use tari_storage::lmdb_store::LMDBError; use thiserror::Error; use tokio::task; -use crate::{blocks::BlockError, chain_storage::MmrTree, proof_of_work::PowError, validation::ValidationError}; +use crate::{ + blocks::BlockError, + chain_storage::MmrTree, + proof_of_work::PowError, + transactions::transaction::TransactionError, + validation::ValidationError, +}; #[derive(Debug, Error)] pub enum ChainStorageError { @@ -117,6 +123,10 @@ pub enum ChainStorageError { DatabaseResyncRequired(&'static str), #[error("Block error: {0}")] BlockError(#[from] BlockError), + #[error("Transaction Error: {0}")] + TransactionError(#[from] TransactionError), + #[error("Could not convert data:{0}")] + ConversionError(String), } impl ChainStorageError { diff --git a/base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs b/base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs index b47ac1e417..fbd8a7dbe1 100644 --- a/base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs +++ b/base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs @@ -102,7 +102,7 @@ use crate::{ }, transactions::{ aggregated_body::AggregateBody, - transaction::{TransactionInput, TransactionKernel, TransactionOutput}, + transaction::{TransactionError, TransactionInput, TransactionKernel, TransactionOutput}, }, }; @@ -645,7 +645,7 @@ impl LMDBDatabase { lmdb_delete( txn, &self.utxo_commitment_index, - input.commitment().as_bytes(), + input.commitment()?.as_bytes(), "utxo_commitment_index", ) .or_else(|err| match err { @@ -661,8 +661,8 @@ impl LMDBDatabase { "deleted_txo_mmr_position_to_height_index", )?; - if let Some(ref unique_id) = input.features.unique_id { - let parent_public_key = input.features.parent_public_key.as_ref(); + if let Some(ref unique_id) = input.features()?.unique_id { + let parent_public_key = input.features()?.parent_public_key.as_ref(); // Move the "current" UTXO entry to a key containing the spend height let mut key = UniqueIdIndexKey::new(parent_public_key, unique_id.as_slice()); let expected_output_hash = lmdb_get::<_, HashOutput>(txn, &self.unique_id_index, key.as_bytes())? @@ -708,14 +708,14 @@ impl LMDBDatabase { )?; } - let hash = input.hash(); + let hash = input.canonical_hash()?; let key = InputKey::new(header_hash, mmr_position, &hash); lmdb_insert( txn, &*self.inputs_db, key.as_bytes(), &TransactionInputRowDataRef { - input, + input: &input.to_compact(), header_hash, mmr_position, hash: &hash, @@ -994,12 +994,39 @@ impl LMDBDatabase { if output_rows.iter().any(|r| r.hash == output_hash) { continue; } - trace!(target: LOG_TARGET, "Input moved to UTXO set: {}", row.input); + let mut input = row.input.clone(); + + let utxo_mined_info = + self.fetch_output_in_txn(txn, &output_hash)? + .ok_or_else(|| ChainStorageError::ValueNotFound { + entity: "UTXO", + field: "hash", + value: output_hash.to_hex(), + })?; + + match utxo_mined_info.output { + PrunedOutput::Pruned { .. } => { + debug!(target: LOG_TARGET, "Output Transaction Input is spending is pruned"); + return Err(ChainStorageError::TransactionError( + TransactionError::MissingTransactionInputData, + )); + }, + PrunedOutput::NotPruned { output } => { + input.add_output_data( + output.features, + output.commitment, + output.script, + output.sender_offset_public_key, + ); + }, + } + + trace!(target: LOG_TARGET, "Input moved to UTXO set: {}", input); lmdb_insert( txn, &*self.utxo_commitment_index, - row.input.commitment.as_bytes(), - &row.input.output_hash(), + input.commitment()?.as_bytes(), + &input.output_hash(), "utxo_commitment_index", )?; lmdb_delete( @@ -1008,8 +1035,9 @@ impl LMDBDatabase { &row.mmr_position, "deleted_txo_mmr_position_to_height_index", )?; - if let Some(unique_id) = row.input.features.unique_asset_id() { - let mut key = UniqueIdIndexKey::new(row.input.features.parent_public_key.as_ref(), unique_id); + + if let Some(unique_id) = input.features()?.unique_asset_id() { + let mut key = UniqueIdIndexKey::new(input.features()?.parent_public_key.as_ref(), unique_id); // The output that made this input that is being unspent is now at the head lmdb_replace(txn, &self.unique_id_index, key.as_bytes(), &output_hash)?; @@ -1201,7 +1229,7 @@ impl LMDBDatabase { target: LOG_TARGET, "Input {} spends output from current block (0-conf)", input ); - spent_zero_conf_commitments.push(&input.commitment); + spent_zero_conf_commitments.push(input.commitment()?); index }, None => return Err(ChainStorageError::UnspendableInput), @@ -1213,7 +1241,7 @@ impl LMDBDatabase { index ))); } - debug!(target: LOG_TARGET, "Inserting input `{}`", input.commitment.to_hex()); + debug!(target: LOG_TARGET, "Inserting input `{}`", input.commitment()?.to_hex()); self.insert_input(txn, current_header_at_height.height, &block_hash, input, index)?; } @@ -1900,7 +1928,7 @@ impl BlockchainBackend for LMDBDatabase { fn fetch_output(&self, output_hash: &HashOutput) -> Result, ChainStorageError> { debug!(target: LOG_TARGET, "Fetch output: {}", output_hash.to_hex()); let txn = self.read_transaction()?; - self.fetch_output_in_txn(&txn, output_hash) + self.fetch_output_in_txn(&*txn, output_hash) } fn fetch_unspent_output_hash_by_commitment( diff --git a/base_layer/core/src/mempool/proto/mempool_request.rs b/base_layer/core/src/mempool/proto/mempool_request.rs index c235ee9146..6509fc7bbc 100644 --- a/base_layer/core/src/mempool/proto/mempool_request.rs +++ b/base_layer/core/src/mempool/proto/mempool_request.rs @@ -48,15 +48,17 @@ impl TryInto for ProtoMempoolRequest { } } -impl From for ProtoMempoolRequest { - fn from(request: MempoolRequest) -> Self { +impl TryFrom for ProtoMempoolRequest { + type Error = String; + + fn try_from(request: MempoolRequest) -> Result { use MempoolRequest::*; - match request { + Ok(match request { GetStats => ProtoMempoolRequest::GetStats(true), GetState => ProtoMempoolRequest::GetState(true), GetTxStateByExcessSig(excess_sig) => ProtoMempoolRequest::GetTxStateByExcessSig(excess_sig.into()), - SubmitTransaction(tx) => ProtoMempoolRequest::SubmitTransaction(tx.into()), - } + SubmitTransaction(tx) => ProtoMempoolRequest::SubmitTransaction(tx.try_into()?), + }) } } diff --git a/base_layer/core/src/mempool/proto/mempool_response.rs b/base_layer/core/src/mempool/proto/mempool_response.rs index 58978d0ce7..900115c4ec 100644 --- a/base_layer/core/src/mempool/proto/mempool_response.rs +++ b/base_layer/core/src/mempool/proto/mempool_response.rs @@ -63,16 +63,18 @@ impl TryFrom for MempoolServiceResponse { } } -impl From for ProtoMempoolResponse { - fn from(response: MempoolResponse) -> Self { +impl TryFrom for ProtoMempoolResponse { + type Error = String; + + fn try_from(response: MempoolResponse) -> Result { use MempoolResponse::*; - match response { + Ok(match response { Stats(stats_response) => ProtoMempoolResponse::Stats(stats_response.into()), - State(state_response) => ProtoMempoolResponse::State(state_response.into()), + State(state_response) => ProtoMempoolResponse::State(state_response.try_into()?), TxStorage(tx_storage_response) => { let tx_storage_response: ProtoTxStorageResponse = tx_storage_response.into(); ProtoMempoolResponse::TxStorage(tx_storage_response.into()) }, - } + }) } } diff --git a/base_layer/core/src/mempool/proto/state_response.rs b/base_layer/core/src/mempool/proto/state_response.rs index fa0c205e49..1767adf747 100644 --- a/base_layer/core/src/mempool/proto/state_response.rs +++ b/base_layer/core/src/mempool/proto/state_response.rs @@ -25,9 +25,12 @@ use std::convert::{TryFrom, TryInto}; use tari_common_types::types::{PrivateKey, PublicKey, Signature}; use tari_crypto::tari_utilities::{ByteArray, ByteArrayError}; -use crate::mempool::{ - proto::mempool::{Signature as ProtoSignature, StateResponse as ProtoStateResponse}, - StateResponse, +use crate::{ + mempool::{ + proto::mempool::{Signature as ProtoSignature, StateResponse as ProtoStateResponse}, + StateResponse, + }, + proto::{mempool::Signature as SignatureProto, types::Transaction}, }; //---------------------------------- Signature --------------------------------------------// @@ -74,11 +77,21 @@ impl TryFrom for StateResponse { } } -impl From for ProtoStateResponse { - fn from(state: StateResponse) -> Self { - Self { - unconfirmed_pool: state.unconfirmed_pool.into_iter().map(Into::into).collect(), - reorg_pool: state.reorg_pool.into_iter().map(Into::into).collect(), - } +impl TryFrom for ProtoStateResponse { + type Error = String; + + fn try_from(state: StateResponse) -> Result { + Ok(Self { + unconfirmed_pool: state + .unconfirmed_pool + .into_iter() + .map(TryInto::try_into) + .collect::, _>>()?, + reorg_pool: state + .reorg_pool + .into_iter() + .map(Into::into) + .collect::>(), + }) } } diff --git a/base_layer/core/src/mempool/rpc/service.rs b/base_layer/core/src/mempool/rpc/service.rs index 30e025fef3..cb9f199f51 100644 --- a/base_layer/core/src/mempool/rpc/service.rs +++ b/base_layer/core/src/mempool/rpc/service.rs @@ -64,7 +64,10 @@ impl MempoolService for MempoolRpcService { async fn get_state(&self, _: Request<()>) -> Result, RpcStatus> { let state = self.mempool().get_state().await.map_err(to_internal_error)?; - Ok(Response::new(state.into())) + Ok(Response::new(state.try_into().map_err(|e: String| { + error!(target: LOG_TARGET, "Internal error: {}", e); + RpcStatus::general(e) + })?)) } async fn get_transaction_state_by_excess_sig( diff --git a/base_layer/core/src/mempool/rpc/test.rs b/base_layer/core/src/mempool/rpc/test.rs index 4cbb4304fb..1dc43e7c8e 100644 --- a/base_layer/core/src/mempool/rpc/test.rs +++ b/base_layer/core/src/mempool/rpc/test.rs @@ -64,6 +64,8 @@ mod get_stats { } mod get_state { + use std::convert::TryInto; + use super::*; use crate::mempool::{MempoolService, StateResponse}; @@ -79,7 +81,7 @@ mod get_state { let resp = service.get_state(req_mock.request_no_context(())).await.unwrap(); let stats = resp.into_message(); - assert_eq!(stats, expected_state.into()); + assert_eq!(stats, expected_state.try_into().unwrap()); assert_eq!(mempool.get_call_count(), 1); } } diff --git a/base_layer/core/src/mempool/service/error.rs b/base_layer/core/src/mempool/service/error.rs index 5669098f06..b85e7063d1 100644 --- a/base_layer/core/src/mempool/service/error.rs +++ b/base_layer/core/src/mempool/service/error.rs @@ -48,4 +48,6 @@ pub enum MempoolServiceError { TransportChannelError(#[from] TransportChannelError), #[error("Failed to send broadcast message")] BroadcastFailed, + #[error("Conversion error: '{0}'")] + ConversionError(String), } diff --git a/base_layer/core/src/mempool/service/service.rs b/base_layer/core/src/mempool/service/service.rs index 27e3459b6d..44569bcef4 100644 --- a/base_layer/core/src/mempool/service/service.rs +++ b/base_layer/core/src/mempool/service/service.rs @@ -20,7 +20,11 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::{convert::TryInto, sync::Arc, time::Duration}; +use std::{ + convert::{TryFrom, TryInto}, + sync::Arc, + time::Duration, +}; use futures::{pin_mut, stream::StreamExt, Stream}; use log::*; @@ -343,7 +347,7 @@ async fn handle_incoming_request( let message = mempool_proto::MempoolServiceResponse { request_key: inner_msg.request_key, - response: Some(response.into()), + response: Some(response.try_into().map_err(MempoolServiceError::ConversionError)?), }; outbound_message_service @@ -396,7 +400,7 @@ async fn handle_outbound_request( let request_key = generate_request_key(&mut OsRng); let service_request = mempool_proto::MempoolServiceRequest { request_key, - request: Some(request.into()), + request: Some(request.try_into().map_err(MempoolServiceError::ConversionError)?), }; let send_result = outbound_message_service @@ -493,7 +497,10 @@ async fn handle_outbound_tx( NodeDestination::Unknown, OutboundEncryption::ClearText, exclude_peers, - OutboundDomainMessage::new(TariMessageType::NewTransaction, proto::types::Transaction::from(tx)), + OutboundDomainMessage::new( + TariMessageType::NewTransaction, + proto::types::Transaction::try_from(tx).map_err(MempoolServiceError::ConversionError)?, + ), ) .await; diff --git a/base_layer/core/src/mempool/sync_protocol/mod.rs b/base_layer/core/src/mempool/sync_protocol/mod.rs index 2cfae149ef..302596a3f6 100644 --- a/base_layer/core/src/mempool/sync_protocol/mod.rs +++ b/base_layer/core/src/mempool/sync_protocol/mod.rs @@ -528,9 +528,15 @@ where TSubstream: AsyncRead + AsyncWrite + Unpin async fn write_transactions(&mut self, transactions: Vec>) -> Result<(), MempoolProtocolError> { let txns = transactions.into_iter().take(self.config.initial_sync_max_transactions) - .map(|txn| { - proto::TransactionItem { - transaction: Some(Clone::clone(&*txn).into()), + .filter_map(|txn| { + match shared_proto::types::Transaction::try_from((*txn).clone()) { + Ok(txn) => Some(proto::TransactionItem { + transaction: Some(txn), + }), + Err(e) => { + warn!(target: LOG_TARGET, "Could not convert transaction: {}", e); + None + } } }) // Write an empty `TransactionItem` to indicate we're done diff --git a/base_layer/core/src/proto/block.rs b/base_layer/core/src/proto/block.rs index 1fa13b0553..60fd6af5d9 100644 --- a/base_layer/core/src/proto/block.rs +++ b/base_layer/core/src/proto/block.rs @@ -51,12 +51,14 @@ impl TryFrom for Block { } } -impl From for proto::Block { - fn from(block: Block) -> Self { - Self { +impl TryFrom for proto::Block { + type Error = String; + + fn try_from(block: Block) -> Result { + Ok(Self { header: Some(block.header.into()), - body: Some(block.body.into()), - } + body: Some(block.body.try_into()?), + }) } } @@ -92,19 +94,21 @@ impl TryFrom for HistoricalBlock { } } -impl From for proto::HistoricalBlock { - fn from(block: HistoricalBlock) -> Self { +impl TryFrom for proto::HistoricalBlock { + type Error = String; + + fn try_from(block: HistoricalBlock) -> Result { let pruned_output_hashes = block.pruned_outputs().iter().map(|x| x.0.clone()).collect(); let pruned_witness_hash = block.pruned_outputs().iter().map(|x| x.1.clone()).collect(); let (block, accumulated_data, confirmations, pruned_input_count) = block.dissolve(); - Self { + Ok(Self { confirmations, accumulated_data: Some(accumulated_data.into()), - block: Some(block.into()), + block: Some(block.try_into()?), pruned_output_hashes, pruned_witness_hash, pruned_input_count, - } + }) } } @@ -169,15 +173,17 @@ impl TryFrom for NewBlockTemplate { } } -impl From for proto::NewBlockTemplate { - fn from(block_template: NewBlockTemplate) -> Self { - Self { +impl TryFrom for proto::NewBlockTemplate { + type Error = String; + + fn try_from(block_template: NewBlockTemplate) -> Result { + Ok(Self { header: Some(block_template.header.into()), - body: Some(block_template.body.into()), + body: Some(block_template.body.try_into()?), target_difficulty: block_template.target_difficulty.as_u64(), reward: block_template.reward.0, total_fees: block_template.total_fees.0, - } + }) } } diff --git a/base_layer/core/src/proto/mod.rs b/base_layer/core/src/proto/mod.rs index f097724dae..cf90e020d0 100644 --- a/base_layer/core/src/proto/mod.rs +++ b/base_layer/core/src/proto/mod.rs @@ -22,7 +22,7 @@ //! Imports of code generated from protobuf files -mod transaction; +pub mod transaction; mod types_impls; pub mod base_node { diff --git a/base_layer/core/src/proto/transaction.proto b/base_layer/core/src/proto/transaction.proto index 174fee741e..90ce2283f5 100644 --- a/base_layer/core/src/proto/transaction.proto +++ b/base_layer/core/src/proto/transaction.proto @@ -42,6 +42,8 @@ message TransactionInput { ComSignature script_signature = 6; // The offset pubkey, K_O bytes sender_offset_public_key = 7; + // The hash of the output this input is spending + bytes output_hash = 8; } // Output for a transaction, defining the new ownership of coins that are being transferred. The commitment is a diff --git a/base_layer/core/src/proto/transaction.rs b/base_layer/core/src/proto/transaction.rs index 4c748ae9e3..1e87c90c63 100644 --- a/base_layer/core/src/proto/transaction.rs +++ b/base_layer/core/src/proto/transaction.rs @@ -101,46 +101,88 @@ impl TryFrom for TransactionInput { type Error = String; fn try_from(input: proto::types::TransactionInput) -> Result { - let features = input - .features - .map(TryInto::try_into) - .ok_or_else(|| "transaction output features not provided".to_string())??; - - let commitment = input - .commitment - .map(|commit| Commitment::from_bytes(&commit.data)) - .ok_or_else(|| "Transaction output commitment not provided".to_string())? - .map_err(|err| err.to_string())?; - let script_signature = input .script_signature .ok_or_else(|| "script_signature not provided".to_string())? .try_into() .map_err(|err: ByteArrayError| err.to_string())?; - let sender_offset_public_key = - PublicKey::from_bytes(input.sender_offset_public_key.as_bytes()).map_err(|err| format!("{:?}", err))?; - - Ok(Self { - features, - commitment, - script: TariScript::from_bytes(input.script.as_slice()).map_err(|err| format!("{:?}", err))?, - input_data: ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?, - script_signature, - sender_offset_public_key, - }) + // Check if the received Transaction input is in compact form or not + if let Some(commitment) = input.commitment { + let commitment = Commitment::from_bytes(&commitment.data).map_err(|e| e.to_string())?; + let features = input + .features + .map(TryInto::try_into) + .ok_or_else(|| "transaction output features not provided".to_string())??; + + let sender_offset_public_key = + PublicKey::from_bytes(input.sender_offset_public_key.as_bytes()).map_err(|err| format!("{:?}", err))?; + + Ok(TransactionInput::new_with_output_data( + features, + commitment, + TariScript::from_bytes(input.script.as_slice()).map_err(|err| format!("{:?}", err))?, + ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?, + script_signature, + sender_offset_public_key, + )) + } else { + if input.output_hash.is_empty() { + return Err("Compact Transaction Input does not contain `output_hash`".to_string()); + } + Ok(TransactionInput::new_with_output_hash( + input.output_hash, + ExecutionStack::from_bytes(input.input_data.as_slice()).map_err(|err| format!("{:?}", err))?, + script_signature, + )) + } } } -impl From for proto::types::TransactionInput { - fn from(input: TransactionInput) -> Self { - Self { - features: Some(input.features.into()), - commitment: Some(input.commitment.into()), - script: input.script.as_bytes(), - input_data: input.input_data.as_bytes(), - script_signature: Some(input.script_signature.into()), - sender_offset_public_key: input.sender_offset_public_key.as_bytes().to_vec(), +impl TryFrom for proto::types::TransactionInput { + type Error = String; + + fn try_from(input: TransactionInput) -> Result { + if input.is_compact() { + let output_hash = input.output_hash(); + Ok(Self { + features: None, + commitment: None, + script: Vec::new(), + input_data: input.input_data.as_bytes(), + script_signature: Some(input.script_signature.into()), + sender_offset_public_key: Vec::new(), + output_hash, + }) + } else { + Ok(Self { + features: Some( + input + .features() + .map_err(|_| "Non-compact Transaction input should contain features".to_string())? + .clone() + .into(), + ), + commitment: Some( + input + .commitment() + .map_err(|_| "Non-compact Transaction input should contain commitment".to_string())? + .clone() + .into(), + ), + script: input + .script() + .map_err(|_| "Non-compact Transaction input should contain script".to_string())? + .as_bytes(), + input_data: input.input_data.as_bytes(), + script_signature: Some(input.script_signature.clone().into()), + sender_offset_public_key: input + .sender_offset_public_key() + .map_err(|_| "Non-compact Transaction input should contain sender_offset_public_key".to_string())? + .as_bytes() + .to_vec(), + output_hash: Vec::new(), + }) } } } @@ -361,14 +403,19 @@ impl TryFrom for AggregateBody { } } -impl From for proto::types::AggregateBody { - fn from(body: AggregateBody) -> Self { +impl TryFrom for proto::types::AggregateBody { + type Error = String; + + fn try_from(body: AggregateBody) -> Result { let (i, o, k) = body.dissolve(); - Self { - inputs: i.into_iter().map(Into::into).collect(), + Ok(Self { + inputs: i + .into_iter() + .map(TryInto::try_into) + .collect::, _>>()?, outputs: o.into_iter().map(Into::into).collect(), kernels: k.into_iter().map(Into::into).collect(), - } + }) } } @@ -401,12 +448,14 @@ impl TryFrom for Transaction { } } -impl From for proto::types::Transaction { - fn from(tx: Transaction) -> Self { - Self { +impl TryFrom for proto::types::Transaction { + type Error = String; + + fn try_from(tx: Transaction) -> Result { + Ok(Self { offset: Some(tx.offset.into()), - body: Some(tx.body.into()), + body: Some(tx.body.try_into()?), script_offset: Some(tx.script_offset.into()), - } + }) } } diff --git a/base_layer/core/src/transactions/aggregated_body.rs b/base_layer/core/src/transactions/aggregated_body.rs index b4673b3f5e..b0019da1c5 100644 --- a/base_layer/core/src/transactions/aggregated_body.rs +++ b/base_layer/core/src/transactions/aggregated_body.rs @@ -328,7 +328,7 @@ impl AggregateBody { /// This function will check all stxo to ensure that feature flags where followed pub fn check_stxo_rules(&self, height: u64) -> Result<(), TransactionError> { for input in self.inputs() { - if input.features.maturity > height { + if input.features()?.maturity > height { warn!( target: LOG_TARGET, "Input found that has not yet matured to spending height: {}", input @@ -377,10 +377,16 @@ impl AggregateBody { } /// Calculate the sum of the outputs - inputs - fn sum_commitments(&self) -> Commitment { - let sum_inputs = &self.inputs.iter().map(|i| &i.commitment).sum::(); + fn sum_commitments(&self) -> Result { + let sum_inputs = &self + .inputs + .iter() + .map(|i| i.commitment()) + .collect::, _>>()? + .into_iter() + .sum::(); let sum_outputs = &self.outputs.iter().map(|o| &o.commitment).sum::(); - sum_outputs - sum_inputs + Ok(sum_outputs - sum_inputs) } /// Calculate the sum of the kernels, taking into account the provided offset, and their constituent fees @@ -409,7 +415,7 @@ impl AggregateBody { ) -> Result<(), TransactionError> { trace!(target: LOG_TARGET, "Checking kernel total"); let KernelSum { sum: excess, fees } = self.sum_kernels(offset_and_reward); - let sum_io = self.sum_commitments(); + let sum_io = self.sum_commitments()?; trace!(target: LOG_TARGET, "Total outputs - inputs:{}", sum_io.to_hex()); let fees = factory.commit_value(&PrivateKey::default(), fees.into()); trace!( @@ -442,7 +448,7 @@ impl AggregateBody { let prev_hash: [u8; 32] = prev_header.unwrap_or_default().as_slice().try_into().unwrap_or([0; 32]); let height = height.unwrap_or_default(); for input in &self.inputs { - let context = ScriptContext::new(height, &prev_hash, &input.commitment); + let context = ScriptContext::new(height, &prev_hash, input.commitment()?); input_keys = input_keys + input.run_and_verify_script(factory, Some(context))?; } @@ -516,6 +522,16 @@ impl AggregateBody { .iter() .fold(0, |max_timelock, kernel| max(max_timelock, kernel.lock_height)) } + + /// Return a cloned version of self with TransactionInputs in their compact form + pub fn to_compact(&self) -> Self { + Self { + sorted: self.sorted, + inputs: self.inputs.iter().map(|i| i.to_compact()).collect(), + outputs: self.outputs.clone(), + kernels: self.kernels.clone(), + } + } } impl PartialEq for AggregateBody { diff --git a/base_layer/core/src/transactions/transaction/error.rs b/base_layer/core/src/transactions/transaction/error.rs index bbe8e18ffa..7e23e5f251 100644 --- a/base_layer/core/src/transactions/transaction/error.rs +++ b/base_layer/core/src/transactions/transaction/error.rs @@ -60,4 +60,6 @@ pub enum TransactionError { ScriptOffset, #[error("Error executing script: {0}")] ScriptExecutionError(String), + #[error("TransactionInput is missing the data from the output being spent")] + MissingTransactionInputData, } diff --git a/base_layer/core/src/transactions/transaction/mod.rs b/base_layer/core/src/transactions/transaction/mod.rs index 58bf2f53ab..a4b3e7c80c 100644 --- a/base_layer/core/src/transactions/transaction/mod.rs +++ b/base_layer/core/src/transactions/transaction/mod.rs @@ -143,8 +143,7 @@ mod test { let input = i .as_transaction_input(&factory) .expect("Should be able to create transaction input"); - assert_eq!(input.features, OutputFeatures::default()); - assert!(input.opened_by(&i, &factory)); + assert!(input.opened_by(&i, &factory).unwrap()); } #[test] @@ -283,8 +282,8 @@ mod test { let input_data = ExecutionStack::default(); let script_signature = ComSignature::default(); let offset_pub_key = PublicKey::default(); - let mut input = TransactionInput::new( - OutputFeatures::default(), + let mut input = TransactionInput::new_with_output_data( + OutputFeatures::with_maturity(5), c, script, input_data, @@ -296,7 +295,7 @@ mod test { let mut tx = Transaction::new(Vec::new(), Vec::new(), Vec::new(), 0.into(), 0.into()); // lets add time locks - input.features.maturity = 5; + input.set_maturity(5).unwrap(); kernel.lock_height = 2; tx.body.add_input(input.clone()); tx.body.add_kernel(kernel.clone()); @@ -307,7 +306,7 @@ mod test { assert_eq!(tx.max_kernel_timelock(), 2); assert_eq!(tx.min_spendable_height(), 5); - input.features.maturity = 4; + input.set_maturity(4).unwrap(); kernel.lock_height = 3; tx.body.add_input(input.clone()); tx.body.add_kernel(kernel.clone()); @@ -316,7 +315,7 @@ mod test { assert_eq!(tx.max_kernel_timelock(), 3); assert_eq!(tx.min_spendable_height(), 5); - input.features.maturity = 2; + input.set_maturity(2).unwrap(); kernel.lock_height = 10; tx.body.add_input(input); tx.body.add_kernel(kernel); diff --git a/base_layer/core/src/transactions/transaction/transaction.rs b/base_layer/core/src/transactions/transaction/transaction.rs index 366602c107..d5687ee776 100644 --- a/base_layer/core/src/transactions/transaction/transaction.rs +++ b/base_layer/core/src/transactions/transaction/transaction.rs @@ -36,7 +36,7 @@ use tari_crypto::tari_utilities::hex::Hex; use crate::transactions::{ aggregated_body::AggregateBody, tari_amount::{uT, MicroTari}, - transaction::{TransactionError, TransactionInput, TransactionKernel, TransactionOutput}, + transaction::{OutputFeatures, TransactionError, TransactionInput, TransactionKernel, TransactionOutput}, weight::TransactionWeight, CryptoFactories, }; @@ -113,16 +113,24 @@ impl Transaction { /// Returns the minimum maturity of the input UTXOs pub fn min_input_maturity(&self) -> u64 { self.body.inputs().iter().fold(u64::MAX, |min_maturity, input| { - min(min_maturity, input.features.maturity) + min( + min_maturity, + input + .features() + .unwrap_or(&OutputFeatures::with_maturity(std::u64::MAX)) + .maturity, + ) }) } /// Returns the maximum maturity of the input UTXOs pub fn max_input_maturity(&self) -> u64 { - self.body - .inputs() - .iter() - .fold(0, |max_maturity, input| max(max_maturity, input.features.maturity)) + self.body.inputs().iter().fold(0, |max_maturity, input| { + max( + max_maturity, + input.features().unwrap_or(&OutputFeatures::with_maturity(0)).maturity, + ) + }) } /// Returns the maximum time lock of the kernels inside of the transaction diff --git a/base_layer/core/src/transactions/transaction/transaction_input.rs b/base_layer/core/src/transactions/transaction/transaction_input.rs index 96a996496c..859caff4f9 100644 --- a/base_layer/core/src/transactions/transaction/transaction_input.rs +++ b/base_layer/core/src/transactions/transaction/transaction_input.rs @@ -30,41 +30,58 @@ use std::{ use blake2::Digest; use serde::{Deserialize, Serialize}; -use tari_common_types::types::{Challenge, ComSignature, Commitment, CommitmentFactory, HashDigest, PublicKey}; +use tari_common_types::types::{ + Challenge, + ComSignature, + Commitment, + CommitmentFactory, + HashDigest, + HashOutput, + PublicKey, +}; use tari_crypto::{ commitment::HomomorphicCommitmentFactory, script::{ExecutionStack, ScriptContext, StackItem, TariScript}, tari_utilities::{hex::Hex, ByteArray, Hashable}, }; -use crate::transactions::{ - transaction, - transaction::{transaction_output::TransactionOutput, OutputFeatures, TransactionError, UnblindedOutput}, +use crate::transactions::transaction::{ + transaction_output::TransactionOutput, + OutputFeatures, + TransactionError, + UnblindedOutput, }; /// A transaction input. /// /// Primarily a reference to an output being spent by the transaction. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct TransactionInput { - /// The features of the output being spent. We will check maturity for all outputs. - pub features: OutputFeatures, - /// The commitment referencing the output being spent. - pub commitment: Commitment, - /// The serialised script - pub script: TariScript, + /// Either the hash of TransactionOutput that this Input is spending or its data + pub spent_output: SpentOutput, /// The script input data, if any pub input_data: ExecutionStack, /// A signature with k_s, signing the script, input data, and mined height pub script_signature: ComSignature, - /// The offset public key, K_O - pub sender_offset_public_key: PublicKey, } /// An input for a transaction that spends an existing output impl TransactionInput { - /// Create a new Transaction Input - pub fn new( + /// Create a new Transaction Input with just a reference hash of the spent output + pub fn new_with_output_hash( + output_hash: HashOutput, + input_data: ExecutionStack, + script_signature: ComSignature, + ) -> TransactionInput { + TransactionInput { + spent_output: SpentOutput::OutputHash(output_hash), + input_data, + script_signature, + } + } + + /// Create a new Transaction Input with just a reference hash of the spent output + pub fn new_with_output_data( features: OutputFeatures, commitment: Commitment, script: TariScript, @@ -73,13 +90,31 @@ impl TransactionInput { sender_offset_public_key: PublicKey, ) -> TransactionInput { TransactionInput { + spent_output: SpentOutput::OutputData { + features, + commitment, + script, + sender_offset_public_key, + }, + input_data, + script_signature, + } + } + + /// Populate the spent output data fields + pub fn add_output_data( + &mut self, + features: OutputFeatures, + commitment: Commitment, + script: TariScript, + sender_offset_public_key: PublicKey, + ) { + self.spent_output = SpentOutput::OutputData { features, commitment, script, - input_data, - script_signature, sender_offset_public_key, - } + }; } pub fn build_script_challenge( @@ -99,14 +134,45 @@ impl TransactionInput { .to_vec() } - /// Accessor method for the commitment contained in an input - pub fn commitment(&self) -> &Commitment { - &self.commitment + pub fn commitment(&self) -> Result<&Commitment, TransactionError> { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { ref commitment, .. } => Ok(commitment), + } + } + + pub fn features(&self) -> Result<&OutputFeatures, TransactionError> { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { ref features, .. } => Ok(features), + } + } + + pub fn script(&self) -> Result<&TariScript, TransactionError> { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { ref script, .. } => Ok(script), + } + } + + pub fn sender_offset_public_key(&self) -> Result<&PublicKey, TransactionError> { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { + ref sender_offset_public_key, + .. + } => Ok(sender_offset_public_key), + } } /// Checks if the given un-blinded input instance corresponds to this blinded Transaction Input - pub fn opened_by(&self, input: &UnblindedOutput, factory: &CommitmentFactory) -> bool { - factory.open(&input.spending_key, &input.value.into(), &self.commitment) + pub fn opened_by(&self, input: &UnblindedOutput, factory: &CommitmentFactory) -> Result { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { ref commitment, .. } => { + Ok(factory.open(&input.spending_key, &input.value.into(), commitment)) + }, + } } /// This will check if the input and the output is the same transactional output by looking at the commitment and @@ -119,11 +185,17 @@ impl TransactionInput { /// public key. pub fn run_script(&self, context: Option) -> Result { let context = context.unwrap_or_default(); - match self.script.execute_with_context(&self.input_data, &context)? { - StackItem::PublicKey(pubkey) => Ok(pubkey), - _ => Err(TransactionError::ScriptExecutionError( - "The script executed successfully but it did not leave a public key on the stack".to_string(), - )), + + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { ref script, .. } => { + match script.execute_with_context(&self.input_data, &context)? { + StackItem::PublicKey(pubkey) => Ok(pubkey), + _ => Err(TransactionError::ScriptExecutionError( + "The script executed successfully but it did not leave a public key on the stack".to_string(), + )), + } + }, } } @@ -132,22 +204,31 @@ impl TransactionInput { public_script_key: &PublicKey, factory: &CommitmentFactory, ) -> Result<(), TransactionError> { - let challenge = TransactionInput::build_script_challenge( - self.script_signature.public_nonce(), - &self.script, - &self.input_data, - public_script_key, - &self.commitment, - ); - if self - .script_signature - .verify_challenge(&(&self.commitment + public_script_key), &challenge, factory) - { - Ok(()) - } else { - Err(TransactionError::InvalidSignatureError( - "Verifying script signature".to_string(), - )) + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { + ref script, + ref commitment, + .. + } => { + let challenge = TransactionInput::build_script_challenge( + self.script_signature.public_nonce(), + script, + &self.input_data, + public_script_key, + commitment, + ); + if self + .script_signature + .verify_challenge(&(commitment + public_script_key), &challenge, factory) + { + Ok(()) + } else { + Err(TransactionError::InvalidSignatureError( + "Verifying script signature".to_string(), + )) + } + }, } } @@ -164,55 +245,132 @@ impl TransactionInput { } /// Returns true if this input is mature at the given height, otherwise false - pub fn is_mature_at(&self, block_height: u64) -> bool { - self.features.maturity <= block_height + pub fn is_mature_at(&self, block_height: u64) -> Result { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { ref features, .. } => Ok(features.maturity <= block_height), + } } /// Returns the hash of the output data contained in this input. /// This hash matches the hash of a transaction output that this input spends. pub fn output_hash(&self) -> Vec { - transaction::hash_output(&self.features, &self.commitment, &self.script) + match self.spent_output { + SpentOutput::OutputHash(ref h) => h.clone(), + SpentOutput::OutputData { + ref commitment, + ref script, + ref features, + .. + } => HashDigest::new() + .chain(features.to_v1_bytes()) + .chain(commitment.as_bytes()) + .chain(script.as_bytes()) + .finalize() + .to_vec(), + } } -} -/// Implement the canonical hashing function for TransactionInput for use in ordering -impl Hashable for TransactionInput { - fn hash(&self) -> Vec { - HashDigest::new() - .chain(self.features.to_v1_bytes()) - .chain(self.commitment.as_bytes()) - .chain(self.script.as_bytes()) - .chain(self.sender_offset_public_key.as_bytes()) - .chain(self.script_signature.u().as_bytes()) - .chain(self.script_signature.v().as_bytes()) - .chain(self.script_signature.public_nonce().as_bytes()) - .chain(self.input_data.as_bytes()) - .finalize() - .to_vec() + pub fn is_compact(&self) -> bool { + matches!(self.spent_output, SpentOutput::OutputHash(_)) + } + + /// Implement the canonical hashing function for TransactionInput for use in ordering + pub fn canonical_hash(&self) -> Result, TransactionError> { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { + ref features, + ref commitment, + ref script, + ref sender_offset_public_key, + } => Ok(HashDigest::new() + .chain(features.to_v1_bytes()) + .chain(commitment.as_bytes()) + .chain(script.as_bytes()) + .chain(sender_offset_public_key.as_bytes()) + .chain(self.script_signature.u().as_bytes()) + .chain(self.script_signature.v().as_bytes()) + .chain(self.script_signature.public_nonce().as_bytes()) + .chain(self.input_data.as_bytes()) + .finalize() + .to_vec()), + } + } + + pub fn set_maturity(&mut self, maturity: u64) -> Result<(), TransactionError> { + if let SpentOutput::OutputData { ref mut features, .. } = self.spent_output { + features.maturity = maturity; + Ok(()) + } else { + Err(TransactionError::MissingTransactionInputData) + } + } + + /// Return a clone of this Input into its compact form + pub fn to_compact(&self) -> Self { + Self { + spent_output: match &self.spent_output { + SpentOutput::OutputHash(h) => SpentOutput::OutputHash(h.clone()), + SpentOutput::OutputData { .. } => SpentOutput::OutputHash(self.output_hash()), + }, + input_data: self.input_data.clone(), + script_signature: self.script_signature.clone(), + } } } impl Display for TransactionInput { fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { - write!( - fmt, - "{} [{:?}], Script hash: ({}), Offset_Pubkey: ({})", - self.commitment.to_hex(), - self.features, - self.script, - self.sender_offset_public_key.to_hex() - ) + match self.spent_output { + SpentOutput::OutputHash(ref h) => write!(fmt, "Input spending Output hash: {}", h.to_hex()), + SpentOutput::OutputData { + ref commitment, + ref script, + ref features, + ref sender_offset_public_key, + } => write!( + fmt, + "{} [{:?}], Script hash: ({}), Offset_Pubkey: ({})", + commitment.to_hex(), + features, + script, + sender_offset_public_key.to_hex() + ), + } + } +} + +impl PartialEq for TransactionInput { + fn eq(&self, other: &Self) -> bool { + self.output_hash() == other.output_hash() && + self.script_signature == other.script_signature && + self.input_data == other.input_data } } +impl Eq for TransactionInput {} + impl PartialOrd for TransactionInput { fn partial_cmp(&self, other: &Self) -> Option { - self.commitment.partial_cmp(&other.commitment) + self.output_hash().partial_cmp(&other.output_hash()) } } impl Ord for TransactionInput { fn cmp(&self, other: &Self) -> Ordering { - self.commitment.cmp(&other.commitment) + self.output_hash().cmp(&other.output_hash()) } } + +#[derive(Clone, Debug, Serialize, Deserialize)] +#[allow(clippy::large_enum_variant)] +pub enum SpentOutput { + OutputHash(HashOutput), + OutputData { + features: OutputFeatures, + commitment: Commitment, + script: TariScript, + sender_offset_public_key: PublicKey, + }, +} diff --git a/base_layer/core/src/transactions/transaction/transaction_output.rs b/base_layer/core/src/transactions/transaction/transaction_output.rs index 183f8f0345..1c353cb346 100644 --- a/base_layer/core/src/transactions/transaction/transaction_output.rs +++ b/base_layer/core/src/transactions/transaction/transaction_output.rs @@ -181,7 +181,7 @@ impl TransactionOutput { /// This will ignore the output range proof #[inline] pub fn is_equal_to(&self, output: &TransactionInput) -> bool { - self.commitment == output.commitment && self.features == output.features + self.hash() == output.output_hash() } /// Returns true if the output is a coinbase, otherwise false diff --git a/base_layer/core/src/transactions/transaction/unblinded_output.rs b/base_layer/core/src/transactions/transaction/unblinded_output.rs index ce7caa5237..f291fc375b 100644 --- a/base_layer/core/src/transactions/transaction/unblinded_output.rs +++ b/base_layer/core/src/transactions/transaction/unblinded_output.rs @@ -42,7 +42,7 @@ use crate::{ tari_amount::MicroTari, transaction, transaction::{ - transaction_input::TransactionInput, + transaction_input::{SpentOutput, TransactionInput}, transaction_output::TransactionOutput, OutputFeatures, TransactionError, @@ -120,12 +120,28 @@ impl UnblindedOutput { .map_err(|_| TransactionError::InvalidSignatureError("Generating script signature".to_string()))?; Ok(TransactionInput { - features: self.features.clone(), - commitment, - script: self.script.clone(), + spent_output: SpentOutput::OutputData { + features: self.features.clone(), + commitment, + script: self.script.clone(), + sender_offset_public_key: self.sender_offset_public_key.clone(), + }, input_data: self.input_data.clone(), script_signature, - sender_offset_public_key: self.sender_offset_public_key.clone(), + }) + } + + /// Commits an UnblindedOutput into a TransactionInput that only contains the hash of the spent output data + pub fn as_compact_transaction_input( + &self, + factory: &CommitmentFactory, + ) -> Result { + let input = self.as_transaction_input(factory)?; + + Ok(TransactionInput { + spent_output: SpentOutput::OutputHash(input.output_hash()), + input_data: input.input_data, + script_signature: input.script_signature, }) } diff --git a/base_layer/core/src/validation/block_validators/async_validator.rs b/base_layer/core/src/validation/block_validators/async_validator.rs index aa27ff65d9..7f95567e93 100644 --- a/base_layer/core/src/validation/block_validators/async_validator.rs +++ b/base_layer/core/src/validation/block_validators/async_validator.rs @@ -32,7 +32,7 @@ use tokio::task; use super::LOG_TARGET; use crate::{ blocks::{Block, BlockHeader}, - chain_storage::{async_db::AsyncBlockchainDb, BlockchainBackend}, + chain_storage::{async_db::AsyncBlockchainDb, BlockchainBackend, PrunedOutput}, consensus::ConsensusManager, iterators::NonOverlappingIntegerPairIter, transactions::{ @@ -95,6 +95,7 @@ impl BlockValidator { // Start all validation tasks concurrently let kernels_task = self.start_kernel_validation(&valid_header, kernels); + let inputs_task = self.start_input_validation(&valid_header, outputs.iter().map(|o| o.hash()).collect(), inputs); @@ -249,7 +250,7 @@ impl BlockValidator { &self, header: &BlockHeader, output_hashes: Vec, - inputs: Vec, + mut inputs: Vec, ) -> AbortOnDropJoinHandle> { let block_height = header.height; let commitment_factory = self.factories.commitment.clone(); @@ -263,13 +264,37 @@ impl BlockValidator { let mut not_found_inputs = Vec::new(); let db = db.db_read_access()?; + // Check for duplicates and/or incorrect sorting for (i, input) in inputs.iter().enumerate() { - // Check for duplicates and/or incorrect sorting if i > 0 && input <= &inputs[i - 1] { return Err(ValidationError::UnsortedOrDuplicateInput); } + } + + for input in inputs.iter_mut() { + // Read the spent_output for this compact input + if input.is_compact() { + let output_mined_info = match db.fetch_output(&input.output_hash())? { + None => return Err(ValidationError::TransactionInputSpentOutputMissing), + Some(o) => o, + }; + + match output_mined_info.output { + PrunedOutput::Pruned { .. } => { + return Err(ValidationError::TransactionInputSpendsPrunedOutput); + }, + PrunedOutput::NotPruned { output } => { + input.add_output_data( + output.features, + output.commitment, + output.script, + output.sender_offset_public_key, + ); + }, + } + } - if !input.is_mature_at(block_height) { + if !input.is_mature_at(block_height)? { warn!( target: LOG_TARGET, "Input found that has not yet matured to spending height: {}", block_height @@ -281,12 +306,12 @@ impl BlockValidator { Err(ValidationError::UnknownInput) => { // Check if the input spends from the current block let output_hash = input.output_hash(); - if output_hashes.iter().all(|hash| *hash != output_hash) { + if output_hashes.iter().all(|hash| hash != &output_hash) { warn!( target: LOG_TARGET, "Validation failed due to input: {} which does not exist yet", input ); - not_found_inputs.push(output_hash); + not_found_inputs.push(output_hash.clone()); } }, Err(err) => return Err(err), @@ -295,12 +320,16 @@ impl BlockValidator { // Once we've found unknown inputs, the aggregate data will be discarded and there is no reason to run // the tari script + let commitment = match input.commitment() { + Ok(c) => c, + Err(e) => return Err(ValidationError::from(e)), + }; if not_found_inputs.is_empty() { - let context = ScriptContext::new(height, &prev_hash, &input.commitment); + let context = ScriptContext::new(height, &prev_hash, commitment); // lets count up the input script public keys aggregate_input_key = aggregate_input_key + input.run_and_verify_script(&commitment_factory, Some(context))?; - commitment_sum = &commitment_sum + &input.commitment; + commitment_sum = &commitment_sum + input.commitment()?; } } diff --git a/base_layer/core/src/validation/error.rs b/base_layer/core/src/validation/error.rs index 1c934630ad..aa21c66fd2 100644 --- a/base_layer/core/src/validation/error.rs +++ b/base_layer/core/src/validation/error.rs @@ -96,6 +96,10 @@ pub enum ValidationError { IncorrectPreviousHash { expected: String, block_hash: String }, #[error("Async validation task failed: {0}")] AsyncTaskFailed(#[from] task::JoinError), + #[error("Could not find the Output being spent by Transaction Input")] + TransactionInputSpentOutputMissing, + #[error("Output being spent by Transaction Input has already been pruned")] + TransactionInputSpendsPrunedOutput, #[error("Bad block with hash {hash} found")] BadBlockFound { hash: String }, #[error("Script exceeded maximum script size, expected less than {max_script_size} but was {actual_script_size}")] diff --git a/base_layer/core/src/validation/helpers.rs b/base_layer/core/src/validation/helpers.rs index cfca45466b..04f7c9c106 100644 --- a/base_layer/core/src/validation/helpers.rs +++ b/base_layer/core/src/validation/helpers.rs @@ -363,16 +363,23 @@ pub fn check_inputs_are_utxos(db: &B, body: &AggregateBody .collect::>(); for input in body.inputs() { // If spending a unique_id, a new output must contain the unique id - if let Some(ref unique_id) = input.features.unique_id { + if let Some(ref unique_id) = input.features()?.unique_id { let exactly_one = output_unique_ids .iter() - .filter(|(parent_public_key, output_unique_id)| { - input.features.parent_public_key.as_ref() == *parent_public_key && unique_id == *output_unique_id + .filter_map(|(parent_public_key, output_unique_id)| match input.features() { + Ok(features) => { + if features.parent_public_key.as_ref() == *parent_public_key && unique_id == *output_unique_id { + Some(Ok((parent_public_key, output_unique_id))) + } else { + None + } + }, + Err(e) => Some(Err(e)), }) .take(2) - .collect::>(); + .collect::, TransactionError>>()?; // Unless a burn flag is present - if input.features.flags.contains(OutputFlags::BURN_NON_FUNGIBLE) { + if input.features()?.flags.contains(OutputFlags::BURN_NON_FUNGIBLE) { if !exactly_one.is_empty() { return Err(ValidationError::UniqueIdBurnedButPresentInOutputs); } @@ -395,7 +402,7 @@ pub fn check_inputs_are_utxos(db: &B, body: &AggregateBody let output_hashes = output_hashes.as_ref().unwrap(); let output_hash = input.output_hash(); - if output_hashes.iter().any(|output| *output == output_hash) { + if output_hashes.iter().any(|output| output == &output_hash) { continue; } @@ -403,7 +410,7 @@ pub fn check_inputs_are_utxos(db: &B, body: &AggregateBody target: LOG_TARGET, "Validation failed due to input: {} which does not exist yet", input ); - not_found_inputs.push(output_hash); + not_found_inputs.push(output_hash.clone()); }, Err(err) => { return Err(err); @@ -421,7 +428,7 @@ pub fn check_inputs_are_utxos(db: &B, body: &AggregateBody /// This function checks that an input is a valid spendable UTXO pub fn check_input_is_utxo(db: &B, input: &TransactionInput) -> Result<(), ValidationError> { let output_hash = input.output_hash(); - if let Some(utxo_hash) = db.fetch_unspent_output_hash_by_commitment(&input.commitment)? { + if let Some(utxo_hash) = db.fetch_unspent_output_hash_by_commitment(input.commitment()?)? { // We know that the commitment exists in the UTXO set. Check that the output hash matches (i.e. all fields // like output features match) if utxo_hash == output_hash { @@ -442,9 +449,9 @@ pub fn check_input_is_utxo(db: &B, input: &TransactionInpu return Err(ValidationError::BlockError(BlockValidationError::InvalidInput)); } - if let Some(unique_id) = &input.features.unique_id { + if let Some(unique_id) = &input.features()?.unique_id { if let Some(utxo_hash) = - db.fetch_utxo_by_unique_id(input.features.parent_public_key.as_ref(), unique_id, None)? + db.fetch_utxo_by_unique_id(input.features()?.parent_public_key.as_ref(), unique_id, None)? { // Check that it is the same utxo in which the unique_id was created if utxo_hash.output.hash() == output_hash { @@ -706,12 +713,25 @@ pub fn check_kernel_lock_height(height: u64, kernels: &[TransactionKernel]) -> R /// Checks that all inputs have matured at the given height pub fn check_maturity(height: u64, inputs: &[TransactionInput]) -> Result<(), TransactionError> { - if let Some(input) = inputs.iter().find(|input| !input.is_mature_at(height)) { - warn!( - target: LOG_TARGET, - "Input found that has not yet matured to spending height: {}", input - ); - return Err(TransactionError::InputMaturity); + if let Err(e) = inputs + .iter() + .map(|input| match input.is_mature_at(height) { + Ok(mature) => { + if !mature { + warn!( + target: LOG_TARGET, + "Input found that has not yet matured to spending height: {}", input + ); + Err(TransactionError::InputMaturity) + } else { + Ok(0) + } + }, + Err(e) => Err(e), + }) + .sum::>() + { + return Err(e); } Ok(()) } @@ -805,11 +825,12 @@ mod test { mod check_maturity { use super::*; + use crate::transactions::transaction::OutputFeatures; #[test] fn it_checks_the_input_maturity() { - let mut input = TransactionInput::new( - Default::default(), + let input = TransactionInput::new_with_output_data( + OutputFeatures::with_maturity(5), Default::default(), Default::default(), Default::default(), @@ -817,8 +838,6 @@ mod test { Default::default(), ); - input.features.maturity = 5; - assert_eq!( check_maturity(1, &[input.clone()]), Err(TransactionError::InputMaturity) diff --git a/base_layer/core/tests/base_node_rpc.rs b/base_layer/core/tests/base_node_rpc.rs index 0564c8a4c2..5d72d1918a 100644 --- a/base_layer/core/tests/base_node_rpc.rs +++ b/base_layer/core/tests/base_node_rpc.rs @@ -145,7 +145,7 @@ async fn test_base_node_wallet_rpc() { assert_eq!(resp.location, TxLocation::NotStored); // First lets try submit tx2 which will be an orphan tx - let msg = TransactionProto::from(tx2.clone()); + let msg = TransactionProto::try_from(tx2.clone()).unwrap(); let req = request_mock.request_with_context(Default::default(), msg); let resp = TxSubmissionResponse::try_from(service.submit_transaction(req).await.unwrap().into_message()).unwrap(); @@ -175,7 +175,7 @@ async fn test_base_node_wallet_rpc() { .unwrap(); // Check that subitting Tx2 will now be accepted - let msg = TransactionProto::from(tx2); + let msg = TransactionProto::try_from(tx2).unwrap(); let req = request_mock.request_with_context(Default::default(), msg); let resp = service.submit_transaction(req).await.unwrap().into_message(); assert!(resp.accepted); @@ -190,7 +190,7 @@ async fn test_base_node_wallet_rpc() { assert_eq!(resp.location, TxLocation::InMempool); // Now if we submit Tx1 is should return as rejected as AlreadyMined as Tx1's kernel is present - let msg = TransactionProto::from(tx1); + let msg = TransactionProto::try_from(tx1).unwrap(); let req = request_mock.request_with_context(Default::default(), msg); let resp = TxSubmissionResponse::try_from(service.submit_transaction(req).await.unwrap().into_message()).unwrap(); @@ -202,7 +202,7 @@ async fn test_base_node_wallet_rpc() { let tx1b = (*txs1b[0]).clone(); // Now if we submit Tx1 is should return as rejected as AlreadyMined - let msg = TransactionProto::from(tx1b); + let msg = TransactionProto::try_from(tx1b).unwrap(); let req = request_mock.request_with_context(Default::default(), msg); let resp = TxSubmissionResponse::try_from(service.submit_transaction(req).await.unwrap().into_message()).unwrap(); diff --git a/base_layer/core/tests/block_validation.rs b/base_layer/core/tests/block_validation.rs index c045e345af..7a21148952 100644 --- a/base_layer/core/tests/block_validation.rs +++ b/base_layer/core/tests/block_validation.rs @@ -193,6 +193,7 @@ fn add_monero_data(tblock: &mut Block, seed_key: &str) { #[tokio::test] async fn inputs_are_not_malleable() { + let _ = env_logger::try_init(); let mut blockchain = TestBlockchain::with_genesis("GB"); let blocks = blockchain.builder(); diff --git a/base_layer/core/tests/mempool.rs b/base_layer/core/tests/mempool.rs index daced7e38f..3230095068 100644 --- a/base_layer/core/tests/mempool.rs +++ b/base_layer/core/tests/mempool.rs @@ -20,7 +20,7 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::{ops::Deref, sync::Arc, time::Duration}; +use std::{convert::TryFrom, ops::Deref, sync::Arc, time::Duration}; use helpers::{ block_builders::{ @@ -915,7 +915,10 @@ async fn receive_and_propagate_transaction() { .outbound_message_service .send_direct( bob_node.node_identity.public_key().clone(), - OutboundDomainMessage::new(TariMessageType::NewTransaction, proto::types::Transaction::from(tx)), + OutboundDomainMessage::new( + TariMessageType::NewTransaction, + proto::types::Transaction::try_from(tx).unwrap(), + ), ) .await .unwrap(); @@ -923,7 +926,10 @@ async fn receive_and_propagate_transaction() { .outbound_message_service .send_direct( carol_node.node_identity.public_key().clone(), - OutboundDomainMessage::new(TariMessageType::NewTransaction, proto::types::Transaction::from(orphan)), + OutboundDomainMessage::new( + TariMessageType::NewTransaction, + proto::types::Transaction::try_from(orphan).unwrap(), + ), ) .await .unwrap(); diff --git a/base_layer/wallet/src/transaction_service/protocols/transaction_broadcast_protocol.rs b/base_layer/wallet/src/transaction_service/protocols/transaction_broadcast_protocol.rs index 2757bbe8a5..f90ab24cfd 100644 --- a/base_layer/wallet/src/transaction_service/protocols/transaction_broadcast_protocol.rs +++ b/base_layer/wallet/src/transaction_service/protocols/transaction_broadcast_protocol.rs @@ -21,7 +21,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use std::{ - convert::TryFrom, + convert::{TryFrom, TryInto}, sync::Arc, time::{Duration, Instant}, }; @@ -182,7 +182,12 @@ where tx: Transaction, client: &mut BaseNodeWalletRpcClient, ) -> Result { - let response = match client.submit_transaction(tx.into()).await { + let response = match client + .submit_transaction(tx.try_into().map_err(|e| { + TransactionServiceProtocolError::new(self.tx_id, TransactionServiceError::InvalidMessageError(e)) + })?) + .await + { Ok(r) => match TxSubmissionResponse::try_from(r) { Ok(r) => r, Err(_) => { diff --git a/base_layer/wallet/src/transaction_service/storage/models.rs b/base_layer/wallet/src/transaction_service/storage/models.rs index ad590212a8..d0c0702571 100644 --- a/base_layer/wallet/src/transaction_service/storage/models.rs +++ b/base_layer/wallet/src/transaction_service/storage/models.rs @@ -191,8 +191,10 @@ impl CompletedTransaction { pub fn get_unique_id(&self) -> Option { let body = self.transaction.body(); for tx_input in body.inputs() { - if let Some(ref unique_id) = tx_input.features.unique_id { - return Some(unique_id.to_hex()); + if let Ok(features) = tx_input.features() { + if let Some(ref unique_id) = features.unique_id { + return Some(unique_id.to_hex()); + } } } for tx_output in body.outputs() { diff --git a/base_layer/wallet/src/transaction_service/tasks/send_finalized_transaction.rs b/base_layer/wallet/src/transaction_service/tasks/send_finalized_transaction.rs index b1520fa744..20436fd7ae 100644 --- a/base_layer/wallet/src/transaction_service/tasks/send_finalized_transaction.rs +++ b/base_layer/wallet/src/transaction_service/tasks/send_finalized_transaction.rs @@ -1,3 +1,4 @@ +use std::convert::TryInto; // Copyright 2020. The Tari Project // // Redistribution and use in source and binary forms, with or without modification, are permitted provided that the @@ -20,7 +21,6 @@ // CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR // OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH // DAMAGE. - use std::time::Duration; use log::*; @@ -64,7 +64,12 @@ pub async fn send_finalized_transaction_message( TransactionRoutingMechanism::StoreAndForwardOnly => { let finalized_transaction_message = proto::TransactionFinalizedMessage { tx_id: tx_id.into(), - transaction: Some(transaction.clone().into()), + transaction: Some( + transaction + .clone() + .try_into() + .map_err(TransactionServiceError::InvalidMessageError)?, + ), }; let store_and_forward_send_result = send_transaction_finalized_message_store_and_forward( tx_id, @@ -92,7 +97,12 @@ pub async fn send_finalized_transaction_message_direct( ) -> Result<(), TransactionServiceError> { let finalized_transaction_message = proto::TransactionFinalizedMessage { tx_id: tx_id.into(), - transaction: Some(transaction.clone().into()), + transaction: Some( + transaction + .clone() + .try_into() + .map_err(TransactionServiceError::InvalidMessageError)?, + ), }; let mut store_and_forward_send_result = false; let mut direct_send_result = false; diff --git a/base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs b/base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs index 143ce198e8..d841e6814f 100644 --- a/base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs +++ b/base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs @@ -473,7 +473,8 @@ where TBackend: WalletBackend + 'static "UTXO (Commitment: {}) imported into wallet", unblinded_output .as_transaction_input(&self.resources.factories.commitment)? - .commitment + .commitment() + .map_err(WalletError::TransactionError)? .to_hex() ); diff --git a/base_layer/wallet/src/wallet.rs b/base_layer/wallet/src/wallet.rs index e70fe9ed18..cff8e3a392 100644 --- a/base_layer/wallet/src/wallet.rs +++ b/base_layer/wallet/src/wallet.rs @@ -401,7 +401,8 @@ where "UTXO (Commitment: {}) imported into wallet", unblinded_output .as_transaction_input(&self.factories.commitment)? - .commitment + .commitment() + .map_err(WalletError::TransactionError)? .to_hex() ); @@ -436,7 +437,8 @@ where "UTXO (Commitment: {}) imported into wallet", unblinded_output .as_transaction_input(&self.factories.commitment)? - .commitment + .commitment() + .map_err(WalletError::TransactionError)? .to_hex() ); diff --git a/base_layer/wallet/tests/support/comms_rpc.rs b/base_layer/wallet/tests/support/comms_rpc.rs index 89c179f118..be28110bf3 100644 --- a/base_layer/wallet/tests/support/comms_rpc.rs +++ b/base_layer/wallet/tests/support/comms_rpc.rs @@ -648,7 +648,7 @@ impl BaseNodeWalletService for BaseNodeWalletRpcMockService { #[cfg(test)] mod test { - use std::convert::TryFrom; + use std::convert::{TryFrom, TryInto}; use tari_common_types::types::BlindingFactor; use tari_comms::{ @@ -711,7 +711,8 @@ mod test { BlindingFactor::default(), ); - let resp = TxSubmissionResponse::try_from(client.submit_transaction(tx.into()).await.unwrap()).unwrap(); + let resp = + TxSubmissionResponse::try_from(client.submit_transaction(tx.try_into().unwrap()).await.unwrap()).unwrap(); assert_eq!(resp.rejection_reason, TxSubmissionRejectionReason::TimeLocked); let calls = service_state diff --git a/base_layer/wallet/tests/transaction_service/service.rs b/base_layer/wallet/tests/transaction_service/service.rs index 610819ba7d..27f962b51b 100644 --- a/base_layer/wallet/tests/transaction_service/service.rs +++ b/base_layer/wallet/tests/transaction_service/service.rs @@ -1582,7 +1582,7 @@ fn finalize_tx_with_incorrect_pubkey() { let finalized_transaction_message = proto::TransactionFinalizedMessage { tx_id: recipient_reply.tx_id.as_u64(), - transaction: Some(tx.clone().into()), + transaction: Some(tx.clone().try_into().unwrap()), }; runtime @@ -1722,7 +1722,8 @@ fn finalize_tx_with_missing_output() { PrivateKey::random(&mut OsRng), PrivateKey::random(&mut OsRng), ) - .into(), + .try_into() + .unwrap(), ), }; @@ -3091,7 +3092,7 @@ fn test_restarting_transaction_protocols() { let finalized_transaction_message = proto::TransactionFinalizedMessage { tx_id: tx_id.as_u64(), - transaction: Some(tx.into()), + transaction: Some(tx.try_into().unwrap()), }; runtime