Skip to content

Commit

Permalink
chore!: audit corrections (#5545)
Browse files Browse the repository at this point in the history
Description
---
- Updates one integer casting to be safer
- Changes a function privacy as per the docs
- Removes outdated comments
- Makes domain hashes consistent in the repo

Motivation and Context
---
Minor changes for the audit, mostly in the documentation. 

How Has This Been Tested?
---
CI

Breaking Changes
---

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

Message hash changes will make messages between older nodes incompatible
with messages sent by newer nodes.
  • Loading branch information
brianp authored Jun 28, 2023
1 parent e64efae commit 6c3fa50
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ pub struct ChainMetadataServiceInitializer;
impl ServiceInitializer for ChainMetadataServiceInitializer {
async fn initialize(&mut self, context: ServiceInitializerContext) -> Result<(), ServiceInitializationError> {
debug!(target: LOG_TARGET, "Initializing Chain Metadata Service");
// Buffer size set to 1 because only the most recent metadata is applicable
let (publisher, _) = broadcast::channel(20);

let handle = ChainMetadataHandle::new(publisher.clone());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl ChainMetadataService {
/// ## Arguments
/// `liveness` - the liveness service handle
/// `base_node` - the base node service handle
/// `event_publisher` - A broadcast sender for chain metadata events
pub fn new(
liveness: LivenessHandle,
base_node: LocalNodeCommsInterface,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

use std::{
convert::TryFrom,
fmt::{Display, Formatter},
ops::Deref,
time::{Duration, Instant},
Expand Down Expand Up @@ -67,9 +68,9 @@ pub struct PeerMetadata {

impl PeerMetadata {
pub fn to_bytes(&self) -> Vec<u8> {
let size = bincode::serialized_size(self).unwrap();
#[allow(clippy::cast_possible_truncation)]
let mut buf = Vec::with_capacity(size as usize);
let size = usize::try_from(bincode::serialized_size(self).unwrap())
.expect("The serialized size is larger than the platform allows");
let mut buf = Vec::with_capacity(size);
bincode::serialize_into(&mut buf, self).unwrap(); // this should not fail
buf
}
Expand Down
12 changes: 6 additions & 6 deletions base_layer/core/src/transactions/key_manager/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ use crate::{
},
};

hash_domain!(KeyManagerHashingDomain, "base_layer.core.key_manager");
hash_domain!(
KeyManagerHashingDomain,
"com.tari.base_layer.core.transactions.key_manager"
);

pub struct TransactionKeyManagerInner<TBackend> {
key_managers: HashMap<String, RwLock<KeyManager<PublicKey, KeyDigest>>>,
Expand Down Expand Up @@ -288,8 +291,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
Ok(key_id)
}

// Note!: This method may not be made public
pub async fn get_private_key(&self, key_id: &TariKeyId) -> Result<PrivateKey, KeyManagerServiceError> {
pub(crate) async fn get_private_key(&self, key_id: &TariKeyId) -> Result<PrivateKey, KeyManagerServiceError> {
match key_id {
KeyId::Managed { branch, index } => {
let km = self
Expand Down Expand Up @@ -523,7 +525,6 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
Ok(script_offset)
}

// Note!: This method may not be made public
async fn get_metadata_signature_ephemeral_private_key_pair(
&self,
nonce_id: &TariKeyId,
Expand Down Expand Up @@ -764,7 +765,7 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
private_key - &self.get_txo_private_kernel_offset(spending_key_id, nonce_id).await?
};

// We need to check if its in put or output for which we are singing. Signing with an input, we need to sign
// We need to check if its input or output for which we are singing. Signing with an input, we need to sign
// with `-k` while outputs are `k`
let final_signing_key = if txo_type == TxoStage::Output {
private_signing_key
Expand Down Expand Up @@ -799,7 +800,6 @@ where TBackend: KeyManagerBackend<PublicKey> + 'static
// Encrypted data section (transactions > transaction_components > encrypted_data)
// -----------------------------------------------------------------------------------------------------------------

// Note!: This method may not be made public
async fn get_recovery_key(&self) -> Result<PrivateKey, KeyManagerServiceError> {
let recovery_id = KeyId::Managed {
branch: TransactionKeyManagerBranch::DataEncryption.get_branch_key(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ pub use validator_node_signature::{ValidatorNodeHashDomain, ValidatorNodeSignatu

hash_domain!(
ContractAcceptanceHashDomain,
"com.tari.tari-project.base_layer.core.transactions.side_chain.contract_acceptance_challenge",
"com.tari.base_layer.core.transactions.side_chain.contract_acceptance_challenge",
1
);

pub type ContractAcceptanceHasherBlake256 = DomainSeparatedHasher<Blake256, ContractAcceptanceHashDomain>;

hash_domain!(
SignerSignatureHashDomain,
"com.tari.tari-project.base_layer.core.transactions.side_chain.signer_signature",
"com.tari.base_layer.core.transactions.side_chain.signer_signature",
1
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ use tari_common_types::types::{FixedHash, PrivateKey, PublicKey, Signature};
use tari_crypto::{hash::blake2::Blake256, hash_domain, hashing::DomainSeparatedHasher, keys::PublicKey as PublicKeyT};
use tari_utilities::ByteArray;

hash_domain!(ValidatorNodeHashDomain, "com.tari.dan_layer.validator_node", 0);
hash_domain!(
ValidatorNodeHashDomain,
"com.tari.base_layer.core.transactions.side_chain.validator_node",
0
);

#[derive(Debug, Clone, Hash, PartialEq, Eq, Deserialize, Serialize, BorshSerialize, BorshDeserialize)]
pub struct ValidatorNodeSignature {
Expand All @@ -40,7 +44,6 @@ impl ValidatorNodeSignature {
Self { public_key, signature }
}

// TODO: pass in commitment instead of arbitrary message
pub fn sign(private_key: &PrivateKey, msg: &[u8]) -> Self {
let (secret_nonce, public_nonce) = PublicKey::random_keypair(&mut OsRng);
let public_key = PublicKey::from_secret_key(private_key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ pub struct RecoveryData {
// hash domain
hash_domain!(
CalculateTxIdTransactionProtocolHashDomain,
"com.tari.tari-project.base_layer.core.transactions.transaction_protocol.calculate_tx_id",
"com.tari.base_layer.core.transactions.transaction_protocol.calculate_tx_id",
1
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ pub fn check_outputs<B: BlockchainBackend>(
Ok(())
}

/// This function checks the at the body contains no duplicated inputs or outputs.
/// This function checks the body contains no duplicated inputs or outputs.
fn verify_no_duplicated_inputs_outputs(body: &AggregateBody) -> Result<(), ValidationError> {
if body.contains_duplicated_inputs() {
warn!(
Expand All @@ -273,7 +273,7 @@ fn verify_no_duplicated_inputs_outputs(body: &AggregateBody) -> Result<(), Valid
Ok(())
}

/// THis function checks the total burned sum in the header ensuring that every burned output is counted in the total
/// This function checks the total burned sum in the header ensuring that every burned output is counted in the total
/// sum.
#[allow(clippy::mutable_key_type)]
fn check_total_burned(body: &AggregateBody) -> Result<(), ValidationError> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ fn check_maturity(height: u64, inputs: &[TransactionInput]) -> Result<(), Transa
Ok(())
}

/// THis function checks the total burned sum in the header ensuring that every burned output is counted in the total
/// This function checks the total burned sum in the header ensuring that every burned output is counted in the total
/// sum.
#[allow(clippy::mutable_key_type)]
fn check_total_burned(body: &AggregateBody) -> Result<(), ValidationError> {
Expand Down

0 comments on commit 6c3fa50

Please sign in to comment.