Skip to content

Commit

Permalink
Check remaining transaction value & make value balance signs match th…
Browse files Browse the repository at this point in the history
…e spec (#2566)

* Make Amount arithmetic more generic

To modify generated amounts, we need some extra operations on `Amount`.

We also need to extend existing operations to both `NonNegative` and
`NegativeAllowed` amounts.

* Add a constrain method for ValueBalance

* Derive Eq for ValueBalance

* impl Neg for ValueBalance

* Make some Amount arithmetic expectations explicit

* Explain why we use i128 for multiplication

And expand the overflow error details.

* Expand Amount::sum error details

* Make amount::Error field order consistent

* Rename an amount::Error variant to Constraint, so it's clearer

* Add specific pool variants to ValueBalanceError

* Update coinbase remaining value consensus rule comment

This consensus rule was updated recently to include coinbase transactions,
but Zebra doesn't check block subsidy or miner fees yet.

* Add test methods for modifying transparent values and shielded value balances

* Temporarily set values and value balances to zero in proptests

In both generated chains and proptests that construct their own transactions.

Using zero values reduces value calculation and value check test coverage.
A future change will use non-zero values, and fix them so the check passes.

* Add extra fields to remaining transaction value errors

* Swap the transparent value balance sign to match shielded value balances

This makes the signs of all the chain value pools consistent.

* Use a NonNegative constraint for transparent values

This fix:
* makes the type signature match the consensus rules
* avoids having to write code to handle negative values

* Allocate total generated transaction input value to outputs

If there isn't enough input value for an output, set it to zero.

Temporarily reduce all generated values to avoid overflow.
(We'll remove this workaround when we calculate chain value balances.)

* Consistently use ValueBalanceError for ValueBalances

* Make the value balance signs match the spec

And rename and document methods so their signs are clearer.

* Convert amount::Errors to specific pool ValueBalanceErrors

* Move some error changes to the next PR

* Add extra info to remaining transaction value errors (#2585)

* Distinguish between overflow and negative remaining transaction value errors

And make some error types cloneable.

* Add methods for updating chain value pools (#2586)

* Move amount::test to amount::tests:vectors

* Make ValueBalance traits more consistent with Amount

- implement Add and Sub variants with Result and Assign
- derive Hash

* Clarify some comments and expects

* Create ValueBalance update methods for blocks and transactions

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
  • Loading branch information
2 people authored and conradoplg committed Aug 9, 2021
1 parent 5781c7e commit ef25a7c
Show file tree
Hide file tree
Showing 18 changed files with 1,428 additions and 597 deletions.
398 changes: 20 additions & 378 deletions zebra-chain/src/amount.rs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions zebra-chain/src/amount/tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//! Tests for amounts
mod prop;
mod vectors;
373 changes: 373 additions & 0 deletions zebra-chain/src/amount/tests/vectors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,373 @@
//! Fixed test vectors for amounts.
use crate::serialization::ZcashDeserializeInto;

use super::super::*;

use std::{collections::hash_map::RandomState, collections::HashSet, fmt::Debug};

use color_eyre::eyre::Result;

#[test]
fn test_add_bare() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let neg_one: Amount = (-1).try_into()?;

let zero: Amount = Amount::zero();
let new_zero = one + neg_one;

assert_eq!(zero, new_zero?);

Ok(())
}

#[test]
fn test_add_opt_lhs() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let one = Ok(one);
let neg_one: Amount = (-1).try_into()?;

let zero: Amount = Amount::zero();
let new_zero = one + neg_one;

assert_eq!(zero, new_zero?);

Ok(())
}

#[test]
fn test_add_opt_rhs() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let neg_one: Amount = (-1).try_into()?;
let neg_one = Ok(neg_one);

let zero: Amount = Amount::zero();
let new_zero = one + neg_one;

assert_eq!(zero, new_zero?);

Ok(())
}

#[test]
fn test_add_opt_both() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let one = Ok(one);
let neg_one: Amount = (-1).try_into()?;
let neg_one = Ok(neg_one);

let zero: Amount = Amount::zero();
let new_zero = one.and_then(|one| one + neg_one);

assert_eq!(zero, new_zero?);

Ok(())
}

#[test]
fn test_add_assign() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let neg_one: Amount = (-1).try_into()?;
let mut neg_one = Ok(neg_one);

let zero: Amount = Amount::zero();
neg_one += one;
let new_zero = neg_one;

assert_eq!(Ok(zero), new_zero);

Ok(())
}

#[test]
fn test_sub_bare() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let zero: Amount = Amount::zero();

let neg_one: Amount = (-1).try_into()?;
let new_neg_one = zero - one;

assert_eq!(Ok(neg_one), new_neg_one);

Ok(())
}

#[test]
fn test_sub_opt_lhs() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let one = Ok(one);
let zero: Amount = Amount::zero();

let neg_one: Amount = (-1).try_into()?;
let new_neg_one = zero - one;

assert_eq!(Ok(neg_one), new_neg_one);

Ok(())
}

#[test]
fn test_sub_opt_rhs() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let zero: Amount = Amount::zero();
let zero = Ok(zero);

let neg_one: Amount = (-1).try_into()?;
let new_neg_one = zero - one;

assert_eq!(Ok(neg_one), new_neg_one);

Ok(())
}

#[test]
fn test_sub_assign() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let zero: Amount = Amount::zero();
let mut zero = Ok(zero);

let neg_one: Amount = (-1).try_into()?;
zero -= one;
let new_neg_one = zero;

assert_eq!(Ok(neg_one), new_neg_one);

Ok(())
}

#[test]
fn add_with_diff_constraints() -> Result<()> {
zebra_test::init();

let one = Amount::<NonNegative>::try_from(1)?;
let zero: Amount<NegativeAllowed> = Amount::zero();

(zero - one.constrain()).expect("should allow negative");
(zero.constrain() - one).expect_err("shouldn't allow negative");

Ok(())
}

#[test]
fn deserialize_checks_bounds() -> Result<()> {
zebra_test::init();

let big = (MAX_MONEY * 2)
.try_into()
.expect("unexpectedly large constant: multiplied constant should be within range");
let neg = -10;

let mut big_bytes = Vec::new();
(&mut big_bytes)
.write_u64::<LittleEndian>(big)
.expect("unexpected serialization failure: vec should be infalliable");

let mut neg_bytes = Vec::new();
(&mut neg_bytes)
.write_i64::<LittleEndian>(neg)
.expect("unexpected serialization failure: vec should be infalliable");

Amount::<NonNegative>::zcash_deserialize(big_bytes.as_slice())
.expect_err("deserialization should reject too large values");
Amount::<NegativeAllowed>::zcash_deserialize(big_bytes.as_slice())
.expect_err("deserialization should reject too large values");

Amount::<NonNegative>::zcash_deserialize(neg_bytes.as_slice())
.expect_err("NonNegative deserialization should reject negative values");
let amount: Amount<NegativeAllowed> = neg_bytes
.zcash_deserialize_into()
.expect("NegativeAllowed deserialization should allow negative values");

assert_eq!(amount.0, neg);

Ok(())
}

#[test]
fn hash() -> Result<()> {
zebra_test::init();

let one = Amount::<NonNegative>::try_from(1)?;
let another_one = Amount::<NonNegative>::try_from(1)?;
let zero: Amount<NonNegative> = Amount::zero();

let hash_set: HashSet<Amount<NonNegative>, RandomState> = [one].iter().cloned().collect();
assert_eq!(hash_set.len(), 1);

let hash_set: HashSet<Amount<NonNegative>, RandomState> = [one, one].iter().cloned().collect();
assert_eq!(hash_set.len(), 1, "Amount hashes are consistent");

let hash_set: HashSet<Amount<NonNegative>, RandomState> =
[one, another_one].iter().cloned().collect();
assert_eq!(hash_set.len(), 1, "Amount hashes are by value");

let hash_set: HashSet<Amount<NonNegative>, RandomState> = [one, zero].iter().cloned().collect();
assert_eq!(
hash_set.len(),
2,
"Amount hashes are different for different values"
);

Ok(())
}

#[test]
fn ordering_constraints() -> Result<()> {
zebra_test::init();

ordering::<NonNegative, NonNegative>()?;
ordering::<NonNegative, NegativeAllowed>()?;
ordering::<NegativeAllowed, NonNegative>()?;
ordering::<NegativeAllowed, NegativeAllowed>()?;

Ok(())
}

#[allow(clippy::eq_op)]
fn ordering<C1, C2>() -> Result<()>
where
C1: Constraint + Debug,
C2: Constraint + Debug,
{
let zero: Amount<C1> = Amount::zero();
let one = Amount::<C2>::try_from(1)?;
let another_one = Amount::<C1>::try_from(1)?;

assert_eq!(one, one);
assert_eq!(one, another_one, "Amount equality is by value");

assert_ne!(one, zero);
assert_ne!(zero, one);

assert!(one > zero);
assert!(zero < one);
assert!(zero <= one);

let negative_one = Amount::<NegativeAllowed>::try_from(-1)?;
let negative_two = Amount::<NegativeAllowed>::try_from(-2)?;

assert_ne!(negative_one, zero);
assert_ne!(negative_one, one);

assert!(negative_one < zero);
assert!(negative_one <= one);
assert!(zero > negative_one);
assert!(zero >= negative_one);
assert!(negative_two < negative_one);
assert!(negative_one > negative_two);

Ok(())
}

#[test]
fn test_sum() -> Result<()> {
zebra_test::init();

let one: Amount = 1.try_into()?;
let neg_one: Amount = (-1).try_into()?;

let zero: Amount = Amount::zero();

// success
let amounts = vec![one, neg_one, zero];

let sum_ref: Amount = amounts.iter().sum::<Result<Amount, Error>>()?;
let sum_value: Amount = amounts.into_iter().sum::<Result<Amount, Error>>()?;

assert_eq!(sum_ref, sum_value);
assert_eq!(sum_ref, zero);

// above max for Amount error
let max: Amount = MAX_MONEY.try_into()?;
let amounts = vec![one, max];
let integer_sum: i64 = amounts.iter().map(|a| a.0).sum();

let sum_ref = amounts.iter().sum::<Result<Amount, Error>>();
let sum_value = amounts.into_iter().sum::<Result<Amount, Error>>();

assert_eq!(sum_ref, sum_value);
assert_eq!(
sum_ref,
Err(Error::SumOverflow {
partial_sum: integer_sum,
remaining_items: 0
})
);

// below min for Amount error
let min: Amount = (-MAX_MONEY).try_into()?;
let amounts = vec![min, neg_one];
let integer_sum: i64 = amounts.iter().map(|a| a.0).sum();

let sum_ref = amounts.iter().sum::<Result<Amount, Error>>();
let sum_value = amounts.into_iter().sum::<Result<Amount, Error>>();

assert_eq!(sum_ref, sum_value);
assert_eq!(
sum_ref,
Err(Error::SumOverflow {
partial_sum: integer_sum,
remaining_items: 0
})
);

// above max of i64 error
let times: usize = (i64::MAX / MAX_MONEY)
.try_into()
.expect("4392 can always be converted to usize");
let amounts: Vec<Amount> = std::iter::repeat(MAX_MONEY.try_into()?)
.take(times + 1)
.collect();

let sum_ref = amounts.iter().sum::<Result<Amount, Error>>();
let sum_value = amounts.into_iter().sum::<Result<Amount, Error>>();

assert_eq!(sum_ref, sum_value);
assert_eq!(
sum_ref,
Err(Error::SumOverflow {
partial_sum: 4200000000000000,
remaining_items: 4391
})
);

// below min of i64 overflow
let times: usize = (i64::MAX / MAX_MONEY)
.try_into()
.expect("4392 can always be converted to usize");
let neg_max_money: Amount<NegativeAllowed> = (-MAX_MONEY).try_into()?;
let amounts: Vec<Amount<NegativeAllowed>> =
std::iter::repeat(neg_max_money).take(times + 1).collect();

let sum_ref = amounts.iter().sum::<Result<Amount, Error>>();
let sum_value = amounts.into_iter().sum::<Result<Amount, Error>>();

assert_eq!(sum_ref, sum_value);
assert_eq!(
sum_ref,
Err(Error::SumOverflow {
partial_sum: -4200000000000000,
remaining_items: 4391
})
);

Ok(())
}
Loading

0 comments on commit ef25a7c

Please sign in to comment.