Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix ffi import external utxo from faucet #3956

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,11 @@ impl wallet_server::Wallet for WalletGrpcServer {
for o in unblinded_outputs.iter() {
tx_ids.push(
wallet
.import_unblinded_utxo(o.clone(), &CommsPublicKey::default(), "Imported via gRPC".to_string())
.import_unblinded_output_as_non_rewindable(
o.clone(),
&CommsPublicKey::default(),
"Imported via gRPC".to_string(),
)
.await
.map_err(|e| Status::internal(format!("{:?}", e)))?
.into(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use std::{
ops::Shl,
};

use log::*;
use rand::rngs::OsRng;
use serde::{Deserialize, Serialize};
use tari_common_types::types::{
Expand All @@ -44,7 +45,7 @@ use tari_crypto::{
commitment::HomomorphicCommitmentFactory,
keys::{PublicKey as PublicKeyTrait, SecretKey},
range_proof::{RangeProofError, RangeProofService},
tari_utilities::ByteArray,
tari_utilities::{hex::to_hex, ByteArray},
};
use tari_script::{ExecutionStack, TariScript};

Expand All @@ -66,6 +67,8 @@ use crate::{
},
};

pub const LOG_TARGET: &str = "c::tx::tx_components::unblinded_output";

/// An unblinded output is one where the value and spending key (blinding factor) are known. This can be used to
/// build both inputs and outputs (every input comes from an output)
// TODO: Try to get rid of 'Serialize' and 'Deserialize' traits here; see related comment at 'struct RawTransactionInfo'
Expand Down Expand Up @@ -207,10 +210,14 @@ impl UnblindedOutput {

let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, None);
if self.features.recovery_byte != recovery_byte {
return Err(TransactionError::ValidationError(format!(
"Recovery byte set incorrectly - expected {} got {}",
recovery_byte, self.features.recovery_byte
)));
// This is not a hard error by choice; we allow inconsistent recovery byte data into the wallet database
error!(
target: LOG_TARGET,
"Recovery byte set incorrectly - expected {}, got {} for commitment {}",
recovery_byte,
self.features.recovery_byte,
to_hex(commitment.as_bytes()),
);
}

let output = TransactionOutput::new(
Expand Down Expand Up @@ -262,10 +269,14 @@ impl UnblindedOutput {

let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, Some(rewind_data));
if self.features.recovery_byte != recovery_byte {
return Err(TransactionError::ValidationError(format!(
"Recovery byte set incorrectly (with rewind) - expected {} got {}",
recovery_byte, self.features.recovery_byte
)));
// This is not a hard error by choice; we allow inconsistent recovery byte data into the wallet database
error!(
target: LOG_TARGET,
"Recovery byte set incorrectly (with rewind) - expected {}, got {} for commitment {}",
recovery_byte,
self.features.recovery_byte,
to_hex(commitment.as_bytes()),
);
}

let output = TransactionOutput::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ use tari_crypto::{
commitment::HomomorphicCommitmentFactory,
keys::{PublicKey as PublicKeyTrait, SecretKey},
ristretto::pedersen::PedersenCommitmentFactory,
tari_utilities::fixed_set::FixedSet,
tari_utilities::{fixed_set::FixedSet, hex::to_hex},
};
use tari_script::{ExecutionStack, TariScript};
use tari_utilities::ByteArray;

use crate::{
consensus::{ConsensusConstants, ConsensusEncodingSized},
Expand Down Expand Up @@ -216,10 +217,14 @@ impl SenderTransactionInitializer {
let commitment = commitment_factory.commit(&output.spending_key, &PrivateKey::from(output.value));
let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, self.rewind_data.as_ref());
if recovery_byte != output.features.recovery_byte {
return self.clone().build_err(&*format!(
"Recovery byte not valid (expected {}, got {}), cannot add output: {:?}",
recovery_byte, output.features.recovery_byte, output
))?;
// This is not a hard error by choice; we allow inconsistent recovery byte data into the wallet database
error!(
target: LOG_TARGET,
"Recovery byte set incorrectly (with output) - expected {}, got {} for commitment {}",
recovery_byte,
output.features.recovery_byte,
to_hex(commitment.as_bytes()),
);
}
let e = TransactionOutput::build_metadata_signature_challenge(
&output.script,
Expand Down
12 changes: 10 additions & 2 deletions base_layer/wallet/src/output_manager_service/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ pub enum OutputManagerRequest {
CalculateRecoveryByte {
spending_key: PrivateKey,
value: u64,
with_rewind_data: bool,
},
FeeEstimate {
amount: MicroTari,
Expand Down Expand Up @@ -172,7 +173,9 @@ impl fmt::Display for OutputManagerRequest {
RemoveEncryption => write!(f, "RemoveEncryption"),
GetCoinbaseTransaction(_) => write!(f, "GetCoinbaseTransaction"),
GetPublicRewindKeys => write!(f, "GetPublicRewindKeys"),
CalculateRecoveryByte { spending_key, value } => write!(
CalculateRecoveryByte {
spending_key, value, ..
} => write!(
f,
"CalculateRecoveryByte ({},{})",
spending_key.as_bytes().to_vec().to_hex(),
Expand Down Expand Up @@ -630,10 +633,15 @@ impl OutputManagerHandle {
&mut self,
spending_key: PrivateKey,
value: u64,
with_rewind_data: bool,
) -> Result<u8, OutputManagerError> {
match self
.handle
.call(OutputManagerRequest::CalculateRecoveryByte { spending_key, value })
.call(OutputManagerRequest::CalculateRecoveryByte {
spending_key,
value,
with_rewind_data,
})
.await??
{
OutputManagerResponse::RecoveryByte(rk) => Ok(rk),
Expand Down
30 changes: 23 additions & 7 deletions base_layer/wallet/src/output_manager_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,15 @@ where
OutputManagerRequest::GetPublicRewindKeys => Ok(OutputManagerResponse::PublicRewindKeys(Box::new(
self.get_rewind_public_keys(),
))),
OutputManagerRequest::CalculateRecoveryByte { spending_key, value } => Ok(
OutputManagerResponse::RecoveryByte(self.calculate_recovery_byte(spending_key, value)?),
),
OutputManagerRequest::CalculateRecoveryByte {
spending_key,
value,
with_rewind_data,
} => Ok(OutputManagerResponse::RecoveryByte(self.calculate_recovery_byte(
spending_key,
value,
with_rewind_data,
)?)),
OutputManagerRequest::ScanForRecoverableOutputs(outputs) => StandardUtxoRecoverer::new(
self.resources.master_key_manager.clone(),
self.resources.rewind_data.clone(),
Expand Down Expand Up @@ -593,9 +599,19 @@ where
self.validate_outputs()
}

pub fn calculate_recovery_byte(&mut self, spending_key: PrivateKey, value: u64) -> Result<u8, OutputManagerError> {
pub fn calculate_recovery_byte(
&mut self,
spending_key: PrivateKey,
value: u64,
with_rewind_data: bool,
) -> Result<u8, OutputManagerError> {
let commitment = self.resources.factories.commitment.commit_value(&spending_key, value);
let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, Some(&self.resources.rewind_data));
let rewind_data = if with_rewind_data {
Some(&self.resources.rewind_data)
} else {
None
};
let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, rewind_data);
Ok(recovery_byte)
}

Expand Down Expand Up @@ -1326,7 +1342,7 @@ where
}

let (spending_key, script_private_key) = self.get_spend_and_script_keys().await?;
let recovery_byte = self.calculate_recovery_byte(spending_key.clone(), amount.as_u64())?;
let recovery_byte = self.calculate_recovery_byte(spending_key.clone(), amount.as_u64(), true)?;
let output_features = OutputFeatures {
recovery_byte,
unique_id: unique_id.clone(),
Expand Down Expand Up @@ -1672,7 +1688,7 @@ where
let output_amount = amount_per_split;

let (spending_key, script_private_key) = self.get_spend_and_script_keys().await?;
let recovery_byte = self.calculate_recovery_byte(spending_key.clone(), output_amount.as_u64())?;
let recovery_byte = self.calculate_recovery_byte(spending_key.clone(), output_amount.as_u64(), true)?;
let output_features = OutputFeatures {
recovery_byte,
..Default::default()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase {
if start.elapsed().as_millis() > 0 {
trace!(
target: LOG_TARGET,
"sqlite profile - reinstate_cancelled_inbound_output: lock {} + db_op {} = {} ms",
"sqlite profile - add_unvalidated_output: lock {} + db_op {} = {} ms",
acquire_lock.as_millis(),
(start.elapsed() - acquire_lock).as_millis(),
start.elapsed().as_millis()
Expand Down
24 changes: 13 additions & 11 deletions base_layer/wallet/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,10 @@ where
self.updater_service.clone().unwrap()
}

/// Import an external spendable UTXO into the wallet. The output will be added to the Output Manager and made
/// EncumberedToBeReceived. A faux incoming transaction will be created to provide a record of the event. The TxId
/// of the generated transaction is returned.

pub async fn import_utxo(
/// Import an external spendable UTXO into the wallet as a non-rewindable/non-recoverable UTXO. The output will be
/// added to the Output Manager and made EncumberedToBeReceived. A faux incoming transaction will be created to
/// provide a record of the event. The TxId of the generated transaction is returned.
pub async fn import_external_utxo_as_non_rewindable(
&mut self,
amount: MicroTari,
spending_key: &PrivateKey,
Expand Down Expand Up @@ -449,22 +448,24 @@ where
.map_err(WalletError::TransactionError)?
.to_hex();

// As non-rewindable
self.output_manager_service
.add_unvalidated_output(tx_id, unblinded_output, None)
.await?;

info!(
target: LOG_TARGET,
"UTXO (Commitment: {}) imported into wallet as 'ImportStatus::Imported'", commitment_hex
"UTXO (Commitment: {}) imported into wallet as 'ImportStatus::Imported' and is non-rewindable",
commitment_hex
);

Ok(tx_id)
}

/// Import an external spendable UTXO into the wallet. The output will be added to the Output Manager and made
/// spendable. A faux incoming transaction will be created to provide a record of the event. The TxId of the
/// generated transaction is returned.
pub async fn import_unblinded_utxo(
/// Import an external spendable UTXO into the wallet as a non-rewindable/non-recoverable UTXO. The output will be
/// added to the Output Manager and made spendable. A faux incoming transaction will be created to provide a record
/// of the event. The TxId of the generated transaction is returned.
pub async fn import_unblinded_output_as_non_rewindable(
&mut self,
unblinded_output: UnblindedOutput,
source_public_key: &CommsPublicKey,
Expand All @@ -483,13 +484,14 @@ where
)
.await?;

// As non-rewindable
self.output_manager_service
.add_output_with_tx_id(tx_id, unblinded_output.clone(), None)
.await?;

info!(
target: LOG_TARGET,
"UTXO (Commitment: {}) imported into wallet as 'ImportStatus::Imported'",
"UTXO (Commitment: {}) imported into wallet as 'ImportStatus::Imported' and is non-rewindable",
unblinded_output
.as_transaction_input(&self.factories.commitment)?
.commitment()
Expand Down
4 changes: 2 additions & 2 deletions base_layer/wallet/tests/support/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub async fn make_input<R: Rng + CryptoRng>(
// further down the line
if let Some(mut oms) = oms {
if let Ok(val) = oms
.calculate_recovery_byte(utxo.spending_key.clone(), utxo.value.clone().as_u64())
.calculate_recovery_byte(utxo.spending_key.clone(), utxo.value.clone().as_u64(), true)
.await
{
utxo.features.set_recovery_byte(val);
Expand Down Expand Up @@ -99,7 +99,7 @@ pub async fn make_input_with_features<R: Rng + CryptoRng>(
// 'TestParamsHelpers::new()'; this will influence validation of output features and the metadata signature
// further down the line
if let Ok(val) = oms
.calculate_recovery_byte(utxo.spending_key.clone(), utxo.value.clone().as_u64())
.calculate_recovery_byte(utxo.spending_key.clone(), utxo.value.clone().as_u64(), true)
.await
{
utxo.features.set_recovery_byte(val);
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ async fn test_import_utxo() {
let expected_output_hash = output.hash();

let tx_id = alice_wallet
.import_utxo(
.import_external_utxo_as_non_rewindable(
utxo.value,
&utxo.spending_key,
script.clone(),
Expand Down
1 change: 1 addition & 0 deletions base_layer/wallet_ffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ openssl = { version = "0.10", features = ["vendored"] }
rand = "0.8"
thiserror = "1.0.26"
tokio = "1.11"
env_logger = "0.7.0"

# <workaround>
# Temporary workaround until crates utilizing openssl have been updated from security-framework 2.4.0
Expand Down