Skip to content

Commit

Permalink
review comments 3
Browse files Browse the repository at this point in the history
  • Loading branch information
hansieodendaal committed Nov 22, 2023
1 parent 39a75da commit 32f9d00
Show file tree
Hide file tree
Showing 14 changed files with 156 additions and 92 deletions.
20 changes: 9 additions & 11 deletions base_layer/wallet/src/output_manager_service/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use std::{fmt, fmt::Formatter, sync::Arc};

use tari_common_types::{
transaction::TxId,
types::{Commitment, HashOutput, PublicKey},
types::{Commitment, FixedHash, HashOutput, PublicKey},
};
use tari_core::{
covenants::Covenant,
Expand All @@ -44,7 +44,7 @@ use tower::Service;

use crate::output_manager_service::{
error::OutputManagerError,
service::{Balance, OutputStatusesByTxId},
service::{Balance, OutputInfoByTxId},
storage::{
database::OutputBackendQuery,
models::{DbWalletOutput, KnownOneSidedPaymentScript, SpendingPriority},
Expand Down Expand Up @@ -121,7 +121,7 @@ pub enum OutputManagerRequest {
ReinstateCancelledInboundTx(TxId),
CreateClaimShaAtomicSwapTransaction(HashOutput, PublicKey, MicroMinotari),
CreateHtlcRefundTransaction(HashOutput, MicroMinotari),
GetOutputStatusesByTxId(TxId),
GetOutputInfoByTxId(TxId),
}

impl fmt::Display for OutputManagerRequest {
Expand Down Expand Up @@ -209,7 +209,7 @@ impl fmt::Display for OutputManagerRequest {
fee_per_gram,
),

GetOutputStatusesByTxId(t) => write!(f, "GetOutputStatusesByTxId: {}", t),
GetOutputInfoByTxId(t) => write!(f, "GetOutputInfoByTxId: {}", t),
}
}
}
Expand Down Expand Up @@ -244,7 +244,7 @@ pub enum OutputManagerResponse {
CreatePayToSelfWithOutputs { transaction: Box<Transaction>, tx_id: TxId },
ReinstatedCancelledInboundTx,
ClaimHtlcTransaction((TxId, MicroMinotari, MicroMinotari, Transaction)),
OutputStatusesByTxId(OutputStatusesByTxId),
OutputInfoByTxId(OutputInfoByTxId),
CoinPreview((Vec<MicroMinotari>, MicroMinotari)),
}

Expand Down Expand Up @@ -288,6 +288,7 @@ pub struct PublicRewindKeys {
pub struct RecoveredOutput {
pub tx_id: TxId,
pub output: WalletOutput,
pub hash: FixedHash,
}

#[derive(Clone)]
Expand Down Expand Up @@ -764,16 +765,13 @@ impl OutputManagerHandle {
}
}

pub async fn get_output_statuses_for_tx_id(
&mut self,
tx_id: TxId,
) -> Result<OutputStatusesByTxId, OutputManagerError> {
pub async fn get_output_info_for_tx_id(&mut self, tx_id: TxId) -> Result<OutputInfoByTxId, OutputManagerError> {
match self
.handle
.call(OutputManagerRequest::GetOutputStatusesByTxId(tx_id))
.call(OutputManagerRequest::GetOutputInfoByTxId(tx_id))
.await??
{
OutputManagerResponse::OutputStatusesByTxId(output_statuses_by_tx_id) => Ok(output_statuses_by_tx_id),
OutputManagerResponse::OutputInfoByTxId(output_info_by_tx_id) => Ok(output_info_by_tx_id),
_ => Err(OutputManagerError::UnexpectedApiResponse),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
use std::time::Instant;

use log::*;
use tari_common_types::transaction::TxId;
use tari_common_types::{transaction::TxId, types::FixedHash};
use tari_core::transactions::{
key_manager::{TariKeyId, TransactionKeyManagerBranch, TransactionKeyManagerInterface},
tari_amount::MicroMinotari,
Expand Down Expand Up @@ -69,7 +69,7 @@ where

let known_scripts = self.db.get_all_known_one_sided_payment_scripts()?;

let mut rewound_outputs: Vec<(WalletOutput, bool)> = Vec::new();
let mut rewound_outputs: Vec<(WalletOutput, bool, FixedHash)> = Vec::new();
let push_pub_key_script = script!(PushPubKey(Box::default()));
for output in outputs {
let known_script_index = known_scripts.iter().position(|s| s.script == output.script);
Expand All @@ -92,6 +92,7 @@ where
None => continue,
};

let hash = output.hash();
let uo = WalletOutput::new_with_rangeproof(
output.version,
committed_value,
Expand All @@ -109,7 +110,7 @@ where
output.proof.clone(),
);

rewound_outputs.push((uo, known_script_index.is_some()));
rewound_outputs.push((uo, known_script_index.is_some(), hash));
}

let rewind_time = start.elapsed();
Expand All @@ -121,7 +122,7 @@ where
);

let mut rewound_outputs_with_tx_id: Vec<RecoveredOutput> = Vec::new();
for (output, has_known_script) in &mut rewound_outputs {
for (output, has_known_script, hash) in &mut rewound_outputs {
let db_output = DbWalletOutput::from_wallet_output(
output.clone(),
&self.master_key_manager,
Expand All @@ -145,6 +146,7 @@ where
rewound_outputs_with_tx_id.push(RecoveredOutput {
output: output.clone(),
tx_id,
hash: *hash,
});
self.update_outputs_script_private_key_and_update_key_manager_index(output)
.await?;
Expand Down
14 changes: 8 additions & 6 deletions base_layer/wallet/src/output_manager_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,14 +408,14 @@ where
.create_htlc_refund_transaction(output, fee_per_gram)
.await
.map(OutputManagerResponse::ClaimHtlcTransaction),
OutputManagerRequest::GetOutputStatusesByTxId(tx_id) => {
let output_statuses_by_tx_id = self.get_output_status_by_tx_id(tx_id)?;
Ok(OutputManagerResponse::OutputStatusesByTxId(output_statuses_by_tx_id))
OutputManagerRequest::GetOutputInfoByTxId(tx_id) => {
let output_statuses_by_tx_id = self.get_output_info_by_tx_id(tx_id)?;
Ok(OutputManagerResponse::OutputInfoByTxId(output_statuses_by_tx_id))
},
}
}

fn get_output_status_by_tx_id(&self, tx_id: TxId) -> Result<OutputStatusesByTxId, OutputManagerError> {
fn get_output_info_by_tx_id(&self, tx_id: TxId) -> Result<OutputInfoByTxId, OutputManagerError> {
let outputs = self.resources.db.fetch_outputs_by_tx_id(tx_id)?;
let statuses = outputs.clone().into_iter().map(|uo| uo.status).collect();
// We need the maximum mined height and corresponding block hash (faux transactions outputs can have different
Expand All @@ -430,7 +430,7 @@ where
}
}
}
Ok(OutputStatusesByTxId {
Ok(OutputInfoByTxId {
statuses,
mined_height: max_mined_height,
block_hash,
Expand Down Expand Up @@ -2368,6 +2368,7 @@ where
committed_value.into(),
)? {
let spending_key_id = self.resources.key_manager.import_key(spending_key).await?;
let hash = output.hash();
let rewound_output = WalletOutput::new_with_rangeproof(
output.version,
committed_value,
Expand Down Expand Up @@ -2408,6 +2409,7 @@ where
rewound_outputs.push(RecoveredOutput {
output: rewound_output,
tx_id,
hash,
})
},
Err(OutputManagerStorageError::DuplicateOutput) => {
Expand Down Expand Up @@ -2510,7 +2512,7 @@ impl UtxoSelection {
}

#[derive(Debug, Clone)]
pub struct OutputStatusesByTxId {
pub struct OutputInfoByTxId {
pub statuses: Vec<OutputStatus>,
pub(crate) mined_height: Option<u64>,
pub(crate) block_hash: Option<BlockHash>,
Expand Down
6 changes: 5 additions & 1 deletion base_layer/wallet/src/transaction_service/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ pub enum TransactionServiceRequest {
tx_id: Option<TxId>,
current_height: Option<u64>,
mined_timestamp: Option<NaiveDateTime>,
scanned_output: TransactionOutput,
},
SubmitTransactionToSelf(TxId, Transaction, MicroMinotari, MicroMinotari, String),
SetLowPowerMode,
Expand Down Expand Up @@ -212,9 +213,10 @@ impl fmt::Display for TransactionServiceRequest {
tx_id,
current_height,
mined_timestamp,
..
} => write!(
f,
"ImportUtxo (from {}, {}, {} and {:?} and {:?} and {:?} and {:?})",
"ImportUtxo (from {}, {}, {} and {:?} and {:?} and {:?} and {:?}",
source_address, amount, message, import_status, tx_id, current_height, mined_timestamp
),
Self::SubmitTransactionToSelf(tx_id, _, _, _, _) => write!(f, "SubmitTransaction ({})", tx_id),
Expand Down Expand Up @@ -734,6 +736,7 @@ impl TransactionServiceHandle {
tx_id: Option<TxId>,
current_height: Option<u64>,
mined_timestamp: Option<NaiveDateTime>,
scanned_output: TransactionOutput,
) -> Result<TxId, TransactionServiceError> {
match self
.handle
Expand All @@ -745,6 +748,7 @@ impl TransactionServiceHandle {
tx_id,
current_height,
mined_timestamp,
scanned_output,
})
.await??
{
Expand Down
11 changes: 4 additions & 7 deletions base_layer/wallet/src/transaction_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,7 @@ where
tx_id,
current_height,
mined_timestamp,
scanned_output,
} => self
.add_utxo_import_transaction_with_status(
amount,
Expand All @@ -791,7 +792,7 @@ where
tx_id,
current_height,
mined_timestamp,
transaction_validation_join_handles,
scanned_output,
)
.await
.map(TransactionServiceResponse::UtxoImported),
Expand Down Expand Up @@ -2796,9 +2797,7 @@ where
tx_id: Option<TxId>,
current_height: Option<u64>,
mined_timestamp: Option<NaiveDateTime>,
transaction_validation_join_handles: &mut FuturesUnordered<
JoinHandle<Result<OperationId, TransactionServiceProtocolError<OperationId>>>,
>,
scanned_output: TransactionOutput,
) -> Result<TxId, TransactionServiceError> {
let tx_id = if let Some(id) = tx_id { id } else { TxId::new_random() };
self.db.add_utxo_import_transaction_with_status(
Expand All @@ -2810,6 +2809,7 @@ where
import_status.clone(),
current_height,
mined_timestamp,
scanned_output,
)?;
let transaction_event = match import_status {
ImportStatus::Imported => TransactionEvent::TransactionImported(tx_id),
Expand All @@ -2828,9 +2828,6 @@ where
);
e
});
// Because we added new transactions, let try to trigger a validation for them
self.start_transaction_validation_protocol(transaction_validation_join_handles)
.await?;
Ok(tx_id)
}

Expand Down
14 changes: 9 additions & 5 deletions base_layer/wallet/src/transaction_service/storage/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ use tari_common_types::{
transaction::{ImportStatus, TransactionDirection, TransactionStatus, TxId},
types::{BlockHash, PrivateKey},
};
use tari_core::transactions::{tari_amount::MicroMinotari, transaction_components::Transaction};
use tari_core::transactions::{
tari_amount::MicroMinotari,
transaction_components::{Transaction, TransactionOutput},
};

use crate::transaction_service::{
error::TransactionStorageError,
Expand Down Expand Up @@ -127,7 +130,7 @@ pub trait TransactionBackend: Send + Sync + Clone {
mined_in_block: BlockHash,
mined_timestamp: u64,
num_confirmations: u64,
is_confirmed: bool,
must_be_confirmed: bool,
is_faux: bool,
) -> Result<(), TransactionStorageError>;
/// Clears the mined block and height of a transaction
Expand Down Expand Up @@ -641,6 +644,7 @@ where T: TransactionBackend + 'static
import_status: ImportStatus,
current_height: Option<u64>,
mined_timestamp: Option<NaiveDateTime>,
scanned_output: TransactionOutput,
) -> Result<(), TransactionStorageError> {
let transaction = CompletedTransaction::new(
tx_id,
Expand All @@ -650,7 +654,7 @@ where T: TransactionBackend + 'static
MicroMinotari::from(0),
Transaction::new(
Vec::new(),
Vec::new(),
vec![scanned_output],
Vec::new(),
PrivateKey::default(),
PrivateKey::default(),
Expand Down Expand Up @@ -690,7 +694,7 @@ where T: TransactionBackend + 'static
mined_in_block: BlockHash,
mined_timestamp: u64,
num_confirmations: u64,
is_confirmed: bool,
must_be_confirmed: bool,
is_faux: bool,
) -> Result<(), TransactionStorageError> {
self.db.update_mined_height(
Expand All @@ -699,7 +703,7 @@ where T: TransactionBackend + 'static
mined_in_block,
mined_timestamp,
num_confirmations,
is_confirmed,
must_be_confirmed,
is_faux,
)
}
Expand Down
Loading

0 comments on commit 32f9d00

Please sign in to comment.