From ecfbefe9f89067829015c5fe36b8ba11947fdf9d Mon Sep 17 00:00:00 2001 From: Alan Szepieniec Date: Thu, 12 Dec 2024 00:47:38 +0100 Subject: [PATCH] feat: Harden native currency Specifically: sanity-check sum of outputs. Also: fix excessive positive fee test generator. --- .../transaction/primitive_witness.rs | 81 ++++++++++++++----- .../type_scripts/native_currency.rs | 60 ++++++++++---- .../blockchain/type_scripts/neptune_coins.rs | 2 +- 3 files changed, 106 insertions(+), 37 deletions(-) diff --git a/src/models/blockchain/transaction/primitive_witness.rs b/src/models/blockchain/transaction/primitive_witness.rs index 8e6dd45d8..5fdc904c5 100644 --- a/src/models/blockchain/transaction/primitive_witness.rs +++ b/src/models/blockchain/transaction/primitive_witness.rs @@ -1224,62 +1224,105 @@ mod test { } /// A strategy for primitive witnesses with 1 input, 2 outputs, and the - /// given fee. The fee can be negative or even an invalid amount - /// (greater than the maximum number of nau). + /// given fee. The fee can be negative or even an invalid amount: + /// greater than the maximum number of nau. It does *not* work for fees + /// smaller than the minimum number of nau. pub(crate) fn arbitrary_with_fee(fee: NeptuneCoins) -> BoxedStrategy { - let total_amount_strategy = if fee.is_negative() { - (-fee).to_nau().try_into().unwrap()..(NeptuneCoins::MAX_NAU) - } else { - std::convert::TryInto::::try_into(fee.to_nau()).unwrap() - ..(NeptuneCoins::MAX_NAU) + let fee_as_i128 = std::convert::TryInto::::try_into(fee.to_nau()).unwrap(); + let total_amount_strategy = match (fee.is_negative(), fee.abs() > NeptuneCoins::max()) { + (false, false) => { + // positive or zero fee, valid amount + // ensure that total amount > fee + fee_as_i128..NeptuneCoins::MAX_NAU + } + (false, true) => { + // positive fee, greater than max nau + // ensure that total_amount > fee + fee_as_i128..i128::MAX + } + (true, false) => { + // negative fee, valid amount + // timelocked_amount = -fee/2 + // liquid_amount = total_amount - timelocked_amount - fee + // so: + // * total_amount > timelocked_amount + // * total_amount - timelocked_amount - fee <= NeptuneCoins::max + // or rephrased: + // * -fee/2 < total_amount <= NeptuneCoins::max + fee/2 + // ensure that total_amount - fee < MAX_NAU + (-fee_as_i128 >> 1)..(NeptuneCoins::MAX_NAU + fee_as_i128 + 1) + } + (true, true) => { + // negative fee, smaller than min nau + // timelocked_amount = -fee/2 + // liquid_amount = total_amount - timelocked_amount - fee + // so: + // * total_amount > timelocked_amount (otherwise bad sub) + // * total_amount - timelocked_amount - fee <= NeptuneCoins::max (otherwise bad add) + // or rephrased: + // * -fee/2 < total_amount <= NeptuneCoins::max + fee/2 + // except, this can only work if 0 < NeptuneCoins::max + fee + // which would imply that fee was a valid amount. So in + // other words, this case should never happen. + panic!("fees smaller than minimum amount of nau are not supported"); + } }; let num_outputs = 2; + ( total_amount_strategy, arb::(), vec(arb::(), num_outputs), arb::(), + NeptuneCoins::arbitrary_non_negative(), ) .prop_flat_map( - move |(amount, input_address_seed, output_seeds, mut timestamp)| { + move |( + amount, + input_address_seed, + output_seeds, + mut timestamp, + extra_amount, + )| { while timestamp + COINBASE_TIME_LOCK_PERIOD < timestamp { timestamp = Timestamp::millis(timestamp.to_millis() >> 1); } let total_amount = NeptuneCoins::from_raw_i128(amount); + let (input_utxos, input_lock_scripts_and_witnesses) = Self::transaction_inputs_from_address_seeds_and_amounts( &[input_address_seed], &[total_amount], ); + // populate outputs differently depending on sign of fee let output_utxos = if fee.is_negative() { + // If you set a negative fee, then half of the + // absolute value of that fee must be time-locked. let mut timelocked_amount = -fee; timelocked_amount.div_two(); + assert!(total_amount >= timelocked_amount); let timelocked_output = Utxo::new_native_currency( LockScript::hash_lock(output_seeds[0]), timelocked_amount, ) .with_time_lock(timestamp + COINBASE_TIME_LOCK_PERIOD); - let liquid_amount = NeptuneCoins::from_raw_i128( - i128::try_from(total_amount.to_nau()) - .unwrap() - .checked_sub( - i128::try_from(timelocked_amount.to_nau()).unwrap(), - ) - .unwrap() - .wrapping_add(i128::try_from((-fee).to_nau()).unwrap()), - ); + let mut liquid_amount = + total_amount.checked_sub(&timelocked_amount).unwrap(); + liquid_amount = liquid_amount.checked_add(&(-fee)).unwrap(); let liquid_output = Utxo::new_native_currency( LockScript::hash_lock(output_seeds[0]), liquid_amount, ); + assert_eq!(timelocked_amount + liquid_amount + fee, total_amount); + vec![timelocked_output, liquid_output] } else { - let mut first_amount = fee; - first_amount.div_two(); + // positive fee + let mut first_amount = extra_amount; while total_amount .checked_sub(&fee) .unwrap() diff --git a/src/models/blockchain/type_scripts/native_currency.rs b/src/models/blockchain/type_scripts/native_currency.rs index ea7850b55..ba77588c2 100644 --- a/src/models/blockchain/type_scripts/native_currency.rs +++ b/src/models/blockchain/type_scripts/native_currency.rs @@ -51,6 +51,8 @@ const TOO_BIG_COIN_FIELD_SIZE_ERROR: i128 = 1_000_036; const STATE_LENGTH_FOR_TIME_LOCK_NOT_ONE_ERROR: i128 = 1_000_037; const FEE_EXCEEDS_MAX: i128 = 1_000_038; const FEE_EXCEEDS_MIN: i128 = 1_000_039; +const SUM_OF_OUTPUTS_EXCEEDS_MAX: i128 = 1_000_040; +const SUM_OF_OUTPUTS_IS_NEGATIVE: i128 = 1_000_041; /// `NativeCurrency` is the type script that governs Neptune's native currency, /// Neptune coins. @@ -589,6 +591,44 @@ impl ConsensusProgram for NativeCurrency { call {loop_utxos_add_amounts} // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] + // sanity check total output + dup 7 + dup 7 + dup 7 + dup 7 + // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] [total_output] + + {&push_max_amount} + // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] [total_output] [max_nau] + + call {i128_lt} + // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] (max_nau < total_output) + + push 0 eq + // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] (max_nau >= total_output) + + assert error_id {SUM_OF_OUTPUTS_EXCEEDS_MAX} + + push 0 + push 0 + push 0 + push 0 + // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] [0] + + dup 11 + dup 11 + dup 11 + dup 11 + // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] [0] [total_output] + + call {i128_lt} + // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] (total_output < 0) + + push 0 eq + // _ [txkmh] *ncw *coinbase *fee *salted_output_utxos N N *input_utxos[N]_si * * * [total_input] *fee N N *output_utxos[N]_si * * * [total_output] [timelocked_amount] (total_output >= 0) + + assert error_id {SUM_OF_OUTPUTS_IS_NEGATIVE} + // add half of fee to timelocked amount dup 14 push {coin_size-1} add @@ -1527,22 +1567,6 @@ pub mod test { assert_both_rust_and_tasm_halt_gracefully(NativeCurrencyWitness::from(primitive_witness))?; } - #[test] - fn fee_can_be_negative_deterministic() { - let mut test_runner = TestRunner::deterministic(); - for _ in 0..10 { - let fee = NeptuneCoins::arbitrary_non_negative() - .new_tree(&mut test_runner) - .unwrap() - .current(); - let pw = PrimitiveWitness::arbitrary_with_fee(-fee) - .new_tree(&mut test_runner) - .unwrap() - .current(); - assert_both_rust_and_tasm_halt_gracefully(NativeCurrencyWitness::from(pw)).unwrap(); - } - } - #[proptest] fn fee_can_be_negative( #[strategy(NeptuneCoins::arbitrary_non_negative())] _fee: NeptuneCoins, @@ -1552,7 +1576,6 @@ pub mod test { assert_both_rust_and_tasm_halt_gracefully(NativeCurrencyWitness::from(primitive_witness))?; } - #[ignore] #[proptest] fn positive_fee_cannot_exceed_max_nau( #[strategy(invalid_positive_amount())] _fee: NeptuneCoins, @@ -1576,5 +1599,8 @@ pub mod test { NativeCurrencyWitness::from(primitive_witness), &[FEE_EXCEEDS_MIN], ); + + // It is actually impossible to trigger this assert error id -- or is it? + // I'm not convinced. } } diff --git a/src/models/blockchain/type_scripts/neptune_coins.rs b/src/models/blockchain/type_scripts/neptune_coins.rs index a646510be..35c814c1c 100644 --- a/src/models/blockchain/type_scripts/neptune_coins.rs +++ b/src/models/blockchain/type_scripts/neptune_coins.rs @@ -338,7 +338,7 @@ impl CheckedAdd for NeptuneCoins { /// smaller than the maximum number of nau. fn checked_add(&self, v: &Self) -> Option { self.0.checked_add(v.0).and_then(|sum| { - if sum > Self::MAX_NAU || sum < -Self::MAX_NAU { + if !(-Self::MAX_NAU..=Self::MAX_NAU).contains(&sum) { None } else { Some(Self(sum))