Skip to content

Commit

Permalink
feat: gracefully handle recovering a duplicate output in LibWallet (#…
Browse files Browse the repository at this point in the history
…3903)

Description
---
This PR addresses the issue of creating duplicate Faux transaction when an output that has previously been recovered is recovered again. This can happen during reorgs.
It is a small fix to the code that when outputs are scanned to see if they can be rewound only new unique outputs are returned to the caller and duplicate outputs are ignored.
The UTXO scanner will then only create new Faux transactions for the outputs that have not been previously recovered.

Two Rust tests were added to confirm the correct behaviour in output scanning.

This behaviour was already implemented for One-sided payments and a test existed that tested the ignoring of duplicates.

How Has This Been Tested?
---
Rust tests provided
  • Loading branch information
philipr-za authored Mar 25, 2022
1 parent 11412a2 commit bcd1418
Show file tree
Hide file tree
Showing 12 changed files with 254 additions and 71 deletions.
4 changes: 2 additions & 2 deletions applications/tari_console_wallet/src/automation/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use tari_core::transactions::{
use tari_utilities::hex::HexError;
use tari_wallet::{
error::{WalletError, WalletStorageError},
key_manager_service::KeyManagerError,
key_manager_service::KeyManagerServiceError,
output_manager_service::error::OutputManagerError,
transaction_service::error::TransactionServiceError,
};
Expand All @@ -54,7 +54,7 @@ pub enum CommandError {
#[error("Output manager error: `{0}`")]
OutputManagerError(#[from] OutputManagerError),
#[error("Key manager error: `{0}`")]
KeyManagerError(#[from] KeyManagerError),
KeyManagerError(#[from] KeyManagerServiceError),
#[error("Tokio join error `{0}`")]
Join(#[from] JoinError),
#[error("Config error `{0}`")]
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use thiserror::Error;
use crate::{
base_node_service::error::BaseNodeServiceError,
contacts_service::error::ContactsServiceError,
key_manager_service::KeyManagerError as KeyManagerServiceError,
key_manager_service::KeyManagerServiceError,
output_manager_service::error::OutputManagerError,
storage::database::DbKey,
transaction_service::error::TransactionServiceError,
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/src/key_manager_service/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use tari_utilities::{hex::HexError, ByteArrayError};
use crate::error::WalletStorageError;

#[derive(Debug, thiserror::Error)]
pub enum KeyManagerError {
pub enum KeyManagerServiceError {
#[error("Branch does not exist")]
UnknownKeyBranch,
#[error("Master seed does not match stored version")]
Expand Down
16 changes: 8 additions & 8 deletions base_layer/wallet/src/key_manager_service/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use tari_key_manager::cipher_seed::CipherSeed;
use tokio::sync::RwLock;

use crate::key_manager_service::{
error::KeyManagerError,
error::KeyManagerServiceError,
interface::NextKeyResult,
storage::database::{KeyManagerBackend, KeyManagerDatabase},
AddResult,
Expand All @@ -55,31 +55,31 @@ where TBackend: KeyManagerBackend + 'static
impl<TBackend> KeyManagerInterface for KeyManagerHandle<TBackend>
where TBackend: KeyManagerBackend + 'static
{
async fn add_new_branch<T: Into<String> + Send>(&self, branch: T) -> Result<AddResult, KeyManagerError> {
async fn add_new_branch<T: Into<String> + Send>(&self, branch: T) -> Result<AddResult, KeyManagerServiceError> {
(*self.key_manager_inner)
.write()
.await
.add_key_manager_branch(branch.into())
.await
}

async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerError> {
async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerServiceError> {
(*self.key_manager_inner).write().await.apply_encryption(cipher).await
}

async fn remove_encryption(&self) -> Result<(), KeyManagerError> {
async fn remove_encryption(&self) -> Result<(), KeyManagerServiceError> {
(*self.key_manager_inner).write().await.remove_encryption().await
}

async fn get_next_key<T: Into<String> + Send>(&self, branch: T) -> Result<NextKeyResult, KeyManagerError> {
async fn get_next_key<T: Into<String> + Send>(&self, branch: T) -> Result<NextKeyResult, KeyManagerServiceError> {
(*self.key_manager_inner).read().await.get_next_key(branch.into()).await
}

async fn get_key_at_index<T: Into<String> + Send>(
&self,
branch: T,
index: u64,
) -> Result<PrivateKey, KeyManagerError> {
) -> Result<PrivateKey, KeyManagerServiceError> {
(*self.key_manager_inner)
.read()
.await
Expand All @@ -91,7 +91,7 @@ where TBackend: KeyManagerBackend + 'static
&self,
branch: T,
key: &PrivateKey,
) -> Result<u64, KeyManagerError> {
) -> Result<u64, KeyManagerServiceError> {
(*self.key_manager_inner)
.read()
.await
Expand All @@ -103,7 +103,7 @@ where TBackend: KeyManagerBackend + 'static
&self,
branch: T,
index: u64,
) -> Result<(), KeyManagerError> {
) -> Result<(), KeyManagerServiceError> {
(*self.key_manager_inner)
.read()
.await
Expand Down
21 changes: 12 additions & 9 deletions base_layer/wallet/src/key_manager_service/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
use aes_gcm::Aes256Gcm;
use tari_common_types::types::PrivateKey;

use crate::key_manager_service::error::KeyManagerError;
use crate::key_manager_service::error::KeyManagerServiceError;

#[derive(Debug, PartialEq)]
pub enum AddResult {
Expand All @@ -38,26 +38,29 @@ pub struct NextKeyResult {

#[async_trait::async_trait]
pub trait KeyManagerInterface: Clone + Send + Sync + 'static {
async fn add_new_branch<T: Into<String> + Send>(&self, branch: T) -> Result<AddResult, KeyManagerError>;
async fn add_new_branch<T: Into<String> + Send>(&self, branch: T) -> Result<AddResult, KeyManagerServiceError>;

async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerError>;
async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerServiceError>;

async fn remove_encryption(&self) -> Result<(), KeyManagerError>;
async fn remove_encryption(&self) -> Result<(), KeyManagerServiceError>;

async fn get_next_key<T: Into<String> + Send>(&self, branch: T) -> Result<NextKeyResult, KeyManagerError>;
async fn get_next_key<T: Into<String> + Send>(&self, branch: T) -> Result<NextKeyResult, KeyManagerServiceError>;

async fn get_key_at_index<T: Into<String> + Send>(
&self,
branch: T,
index: u64,
) -> Result<PrivateKey, KeyManagerError>;
) -> Result<PrivateKey, KeyManagerServiceError>;

async fn find_key_index<T: Into<String> + Send>(&self, branch: T, key: &PrivateKey)
-> Result<u64, KeyManagerError>;
async fn find_key_index<T: Into<String> + Send>(
&self,
branch: T,
key: &PrivateKey,
) -> Result<u64, KeyManagerServiceError>;

async fn update_current_key_index_if_higher<T: Into<String> + Send>(
&self,
branch: T,
index: u64,
) -> Result<(), KeyManagerError>;
) -> Result<(), KeyManagerServiceError>;
}
40 changes: 22 additions & 18 deletions base_layer/wallet/src/key_manager_service/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const LOG_TARGET: &str = "wallet::Key_manager_mock";
const KEY_MANAGER_MAX_SEARCH_DEPTH: u64 = 1_000_000;
use std::{collections::HashMap, sync::Arc};

use crate::key_manager_service::{error::KeyManagerError, storage::database::KeyManagerState};
use crate::key_manager_service::{error::KeyManagerServiceError, storage::database::KeyManagerState};

#[derive(Clone)]
pub struct KeyManagerMock {
Expand All @@ -53,7 +53,7 @@ impl KeyManagerMock {
}

impl KeyManagerMock {
pub async fn add_key_manager_mock(&self, branch: String) -> Result<AddResult, KeyManagerError> {
pub async fn add_key_manager_mock(&self, branch: String) -> Result<AddResult, KeyManagerServiceError> {
let result = if self.key_managers.read().await.contains_key(&branch) {
AddResult::AlreadyExists
} else {
Expand All @@ -75,27 +75,31 @@ impl KeyManagerMock {
Ok(result)
}

pub async fn get_next_key_mock(&self, branch: String) -> Result<NextKeyResult, KeyManagerError> {
pub async fn get_next_key_mock(&self, branch: String) -> Result<NextKeyResult, KeyManagerServiceError> {
let mut lock = self.key_managers.write().await;
let km = lock.get_mut(&branch).ok_or(KeyManagerError::UnknownKeyBranch)?;
let km = lock.get_mut(&branch).ok_or(KeyManagerServiceError::UnknownKeyBranch)?;
let key = km.next_key()?;
Ok(NextKeyResult {
key: key.k,
index: km.key_index(),
})
}

pub async fn get_key_at_index_mock(&self, branch: String, index: u64) -> Result<PrivateKey, KeyManagerError> {
pub async fn get_key_at_index_mock(
&self,
branch: String,
index: u64,
) -> Result<PrivateKey, KeyManagerServiceError> {
let lock = self.key_managers.read().await;
let km = lock.get(&branch).ok_or(KeyManagerError::UnknownKeyBranch)?;
let km = lock.get(&branch).ok_or(KeyManagerServiceError::UnknownKeyBranch)?;
let key = km.derive_key(index)?;
Ok(key.k)
}

/// Search the specified branch key manager key chain to find the index of the specified key.
pub async fn find_key_index_mock(&self, branch: String, key: &PrivateKey) -> Result<u64, KeyManagerError> {
pub async fn find_key_index_mock(&self, branch: String, key: &PrivateKey) -> Result<u64, KeyManagerServiceError> {
let lock = self.key_managers.read().await;
let km = lock.get(&branch).ok_or(KeyManagerError::UnknownKeyBranch)?;
let km = lock.get(&branch).ok_or(KeyManagerServiceError::UnknownKeyBranch)?;

let current_index = km.key_index();

Expand All @@ -106,17 +110,17 @@ impl KeyManagerMock {
}
}

Err(KeyManagerError::KeyNotFoundInKeyChain)
Err(KeyManagerServiceError::KeyNotFoundInKeyChain)
}

/// If the supplied index is higher than the current UTXO key chain indices then they will be updated.
pub async fn update_current_key_index_if_higher_mock(
&self,
branch: String,
index: u64,
) -> Result<(), KeyManagerError> {
) -> Result<(), KeyManagerServiceError> {
let lock = self.key_managers.write().await;
let km = lock.get(&branch).ok_or(KeyManagerError::UnknownKeyBranch)?;
let km = lock.get(&branch).ok_or(KeyManagerServiceError::UnknownKeyBranch)?;
let current_index = km.key_index();
if index > current_index {
// km.update_key_index(index);
Expand All @@ -128,43 +132,43 @@ impl KeyManagerMock {

#[async_trait::async_trait]
impl KeyManagerInterface for KeyManagerMock {
async fn add_new_branch<T: Into<String> + Send>(&self, branch: T) -> Result<AddResult, KeyManagerError> {
async fn add_new_branch<T: Into<String> + Send>(&self, branch: T) -> Result<AddResult, KeyManagerServiceError> {
self.add_key_manager_mock(branch.into()).await
}

async fn get_next_key<T: Into<String> + Send>(&self, branch: T) -> Result<NextKeyResult, KeyManagerError> {
async fn get_next_key<T: Into<String> + Send>(&self, branch: T) -> Result<NextKeyResult, KeyManagerServiceError> {
self.get_next_key_mock(branch.into()).await
}

async fn get_key_at_index<T: Into<String> + Send>(
&self,
branch: T,
index: u64,
) -> Result<PrivateKey, KeyManagerError> {
) -> Result<PrivateKey, KeyManagerServiceError> {
self.get_key_at_index_mock(branch.into(), index).await
}

async fn apply_encryption(&self, _cipher: Aes256Gcm) -> Result<(), KeyManagerError> {
async fn apply_encryption(&self, _cipher: Aes256Gcm) -> Result<(), KeyManagerServiceError> {
unimplemented!("Not supported");
}

async fn remove_encryption(&self) -> Result<(), KeyManagerError> {
async fn remove_encryption(&self) -> Result<(), KeyManagerServiceError> {
unimplemented!("Not supported");
}

async fn find_key_index<T: Into<String> + Send>(
&self,
branch: T,
key: &PrivateKey,
) -> Result<u64, KeyManagerError> {
) -> Result<u64, KeyManagerServiceError> {
self.find_key_index_mock(branch.into(), key).await
}

async fn update_current_key_index_if_higher<T: Into<String> + Send>(
&self,
branch: T,
index: u64,
) -> Result<(), KeyManagerError> {
) -> Result<(), KeyManagerServiceError> {
self.update_current_key_index_if_higher_mock(branch.into(), index).await
}
}
2 changes: 1 addition & 1 deletion base_layer/wallet/src/key_manager_service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

mod error;
pub use error::KeyManagerError;
pub use error::KeyManagerServiceError;

mod handle;
pub use handle::KeyManagerHandle;
Expand Down
30 changes: 17 additions & 13 deletions base_layer/wallet/src/key_manager_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const KEY_MANAGER_MAX_SEARCH_DEPTH: u64 = 1_000_000;
use std::collections::HashMap;

use crate::key_manager_service::{
error::KeyManagerError,
error::KeyManagerServiceError,
interface::NextKeyResult,
storage::database::{KeyManagerBackend, KeyManagerDatabase, KeyManagerState},
AddResult,
Expand All @@ -56,7 +56,7 @@ where TBackend: KeyManagerBackend + 'static
}
}

pub async fn add_key_manager_branch(&mut self, branch: String) -> Result<AddResult, KeyManagerError> {
pub async fn add_key_manager_branch(&mut self, branch: String) -> Result<AddResult, KeyManagerServiceError> {
let result = if self.key_managers.contains_key(&branch) {
AddResult::AlreadyExists
} else {
Expand Down Expand Up @@ -84,11 +84,11 @@ where TBackend: KeyManagerBackend + 'static
Ok(result)
}

pub async fn get_next_key(&self, branch: String) -> Result<NextKeyResult, KeyManagerError> {
pub async fn get_next_key(&self, branch: String) -> Result<NextKeyResult, KeyManagerServiceError> {
let mut km = self
.key_managers
.get(&branch)
.ok_or(KeyManagerError::UnknownKeyBranch)?
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.await;
let key = km.next_key()?;
Expand All @@ -99,33 +99,33 @@ where TBackend: KeyManagerBackend + 'static
})
}

pub async fn get_key_at_index(&self, branch: String, index: u64) -> Result<PrivateKey, KeyManagerError> {
pub async fn get_key_at_index(&self, branch: String, index: u64) -> Result<PrivateKey, KeyManagerServiceError> {
let km = self
.key_managers
.get(&branch)
.ok_or(KeyManagerError::UnknownKeyBranch)?
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.await;
let key = km.derive_key(index)?;
Ok(key.k)
}

pub async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerError> {
pub async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerServiceError> {
self.db.apply_encryption(cipher).await?;
Ok(())
}

pub async fn remove_encryption(&self) -> Result<(), KeyManagerError> {
pub async fn remove_encryption(&self) -> Result<(), KeyManagerServiceError> {
self.db.remove_encryption().await?;
Ok(())
}

/// Search the specified branch key manager key chain to find the index of the specified key.
pub async fn find_key_index(&self, branch: String, key: &PrivateKey) -> Result<u64, KeyManagerError> {
pub async fn find_key_index(&self, branch: String, key: &PrivateKey) -> Result<u64, KeyManagerServiceError> {
let km = self
.key_managers
.get(&branch)
.ok_or(KeyManagerError::UnknownKeyBranch)?
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.await;

Expand All @@ -138,15 +138,19 @@ where TBackend: KeyManagerBackend + 'static
}
}

Err(KeyManagerError::KeyNotFoundInKeyChain)
Err(KeyManagerServiceError::KeyNotFoundInKeyChain)
}

/// If the supplied index is higher than the current UTXO key chain indices then they will be updated.
pub async fn update_current_key_index_if_higher(&self, branch: String, index: u64) -> Result<(), KeyManagerError> {
pub async fn update_current_key_index_if_higher(
&self,
branch: String,
index: u64,
) -> Result<(), KeyManagerServiceError> {
let mut km = self
.key_managers
.get(&branch)
.ok_or(KeyManagerError::UnknownKeyBranch)?
.ok_or(KeyManagerServiceError::UnknownKeyBranch)?
.lock()
.await;
let current_index = km.key_index();
Expand Down
Loading

0 comments on commit bcd1418

Please sign in to comment.