Generate both spend and output values for actions.

src/builder.rs

@@ -177,7 +177,7 @@ pub struct Builder {
 }
 
 impl Builder {
-    /// Construct a new empty builder for an Orchard bundle.
+    /// Constructs a new empty builder for an Orchard bundle.
     pub fn new(flags: Flags, anchor: Anchor) -> Self {
         Builder {
             spends: vec![],
@@ -291,6 +291,10 @@ impl Builder {
             })
             .ok_or(OverflowError)?;
 
+        let result_value_balance: V = i64::try_from(value_balance)
+            .map_err(Error::ValueSum)
+            .and_then(|i| V::try_from(i).map_err(|_| Error::ValueSum(value::OverflowError)))?;
+
         // Compute the transaction binding signing key.
         let bsk = pre_actions
             .iter()
@@ -315,14 +319,10 @@ impl Builder {
             .collect();
         let proof = Proof::create(pk, &circuits, &instances)?;
 
-        let value_balance: V = i64::try_from(value_balance)
-            .map_err(Error::ValueSum)
-            .and_then(|i| V::try_from(i).map_err(|_| Error::ValueSum(value::OverflowError)))?;
-
         Ok(Bundle::from_parts(
             NonEmpty::from_vec(actions).unwrap(),
             flags,
-            value_balance,
+            result_value_balance,
             anchor,
             Unauthorized { proof, bsk },
         ))
@@ -477,7 +477,7 @@ pub mod testing {
         note::testing::arb_note,
         tree::{Anchor, MerklePath},
         value::{
-            testing::{arb_positive_note_value, MAX_MONEY},
+            testing::{arb_positive_note_value, MAX_NOTE_VALUE},
             NoteValue,
         },
         Address, Note,
@@ -534,15 +534,24 @@ pub mod testing {
 
     prop_compose! {
         /// Produce a random valid Orchard bundle.
-        fn arb_bundle_inputs(sk: SpendingKey)(
+        fn arb_bundle_inputs(sk: SpendingKey)
+        (
+            n_notes in 1..30,
+            n_recipients in 1..30,
+        )
+        (
             anchor in prop::array::uniform32(prop::num::u8::ANY).prop_map(Anchor),
-            // generate note values that we're certain won't exceed MAX_MONEY in total
+            // generate note values that we're certain won't exceed MAX_NOTE_VALUE in total
-            notes in vec(arb_positive_note_value(MAX_MONEY as u64 / 10000).prop_flat_map(arb_note), 1..30),
+            notes in vec(
-            recipient_amounts in vec(
+                arb_positive_note_value(MAX_NOTE_VALUE / n_notes as u64).prop_flat_map(arb_note),
-                arb_address().prop_flat_map(
+                n_notes as usize
-                    |a| arb_positive_note_value(MAX_MONEY as u64 / 10000).prop_map(move |v| (a.clone(), v))
             ),
-                1..30
+            recipient_amounts in vec(
+                arb_address().prop_flat_map(move |a| {
+                    arb_positive_note_value(MAX_NOTE_VALUE / n_recipients as u64)
+                        .prop_map(move |v| (a.clone(), v))
+                }),
+                n_recipients as usize
             ),
             rng_seed in prop::array::uniform32(prop::num::u8::ANY)
         ) -> ArbitraryBundleInputs<StdRng, rand_7::rngs::StdRng> {

src/bundle.rs

@@ -26,7 +26,7 @@ pub struct Action<A> {
     rk: redpallas::VerificationKey<SpendAuth>,
     /// A commitment to the new note being created.
     cmx: ExtractedNoteCommitment,
-    /// The transmitted note ciphertext
+    /// The transmitted note ciphertext.
     encrypted_note: TransmittedNoteCiphertext,
     /// A commitment to the net value created or consumed by this action.
     cv_net: ValueCommitment,
@@ -356,6 +356,7 @@ pub struct BundleAuthorizingCommitment;
 pub mod testing {
     use nonempty::NonEmpty;
     use rand_7::{rngs::StdRng, SeedableRng};
+    use reddsa::orchard::SpendAuth;
 
     use proptest::collection::vec;
     use proptest::prelude::*;
@@ -371,9 +372,10 @@ pub mod testing {
             testing::{
                 arb_binding_signing_key, arb_spendauth_signing_key, arb_spendauth_verification_key,
             },
+            Signature,
         },
         value::{
-            testing::{arb_positive_note_value, MAX_MONEY},
+            testing::{arb_nonnegative_note_value, MAX_NOTE_VALUE},
             NoteValue, ValueCommitTrapdoor, ValueCommitment, ValueSum,
         },
         Anchor,
@@ -391,14 +393,14 @@ pub mod testing {
 
     prop_compose! {
         /// Generate an action without authorization data.
-        pub fn arb_unauthorized_action(value: NoteValue)(
+        pub fn arb_unauthorized_action(spend_value: NoteValue, output_value: NoteValue)(
             nf in arb_nullifier(),
             rk in arb_spendauth_verification_key(),
-            note in arb_note(value),
+            note in arb_note(output_value),
         ) -> Action<()> {
             let cmx = ExtractedNoteCommitment::from(note.commitment());
             let cv_net = ValueCommitment::derive(
-                (note.value() - NoteValue::zero()).unwrap(),
+                (spend_value - output_value).unwrap(),
                 ValueCommitTrapdoor::zero()
             );
             // FIXME: make a real one from the note.
@@ -418,18 +420,34 @@ pub mod testing {
         }
     }
 
+    /// Generate an unauthorized action having spend and output values less than MAX_NOTE_VALUE / n_actions.
+    pub fn arb_unauthorized_action_n(
+        n_actions: usize,
+    ) -> impl Strategy<Value = (ValueSum, Action<()>)> {
+        arb_nonnegative_note_value(MAX_NOTE_VALUE / n_actions as u64).prop_flat_map(
+            move |spend_value| {
+                arb_nonnegative_note_value(MAX_NOTE_VALUE / n_actions as u64).prop_flat_map(
+                    move |output_value| {
+                        arb_unauthorized_action(spend_value, output_value)
+                            .prop_map(move |a| ((spend_value - output_value).unwrap(), a))
+                    },
+                )
+            },
+        )
+    }
+
     prop_compose! {
         /// Generate an action with invalid (random) authorization data.
-        pub fn arb_action(value: NoteValue)(
+        pub fn arb_action(spend_value: NoteValue, output_value: NoteValue)(
             nf in arb_nullifier(),
             sk in arb_spendauth_signing_key(),
-            note in arb_note(value),
+            note in arb_note(output_value),
             rng_seed in prop::array::uniform32(prop::num::u8::ANY),
             fake_sighash in prop::array::uniform32(prop::num::u8::ANY),
-        ) -> Action<redpallas::Signature<reddsa::orchard::SpendAuth>> {
+        ) -> Action<Signature<SpendAuth>> {
             let cmx = ExtractedNoteCommitment::from(note.commitment());
             let cv_net = ValueCommitment::derive(
-                (note.value() - NoteValue::zero()).unwrap(),
+                (spend_value - output_value).unwrap(),
                 ValueCommitTrapdoor::zero()
             );
 
@@ -453,6 +471,22 @@ pub mod testing {
         }
     }
 
+    /// Generate an authorized action having spend and output values less than MAX_NOTE_VALUE / n_actions.
+    pub fn arb_action_n(
+        n_actions: usize,
+    ) -> impl Strategy<Value = (ValueSum, Action<Signature<SpendAuth>>)> {
+        arb_nonnegative_note_value(MAX_NOTE_VALUE / n_actions as u64).prop_flat_map(
+            move |spend_value| {
+                arb_nonnegative_note_value(MAX_NOTE_VALUE / n_actions as u64).prop_flat_map(
+                    move |output_value| {
+                        arb_action(spend_value, output_value)
+                            .prop_map(move |a| ((spend_value - output_value).unwrap(), a))
+                    },
+                )
+            },
+        )
+    }
+
     prop_compose! {
         /// Create an arbitrary set of flags.
         pub fn arb_flags()(spends_enabled in prop::bool::ANY, outputs_enabled in prop::bool::ANY) -> Flags {
@@ -464,22 +498,19 @@ pub mod testing {
         /// Generate an arbitrary unauthorized bundle. This bundle does not
         /// necessarily respect consensus rules; for that use
         /// [`crate::builder::testing::arb_bundle`]
-        pub fn arb_unauthorized_bundle()(
+        pub fn arb_unauthorized_bundle()
-            acts in vec(
+        ( n_actions in 1usize..100 )
-                arb_positive_note_value(MAX_MONEY as u64 / 10000).prop_flat_map(|v|
+        (
-                    arb_unauthorized_action(v).prop_map(move |a| (v, a))
+            acts in vec(arb_unauthorized_action_n(n_actions), n_actions),
-                ),
-                1..10
-            ),
             flags in arb_flags(),
             anchor in prop::array::uniform32(prop::num::u8::ANY).prop_map(Anchor)
         ) -> Bundle<Unauthorized, ValueSum> {
-            let (note_values, actions): (Vec<NoteValue>, Vec<Action<_>>) = acts.into_iter().unzip();
+            let (balances, actions): (Vec<ValueSum>, Vec<Action<_>>) = acts.into_iter().unzip();
 
             Bundle::from_parts(
                 NonEmpty::from_vec(actions).unwrap(),
                 flags,
-                note_values.into_iter().map(ValueSum::from).sum::<Result<ValueSum, _>>().unwrap(),
+                balances.into_iter().sum::<Result<ValueSum, _>>().unwrap(),
                 anchor,
                 Unauthorized
             )
@@ -490,13 +521,10 @@ pub mod testing {
         /// Generate an arbitrary bundle with fake authorization data. This bundle does not
         /// necessarily respect consensus rules; for that use
         /// [`crate::builder::testing::arb_bundle`]
-        pub fn arb_bundle()(
+        pub fn arb_bundle()
-            acts in vec(
+        ( n_actions in 1usize..100 )
-                arb_positive_note_value(MAX_MONEY as u64 / 10000).prop_flat_map(|v|
+        (
-                    arb_action(v).prop_map(move |a| (v, a))
+            acts in vec(arb_action_n(n_actions), n_actions),
-                ),
-                1..10
-            ),
             flags in arb_flags(),
             anchor in prop::array::uniform32(prop::num::u8::ANY).prop_map(Anchor),
             sk in arb_binding_signing_key(),
@@ -504,13 +532,13 @@ pub mod testing {
             fake_proof in vec(prop::num::u8::ANY, 1973),
             fake_sighash in prop::array::uniform32(prop::num::u8::ANY),
         ) -> Bundle<Authorized, ValueSum> {
-            let (note_values, actions): (Vec<NoteValue>, Vec<Action<_>>) = acts.into_iter().unzip();
+            let (balances, actions): (Vec<ValueSum>, Vec<Action<_>>) = acts.into_iter().unzip();
             let rng = StdRng::from_seed(rng_seed);
 
             Bundle::from_parts(
                 NonEmpty::from_vec(actions).unwrap(),
                 flags,
-                note_values.into_iter().map(ValueSum::from).sum::<Result<ValueSum, _>>().unwrap(),
+                balances.into_iter().sum::<Result<ValueSum, _>>().unwrap(),
                 anchor,
                 Authorized {
                     proof: Proof(fake_proof),

src/value.rs

@@ -31,6 +31,15 @@ use subtle::CtOption;
 
 use crate::primitives::redpallas::{self, Binding};
 
+use std::ops::RangeInclusive;
+
+/// The valid range of the scalar multiplication used in ValueCommit^Orchard.
+///
+/// Defined in a note in [Zcash Protocol Spec § 4.17.4: Action Statement (Orchard)][actionstatement].
+///
+/// [actionstatement]: https://zips.z.cash/protocol/nu5.pdf#actionstatement
+pub const VALUE_SUM_RANGE: RangeInclusive<i128> = -(u64::MAX as i128)..=u64::MAX as i128;
+
 /// A value operation overflowed.
 #[derive(Debug)]
 pub struct OverflowError;
@@ -74,7 +83,7 @@ impl Sub for NoteValue {
         let a = self.0 as i128;
         let b = rhs.0 as i128;
         a.checked_sub(b)
-            .filter(|v| v > &(-(std::u64::MAX as i128)) && v < &(std::u64::MAX as i128))
+            .filter(|v| VALUE_SUM_RANGE.contains(v))
             .map(ValueSum)
     }
 }
@@ -105,7 +114,7 @@ impl Add for ValueSum {
     fn add(self, rhs: Self) -> Self::Output {
         self.0
             .checked_add(rhs.0)
-            .filter(|v| v > &(-(std::u64::MAX as i128)) && v < &(std::u64::MAX as i128))
+            .filter(|v| VALUE_SUM_RANGE.contains(v))
             .map(ValueSum)
     }
 }
@@ -253,9 +262,8 @@ pub mod testing {
 
     use super::{NoteValue, ValueCommitTrapdoor, ValueSum};
 
-    /// Zcash's maximum money amount. Used as a bound in proptests so we don't artifically
+    /// Maximum note value.
-    /// overflow `ValueSum`'s size.
+    pub const MAX_NOTE_VALUE: u64 = u64::MAX - 1;
-    pub const MAX_MONEY: i64 = 21_000_000 * 1_0000_0000;
 
     prop_compose! {
         /// Generate an arbitrary Pallas scalar.
@@ -283,7 +291,15 @@ pub mod testing {
 
     prop_compose! {
         /// Generate an arbitrary value in the range of valid nonnegative Zcash amounts.
-        pub fn arb_note_value()(value in 0u64..(MAX_MONEY as u64)) -> NoteValue {
+        pub fn arb_note_value()(value in 0u64..MAX_NOTE_VALUE) -> NoteValue {
+            NoteValue(value)
+        }
+    }
+
+    prop_compose! {
+        /// Generate an arbitrary value in the range of valid positive Zcash amounts
+        /// less than a specified value.
+        pub fn arb_nonnegative_note_value(max: u64)(value in 0u64..max) -> NoteValue {
             NoteValue(value)
         }
     }
@@ -302,11 +318,13 @@ mod tests {
     use proptest::prelude::*;
 
     use super::{
-        testing::{arb_trapdoor, arb_value_sum, MAX_MONEY},
+        testing::{arb_trapdoor, arb_value_sum},
         OverflowError, ValueCommitTrapdoor, ValueCommitment, ValueSum,
     };
     use crate::primitives::redpallas;
 
+    const MAX_MONEY: i64 = 21_000_000 * 1_0000_0000;
+
     proptest! {
         #[test]
         fn bsk_consistent_with_bvk(