From fb600061c429b00f22bb2d9041d2730f6a7d1112 Mon Sep 17 00:00:00 2001 From: Philip Robinson Date: Mon, 18 Oct 2021 08:50:13 +0200 Subject: [PATCH] feat!: Provide a compact form of TransactionInput This PR updates the TransactionInput struct to support containing the duplicated Output data of the output it is spending OR just the hash of the output. This is done in the original TransactionInput as opposed to creating a new struct to allow for easier reuse of all the code that already supports the TransactionInput. Methods are provided to: - Create a new compact or full TransactionInput - Return a compact form of the current TransactionInput - Provide the output data that is referenced in the TransactionInput This PR also updates the RPC definitions and all the conversion methods to support both forms of the TransactionInput The reason for this change is to reduce the amount of duplicate data stored in the Blockchain db and sent across the wire during Block sync. Currently the input is stored with all the duplicated data from the output it is spending along side the spent output in the database which is a duplication of data. Now the TransactionInput is converted into its compact form with just a reference to the spent output before being stored in the database and before being sent across the wire. When the Input is read from the database or received then the associated output data is retrieved from the local datastore. TODO: apply the compact form of the TransactionInput to transactions submitted to the base node mempool. This will further reduce data sent from wallets to the base nodes. --- applications/tari_app_grpc/proto/types.proto | 2 + .../src/conversions/aggregate_body.rs | 14 +- .../tari_app_grpc/src/conversions/block.rs | 12 +- .../src/conversions/historical_block.rs | 8 +- .../src/conversions/new_block_template.rs | 14 +- .../src/conversions/transaction.rs | 12 +- .../src/conversions/transaction_input.rs | 122 ++++-- .../src/grpc/base_node_grpc_server.rs | 23 +- .../src/automation/commands.rs | 6 +- .../src/grpc/wallet_grpc_server.rs | 4 +- .../src/common/merge_mining.rs | 4 +- .../tari_merge_mining_proxy/src/error.rs | 2 + .../tari_stratum_transcoder/src/error.rs | 2 + .../tari_stratum_transcoder/src/proxy.rs | 5 +- .../core/src/base_node/proto/request.rs | 12 +- .../core/src/base_node/proto/response.rs | 58 +-- base_layer/core/src/base_node/proto/rpc.rs | 13 +- .../core/src/base_node/service/service.rs | 4 +- .../core/src/base_node/sync/rpc/service.rs | 13 +- base_layer/core/src/blocks/block.rs | 8 + .../src/chain_storage/blockchain_database.rs | 35 +- base_layer/core/src/chain_storage/error.rs | 11 +- .../core/src/chain_storage/lmdb_db/lmdb_db.rs | 153 +++++--- .../core/src/mempool/proto/mempool_request.rs | 12 +- .../src/mempool/proto/mempool_response.rs | 12 +- .../core/src/mempool/proto/state_response.rs | 32 +- base_layer/core/src/mempool/rpc/service.rs | 5 +- base_layer/core/src/mempool/rpc/test.rs | 3 +- base_layer/core/src/mempool/service/error.rs | 2 + .../core/src/mempool/service/service.rs | 15 +- .../core/src/mempool/sync_protocol/mod.rs | 12 +- base_layer/core/src/proto/block.rs | 36 +- base_layer/core/src/proto/transaction.proto | 2 + base_layer/core/src/proto/transaction.rs | 131 ++++--- .../core/src/transactions/aggregated_body.rs | 26 +- .../core/src/transactions/transaction.rs | 348 +++++++++++++----- .../block_validators/async_validator.rs | 40 +- base_layer/core/src/validation/error.rs | 4 + base_layer/core/src/validation/helpers.rs | 38 +- base_layer/core/tests/base_node_rpc.rs | 8 +- base_layer/core/tests/block_validation.rs | 1 + base_layer/core/tests/mempool.rs | 18 +- .../recovery/standard_outputs_recoverer.rs | 2 +- .../transaction_broadcast_protocol.rs | 9 +- .../tasks/send_finalized_transaction.rs | 15 +- .../src/utxo_scanner_service/utxo_scanning.rs | 3 +- base_layer/wallet/src/wallet.rs | 6 +- base_layer/wallet/tests/support/comms_rpc.rs | 5 +- .../tests/transaction_service/service.rs | 7 +- 49 files changed, 934 insertions(+), 395 deletions(-) diff --git a/applications/tari_app_grpc/proto/types.proto b/applications/tari_app_grpc/proto/types.proto index 49306660b75..1e5f2f7ec5c 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 bd8a2888005..4974da0a59d 100644 --- a/applications/tari_app_grpc/src/conversions/aggregate_body.rs +++ b/applications/tari_app_grpc/src/conversions/aggregate_body.rs @@ -25,14 +25,16 @@ use tari_core::{proto::utils::try_convert_all, transactions::aggregated_body::Ag 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() @@ -43,7 +45,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 4550b896843..b45caf5c1b0 100644 --- a/applications/tari_app_grpc/src/conversions/block.rs +++ b/applications/tari_app_grpc/src/conversions/block.rs @@ -24,12 +24,14 @@ use crate::tari_rpc as grpc; use std::convert::{TryFrom, TryInto}; use tari_core::blocks::Block; -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 bf2022703a3..58a9bd2a199 100644 --- a/applications/tari_app_grpc/src/conversions/historical_block.rs +++ b/applications/tari_app_grpc/src/conversions/historical_block.rs @@ -21,7 +21,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use crate::tari_rpc as grpc; -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; use tari_core::chain_storage::{ChainStorageError, HistoricalBlock}; impl TryFrom for grpc::HistoricalBlock { @@ -30,7 +30,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 15c41c499b0..cd881ec571c 100644 --- a/applications/tari_app_grpc/src/conversions/new_block_template.rs +++ b/applications/tari_app_grpc/src/conversions/new_block_template.rs @@ -28,8 +28,10 @@ use tari_core::{ crypto::tari_utilities::ByteArray, proof_of_work::ProofOfWork, }; -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, @@ -41,14 +43,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() @@ -63,7 +65,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 cb9e91f63af..4235a89dd6a 100644 --- a/applications/tari_app_grpc/src/conversions/transaction.rs +++ b/applications/tari_app_grpc/src/conversions/transaction.rs @@ -27,13 +27,15 @@ use tari_core::{ transactions::transaction::Transaction, }; -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 48eebe04ad7..0aa73a76406 100644 --- a/applications/tari_app_grpc/src/conversions/transaction_input.rs +++ b/applications/tari_app_grpc/src/conversions/transaction_input.rs @@ -26,61 +26,105 @@ 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, }; 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(grpc::OutputFeatures { - flags: input.features.flags.bits() as u32, - maturity: input.features.maturity, - }), - 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(grpc::OutputFeatures { + flags: features.flags.bits() as u32, + maturity: features.maturity, + }), + 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 7db518cbcc8..6bb1a9501aa 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 @@ -266,9 +266,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 { @@ -439,7 +456,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, }; @@ -478,7 +495,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 dde91c74f42..d45faa5363a 100644 --- a/applications/tari_console_wallet/src/automation/commands.rs +++ b/applications/tari_console_wallet/src/automation/commands.rs @@ -55,6 +55,7 @@ use tari_core::{ }, }; use tari_wallet::{ + error::WalletError, output_manager_service::{handle::OutputManagerHandle, TxId}, transaction_service::handle::{TransactionEvent, TransactionServiceHandle}, WalletSqlite, @@ -752,7 +753,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 cf20599a9b4..58959efed41 100644 --- a/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs +++ b/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs @@ -1,6 +1,6 @@ use futures::{channel::mpsc, future, SinkExt}; use log::*; -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; use tari_app_grpc::{ conversions::naive_datetime_to_timestamp, tari_rpc, @@ -131,7 +131,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_merge_mining_proxy/src/common/merge_mining.rs b/applications/tari_merge_mining_proxy/src/common/merge_mining.rs index 7d232069b72..f013cd98a2c 100644 --- a/applications/tari_merge_mining_proxy/src/common/merge_mining.rs +++ b/applications/tari_merge_mining_proxy/src/common/merge_mining.rs @@ -21,7 +21,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use crate::error::MmProxyError; -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; use tari_app_grpc::tari_rpc as grpc; use tari_core::{ blocks::NewBlockTemplate, @@ -40,5 +40,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 7affa3a6e34..f05888e537a 100644 --- a/applications/tari_merge_mining_proxy/src/error.rs +++ b/applications/tari_merge_mining_proxy/src/error.rs @@ -79,6 +79,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), } impl From for MmProxyError { diff --git a/applications/tari_stratum_transcoder/src/error.rs b/applications/tari_stratum_transcoder/src/error.rs index 67094cdc927..6025e8cc7db 100644 --- a/applications/tari_stratum_transcoder/src/error.rs +++ b/applications/tari_stratum_transcoder/src/error.rs @@ -65,6 +65,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 c165c32934a..588824b4420 100644 --- a/applications/tari_stratum_transcoder/src/proxy.rs +++ b/applications/tari_stratum_transcoder/src/proxy.rs @@ -35,7 +35,7 @@ use json::json; use jsonrpc::error::StandardError; use serde_json as json; use std::{ - convert::TryFrom, + convert::{TryFrom, TryInto}, future::Future, net::SocketAddr, pin::Pin, @@ -295,7 +295,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/request.rs b/base_layer/core/src/base_node/proto/request.rs index bf766bffc23..5d58a3ea7c1 100644 --- a/base_layer/core/src/base_node/proto/request.rs +++ b/base_layer/core/src/base_node/proto/request.rs @@ -87,10 +87,12 @@ impl TryInto for ProtoNodeCommsRequest { } } -impl From for ProtoNodeCommsRequest { - fn from(request: ci::NodeCommsRequest) -> Self { +impl TryFrom for ProtoNodeCommsRequest { + type Error = String; + + fn try_from(request: ci::NodeCommsRequest) -> Result { use ci::NodeCommsRequest::*; - match request { + Ok(match request { GetChainMetadata => ProtoNodeCommsRequest::GetChainMetadata(true), FetchHeaders(block_heights) => ProtoNodeCommsRequest::FetchHeaders(block_heights.into()), FetchHeadersWithHashes(block_hashes) => ProtoNodeCommsRequest::FetchHeadersWithHashes(block_hashes.into()), @@ -117,9 +119,9 @@ impl From for ProtoNodeCommsRequest { max_weight: request.max_weight, }) }, - GetNewBlock(block_template) => ProtoNodeCommsRequest::GetNewBlock(block_template.into()), + GetNewBlock(block_template) => ProtoNodeCommsRequest::GetNewBlock(block_template.try_into()?), FetchKernelByExcessSig(signature) => ProtoNodeCommsRequest::FetchKernelByExcessSig(signature.into()), - } + }) } } diff --git a/base_layer/core/src/base_node/proto/response.rs b/base_layer/core/src/base_node/proto/response.rs index ef88be684f5..f4a6df76210 100644 --- a/base_layer/core/src/base_node/proto/response.rs +++ b/base_layer/core/src/base_node/proto/response.rs @@ -42,7 +42,7 @@ use crate::{ tari_utilities::convert::try_convert_all, }; use std::{ - convert::TryInto, + convert::{TryFrom, TryInto}, iter::{FromIterator, Iterator}, }; @@ -92,41 +92,51 @@ impl TryInto for ProtoNodeCommsResponse { } } -impl From for ProtoNodeCommsResponse { - fn from(response: ci::NodeCommsResponse) -> Self { +impl TryFrom for ProtoNodeCommsResponse { + type Error = String; + + fn try_from(response: ci::NodeCommsResponse) -> Result { use ci::NodeCommsResponse::*; match response { - ChainMetadata(chain_metadata) => ProtoNodeCommsResponse::ChainMetadata(chain_metadata.into()), + ChainMetadata(chain_metadata) => Ok(ProtoNodeCommsResponse::ChainMetadata(chain_metadata.into())), TransactionKernels(kernels) => { let kernels = kernels.into_iter().map(Into::into).collect(); - ProtoNodeCommsResponse::TransactionKernels(kernels) + Ok(ProtoNodeCommsResponse::TransactionKernels(kernels)) }, BlockHeaders(headers) => { let block_headers = headers.into_iter().map(Into::into).collect(); - ProtoNodeCommsResponse::BlockHeaders(block_headers) + Ok(ProtoNodeCommsResponse::BlockHeaders(block_headers)) }, - BlockHeader(header) => ProtoNodeCommsResponse::BlockHeader(header.into()), - HistoricalBlock(block) => ProtoNodeCommsResponse::HistoricalBlock((*block).into()), + BlockHeader(header) => Ok(ProtoNodeCommsResponse::BlockHeader(header.into())), + HistoricalBlock(block) => Ok(ProtoNodeCommsResponse::HistoricalBlock((*block).try_into()?)), FetchHeadersAfterResponse(headers) => { let block_headers = headers.into_iter().map(Into::into).collect(); - ProtoNodeCommsResponse::FetchHeadersAfterResponse(block_headers) + Ok(ProtoNodeCommsResponse::FetchHeadersAfterResponse(block_headers)) }, TransactionOutputs(outputs) => { let outputs = outputs.into_iter().map(Into::into).collect(); - ProtoNodeCommsResponse::TransactionOutputs(outputs) + Ok(ProtoNodeCommsResponse::TransactionOutputs(outputs)) }, HistoricalBlocks(historical_blocks) => { - let historical_blocks = historical_blocks.into_iter().map(Into::into).collect(); - ProtoNodeCommsResponse::HistoricalBlocks(historical_blocks) + let historical_blocks = historical_blocks + .into_iter() + .map(TryInto::try_into) + .collect::, _>>()? + .into_iter() + .map(Into::into) + .collect(); + Ok(ProtoNodeCommsResponse::HistoricalBlocks(historical_blocks)) }, - NewBlockTemplate(block_template) => ProtoNodeCommsResponse::NewBlockTemplate(block_template.into()), - NewBlock { success, error, block } => ProtoNodeCommsResponse::NewBlock(ProtoNewBlockResponse { + NewBlockTemplate(block_template) => { + Ok(ProtoNodeCommsResponse::NewBlockTemplate(block_template.try_into()?)) + }, + NewBlock { success, error, block } => Ok(ProtoNodeCommsResponse::NewBlock(ProtoNewBlockResponse { success, error: error.unwrap_or_else(|| "".to_string()), - block: block.map(|b| b.into()), - }), - TargetDifficulty(difficulty) => ProtoNodeCommsResponse::TargetDifficulty(difficulty.as_u64()), - MmrNodes(added, deleted) => ProtoNodeCommsResponse::MmrNodes(ProtoMmrNodes { added, deleted }), + block: block.map(|b| b.try_into()).transpose()?, + })), + TargetDifficulty(difficulty) => Ok(ProtoNodeCommsResponse::TargetDifficulty(difficulty.as_u64())), + MmrNodes(added, deleted) => Ok(ProtoNodeCommsResponse::MmrNodes(ProtoMmrNodes { added, deleted })), } } } @@ -153,11 +163,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 e28355aeb2b..14f0e8df73b 100644 --- a/base_layer/core/src/base_node/proto/rpc.rs +++ b/base_layer/core/src/base_node/proto/rpc.rs @@ -21,13 +21,16 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use crate::{blocks::Block, chain_storage::PrunedOutput, proto::base_node as proto, tari_utilities::Hashable}; +use std::convert::{TryFrom, TryInto}; -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 1d66cbf1b1d..34fc2a0c053 100644 --- a/base_layer/core/src/base_node/service/service.rs +++ b/base_layer/core/src/base_node/service/service.rs @@ -400,7 +400,7 @@ async fn handle_incoming_request( let message = proto::BaseNodeServiceResponse { request_key: inner_msg.request_key, - response: Some(response.into()), + response: Some(response.try_into().map_err(BaseNodeServiceError::InvalidResponse)?), is_synced, }; @@ -501,7 +501,7 @@ async fn handle_outbound_request( let request_key = generate_request_key(&mut OsRng); let service_request = proto::BaseNodeServiceRequest { request_key, - request: Some(request.into()), + 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 9e20df058a4..7fc64e2a554 100644 --- a/base_layer/core/src/base_node/sync/rpc/service.rs +++ b/base_layer/core/src/base_node/sync/rpc/service.rs @@ -38,6 +38,7 @@ use crate::{ use log::*; use std::{ cmp, + convert::TryFrom, sync::{Arc, Weak}, }; use tari_comms::{ @@ -175,9 +176,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 15c4d9c8efb..5e64daa0dcc 100644 --- a/base_layer/core/src/blocks/block.rs +++ b/base_layer/core/src/blocks/block.rs @@ -136,6 +136,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 6053ccef959..a4765992540 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -50,7 +50,7 @@ use crate::{ consensus::{chain_strength_comparer::ChainStrengthComparer, ConsensusConstants, ConsensusManager}, proof_of_work::{monero_rx::MoneroPowData, PowAlgorithm, TargetDifficultyWindow}, tari_utilities::epoch_time::EpochTime, - transactions::transaction::TransactionKernel, + transactions::transaction::{TransactionInput, TransactionKernel}, validation::{ helpers::calc_median_timestamp, DifficultyCalculator, @@ -1085,7 +1085,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 @@ -1302,7 +1302,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 ac9208025e6..a9399d08133 100644 --- a/base_layer/core/src/chain_storage/error.rs +++ b/base_layer/core/src/chain_storage/error.rs @@ -20,7 +20,12 @@ // 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 crate::{chain_storage::MmrTree, proof_of_work::PowError, validation::ValidationError}; +use crate::{ + chain_storage::MmrTree, + proof_of_work::PowError, + transactions::transaction::TransactionError, + validation::ValidationError, +}; use lmdb_zero::error; use tari_mmr::{error::MerkleMountainRangeError, MerkleProofError}; use tari_storage::lmdb_store::LMDBError; @@ -116,6 +121,10 @@ pub enum ChainStorageError { DbTransactionTooLarge(usize), #[error("DB needs to be resynced: {0}")] DatabaseResyncRequired(&'static str), + #[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 f58eea66bec..6e8235d109b 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 @@ -66,7 +66,7 @@ use crate::{ crypto::tari_utilities::hex::to_hex, transactions::{ aggregated_body::AggregateBody, - transaction::{TransactionInput, TransactionKernel, TransactionOutput}, + transaction::{TransactionError, TransactionInput, TransactionKernel, TransactionOutput}, }, }; use croaring::Bitmap; @@ -606,7 +606,7 @@ impl LMDBDatabase { lmdb_delete( txn, &self.utxo_commitment_index, - input.commitment().as_bytes(), + input.commitment()?.as_bytes(), "utxo_commitment_index", )?; lmdb_insert( @@ -617,14 +617,14 @@ impl LMDBDatabase { "deleted_txo_mmr_position_to_height_index", )?; - let hash = input.hash(); + let hash = input.canonical_hash()?; let key = format!("{}-{:010}-{}", header_hash.to_hex(), mmr_position, hash.to_hex()); lmdb_insert( txn, &*self.inputs_db, key.as_str(), &TransactionInputRowData { - input, + input: input.to_compact(), header_hash, mmr_position, hash, @@ -901,12 +901,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( @@ -1081,7 +1108,7 @@ impl LMDBDatabase { } for input in inputs { - total_utxo_sum = &total_utxo_sum - &input.commitment; + total_utxo_sum = &total_utxo_sum - input.commitment()?; let index = self .fetch_mmr_leaf_index(&**txn, MmrTree::Utxo, &input.output_hash())? .ok_or(ChainStorageError::UnspendableInput)?; @@ -1091,7 +1118,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.clone(), input, index)?; } @@ -1305,6 +1332,63 @@ impl LMDBDatabase { fn fetch_last_header_in_txn(&self, txn: &ConstTransaction<'_>) -> Result, ChainStorageError> { lmdb_last(txn, &self.headers_db) } + + fn fetch_output_in_txn( + &self, + txn: &ConstTransaction<'_>, + output_hash: &HashOutput, + ) -> Result, ChainStorageError> { + if let Some((index, key)) = + lmdb_get::<_, (u32, String)>(txn, &self.txos_hash_to_index_db, output_hash.as_slice())? + { + debug!( + target: LOG_TARGET, + "Fetch output: {} Found ({}, {})", + output_hash.to_hex(), + index, + key + ); + match lmdb_get::<_, TransactionOutputRowData>(txn, &self.utxos_db, key.as_str())? { + Some(TransactionOutputRowData { + output: Some(o), + mmr_position, + mined_height, + header_hash, + .. + }) => Ok(Some(UtxoMinedInfo { + output: PrunedOutput::NotPruned { output: o }, + mmr_position, + mined_height, + header_hash, + })), + Some(TransactionOutputRowData { + output: None, + mmr_position, + mined_height, + hash, + witness_hash, + header_hash, + .. + }) => Ok(Some(UtxoMinedInfo { + output: PrunedOutput::Pruned { + output_hash: hash, + witness_hash, + }, + mmr_position, + mined_height, + header_hash, + })), + _ => Ok(None), + } + } else { + debug!( + target: LOG_TARGET, + "Fetch output: {} NOT found in index", + output_hash.to_hex() + ); + Ok(None) + } + } } pub fn create_recovery_lmdb_database>(path: P) -> Result<(), ChainStorageError> { @@ -1821,56 +1905,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()?; - if let Some((index, key)) = - lmdb_get::<_, (u32, String)>(&txn, &self.txos_hash_to_index_db, output_hash.as_slice())? - { - debug!( - target: LOG_TARGET, - "Fetch output: {} Found ({}, {})", - output_hash.to_hex(), - index, - key - ); - match lmdb_get::<_, TransactionOutputRowData>(&txn, &self.utxos_db, key.as_str())? { - Some(TransactionOutputRowData { - output: Some(o), - mmr_position, - mined_height, - header_hash, - .. - }) => Ok(Some(UtxoMinedInfo { - output: PrunedOutput::NotPruned { output: o }, - mmr_position, - mined_height, - header_hash, - })), - Some(TransactionOutputRowData { - output: None, - mmr_position, - mined_height, - hash, - witness_hash, - header_hash, - .. - }) => Ok(Some(UtxoMinedInfo { - output: PrunedOutput::Pruned { - output_hash: hash, - witness_hash, - }, - mmr_position, - mined_height, - header_hash, - })), - _ => Ok(None), - } - } else { - debug!( - target: LOG_TARGET, - "Fetch output: {} NOT found in index", - output_hash.to_hex() - ); - Ok(None) - } + 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 72b31382fe1..bbf4b39f2cb 100644 --- a/base_layer/core/src/mempool/proto/mempool_request.rs +++ b/base_layer/core/src/mempool/proto/mempool_request.rs @@ -46,15 +46,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 f50bb4d4f8a..716f8796ce8 100644 --- a/base_layer/core/src/mempool/proto/mempool_response.rs +++ b/base_layer/core/src/mempool/proto/mempool_response.rs @@ -62,16 +62,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 8b3af21ac0d..37c9bb5c4de 100644 --- a/base_layer/core/src/mempool/proto/state_response.rs +++ b/base_layer/core/src/mempool/proto/state_response.rs @@ -20,10 +20,14 @@ // 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 crate::mempool::{proto::mempool::StateResponse as ProtoStateResponse, StateResponse}; +use crate::{ + mempool::{ + proto::mempool::{Signature as ProtoSignature, StateResponse as ProtoStateResponse}, + StateResponse, + }, + proto::{mempool::Signature as SignatureProto, types::Transaction}, +}; use std::convert::{TryFrom, TryInto}; -// use crate::transactions::proto::types::Signature as ProtoSignature; -use crate::mempool::proto::mempool::Signature as ProtoSignature; use tari_common_types::types::{PrivateKey, PublicKey, Signature}; use tari_crypto::tari_utilities::{ByteArray, ByteArrayError}; @@ -71,11 +75,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 1444107e077..2b851037722 100644 --- a/base_layer/core/src/mempool/rpc/service.rs +++ b/base_layer/core/src/mempool/rpc/service.rs @@ -62,7 +62,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 a9cbb2ee490..7ac2fe985b4 100644 --- a/base_layer/core/src/mempool/rpc/test.rs +++ b/base_layer/core/src/mempool/rpc/test.rs @@ -65,6 +65,7 @@ mod get_stats { mod get_state { use super::*; use crate::mempool::{MempoolService, StateResponse}; + use std::convert::TryInto; #[tokio::test] async fn it_returns_the_state() { @@ -78,7 +79,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 5e71ac945c9..8c3b64a440d 100644 --- a/base_layer/core/src/mempool/service/error.rs +++ b/base_layer/core/src/mempool/service/error.rs @@ -47,4 +47,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 137dd9d0a02..0687d1c01d8 100644 --- a/base_layer/core/src/mempool/service/service.rs +++ b/base_layer/core/src/mempool/service/service.rs @@ -41,7 +41,11 @@ use crate::{ use futures::{pin_mut, stream::StreamExt, Stream}; use log::*; use rand::rngs::OsRng; -use std::{convert::TryInto, sync::Arc, time::Duration}; +use std::{ + convert::{TryFrom, TryInto}, + sync::Arc, + time::Duration, +}; use tari_common_types::waiting_requests::{generate_request_key, RequestKey, WaitingRequests}; use tari_comms::peer_manager::NodeId; use tari_comms_dht::{ @@ -341,7 +345,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 @@ -394,7 +398,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 @@ -491,7 +495,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 dc133f744d7..73145ca4da1 100644 --- a/base_layer/core/src/mempool/sync_protocol/mod.rs +++ b/base_layer/core/src/mempool/sync_protocol/mod.rs @@ -527,9 +527,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 50778f5a924..0b7327768a9 100644 --- a/base_layer/core/src/proto/block.rs +++ b/base_layer/core/src/proto/block.rs @@ -50,12 +50,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()?), + }) } } @@ -91,19 +93,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, - } + }) } } @@ -168,15 +172,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/transaction.proto b/base_layer/core/src/proto/transaction.proto index 08a1e9775d4..a405c047773 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 93bbfbd46c3..0d0f2ba6e30 100644 --- a/base_layer/core/src/proto/transaction.rs +++ b/base_layer/core/src/proto/transaction.rs @@ -95,46 +95,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: Vec::new(), + 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(), + }) } } } @@ -228,14 +270,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(), - } + }) } } @@ -268,12 +315,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 96df03e91a8..216d7058afd 100644 --- a/base_layer/core/src/transactions/aggregated_body.rs +++ b/base_layer/core/src/transactions/aggregated_body.rs @@ -306,7 +306,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 @@ -352,10 +352,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 @@ -384,7 +390,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!( @@ -474,6 +480,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.rs b/base_layer/core/src/transactions/transaction.rs index 0de8edba7af..5c0b1b3109f 100644 --- a/base_layer/core/src/transactions/transaction.rs +++ b/base_layer/core/src/transactions/transaction.rs @@ -65,6 +65,7 @@ use tari_common_types::types::{ Commitment, CommitmentFactory, HashDigest, + HashOutput, MessageHash, PrivateKey, PublicKey, @@ -203,6 +204,8 @@ pub enum TransactionError { ScriptOffset, #[error("Error executing script: {0}")] ScriptExecutionError(String), + #[error("TransactionInput is missing the data from the output being spent")] + MissingTransactionInputData, } //----------------------------------------- UnblindedOutput ----------------------------------------------------// @@ -272,12 +275,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, }) } @@ -375,26 +394,33 @@ impl Ord for 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, @@ -403,13 +429,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( @@ -430,13 +474,48 @@ impl TransactionInput { } /// 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), + } + } + + /// Accessor method for the commitment contained in an input + pub fn features(&self) -> Result<&OutputFeatures, TransactionError> { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { ref features, .. } => Ok(features), + } + } + + /// Accessor method for the commitment contained in an input + pub fn script(&self) -> Result<&TariScript, TransactionError> { + match self.spent_output { + SpentOutput::OutputHash(_) => Err(TransactionError::MissingTransactionInputData), + SpentOutput::OutputData { ref script, .. } => Ok(script), + } + } + + /// Accessor method for the commitment contained in an input + 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 @@ -448,11 +527,14 @@ impl TransactionInput { /// This will run the script contained in the TransactionInput, returning either a script error or the resulting /// public key. pub fn run_script(&self) -> Result { - match self.script.execute(&self.input_data)? { - 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(&self.input_data)? { + 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(), + )), + }, } } @@ -461,22 +543,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(), + )) + } + }, } } @@ -489,64 +580,136 @@ 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 { - HashDigest::new() - .chain(self.features.to_bytes()) - .chain(self.commitment.as_bytes()) - .chain(self.script.as_bytes()) - .finalize() - .to_vec() + match self.spent_output { + SpentOutput::OutputHash(ref h) => h.clone(), + SpentOutput::OutputData { + ref commitment, + ref script, + ref features, + .. + } => HashDigest::new() + .chain(features.to_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_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_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, + }, +} + //---------------------------------------- TransactionOutput ----------------------------------------------------// /// Output for a transaction, defining the new ownership of coins that are being transferred. The commitment is a @@ -658,7 +821,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 @@ -1175,16 +1338,24 @@ impl Transaction { /// Returns the minimum maturity of the input UTXOs pub fn min_input_maturity(&self) -> u64 { self.body.inputs().iter().fold(std::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 @@ -1367,8 +1538,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] @@ -1507,8 +1677,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, @@ -1520,7 +1690,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()); @@ -1531,7 +1701,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()); @@ -1540,7 +1710,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/validation/block_validators/async_validator.rs b/base_layer/core/src/validation/block_validators/async_validator.rs index 4acab94d651..a689be79703 100644 --- a/base_layer/core/src/validation/block_validators/async_validator.rs +++ b/base_layer/core/src/validation/block_validators/async_validator.rs @@ -22,7 +22,7 @@ use super::LOG_TARGET; use crate::{ blocks::{Block, BlockHeader}, - chain_storage::{async_db::AsyncBlockchainDb, BlockchainBackend}, + chain_storage::{async_db::AsyncBlockchainDb, BlockchainBackend, PrunedOutput}, consensus::ConsensusManager, crypto::tari_utilities::Hashable, iterators::NonOverlappingIntegerPairIter, @@ -236,12 +236,38 @@ impl BlockValidator { let mut not_found_inputs = Vec::new(); let db = db.db_read_access()?; for (i, input) in inputs.iter().enumerate() { + // Read the spent_output for this compact input + let 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 } => { + let mut full_input = input.clone(); + full_input.add_output_data( + output.features, + output.commitment, + output.script, + output.sender_offset_public_key, + ); + full_input + }, + } + } else { + input.clone() + }; + // Check for duplicates and/or incorrect sorting - if i > 0 && input <= &inputs[i - 1] { + if i > 0 && input <= inputs[i - 1] { return Err(ValidationError::UnsortedOrDuplicateInput); } - 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 @@ -249,16 +275,16 @@ impl BlockValidator { return Err(TransactionError::InputMaturity.into()); } - match helpers::check_input_is_utxo(&*db, input) { + match helpers::check_input_is_utxo(&*db, &input) { 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), @@ -270,7 +296,7 @@ impl BlockValidator { if not_found_inputs.is_empty() { // lets count up the input script public keys aggregate_input_key = aggregate_input_key + input.run_and_verify_script(&commitment_factory)?; - 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 10f5225d04e..ee4ae528e70 100644 --- a/base_layer/core/src/validation/error.rs +++ b/base_layer/core/src/validation/error.rs @@ -87,6 +87,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, } // ChainStorageError has a ValidationError variant, so to prevent a cyclic dependency we use a string representation in diff --git a/base_layer/core/src/validation/helpers.rs b/base_layer/core/src/validation/helpers.rs index ef6bb8e0db3..65347232667 100644 --- a/base_layer/core/src/validation/helpers.rs +++ b/base_layer/core/src/validation/helpers.rs @@ -341,7 +341,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; } @@ -349,7 +349,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); @@ -367,7 +367,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 { @@ -566,12 +566,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(()) } @@ -665,11 +678,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(), @@ -677,8 +691,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 3b1523d1438..5705bfa4a99 100644 --- a/base_layer/core/tests/base_node_rpc.rs +++ b/base_layer/core/tests/base_node_rpc.rs @@ -162,7 +162,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(); @@ -192,7 +192,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); @@ -207,7 +207,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(); @@ -219,7 +219,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 8b7303ed47b..4d8a4340631 100644 --- a/base_layer/core/tests/block_validation.rs +++ b/base_layer/core/tests/block_validation.rs @@ -199,6 +199,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 011fbf794e5..3c41b50f1a6 100644 --- a/base_layer/core/tests/mempool.rs +++ b/base_layer/core/tests/mempool.rs @@ -21,11 +21,6 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. // use crate::helpers::database::create_store; -use std::{ops::Deref, sync::Arc, time::Duration}; - -use tari_crypto::{keys::PublicKey as PublicKeyTrait, script}; -use tempfile::tempdir; - use helpers::{ block_builders::{ chain_block, @@ -39,6 +34,7 @@ use helpers::{ sample_blockchains::{create_new_blockchain, create_new_blockchain_with_constants}, }; use randomx_rs::RandomXFlag; +use std::{convert::TryFrom, ops::Deref, sync::Arc, time::Duration}; use tari_common::configuration::Network; use tari_common_types::types::{Commitment, PrivateKey, PublicKey, Signature}; use tari_comms_dht::domain_message::OutboundDomainMessage; @@ -64,8 +60,10 @@ use tari_core::{ txn_schema, validation::transaction_validators::{TxConsensusValidator, TxInputAndMaturityValidator}, }; +use tari_crypto::{keys::PublicKey as PublicKeyTrait, script}; use tari_p2p::{services::liveness::LivenessConfig, tari_message::TariMessageType}; use tari_test_utils::async_assert_eventually; +use tempfile::tempdir; #[allow(dead_code)] mod helpers; @@ -888,7 +886,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(); @@ -896,7 +897,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/output_manager_service/recovery/standard_outputs_recoverer.rs b/base_layer/wallet/src/output_manager_service/recovery/standard_outputs_recoverer.rs index f3386a30eac..2fe4445dcd8 100644 --- a/base_layer/wallet/src/output_manager_service/recovery/standard_outputs_recoverer.rs +++ b/base_layer/wallet/src/output_manager_service/recovery/standard_outputs_recoverer.rs @@ -128,7 +128,7 @@ where TBackend: OutputManagerBackend + 'static "Output {} with value {} with {} recovered", output .as_transaction_input(&self.factories.commitment)? - .commitment + .commitment()? .to_hex(), output.value, output.features, 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 61b83891de2..a501fca6a0a 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 @@ -36,7 +36,7 @@ use crate::{ use futures::FutureExt; use log::*; use std::{ - convert::TryFrom, + convert::{TryFrom, TryInto}, sync::Arc, time::{Duration, Instant}, }; @@ -180,7 +180,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::ConversionError(e)) + })?) + .await + { Ok(r) => match TxSubmissionResponse::try_from(r) { Ok(r) => r, Err(_) => { 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 db28a7db584..e41815aab6c 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 @@ -63,7 +64,12 @@ pub async fn send_finalized_transaction_message( TransactionRoutingMechanism::StoreAndForwardOnly => { let finalized_transaction_message = proto::TransactionFinalizedMessage { tx_id, - transaction: Some(transaction.clone().into()), + transaction: Some( + transaction + .clone() + .try_into() + .map_err(TransactionServiceError::ConversionError)?, + ), }; let store_and_forward_send_result = send_transaction_finalized_message_store_and_forward( tx_id, @@ -91,7 +97,12 @@ pub async fn send_finalized_transaction_message_direct( ) -> Result<(), TransactionServiceError> { let finalized_transaction_message = proto::TransactionFinalizedMessage { tx_id, - transaction: Some(transaction.clone().into()), + transaction: Some( + transaction + .clone() + .try_into() + .map_err(TransactionServiceError::ConversionError)?, + ), }; 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_scanning.rs b/base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs index c12d1bdcce1..baf0a12f9fb 100644 --- a/base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs +++ b/base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs @@ -568,7 +568,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 01c55ab3e61..c13f200c071 100644 --- a/base_layer/wallet/src/wallet.rs +++ b/base_layer/wallet/src/wallet.rs @@ -382,7 +382,8 @@ where "UTXO (Commitment: {}) imported into wallet", unblinded_output .as_transaction_input(&self.factories.commitment)? - .commitment + .commitment() + .map_err(WalletError::TransactionError)? .to_hex() ); @@ -417,7 +418,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 3a4f306fc4d..9de87ccc2f1 100644 --- a/base_layer/wallet/tests/support/comms_rpc.rs +++ b/base_layer/wallet/tests/support/comms_rpc.rs @@ -654,7 +654,7 @@ mod test { test_utils::node_identity::build_node_identity, }; - use std::convert::TryFrom; + use std::convert::{TryFrom, TryInto}; use tari_common_types::types::BlindingFactor; use tari_core::{ base_node::{ @@ -709,7 +709,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 b7cff07ba99..2f963561000 100644 --- a/base_layer/wallet/tests/transaction_service/service.rs +++ b/base_layer/wallet/tests/transaction_service/service.rs @@ -1439,7 +1439,7 @@ fn finalize_tx_with_incorrect_pubkey() { let finalized_transaction_message = proto::TransactionFinalizedMessage { tx_id: recipient_reply.tx_id, - transaction: Some(tx.clone().into()), + transaction: Some(tx.clone().try_into().unwrap()), }; runtime @@ -1576,7 +1576,8 @@ fn finalize_tx_with_missing_output() { PrivateKey::random(&mut OsRng), PrivateKey::random(&mut OsRng), ) - .into(), + .try_into() + .unwrap(), ), }; @@ -2939,7 +2940,7 @@ fn test_restarting_transaction_protocols() { let finalized_transaction_message = proto::TransactionFinalizedMessage { tx_id, - transaction: Some(tx.into()), + transaction: Some(tx.try_into().unwrap()), }; runtime