From 64f67df94eaa6d1322514a2fdcd8bf0e33858246 Mon Sep 17 00:00:00 2001 From: David Rusu Date: Mon, 10 May 2021 14:17:54 -0400 Subject: [PATCH] fix(mint): validate output numbering --- src/dbc.rs | 9 ++++---- src/error.rs | 2 ++ src/lib.rs | 25 +++++++++++++++++++++ src/mint.rs | 62 ++++++++++++++++++++++++++++++++++++++++------------ 4 files changed, 79 insertions(+), 19 deletions(-) diff --git a/src/dbc.rs b/src/dbc.rs index e66a8e4..db8a20b 100644 --- a/src/dbc.rs +++ b/src/dbc.rs @@ -59,7 +59,7 @@ mod tests { use quickcheck_macros::quickcheck; - use crate::tests::TinyInt; + use crate::tests::{NonZeroTinyInt, TinyInt}; use crate::{Mint, MintRequest}; fn divide(amount: u64, n_ways: u8) -> impl Iterator { @@ -110,9 +110,8 @@ mod tests { } #[quickcheck] - #[ignore] fn prop_dbc_validation( - n_inputs: TinyInt, // # of input DBC's + n_inputs: NonZeroTinyInt, // # of input DBC's n_valid_sigs: TinyInt, // # of valid sigs n_wrong_signer_sigs: TinyInt, // # of valid sigs from unrecognized authority n_wrong_msg_sigs: TinyInt, // # of sigs from recognized authority signing wrong message @@ -229,7 +228,7 @@ mod tests { Ok(()) => { assert!(dbc.transaction.outputs.contains(&dbc.content.hash())); assert!(n_inputs.coerce::() > 0); - assert!(n_valid_sigs >= n_inputs); + assert!(n_valid_sigs.coerce::() >= n_inputs.coerce::()); assert_eq!(dbc.amount(), amount); assert_eq!(n_extra_input_sigs.coerce::(), 0); assert_eq!(n_wrong_signer_sigs.coerce::(), 0); @@ -239,7 +238,7 @@ mod tests { assert_eq!(n_drop_parents.coerce::(), 0); } Err(Error::MissingSignatureForInput) => { - assert!(n_valid_sigs < n_inputs); + assert!(n_valid_sigs.coerce::() < n_inputs.coerce::()); } Err(Error::Ed25519(_)) => { assert!(n_wrong_msg_sigs.coerce::() > 0); diff --git a/src/error.rs b/src/error.rs index 0f8262b..ebd1c34 100644 --- a/src/error.rs +++ b/src/error.rs @@ -30,6 +30,8 @@ pub enum Error { MissingSignatureForInput, #[error("Mint request doesn't balance out sum(input) == sum(output)")] DbcMintRequestDoesNotBalance, + #[error("Outputs must be numbered 0..N where N = # of outputs")] + OutputsAreNotNumberedCorrectly, #[error("DBC already spent in transaction: {transaction:?}")] DbcAlreadySpent { transaction: crate::DbcTransaction, diff --git a/src/lib.rs b/src/lib.rs index deb266c..2591bd6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -67,6 +67,31 @@ mod tests { } } + #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] + pub struct NonZeroTinyInt(u8); + + impl NonZeroTinyInt { + pub fn coerce>(self) -> T { + self.0.into() + } + } + + impl std::fmt::Debug for NonZeroTinyInt { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } + } + + impl Arbitrary for NonZeroTinyInt { + fn arbitrary(g: &mut Gen) -> Self { + Self(u8::arbitrary(g) % 4 + 1) + } + + fn shrink(&self) -> Box> { + Box::new((0..(self.0)).into_iter().rev().map(Self)) + } + } + #[derive(Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct TinyVec(Vec); diff --git a/src/mint.rs b/src/mint.rs index 0f4e2f0..5c97e5e 100644 --- a/src/mint.rs +++ b/src/mint.rs @@ -13,7 +13,7 @@ // input is vaid // Outputs <= input value -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use crate::{ Dbc, DbcContent, DbcContentHash, DbcTransaction, Error, KeyCache, KeyManager, PublicKey, @@ -112,6 +112,7 @@ impl Mint { )> { mint_request.verify_transaction_balances()?; self.validate_transaction_input_dbcs(&mint_request.inputs)?; + self.validate_transaction_outputs(&mint_request.outputs)?; let transaction = mint_request.to_transaction(); let transaction_sigs = self.sign_transaction(&transaction); @@ -143,6 +144,21 @@ impl Mint { Ok(()) } + fn validate_transaction_outputs(&self, outputs: &HashSet) -> Result<()> { + let number_set = outputs + .iter() + .map(|dbc_content| dbc_content.output_number.into()) + .collect::>(); + + let expected_number_set = (0..outputs.len()).into_iter().collect::>(); + + if number_set != expected_number_set { + return Err(Error::OutputsAreNotNumberedCorrectly); + } + + Ok(()) + } + fn sign_transaction( &self, transaction: &DbcTransaction, @@ -272,7 +288,7 @@ mod tests { #[quickcheck] fn prop_dbc_transaction_many_to_many( input_amounts: TinyVec, - output_amounts: TinyVec, + output_amounts: TinyVec<(TinyInt, TinyInt)>, ) { let genesis_amount = input_amounts.vec().iter().map(|i| i.coerce::()).sum(); @@ -308,8 +324,13 @@ mod tests { let outputs: HashSet<_> = output_amounts .vec() .iter() - .enumerate() - .map(|(i, amount)| DbcContent::new(input_hashes.clone(), amount.coerce(), i as u8)) + .map(|(output_number, amount)| { + DbcContent::new( + input_hashes.clone(), + amount.coerce(), + output_number.coerce(), + ) + }) .collect(); let mint_request = MintRequest { @@ -319,10 +340,13 @@ mod tests { let many_to_many_result = genesis.reissue(mint_request.clone()); + let output_amounts_set: BTreeSet<_> = output_amounts.vec().into_iter().collect(); + let output_amount: u64 = output_amounts_set + .iter() + .map(|(_, amount)| amount.coerce::()) + .sum(); match many_to_many_result { Ok((transaction, transaction_sigs)) => { - let output_amount: u64 = - output_amounts.vec().iter().map(|i| i.coerce::()).sum(); assert_eq!(genesis_amount, output_amount); let output_dbcs: Vec<_> = outputs @@ -345,25 +369,35 @@ mod tests { output_dbcs.iter().map(|dbc| dbc.amount()).sum::(), output_amount ); + + // The outputs should have been uniquely number from 0 to N (N = # of outputs) + assert_eq!( + output_amounts_set + .iter() + .map(|(output_number, _)| output_number.coerce()) + .collect::>(), + (0..output_amounts_set.len()).into_iter().collect() + ); } Err(Error::DbcMintRequestDoesNotBalance) => { - let output_amount: u64 = - output_amounts.vec().iter().map(|i| i.coerce::()).sum(); assert_ne!(genesis_amount, output_amount); } Err(Error::TransactionMustHaveAnInput) => { assert_eq!(input_amounts.vec().len(), 0); } + Err(Error::OutputsAreNotNumberedCorrectly) => { + assert_ne!( + output_amounts_set + .iter() + .map(|(output_number, _)| output_number.coerce()) + .collect::>(), + (0..output_amounts_set.len()).into_iter().collect() + ); + } err => panic!("Unexpected reissue err {:#?}", err), } } - #[quickcheck] - #[ignore] - fn prop_dbc_ensure_outputs_are_numbered_uniquely() { - todo!() - } - #[quickcheck] #[ignore] fn prop_in_progress_transaction_can_be_continued_across_churn() {