Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: BSD-3-Clause

use anyhow::anyhow;
use tari_ootle_wallet_sdk::apis::key_manager::KeyBranch;
use tari_ootle_wallet_sdk::models::KeyBranch;
use tari_template_lib_types::crypto::RistrettoPublicKeyBytes;
use tari_wallet_daemon_client::{
types::{AuthLoginAcceptRequest, AuthLoginRequest, AuthLoginResponse, WebauthnFinishAuthRequest},
Expand Down
2 changes: 1 addition & 1 deletion applications/tari_wallet_cli/src/command/key.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.

use clap::Subcommand;
use tari_ootle_wallet_sdk::{apis::key_manager::KeyBranch, models::KeyId};
use tari_ootle_wallet_sdk::models::{KeyBranch, KeyId};
use tari_template_lib::prelude::RistrettoPublicKeyBytes;
use tari_wallet_daemon_client::WalletDaemonClient;

Expand Down
47 changes: 31 additions & 16 deletions applications/tari_walletd/src/handlers/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ use tari_ootle_wallet_crypto::{
use tari_ootle_wallet_sdk::{
apis::{
confidential_transfer::ConfidentialTransferParams,
key_manager::KeyBranch,
stealth_transfer::StealthTransferParams,
substate::ValidatorScanResult,
},
models::NewAccountData,
models::{KeyBranch, KeyId, NewAccountData},
};
use tari_ootle_wallet_sdk_services::events::TransactionSubmittedEvent;
use tari_template_builtin::ACCOUNT_TEMPLATE_ADDRESS;
Expand Down Expand Up @@ -429,6 +428,8 @@ pub async fn handle_claim_burn(
.ok_or_else(|| invalid_params("account", Some("cannot claim burn to an account without an owner key")))?;

let network = sdk.config_api().get_network()?;
// We derive secrets directly here because claim burn is a unique case, making it difficult to use the higher
// level stealth output api that takes care of keys but assumes that this is a regular transfer.
let claim_nonce_keypair = sdk
.key_manager_api()
.derive_keypair(KeyBranch::Nonce, owner_nonce_key_index)?;
Expand Down Expand Up @@ -553,7 +554,14 @@ pub async fn handle_claim_burn(
.pay_fee_stealth(pay_fee_and_mint_output)
})
.add_input(XTR)
.build_and_seal(claim_nonce_keypair.secret_key());
.build();

// The signer does not authorize this transaction, as the claim burn instruction is authorized by the proofs. So we
// can sign with any key.
let nonce = sdk.key_manager_api().next_public_key(KeyBranch::Nonce)?;
let transaction = sdk
.local_signer_api()
.sign(KeyBranch::Nonce, nonce.key_id, transaction)?;

let tx_id = context.transaction_service().submit_transaction(transaction).await?;

Expand Down Expand Up @@ -635,8 +643,6 @@ pub async fn handle_create_free_test_coins(
);
}

let account_owner_key = sdk.key_manager_api().get_account_owner_key(account_owner_key_id)?;

let transaction = context
.transaction_builder()
.with_fee_instructions_builder(|fee_builder| {
Expand All @@ -656,7 +662,11 @@ pub async fn handle_create_free_test_coins(
.call_method(*account.component_address(), "pay_fee", args![max_fee])
})
.with_inputs(inputs.into_iter().map(|input| input.into_unversioned()))
.build_and_seal(&account_owner_key.secret);
.build();

let transaction = sdk
.local_signer_api()
.sign(KeyBranch::Account, account_owner_key_id, transaction)?;

info!(
target: LOG_TARGET,
Expand Down Expand Up @@ -815,7 +825,6 @@ pub async fn handle_transfer(

// build the transaction
let max_fee = req.max_fee.unwrap_or(DEFAULT_FEE);
let account_owner_key = sdk.key_manager_api().get_account_owner_key(account_owner_key_id)?;

let transaction = builder
.with_dry_run(req.dry_run)
Expand Down Expand Up @@ -844,7 +853,11 @@ pub async fn handle_transfer(
}
})
.with_inputs(inputs.into_iter().map(|req| req.into_unversioned()))
.build_and_seal(&account_owner_key.secret);
.build();

let transaction = sdk
.local_signer_api()
.sign(KeyBranch::Account, account_owner_key_id, transaction)?;

// If dry run we can return the result immediately
if req.dry_run {
Expand Down Expand Up @@ -976,18 +989,20 @@ pub async fn handle_stealth_transfer(

let must_sign_with_account_key =
transfer.fee_inputs.revealed.is_positive() || transfer.transfer_inputs.revealed.is_positive();
let signer_key = if must_sign_with_account_key {
sdk.key_manager_api().get_account_owner_key(owner_key_id)?

let transaction = transfer.transaction.authorized_sealed_signer().build(vec![]);

let (key_branch, key_id) = if must_sign_with_account_key {
(KeyBranch::Account, owner_key_id)
} else {
// Since we don't require account auth, use a throwaway nonce to sign the transaction
sdk.key_manager_api().next_key(KeyBranch::Nonce)?.into()
(
KeyBranch::Nonce,
KeyId::derived(sdk.key_manager_api().next_derived_key_index(KeyBranch::Nonce)?),
)
};

let transaction = transfer
.transaction
.authorized_sealed_signer()
.build(vec![])
.seal(&signer_key.secret);
let transaction = sdk.local_signer_api().sign(key_branch, key_id, transaction)?;

// TODO: if submitting fails we need to unlock the inputs again
if req.dry_run {
Expand Down
9 changes: 2 additions & 7 deletions applications/tari_walletd/src/handlers/confidential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ use serde_json::json;
use tari_crypto::{commitment::HomomorphicCommitmentFactory, keys::PublicKey as _, ristretto::RistrettoPublicKey};
use tari_engine_types::{crypto::get_commitment_factory, ToByteType};
use tari_ootle_wallet_crypto::{AlwaysMissLookupTable, IoReaderValueLookup, UnblindedOutputWitness};
use tari_ootle_wallet_sdk::{
apis::key_manager::KeyBranch,
models::{ConfidentialOutputModel, OutputStatus},
};
use tari_ootle_wallet_sdk::models::{ConfidentialOutputModel, KeyBranch, OutputStatus};
use tari_template_lib::types::Amount;
use tari_wallet_daemon_client::{
permissions::JrpcPermission,
Expand Down Expand Up @@ -290,9 +287,7 @@ pub async fn handle_view_vault_balance(
.ok_or_else(|| invalid_params("vault_id", Some("Vault does not contain a confidential resource")))?;

// Get view secret key
let view_key = sdk
.key_manager_api()
.derive_key(KeyBranch::ElgamalEncryptionViewKey, req.view_key_id)?;
let view_key = sdk.key_manager_api().get_elgamal_encrypted_view_key(req.view_key_id)?;

let value_range = req.minimum_expected_value.unwrap_or(0)..=req.maximum_expected_value.unwrap_or(10_000_000_000);

Expand Down
11 changes: 5 additions & 6 deletions applications/tari_walletd/src/handlers/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
// SPDX-License-Identifier: BSD-3-Clause

use axum_extra::headers::authorization::Bearer;
use tari_crypto::{keys::PublicKey as PublicKeyTrait, ristretto::RistrettoPublicKey};
use tari_engine_types::ToByteType;
use tari_ootle_wallet_sdk::apis::key_manager::KeyBranch;
use tari_ootle_wallet_sdk::models::{KeyBranch, KeyId};
use tari_wallet_daemon_client::{
permissions::JrpcPermission,
types::{
Expand All @@ -29,11 +28,11 @@ pub async fn handle_create(
let key_manager = sdk.key_manager_api();
let key = req
.specific_index
.map(|idx| key_manager.derive_key(req.branch, idx))
.unwrap_or_else(|| key_manager.next_key(req.branch))?;
.map(|idx| key_manager.get_public_key(req.branch, KeyId::derived(idx)))
.unwrap_or_else(|| key_manager.next_public_key(req.branch))?;
Ok(KeysCreateResponse {
id: key.key_index,
public_key: RistrettoPublicKey::from_secret_key(&key.key).to_byte_type(),
id: key.key_id.derived_index().expect("Key is derived"),
public_key: key.public_key.to_byte_type(),
})
Comment on lines +34 to 36

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid panic on non-derived key; map to a user error.

expect("Key is derived") will crash the handler if invariants change. Convert this to a proper error and return 400/500 instead.

Apply this diff to handle the error explicitly:

-    Ok(KeysCreateResponse {
-        id: key.key_id.derived_index().expect("Key is derived"),
-        public_key: key.public_key.to_byte_type(),
-    })
+    let id = key
+        .key_id
+        .derived_index()
+        .ok_or_else(|| anyhow::anyhow!("Expected a derived key from key_manager_api()"))?;
+    Ok(KeysCreateResponse {
+        id,
+        public_key: key.public_key.to_byte_type(),
+    })
🤖 Prompt for AI Agents
In applications/tari_walletd/src/handlers/keys.rs around lines 34 to 36, the
code calls key.key_id.derived_index().expect("Key is derived") which can panic;
replace the expect with explicit error handling that maps a non-derived key into
a user-facing error and returns an appropriate HTTP response (e.g., 400 Bad
Request) or Result::Err for internal handlers. Change the handler to propagate
the Result from derived_index() (or match its Option) and construct a clear
error variant/message when the key is not derived, then return that error (or
convert it to a response with status 400/500) instead of panicking.

}

Expand Down
35 changes: 22 additions & 13 deletions applications/tari_walletd/src/handlers/nfts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use tari_engine_types::{
ToByteType,
};
use tari_ootle_common_types::{optional::Optional, SubstateRequirement};
use tari_ootle_wallet_sdk::apis::substate::ValidatorScanResult;
use tari_ootle_wallet_sdk::{apis::substate::ValidatorScanResult, models::KeyBranch};
use tari_template_builtin::ACCOUNT_TEMPLATE_ADDRESS;
use tari_template_lib::{
constants::{NFT_FAUCET_COMPONENT_ADDRESS, NFT_FAUCET_RESOURCE_ADDRESS},
Expand Down Expand Up @@ -85,7 +85,6 @@ pub async fn handle_mint_faucet(
req: MintFaucetNftRequest,
) -> Result<MintFaucetNftResponse, anyhow::Error> {
let sdk = context.wallet_sdk();
let key_manager_api = sdk.key_manager_api();
context.check_auth(token, &[JrpcPermission::Admin])?;

let account = get_account(&req.account, &sdk.accounts_api())?;
Expand All @@ -95,8 +94,6 @@ pub async fn handle_mint_faucet(
.owner_key_id
.ok_or_else(|| invalid_params("account", Some("The account does not have an owner key ID")))?;

let signing_key = key_manager_api.get_account_owner_key(account_owner_key_id)?;

info!(target: LOG_TARGET, "🎮 Minting new NFT with metadata {}", req.mutable_data);

let mutable_data = convert_json_to_cbor(req.mutable_data).map_err(|e| invalid_params("mutable_data", Some(e)))?;
Expand All @@ -122,7 +119,11 @@ pub async fn handle_mint_faucet(
.with_inputs(inputs.into_iter().map(|input| input.into_unversioned()))
.add_input(NFT_FAUCET_COMPONENT_ADDRESS)
.add_input(NFT_FAUCET_RESOURCE_ADDRESS)
.build_and_seal(&signing_key.secret);
.build();

let transaction = sdk
.local_signer_api()
.sign(KeyBranch::Account, account_owner_key_id, transaction)?;

let mut events = context.notifier().subscribe();
let tx_id = context.transaction_service().submit_transaction(transaction).await?;
Expand Down Expand Up @@ -292,21 +293,29 @@ pub async fn handle_transfer(
.call_method(target_account_address, "deposit", args![Workspace(format!("b-{i}"))]);
}

let fee_owner_key = sdk.key_manager_api().get_account_owner_key(fee_payer_key_id)?;

let source_account_secret_key = sdk.key_manager_api().get_account_owner_key(account_owner_key_id)?;
let fee_owner_key = sdk
.key_manager_api()
.get_public_key(KeyBranch::Account, fee_payer_key_id)?;

let transaction = builder
.with_dry_run(req.dry_run)
.fee_transaction_pay_from_component(fee_payer_account_address, req.max_fee)
.with_inputs(inputs.into_iter().map(|input| input.into_unversioned()))
// Seal signer is the fee payer account
.with_authorized_seal_signer()
.add_signer(
&fee_owner_key.to_public_key().to_byte_type(),
&source_account_secret_key.secret,
)
.build_and_seal(&fee_owner_key.secret);
.then(|builder| {
sdk.local_signer_api().sign_with_context(
KeyBranch::Account,
account_owner_key_id,
&fee_owner_key.public_key().to_byte_type(),
builder,
)
})?
.build();

let transaction = sdk
.local_signer_api()
.sign(KeyBranch::Account, account_owner_key_id, transaction)?;

// if dry run, we can return the result immediately
if req.dry_run {
Expand Down
5 changes: 1 addition & 4 deletions applications/tari_walletd/src/handlers/stealth_utxos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use axum_extra::headers::authorization::Bearer;
use indexmap::IndexMap;
use log::info;
use tari_ootle_wallet_crypto::{AlwaysMissLookupTable, IoReaderValueLookup};
use tari_ootle_wallet_sdk::apis::key_manager::KeyBranch;
use tari_template_lib::models::UtxoAddress;
use tari_wallet_daemon_client::{
permissions::JrpcPermission,
Expand Down Expand Up @@ -80,9 +79,7 @@ pub async fn handle_decrypt_value(
let substates = sdk.substate_api().get_substates_from_network(utxo_ids).await?;

// Get view secret key
let view_key = sdk
.key_manager_api()
.derive_key(KeyBranch::ElgamalEncryptionViewKey, req.view_key_id)?;
let view_key = sdk.key_manager_api().get_elgamal_encrypted_view_key(req.view_key_id)?;

let value_range = req.minimum_expected_value.unwrap_or(0)..=req.maximum_expected_value.unwrap_or(10_000_000_000);

Expand Down
45 changes: 26 additions & 19 deletions applications/tari_walletd/src/handlers/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use log::*;
use tari_engine_types::ToByteType;
use tari_ootle_common_types::{optional::Optional, Epoch, Network};
use tari_ootle_wallet_sdk::{
apis::{config::ConfigKey, key_manager::KeyBranch, transaction::TransactionApiError},
apis::{config::ConfigKey, transaction::TransactionApiError},
models::KeyBranch,
network::WalletQueryErrorStatus,
};
use tari_ootle_wallet_sdk_services::{events::WalletEvent, transaction_service::TransactionServiceError};
Expand Down Expand Up @@ -104,8 +105,7 @@ pub async fn handle_submit(
let sdk = context.wallet_sdk();
let key_api = sdk.key_manager_api();
// Fetch the key to sign the transaction
// TODO: Ideally the SDK should take care of signing the transaction internally
let key = key_api.get_key_or_active(KeyBranch::Account, req.signing_key_id)?;
let signing_key = key_api.get_key_or_active(KeyBranch::Account, req.signing_key_id)?;

let detected_inputs = if req.detect_inputs {
// If we are not overriding inputs, we will use inputs that we know about in the local substate id db
Expand Down Expand Up @@ -149,7 +149,10 @@ pub async fn handle_submit(
.transaction_builder()
.with_unsigned_transaction(req.transaction)
.with_inputs(detected_inputs)
.build_and_seal(&key.secret);
.build();
let transaction = sdk
.local_signer_api()
.sign(KeyBranch::Account, signing_key.key_id, transaction)?;

if log_enabled!(log::Level::Debug) {
for input in transaction.inputs() {
Expand Down Expand Up @@ -212,7 +215,6 @@ pub async fn handle_submit_dry_run(
let sdk = context.wallet_sdk();
let key_api = sdk.key_manager_api();
// Fetch the key to sign the transaction
// TODO: Ideally the SDK should take care of signing the transaction internally
let key = key_api.get_key_or_active(KeyBranch::Account, req.signing_key_id)?;

let detected_inputs = if req.detect_inputs {
Expand Down Expand Up @@ -242,7 +244,10 @@ pub async fn handle_submit_dry_run(
.with_unsigned_transaction(req.transaction)
.with_inputs(detected_inputs)
.with_dry_run(true)
.build_and_seal(&key.secret);
.build();
let transaction = sdk
.local_signer_api()
.sign(KeyBranch::Account, key.key_id, transaction)?;

for proof_id in req.proof_ids {
// update the proofs table with the corresponding transaction hash
Expand Down Expand Up @@ -307,13 +312,11 @@ pub async fn handle_submit_manifest(
let key = sdk
.key_manager_api()
.get_key_or_active(KeyBranch::Account, Some(signing_key_id))?;
let seal_signer_pk = key.to_public_key();

let network = context.wallet_sdk().config_api().get::<Network>(ConfigKey::Network)?;

let fee_amount = req.max_fee;

let acc_key = sdk.key_manager_api().get_account_owner_key(account_owner_key_id)?;
let builder = Transaction::builder()
.for_network(network.as_byte())
.with_fee_instructions_builder(|builder| {
Expand All @@ -326,28 +329,32 @@ pub async fn handle_submit_manifest(
.with_instructions(instructions.instructions)
.then(|builder| {
if signing_key_id == account_owner_key_id {
builder
Ok(builder)
} else {
builder.add_signer(&seal_signer_pk.to_byte_type(), &acc_key.secret)
sdk.local_signer_api().sign_with_context(
KeyBranch::Account,
signing_key_id,
&key.public_key().to_byte_type(),
builder,
)
}
});
})?;
let signatures = builder.signatures().to_vec();
let mut transaction = builder.build_unsigned_transaction();
let transaction = builder.with_dry_run(req.dry_run).build_unsigned_transaction();

// Detect inputs
let substates = transaction.to_referenced_substates()?;
let substates = substates.into_iter().collect::<Vec<_>>();
let substates = transaction.to_referenced_substates()?.into_iter().collect::<Vec<_>>();
let dependencies = sdk.substate_api().locate_dependent_substates(&substates, true).await?;
let inputs = dependencies.into_iter().map(|input| input.into_unversioned());

// set currently requested dry run status
transaction.set_dry_run(req.dry_run);

let transaction = transaction
.with_inputs(inputs)
.authorized_sealed_signer()
.build(signatures)
.seal(&key.secret);
.build(signatures);

let transaction = sdk
.local_signer_api()
.sign(KeyBranch::Account, key.key_id, transaction)?;

if req.dry_run {
let exec_result = context
Expand Down
Loading
Loading