Skip to content

Commit

Permalink
fix(core/keymanager): use tokio rwlock for keymanagers (#5494)
Browse files Browse the repository at this point in the history
Description
---
Replaces futures-rs Mutex with tokio's RwLock for key manager locks

Motivation and Context
---
Allow multiple concurrent reads for keymanagers. In general, we prefer
to use tokio's locks with the tokio runtime (no investigation done into
performance differences).

If there was a particular reason to use futures-rs Mutex we can close
this.

How Has This Been Tested?
---
Existing tests, this change is not benchmarked.

What process can a PR reviewer use to test or verify this change?
---

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
  • Loading branch information
sdbondi authored Jun 23, 2023
1 parent 822dac6 commit 229aee0
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 37 deletions.
18 changes: 9 additions & 9 deletions base_layer/core/src/transactions/key_manager/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
use std::{collections::HashMap, ops::Shl};

use futures::lock::Mutex;
use log::*;
use rand::rngs::OsRng;
use strum::IntoEnumIterator;
Expand Down Expand Up @@ -53,6 +52,7 @@ use tari_key_manager::{
},
};
use tari_utilities::{hex::Hex, ByteArray};
use tokio::sync::RwLock;

use crate::{
one_sided::diffie_hellman_stealth_domain_hasher,
Expand Down Expand Up @@ -87,7 +87,7 @@ use crate::{
hash_domain!(KeyManagerHashingDomain, "base_layer.core.key_manager");

pub struct TransactionKeyManagerInner<TBackend> {
key_managers: HashMap<String, Mutex<KeyManager<PublicKey, KeyDigest>>>,
key_managers: HashMap<String, RwLock<KeyManager<PublicKey, KeyDigest>>>,
db: KeyManagerDatabase<TBackend, PublicKey>,
master_seed: CipherSeed,
crypto_factories: CryptoFactories,
Expand Down Expand Up @@ -141,7 +141,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
};
self.key_managers.insert(
branch.to_string(),
Mutex::new(KeyManager::<PublicKey, KeyDigest>::from(
RwLock::new(KeyManager::<PublicKey, KeyDigest>::from(
self.master_seed.clone(),
state.branch_seed,
state.primary_key_index,
Expand All @@ -155,7 +155,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
.key_managers
.get(branch)
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.write()
.await;
self.db.increment_key_index(branch)?;
let index = km.increment_key_index(1);
Expand Down Expand Up @@ -186,7 +186,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
.key_managers
.get(branch)
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.read()
.await;
Ok(km.derive_public_key(*index)?.key)
},
Expand Down Expand Up @@ -220,7 +220,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
.key_managers
.get(branch)
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.read()
.await;

let current_index = km.key_index();
Expand All @@ -242,7 +242,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
.key_managers
.get(branch)
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.read()
.await;

let current_index = km.key_index();
Expand All @@ -268,7 +268,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
.key_managers
.get(branch)
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.write()
.await;
let current_index = km.key_index();
if index > current_index {
Expand Down Expand Up @@ -296,7 +296,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
.key_managers
.get(branch)
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.read()
.await;
let key = km.get_private_key(*index)?;
Ok(key)
Expand Down
56 changes: 28 additions & 28 deletions base_layer/core/src/transactions/key_manager/wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl<TBackend> KeyManagerInterface<PublicKey> for TransactionKeyManagerWrapper<T
where TBackend: KeyManagerBackend<PublicKey> + 'static
{
async fn add_new_branch<T: Into<String> + Send>(&self, branch: T) -> Result<AddResult, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.write()
.await
.add_key_manager_branch(&branch.into())
Expand All @@ -104,23 +104,23 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
&self,
branch: T,
) -> Result<(TariKeyId, PublicKey), KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_next_key(&branch.into())
.await
}

async fn get_static_key<T: Into<String> + Send>(&self, branch: T) -> Result<TariKeyId, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_static_key(&branch.into())
.await
}

async fn get_public_key_at_key_id(&self, key_id: &TariKeyId) -> Result<PublicKey, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_public_key_at_key_id(key_id)
Expand All @@ -132,7 +132,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
branch: T,
key: &PublicKey,
) -> Result<u64, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.find_key_index(&branch.into(), key)
Expand All @@ -144,15 +144,15 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
branch: T,
index: u64,
) -> Result<(), KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.update_current_key_index_if_higher(&branch.into(), index)
.await
}

async fn import_key(&self, private_key: PrivateKey) -> Result<TariKeyId, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.import_key(private_key)
Expand All @@ -169,7 +169,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
spend_key_id: &TariKeyId,
value: &PrivateKey,
) -> Result<Commitment, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_commitment(spend_key_id, value)
Expand All @@ -182,7 +182,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
spending_key_id: &TariKeyId,
value: u64,
) -> Result<bool, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.verify_mask(commitment, spending_key_id, value)
Expand All @@ -197,7 +197,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
async fn get_next_spend_and_script_key_ids(
&self,
) -> Result<(TariKeyId, PublicKey, TariKeyId, PublicKey), KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_next_spend_and_script_key_ids()
Expand All @@ -209,7 +209,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
secret_key_id: &TariKeyId,
public_key: &PublicKey,
) -> Result<CommsDHKE, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_diffie_hellman_shared_secret(secret_key_id, public_key)
Expand All @@ -221,7 +221,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
secret_key_id: &TariKeyId,
public_key: &PublicKey,
) -> Result<DomainSeparatedHash<Blake256>, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_diffie_hellman_stealth_domain_hasher(secret_key_id, public_key)
Expand All @@ -233,15 +233,15 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
secret_key_id: &TariKeyId,
offset: PrivateKey,
) -> Result<TariKeyId, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.import_add_offset_to_private_key(secret_key_id, offset)
.await
}

async fn get_spending_key_id(&self, public_spending_key: &PublicKey) -> Result<TariKeyId, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_spending_key_id(public_spending_key)
Expand All @@ -254,7 +254,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
value: u64,
min_value: u64,
) -> Result<RangeProof, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.construct_range_proof(spend_key_id, value, min_value)
Expand All @@ -269,7 +269,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
txi_version: &TransactionInputVersion,
script_message: &[u8; 32],
) -> Result<ComAndPubSignature, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_script_signature(script_key_id, spend_key_id, value, txi_version, script_message)
Expand All @@ -287,7 +287,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
kernel_features: &KernelFeatures,
txo_type: TxoStage,
) -> Result<Signature, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_partial_txo_kernel_signature(
Expand All @@ -308,7 +308,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
spend_key_id: &TariKeyId,
nonce_id: &TariKeyId,
) -> Result<PublicKey, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_txo_kernel_signature_excess_with_offset(spend_key_id, nonce_id)
Expand All @@ -320,7 +320,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
spend_key_id: &TariKeyId,
nonce_id: &TariKeyId,
) -> Result<PrivateKey, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_txo_private_kernel_offset(spend_key_id, nonce_id)
Expand All @@ -333,7 +333,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
custom_recovery_key_id: Option<&TariKeyId>,
value: u64,
) -> Result<EncryptedData, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.encrypt_data_for_recovery(spend_key_id, custom_recovery_key_id, value)
Expand All @@ -345,7 +345,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
output: &TransactionOutput,
custom_recovery_key_id: Option<&TariKeyId>,
) -> Result<(TariKeyId, MicroTari), TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.try_output_key_recovery(output, custom_recovery_key_id)
Expand All @@ -357,7 +357,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
script_key_ids: &[TariKeyId],
sender_offset_key_ids: &[TariKeyId],
) -> Result<PrivateKey, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_script_offset(script_key_ids, sender_offset_key_ids)
Expand All @@ -369,7 +369,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
nonce_id: &TariKeyId,
range_proof_type: RangeProofType,
) -> Result<Commitment, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_metadata_signature_ephemeral_commitment(nonce_id, range_proof_type)
Expand All @@ -385,7 +385,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
metadata_signature_message: &[u8; 32],
range_proof_type: RangeProofType,
) -> Result<ComAndPubSignature, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_metadata_signature(
Expand All @@ -409,7 +409,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
metadata_signature_message: &[u8; 32],
range_proof_type: RangeProofType,
) -> Result<ComAndPubSignature, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_receiver_partial_metadata_signature(
Expand All @@ -433,7 +433,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
txo_version: &TransactionOutputVersion,
metadata_signature_message: &[u8; 32],
) -> Result<ComAndPubSignature, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_sender_partial_metadata_signature(
Expand All @@ -453,7 +453,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
amount: &PrivateKey,
claim_public_key: &PublicKey,
) -> Result<RistrettoComSig, TransactionError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.generate_burn_proof(spending_key, amount, claim_public_key)
Expand All @@ -466,7 +466,7 @@ impl<TBackend> SecretTransactionKeyManagerInterface for TransactionKeyManagerWra
where TBackend: KeyManagerBackend<PublicKey> + 'static
{
async fn get_private_key(&self, key_id: &TariKeyId) -> Result<PrivateKey, KeyManagerServiceError> {
(*self.transaction_key_manager_inner)
self.transaction_key_manager_inner
.read()
.await
.get_private_key(key_id)
Expand Down

0 comments on commit 229aee0

Please sign in to comment.