Skip to content

Commit

Permalink
fix: revert changes to key manager branch seed strings (#3971)
Browse files Browse the repository at this point in the history 
Description
---
During the Key Manager refactor the strings used as branch seeds by the key manager were changed. This broke recovery as the keys generated for the rewind process were wrong and the rewound key index could no longer be found.
  • Loading branch information
@philipr-za
philipr-za committed Mar 29, 2022
1 parent 313d7e2 commit db688aa
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 49 deletions.
4 changes: 2 additions & 2 deletions base_layer/wallet/src/key_manager_service/storage/sqlite_db/mod.rs
View file
Expand Up @@ -94,11 +94,11 @@ impl KeyManagerSqliteDatabase {
if !old_state.is_empty() {
// there should only be 1 if there is an old state.
let spending_km = KeyManagerState {
branch_seed: OutputManagerKeyManagerBranch::Spend.to_string(),
branch_seed: OutputManagerKeyManagerBranch::Spend.get_branch_key(),
primary_key_index: old_state[0].primary_key_index as u64,
};
let spending_script_km = KeyManagerState {
branch_seed: OutputManagerKeyManagerBranch::SpendScript.to_string(),
branch_seed: OutputManagerKeyManagerBranch::SpendScript.get_branch_key(),
primary_key_index: old_state[0].primary_key_index as u64,
};
let mut km_sql_spending = NewKeyManagerStateSql::from(spending_km);
Expand Down
21 changes: 15 additions & 6 deletions base_layer/wallet/src/output_manager_service/recovery/standard_outputs_recoverer.rs
View file
Expand Up @@ -184,29 +184,38 @@ where
let found_index = self
.master_key_manager
.find_key_index(
OutputManagerKeyManagerBranch::Coinbase.to_string(),
OutputManagerKeyManagerBranch::Coinbase.get_branch_key(),
&output.spending_key,
)
.await?;

self.master_key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::CoinbaseScript, found_index)
.get_key_at_index(
OutputManagerKeyManagerBranch::CoinbaseScript.get_branch_key(),
found_index,
)
.await?
} else {
let found_index = self
.master_key_manager
.find_key_index(OutputManagerKeyManagerBranch::Spend, &output.spending_key)
.find_key_index(
OutputManagerKeyManagerBranch::Spend.get_branch_key(),
&output.spending_key,
)
.await?;

self.master_key_manager
.update_current_key_index_if_higher(OutputManagerKeyManagerBranch::Spend, found_index)
.update_current_key_index_if_higher(OutputManagerKeyManagerBranch::Spend.get_branch_key(), found_index)
.await?;
self.master_key_manager
.update_current_key_index_if_higher(OutputManagerKeyManagerBranch::SpendScript, found_index)
.update_current_key_index_if_higher(
OutputManagerKeyManagerBranch::SpendScript.get_branch_key(),
found_index,
)
.await?;

self.master_key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::SpendScript, found_index)
.get_key_at_index(OutputManagerKeyManagerBranch::SpendScript.get_branch_key(), found_index)
.await?
};

Expand Down
34 changes: 13 additions & 21 deletions base_layer/wallet/src/output_manager_service/resources.rs
View file
Expand Up @@ -20,8 +20,6 @@
// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

use std::fmt::{Display, Error, Formatter};

use tari_core::{
consensus::ConsensusConstants,
transactions::{transaction_protocol::RewindData, CryptoFactories},
Expand Down Expand Up @@ -59,24 +57,18 @@ pub enum OutputManagerKeyManagerBranch {
RecoveryByte,
}

impl Display for OutputManagerKeyManagerBranch {
fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), Error> {
let response = match self {
OutputManagerKeyManagerBranch::Spend => "Spend",
OutputManagerKeyManagerBranch::SpendScript => "Script",
OutputManagerKeyManagerBranch::Coinbase => "Coinbase",
OutputManagerKeyManagerBranch::CoinbaseScript => "Coinbase_script",
OutputManagerKeyManagerBranch::RecoveryViewOnly => "Recovery_viewonly",
OutputManagerKeyManagerBranch::RecoveryBlinding => "Recovery_blinding",
OutputManagerKeyManagerBranch::RecoveryByte => "Recovery_byte",
};
fmt.write_str(response)
}
}

#[allow(clippy::from_over_into)]
impl Into<String> for OutputManagerKeyManagerBranch {
fn into(self) -> String {
self.to_string()
impl OutputManagerKeyManagerBranch {
/// Warning: Changing these strings will affect the backwards compatibility of the wallet with older databases or
/// recovery.
pub fn get_branch_key(&self) -> String {
match self {
OutputManagerKeyManagerBranch::Spend => "".to_string(),
OutputManagerKeyManagerBranch::SpendScript => "script".to_string(),
OutputManagerKeyManagerBranch::Coinbase => "coinbase".to_string(),
OutputManagerKeyManagerBranch::CoinbaseScript => "coinbase_script".to_string(),
OutputManagerKeyManagerBranch::RecoveryViewOnly => "recovery_viewonly".to_string(),
OutputManagerKeyManagerBranch::RecoveryBlinding => "recovery_blinding".to_string(),
OutputManagerKeyManagerBranch::RecoveryByte => "Recovery_byte".to_string(),
}
}
}
36 changes: 22 additions & 14 deletions base_layer/wallet/src/output_manager_service/service.rs
View file
Expand Up @@ -134,13 +134,13 @@ where
db.clear_short_term_encumberances().await?;
Self::initialise_key_manager(&key_manager).await?;
let rewind_key = key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryViewOnly, 0)
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryViewOnly.get_branch_key(), 0)
.await?;
let rewind_blinding_key = key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryBlinding, 0)
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryBlinding.get_branch_key(), 0)
.await?;
let recovery_byte_key = key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryByte, 0)
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryByte.get_branch_key(), 0)
.await?;
let rewind_data = RewindData {
rewind_key,
Expand Down Expand Up @@ -171,24 +171,26 @@ where
}

async fn initialise_key_manager(key_manager: &TKeyManagerInterface) -> Result<(), OutputManagerError> {
key_manager.add_new_branch(OutputManagerKeyManagerBranch::Spend).await?;
key_manager
.add_new_branch(OutputManagerKeyManagerBranch::SpendScript)
.add_new_branch(OutputManagerKeyManagerBranch::Spend.get_branch_key())
.await?;
key_manager
.add_new_branch(OutputManagerKeyManagerBranch::Coinbase)
.add_new_branch(OutputManagerKeyManagerBranch::SpendScript.get_branch_key())
.await?;
key_manager
.add_new_branch(OutputManagerKeyManagerBranch::CoinbaseScript)
.add_new_branch(OutputManagerKeyManagerBranch::Coinbase.get_branch_key())
.await?;
key_manager
.add_new_branch(OutputManagerKeyManagerBranch::RecoveryViewOnly)
.add_new_branch(OutputManagerKeyManagerBranch::CoinbaseScript.get_branch_key())
.await?;
key_manager
.add_new_branch(OutputManagerKeyManagerBranch::RecoveryByte)
.add_new_branch(OutputManagerKeyManagerBranch::RecoveryViewOnly.get_branch_key())
.await?;
key_manager
.add_new_branch(OutputManagerKeyManagerBranch::RecoveryByte.get_branch_key())
.await?;
match key_manager
.add_new_branch(OutputManagerKeyManagerBranch::RecoveryBlinding)
.add_new_branch(OutputManagerKeyManagerBranch::RecoveryBlinding.get_branch_key())
.await
{
Ok(_) => Ok(()),
Expand Down Expand Up @@ -717,12 +719,15 @@ where
let result = self
.resources
.master_key_manager
.get_next_key(OutputManagerKeyManagerBranch::Spend)
.get_next_key(OutputManagerKeyManagerBranch::Spend.get_branch_key())
.await?;
let script_key = self
.resources
.master_key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::SpendScript, result.index)
.get_key_at_index(
OutputManagerKeyManagerBranch::SpendScript.get_branch_key(),
result.index,
)
.await?;
Ok((result.key, script_key))
}
Expand Down Expand Up @@ -1050,12 +1055,15 @@ where
let spending_key = self
.resources
.master_key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::Coinbase.to_string(), block_height)
.get_key_at_index(OutputManagerKeyManagerBranch::Coinbase.get_branch_key(), block_height)
.await?;
let script_private_key = self
.resources
.master_key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::CoinbaseScript.to_string(), block_height)
.get_key_at_index(
OutputManagerKeyManagerBranch::CoinbaseScript.get_branch_key(),
block_height,
)
.await?;

let nonce = PrivateKey::random(&mut OsRng);
Expand Down
15 changes: 9 additions & 6 deletions base_layer/wallet/tests/output_manager_service_tests/service.rs
View file
Expand Up @@ -227,15 +227,15 @@ async fn setup_output_manager_service<T: OutputManagerBackend + 'static, U: KeyM
let output_manager_service_handle = OutputManagerHandle::new(oms_request_sender, oms_event_publisher);

let rewind_key = key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryViewOnly, 0)
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryViewOnly.get_branch_key(), 0)
.await
.unwrap();
let rewind_blinding_key = key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryBlinding, 0)
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryBlinding.get_branch_key(), 0)
.await
.unwrap();
let recovery_byte_key = key_manager
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryByte, 0)
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryByte.get_branch_key(), 0)
.await
.unwrap();
let rewind_data = RewindData {
Expand Down Expand Up @@ -2052,12 +2052,15 @@ async fn scan_for_recovery_test() {
for i in 1..=NUM_REWINDABLE {
let spending_key_result = oms
.key_manager_handler
.get_next_key(OutputManagerKeyManagerBranch::Spend)
.get_next_key(OutputManagerKeyManagerBranch::Spend.get_branch_key())
.await
.unwrap();
let script_key = oms
.key_manager_handler
.get_key_at_index(OutputManagerKeyManagerBranch::SpendScript, spending_key_result.index)
.get_key_at_index(
OutputManagerKeyManagerBranch::SpendScript.get_branch_key(),
spending_key_result.index,
)
.await
.unwrap();
let commitment = factories
Expand Down Expand Up @@ -2104,7 +2107,7 @@ async fn scan_for_recovery_test() {

let recovery_byte_key = oms
.key_manager_handler
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryByte, 0)
.get_key_at_index(OutputManagerKeyManagerBranch::RecoveryByte.get_branch_key(), 0)
.await
.unwrap();
let other_rewind_data = RewindData {
Expand Down

0 comments on commit db688aa

Please sign in to comment.