Skip to content

Commit

Permalink
feat: change default script to PushPubKey (tari-project#5653)
Browse files Browse the repository at this point in the history
Description
---
Change the default script to `PushPubKey` to ensure the spending of
outputs from standard transactions is only performed with the use of
specific private keys.

Motivation and Context
---
This change to the default helps enforce that UTXOS can't be spent from
a software wallet, when a hardware wallet is in use.

How Has This Been Tested?
---
Manually, locally, but pretty tests still require updating.

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 -->

---------

Co-authored-by: SW van Heerden <swvheerden@gmail.com>
  • Loading branch information
brianp and SWvheerden committed Aug 30, 2023
1 parent 37bbc29 commit f5b89ad
Show file tree
Hide file tree
Showing 26 changed files with 253 additions and 135 deletions.
Expand Up @@ -744,7 +744,7 @@ where B: BlockchainBackend + 'static
);
if let Err(e) = self
.connectivity
.ban_peer(source_peer.clone(), format!("Peer sen invalid API response"))
.ban_peer(source_peer.clone(), "Peer sent invalid API response".to_string())
.await
{
error!(target: LOG_TARGET, "Failed to ban peer: {}", e);
Expand Down
Expand Up @@ -798,7 +798,7 @@ impl UnconfirmedPool {
#[cfg(test)]
mod test {
use tari_common::configuration::Network;
use tari_script::{inputs, script};
use tari_script::{ExecutionStack, TariScript};

use super::*;
use crate::{
Expand Down Expand Up @@ -924,8 +924,8 @@ mod test {
.with_lock_height(0)
.with_fee_per_gram(5.into())
.with_change_data(
script!(Nop),
inputs!(change.script_key_pk),
TariScript::default(),
ExecutionStack::default(),
change.script_key_id.clone(),
change.spend_key_id.clone(),
Covenant::default(),
Expand Down
27 changes: 27 additions & 0 deletions base_layer/core/src/transactions/key_manager/inner.rs
Expand Up @@ -217,6 +217,33 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
Ok((spend_key_id, spend_public_key, script_key_id, script_public_key))
}

/// Calculates a script key id from the spend key id, if a public key is provided, it will only return a result of
/// the public keys match
pub async fn find_script_key_id_from_spend_key_id(
&self,
spend_key_id: &TariKeyId,
public_script_key: Option<&PublicKey>,
) -> Result<Option<TariKeyId>, KeyManagerServiceError> {
let index = match spend_key_id {
KeyId::Managed { index, .. } => *index,
KeyId::Imported { .. } => return Ok(None),
KeyId::Zero => return Ok(None),
};
let script_key_id = KeyId::Managed {
branch: TransactionKeyManagerBranch::ScriptKey.get_branch_key(),
index,
};

if let Some(key) = public_script_key {
let script_public_key = self.get_public_key_at_key_id(&script_key_id).await?;
if *key == script_public_key {
return Ok(Some(script_key_id));
}
return Ok(None);
}
Ok(Some(script_key_id))
}

/// Search the specified branch key manager key chain to find the index of the specified key.
pub async fn find_key_index(&self, branch: &str, key: &PublicKey) -> Result<u64, KeyManagerServiceError> {
let km = self
Expand Down
6 changes: 6 additions & 0 deletions base_layer/core/src/transactions/key_manager/interface.rs
Expand Up @@ -101,6 +101,12 @@ pub trait TransactionKeyManagerInterface: KeyManagerInterface<PublicKey> {
&self,
) -> Result<(TariKeyId, PublicKey, TariKeyId, PublicKey), KeyManagerServiceError>;

async fn find_script_key_id_from_spend_key_id(
&self,
spend_key_id: &TariKeyId,
public_script_key: Option<&PublicKey>,
) -> Result<Option<TariKeyId>, KeyManagerServiceError>;

async fn get_diffie_hellman_shared_secret(
&self,
secret_key_id: &TariKeyId,
Expand Down
12 changes: 12 additions & 0 deletions base_layer/core/src/transactions/key_manager/wrapper.rs
Expand Up @@ -206,6 +206,18 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
.await
}

async fn find_script_key_id_from_spend_key_id(
&self,
spend_key_id: &TariKeyId,
public_script_key: Option<&PublicKey>,
) -> Result<Option<TariKeyId>, KeyManagerServiceError> {
self.transaction_key_manager_inner
.read()
.await
.find_script_key_id_from_spend_key_id(spend_key_id, public_script_key)
.await
}

async fn get_diffie_hellman_shared_secret(
&self,
secret_key_id: &TariKeyId,
Expand Down
12 changes: 8 additions & 4 deletions base_layer/core/src/transactions/test_helpers.rs
Expand Up @@ -615,8 +615,8 @@ pub async fn create_transaction_with(
.with_fee_per_gram(fee_per_gram)
.with_kernel_features(KernelFeatures::empty())
.with_change_data(
script!(Nop),
inputs!(change.script_key_pk),
TariScript::default(),
ExecutionStack::default(),
change.script_key_id,
change.spend_key_id,
Covenant::default(),
Expand Down Expand Up @@ -658,12 +658,16 @@ pub async fn create_stx_protocol(
.clone();
let mut stx_builder = SenderTransactionProtocol::builder(constants, key_manager.clone());
let change = TestParams::new(key_manager).await;
let script_public_key = key_manager
.get_public_key_at_key_id(&change.script_key_id)
.await
.unwrap();
stx_builder
.with_lock_height(schema.lock_height)
.with_fee_per_gram(schema.fee)
.with_change_data(
script!(Nop),
inputs!(change.script_key_pk),
script!(PushPubKey(Box::new(script_public_key))),
ExecutionStack::default(),
change.script_key_id,
change.spend_key_id,
Covenant::default(),
Expand Down
Expand Up @@ -507,11 +507,12 @@ impl Display for TransactionInput {
..
} => write!(
fmt,
"({}, {}) [{:?}], Script: ({}), Offset_Pubkey: ({}), Input Hash: {}",
"({}, {}) [{:?}], Script: ({}), Input_data : ({}), Offset_Pubkey: ({}), Input Hash: {}",
commitment.to_hex(),
self.output_hash().to_hex(),
features,
script,
self.input_data.to_hex(),
sender_offset_public_key.to_hex(),
self.canonical_hash().to_hex(),
),
Expand Down
Expand Up @@ -1057,7 +1057,7 @@ mod test {
.with_lock_height(0)
.with_fee_per_gram(MicroMinotari(2))
.with_change_data(
script!(Nop),
TariScript::default(),
inputs!(change.script_key_pk),
change.script_key_id.clone(),
change.spend_key_id.clone(),
Expand Down
Expand Up @@ -143,7 +143,7 @@ mod test {
use tari_common_types::types::PublicKey;
use tari_crypto::{keys::PublicKey as PublicKeyTrait, signatures::CommitmentAndPublicKeySignature};
use tari_key_manager::key_manager_service::KeyManagerInterface;
use tari_script::{script, ExecutionStack, TariScript};
use tari_script::{script, ExecutionStack};

use crate::{
covenants::Covenant,
Expand Down Expand Up @@ -255,7 +255,7 @@ mod test {
let m = TransactionMetadata::new(MicroMinotari(100), 0);
let test_params = TestParams::new(&key_manager).await;
let test_params2 = TestParams::new(&key_manager).await;
let script = TariScript::default();
let script = script!(Nop);
let sender_offset_public_key = key_manager
.get_public_key_at_key_id(&test_params.sender_offset_key_id)
.await
Expand Down
Expand Up @@ -581,7 +581,7 @@ where KM: TransactionKeyManagerInterface

#[cfg(test)]
mod test {
use tari_script::{inputs, script, TariScript};
use tari_script::{inputs, script};

use crate::{
covenants::Covenant,
Expand Down Expand Up @@ -696,7 +696,7 @@ mod test {
);

let output = create_wallet_output_with_data(
TariScript::default(),
script!(Nop),
OutputFeatures::default(),
&p,
MicroMinotari(5000) - expected_fee,
Expand Down Expand Up @@ -791,7 +791,7 @@ mod test {
let p = TestParams::new(&key_manager).await;

let output = create_wallet_output_with_data(
TariScript::default(),
script!(Nop),
OutputFeatures::default(),
&p,
MicroMinotari(500),
Expand Down
Expand Up @@ -427,7 +427,7 @@ mod test {
use rand::seq::SliceRandom;
use tari_common::configuration::Network;
use tari_common_types::types::RANGE_PROOF_AGGREGATION_FACTOR;
use tari_script::TariScript;
use tari_script::script;

use super::*;
use crate::{
Expand Down Expand Up @@ -504,7 +504,7 @@ mod test {
100.into(),
&key_manager,
&OutputFeatures::create_burn_output(),
&TariScript::default(),
&script!(Nop),
&Covenant::default(),
0.into(),
)
Expand All @@ -513,7 +513,7 @@ mod test {
101.into(),
&key_manager,
&OutputFeatures::create_burn_output(),
&TariScript::default(),
&script!(Nop),
&Covenant::default(),
0.into(),
)
Expand All @@ -522,7 +522,7 @@ mod test {
102.into(),
&key_manager,
&OutputFeatures::create_burn_output(),
&TariScript::default(),
&script!(Nop),
&Covenant::default(),
0.into(),
)
Expand Down Expand Up @@ -567,7 +567,7 @@ mod test {
100.into(),
&key_manager,
&OutputFeatures::create_burn_output(),
&TariScript::default(),
&script!(Nop),
&Covenant::default(),
0.into(),
)
Expand Down
14 changes: 7 additions & 7 deletions base_layer/core/src/validation/test.rs
Expand Up @@ -25,7 +25,7 @@ use std::{cmp, sync::Arc};
use tari_common::configuration::Network;
use tari_common_types::types::Commitment;
use tari_crypto::commitment::HomomorphicCommitment;
use tari_script::script;
use tari_script::TariScript;
use tari_test_utils::unpack_enum;

use crate::{
Expand Down Expand Up @@ -182,7 +182,7 @@ async fn chain_balance_validation() {
faucet_value,
&key_manager,
&OutputFeatures::default(),
&script!(Nop),
&TariScript::default(),
&Covenant::default(),
MicroMinotari::zero(),
)
Expand Down Expand Up @@ -240,7 +240,7 @@ async fn chain_balance_validation() {
coinbase_value,
&key_manager,
&OutputFeatures::create_coinbase(1, None),
&script!(Nop),
&TariScript::default(),
&Covenant::default(),
MicroMinotari::zero(),
)
Expand Down Expand Up @@ -302,7 +302,7 @@ async fn chain_balance_validation() {
v,
&key_manager,
&OutputFeatures::create_coinbase(1, None),
&script!(Nop),
&TariScript::default(),
&Covenant::default(),
MicroMinotari::zero(),
)
Expand Down Expand Up @@ -367,7 +367,7 @@ async fn chain_balance_validation_burned() {
faucet_value,
&key_manager,
&OutputFeatures::default(),
&script!(Nop),
&TariScript::default(),
&Covenant::default(),
MicroMinotari::zero(),
)
Expand Down Expand Up @@ -425,7 +425,7 @@ async fn chain_balance_validation_burned() {
coinbase_value,
&key_manager,
&OutputFeatures::create_coinbase(1, None),
&script!(Nop),
&TariScript::default(),
&Covenant::default(),
MicroMinotari::zero(),
)
Expand All @@ -451,7 +451,7 @@ async fn chain_balance_validation_burned() {
100.into(),
&key_manager,
&OutputFeatures::create_burn_output(),
&script!(Nop),
&TariScript::default(),
&Covenant::default(),
MicroMinotari::zero(),
)
Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/tests/chain_storage_tests/chain_backend.rs
Expand Up @@ -126,7 +126,7 @@ fn test_utxo_order() {
let mut utxos = Vec::with_capacity(2000);
let version = TransactionOutputVersion::V0;
let features = OutputFeatures::default();
let script = TariScript::default();
let script = script!(Nop);
let proof = RangeProof::default();
let sig = ComAndPubSignature::default();
let covenant = Covenant::default();
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/tests/tests/block_validation.rs
Expand Up @@ -310,7 +310,7 @@ async fn test_orphan_validator() {
let key_manager = create_test_core_key_manager_with_memory_db();
let network = Network::Igor;
let consensus_constants = ConsensusConstantsBuilder::new(network)
.with_max_block_transaction_weight(321)
.with_max_block_transaction_weight(325)
.build();
let (genesis, outputs) = create_genesis_block_with_utxos(&[T, T, T], &consensus_constants, &key_manager).await;
let network = Network::LocalNet;
Expand Down Expand Up @@ -885,7 +885,7 @@ async fn test_block_sync_body_validator() {
matches!(
err,
ValidationError::BlockTooLarge { actual_weight, max_weight } if
actual_weight == 449 && max_weight == 400
actual_weight == 455 && max_weight == 400
),
"{}",
err
Expand Down
17 changes: 4 additions & 13 deletions base_layer/core/tests/tests/mempool.rs
Expand Up @@ -152,7 +152,6 @@ async fn test_insert_and_process_published_block() {
mempool.insert(tx3.clone()).await.unwrap();
mempool.insert(tx5.clone()).await.unwrap();
mempool.process_published_block(blocks[1].to_arc_block()).await.unwrap();

assert_eq!(
mempool
.has_tx_with_excess_sig(orphan.body.kernels()[0].excess_sig.clone())
Expand Down Expand Up @@ -197,18 +196,10 @@ async fn test_insert_and_process_published_block() {
let stats = mempool.stats().await.unwrap();
assert_eq!(stats.unconfirmed_txs, 1);
assert_eq!(stats.reorg_txs, 0);
let expected_weight = consensus_manager
.consensus_constants(0)
.transaction_weight_params()
.calculate(
1,
1,
2,
TestParams::new(&key_manager)
.await
.get_size_for_default_features_and_scripts(2)
.expect("Failed to get size for default features and scripts"),
);
let expected_weight = tx2
.body
.calculate_weight(consensus_manager.consensus_constants(0).transaction_weight_params())
.unwrap();
assert_eq!(stats.unconfirmed_weight, expected_weight);

// Spend tx2, so it goes in Reorg pool
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/tests/tests/node_comms_interface.rs
Expand Up @@ -49,7 +49,7 @@ use tari_core::{
validation::{mocks::MockValidator, transaction::TransactionChainLinkedValidator},
};
use tari_key_manager::key_manager_service::KeyManagerInterface;
use tari_script::{inputs, script, TariScript};
use tari_script::{inputs, script};
use tari_service_framework::reply_channel;
use tokio::sync::{broadcast, mpsc};

Expand Down Expand Up @@ -195,7 +195,7 @@ async fn inbound_fetch_utxos() {
MicroMinotari(10_000),
&key_manager,
&Default::default(),
&TariScript::default(),
&script!(Nop),
&Covenant::default(),
MicroMinotari::zero(),
)
Expand Down

0 comments on commit f5b89ad

Please sign in to comment.