Skip to content

Commit

Permalink
further review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
hansieodendaal committed Oct 12, 2023
1 parent 663c40f commit 861866d
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 19 deletions.
40 changes: 26 additions & 14 deletions base_layer/core/src/transactions/tari_amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,34 +113,40 @@ impl MicroMinotari {
Self(0)
}

pub fn checked_add(self, v: MicroMinotari) -> Option<MicroMinotari> {
self.as_u64().checked_add(v.as_u64()).map(Into::into)
pub fn checked_add<T>(&self, v: T) -> Option<MicroMinotari>
where T: AsRef<MicroMinotari> {
self.as_u64().checked_add(v.as_ref().as_u64()).map(Into::into)
}

pub fn checked_sub(self, v: MicroMinotari) -> Option<MicroMinotari> {
if self >= v {
return Some(self - v);
pub fn checked_sub<T>(&self, v: T) -> Option<MicroMinotari>
where T: AsRef<MicroMinotari> {
if self >= v.as_ref() {
return Some(self - v.as_ref());
}
None
}

pub fn checked_mul(self, v: MicroMinotari) -> Option<MicroMinotari> {
self.as_u64().checked_mul(v.as_u64()).map(Into::into)
pub fn checked_mul<T>(&self, v: T) -> Option<MicroMinotari>
where T: AsRef<MicroMinotari> {
self.as_u64().checked_mul(v.as_ref().as_u64()).map(Into::into)
}

pub fn checked_div(self, v: MicroMinotari) -> Option<MicroMinotari> {
self.as_u64().checked_div(v.as_u64()).map(Into::into)
pub fn checked_div<T>(&self, v: T) -> Option<MicroMinotari>
where T: AsRef<MicroMinotari> {
self.as_u64().checked_div(v.as_ref().as_u64()).map(Into::into)
}

pub fn saturating_sub(self, v: MicroMinotari) -> MicroMinotari {
if self >= v {
return self - v;
pub fn saturating_sub<T>(&self, v: T) -> MicroMinotari
where T: AsRef<MicroMinotari> {
if self >= v.as_ref() {
return self - v.as_ref();
}
Self(0)
}

pub fn saturating_add(self, v: MicroMinotari) -> MicroMinotari {
self.0.saturating_add(v.0).into()
pub fn saturating_add<T>(&self, v: T) -> MicroMinotari
where T: AsRef<MicroMinotari> {
self.0.saturating_add(v.as_ref().0).into()
}

#[inline]
Expand All @@ -153,6 +159,12 @@ impl MicroMinotari {
}
}

impl AsRef<MicroMinotari> for MicroMinotari {
fn as_ref(&self) -> &MicroMinotari {
self
}
}

#[allow(clippy::identity_op)]
impl Display for MicroMinotari {
fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ where KM: TransactionKeyManagerInterface
let total_input_value = [total_to_self, total_amount, fee_without_change]
.iter()
.fold(Ok(MicroMinotari::zero()), |acc, x| {
acc?.checked_add(*x).ok_or("Total input value overflow")
acc?.checked_add(x).ok_or("Total input value overflow")
})?;
let change_amount = total_being_spent.checked_sub(total_input_value);
match change_amount {
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/tests/tests/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ async fn test_fee_overflow() {
kernels.push(TransactionKernel {
version: kernel.version,
features: kernel.features,
fee: (u64::MAX / 2).into(),
fee: (u64::MAX / 2).into(), // This is the adversary's attack!
lock_height: kernel.lock_height,
excess_sig: kernel.excess_sig.clone(),
excess: kernel.excess.clone(),
Expand Down Expand Up @@ -448,7 +448,7 @@ async fn test_fee_overflow() {
let schema = txn_schema!(
from: vec![outputs[1][3].clone()],
to: vec![],
fee: MicroMinotari(u64::MAX / 2),
fee: MicroMinotari(u64::MAX / 2), // This is the adversary's attack!
lock: 0,
features: OutputFeatures::default()
);
Expand Down
8 changes: 6 additions & 2 deletions base_layer/core/tests/tests/node_comms_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ async fn initialize_sender_transaction_protocol_for_overflow_test(
async fn test_sender_transaction_protocol_for_overflow() {
let key_manager = create_test_core_key_manager_with_memory_db();
let script = script!(Nop);
let amount = MicroMinotari(u64::MAX);
let amount = MicroMinotari(u64::MAX); // This is the adversary's attack!
let output_features = OutputFeatures::default();
let covenant = Covenant::default();
let (utxo, spending_key_id, sender_offset_key_id) = create_utxo(
Expand Down Expand Up @@ -401,6 +401,7 @@ async fn test_sender_transaction_protocol_for_overflow() {

// Test overflow in inputs
let txn_schema =
// This is the adversary's attack!
txn_schema!(from: vec![wallet_output.clone(), wallet_output.clone()], to: vec![MicroMinotari(5_000)]);
let stx_builder = initialize_sender_transaction_protocol_for_overflow_test(&key_manager, txn_schema).await;
assert_eq!(
Expand All @@ -410,6 +411,7 @@ async fn test_sender_transaction_protocol_for_overflow() {

// Test overflow in outputs to self
let txn_schema =
// This is the adversary's attack!
txn_schema!(from: vec![wallet_output.clone()], to: vec![MicroMinotari(5_000), MicroMinotari(u64::MAX)]);
let stx_builder = initialize_sender_transaction_protocol_for_overflow_test(&key_manager, txn_schema).await;
assert_eq!(
Expand All @@ -418,7 +420,9 @@ async fn test_sender_transaction_protocol_for_overflow() {
);

// Test overflow in total input value (inputs + outputs to self + fee)
let txn_schema = txn_schema!(from: vec![wallet_output], to: vec![MicroMinotari(u64::MAX)]);
let txn_schema =
// This is the adversary's attack!
txn_schema!(from: vec![wallet_output], to: vec![MicroMinotari(u64::MAX)]);
let stx_builder = initialize_sender_transaction_protocol_for_overflow_test(&key_manager, txn_schema).await;
assert_eq!(
format!("{:?}", stx_builder.build().await.unwrap_err()),
Expand Down

0 comments on commit 861866d

Please sign in to comment.