From 834eff946c7419ca3c4c927cbe7899ff5dcb0a8f Mon Sep 17 00:00:00 2001 From: Philip Robinson Date: Wed, 5 Jan 2022 13:02:32 +0200 Subject: [PATCH] feat!: provide a compact form of TransactionInput (#3460) Description --- 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 Motivation and Context --- 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. How Has This Been Tested? --- cargo test --- 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 | 116 ++++--- .../src/grpc/base_node_grpc_server.rs | 23 +- .../src/automation/commands.rs | 6 +- .../src/grpc/wallet_grpc_server.rs | 2 +- .../src/ui/state/app_state.rs | 6 +- .../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/response.rs | 20 +- base_layer/core/src/base_node/proto/rpc.rs | 14 +- .../core/src/base_node/service/service.rs | 2 +- .../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 | 12 +- .../core/src/chain_storage/lmdb_db/lmdb_db.rs | 56 +++- .../core/src/mempool/proto/mempool_request.rs | 12 +- .../src/mempool/proto/mempool_response.rs | 12 +- .../core/src/mempool/proto/state_response.rs | 31 +- base_layer/core/src/mempool/rpc/service.rs | 5 +- base_layer/core/src/mempool/rpc/test.rs | 4 +- 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/mod.rs | 2 +- base_layer/core/src/proto/transaction.proto | 2 + base_layer/core/src/proto/transaction.rs | 131 +++++--- .../core/src/transactions/aggregated_body.rs | 28 +- .../src/transactions/transaction/error.rs | 2 + .../core/src/transactions/transaction/mod.rs | 13 +- .../transactions/transaction/transaction.rs | 20 +- .../transaction/transaction_input.rs | 302 +++++++++++++----- .../transaction/transaction_output.rs | 2 +- .../transaction/unblinded_output.rs | 26 +- .../block_validators/async_validator.rs | 45 ++- base_layer/core/src/validation/error.rs | 4 + base_layer/core/src/validation/helpers.rs | 59 ++-- base_layer/core/tests/base_node_rpc.rs | 8 +- base_layer/core/tests/block_validation.rs | 1 + base_layer/core/tests/mempool.rs | 12 +- .../transaction_broadcast_protocol.rs | 9 +- .../src/transaction_service/storage/models.rs | 6 +- .../tasks/send_finalized_transaction.rs | 16 +- .../utxo_scanner_service/utxo_scanner_task.rs | 3 +- base_layer/wallet/src/wallet.rs | 6 +- base_layer/wallet/tests/support/comms_rpc.rs | 5 +- .../tests/transaction_service/service.rs | 7 +- 55 files changed, 891 insertions(+), 335 deletions(-) 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