Skip to content

Commit

Permalink
chore: remove mutex (#4617)
Browse files Browse the repository at this point in the history
Description
---
Removes mutex added in #4613 

Motivation and Context
---
While the race condition is definitely possible in code, all code from the OMS runs synchronously, as shown by the fact that almost all functions have `&mut self` and rust blocks mut borrow across threads. This means that there is no race condition possible as this is all called synchronously. 

How Has This Been Tested?
---
  • Loading branch information
SWvheerden committed Sep 6, 2022
1 parent 99cef05 commit 50d46a6
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 62 deletions.
47 changes: 0 additions & 47 deletions base_layer/wallet/src/output_manager_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1134,56 +1134,9 @@ where
)?)
}

// let mut change_keys = None;
//
// let fee = Fee::calculate(fee_per_gram, 1, inputs.len(), 1);
// let change_value = total.saturating_sub(fee);
// if change_value > 0.into() {
// let (spending_key, script_private_key) = self
// .resources
// .master_key_manager
// .get_next_spend_and_script_key()
// .await?;
// change_keys = Some((spending_key.clone(), script_private_key.clone()));
// builder.with_change_secret(spending_key);
// builder.with_rewindable_outputs(&self.resources.rewind_data.clone());
// builder.with_change_script(
// script!(Nop),
// inputs!(PublicKey::from_secret_key(&script_private_key)),
// script_private_key,
// );
// }

let mut stp = builder
.build(&self.resources.factories, None, u64::MAX)
.map_err(|e| OutputManagerError::BuildError(e.message))?;
// if let Some((spending_key, script_private_key)) = change_keys {
// // let change_script_offset_public_key = stp.get_change_sender_offset_public_key()?.ok_or_else(|| {
// // OutputManagerError::BuildError(
// // "There should be a change script offset public key available".to_string(),
// // )
// // })?;
//
// let sender_offset_private_key = PrivateKey::random(&mut OsRng);
// let sender_offset_public_key = PublicKey::from_secret_key(&sender_offset_private_key);
//
// let public_offset_commitment_private_key = PrivateKey::random(&mut OsRng);
// let public_offset_commitment_pub_key = PublicKey::from_secret_key(&public_offset_commitment_private_key);
//
// let mut output_builder = UnblindedOutputBuilder::new(stp.get_change_amount()?, spending_key)
// .with_script(script!(Nop))
// .with_input_data(inputs!(PublicKey::from_secret_key(&script_private_key)))
// .with_script_private_key(script_private_key);
//
// output_builder.sign_as_receiver(sender_offset_public_key, public_offset_commitment_pub_key)?;
// output_builder.sign_as_sender(&sender_offset_private_key)?;
//

// let change_output =
// DbUnblindedOutput::from_unblinded_output(output_builder.try_build()?, &self.resources.factories)?;
//
// db_outputs.push(change_output);
// }

if let Some(unblinded_output) = stp.get_change_unblinded_output()? {
db_outputs.push(DbUnblindedOutput::rewindable_from_unblinded_output(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

use std::{
convert::{TryFrom, TryInto},
sync::{Arc, Mutex, RwLock},
sync::{Arc, RwLock},
};

use chacha20poly1305::XChaCha20Poly1305;
Expand Down Expand Up @@ -68,15 +68,13 @@ const LOG_TARGET: &str = "wallet::output_manager_service::database::wallet";
pub struct OutputManagerSqliteDatabase {
database_connection: WalletDbConnection,
cipher: Arc<RwLock<Option<XChaCha20Poly1305>>>,
encumber_lock: Arc<Mutex<()>>,
}

impl OutputManagerSqliteDatabase {
pub fn new(database_connection: WalletDbConnection, cipher: Option<XChaCha20Poly1305>) -> Self {
Self {
database_connection,
cipher: Arc::new(RwLock::new(cipher)),
encumber_lock: Arc::new(Mutex::new(())),
}
}

Expand Down Expand Up @@ -663,12 +661,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase {
let start = Instant::now();
let conn = self.database_connection.get_pooled_connection()?;
let acquire_lock = start.elapsed();
// We need to ensure that this whole encumber operation happens inside of a mutex to ensure thread safety as the
// transaction first check checks if it can encumber then encumbers them.
let _guard = self
.encumber_lock
.lock()
.map_err(|e| OutputManagerStorageError::UnexpectedResult(format!("Encumber lock poisoned: {}", e)))?;

let mut outputs_to_be_spent = Vec::with_capacity(outputs_to_send.len());

Expand Down Expand Up @@ -723,12 +715,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase {
let conn = self.database_connection.get_pooled_connection()?;
let acquire_lock = start.elapsed();

// We need to ensure that this whole encumber operation happens inside of a mutex to ensure thread safety as the
// transaction first check checks if it can encumber then encumbers them.
let _guard = self
.encumber_lock
.lock()
.map_err(|e| OutputManagerStorageError::UnexpectedResult(format!("Encumber lock poisoned: {}", e)))?;
let outputs_to_be_received =
OutputSql::find_by_tx_id_and_status(tx_id, OutputStatus::ShortTermEncumberedToBeReceived, &conn)?;
for o in &outputs_to_be_received {
Expand Down

0 comments on commit 50d46a6

Please sign in to comment.