Separate Amount::{from_i64, from_nonnegative_i64} APIs

This is more intuitive than a boolean flag for handling non-negative
Amounts stored in i64 values.
This commit is contained in:
Jack Grigg
2019-08-08 00:55:23 +01:00
parent 59ed258c7f
commit 7c07914bfd
6 changed files with 46 additions and 32 deletions

View File

@@ -705,7 +705,7 @@ pub extern "system" fn librustzcash_sapling_final_check(
binding_sig: *const [c_uchar; 64],
sighash_value: *const [c_uchar; 32],
) -> bool {
let value_balance = match Amount::from_i64(value_balance, true) {
let value_balance = match Amount::from_i64(value_balance) {
Ok(vb) => vb,
Err(()) => return false,
};
@@ -1028,7 +1028,7 @@ pub extern "system" fn librustzcash_sapling_binding_sig(
sighash: *const [c_uchar; 32],
result: *mut [c_uchar; 64],
) -> bool {
let value_balance = match Amount::from_i64(value_balance, true) {
let value_balance = match Amount::from_i64(value_balance) {
Ok(vb) => vb,
Err(()) => return false,
};

View File

@@ -564,7 +564,7 @@ mod tests {
let to = extfvk.default_address().unwrap().1;
let mut builder = Builder::new(0);
match builder.add_sapling_output(ovk, to, Amount::from_i64(-1, true).unwrap(), None) {
match builder.add_sapling_output(ovk, to, Amount::from_i64(-1).unwrap(), None) {
Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount),
Ok(_) => panic!("Should have failed"),
}
@@ -575,7 +575,7 @@ mod tests {
let mut builder = Builder::new(0);
match builder.add_transparent_output(
&TransparentAddress::PublicKey([0; 20]),
Amount::from_i64(-1, true).unwrap(),
Amount::from_i64(-1).unwrap(),
) {
Err(e) => assert_eq!(e.kind(), &ErrorKind::InvalidAmount),
Ok(_) => panic!("Should have failed"),
@@ -596,7 +596,7 @@ mod tests {
match builder.build(1, MockTxProver) {
Err(e) => assert_eq!(
e.kind(),
&ErrorKind::ChangeIsNegative(Amount::from_i64(-10000, true).unwrap())
&ErrorKind::ChangeIsNegative(Amount::from_i64(-10000).unwrap())
),
Ok(_) => panic!("Should have failed"),
}
@@ -621,7 +621,7 @@ mod tests {
match builder.build(1, MockTxProver) {
Err(e) => assert_eq!(
e.kind(),
&ErrorKind::ChangeIsNegative(Amount::from_i64(-60000, true).unwrap())
&ErrorKind::ChangeIsNegative(Amount::from_i64(-60000).unwrap())
),
Ok(_) => panic!("Should have failed"),
}
@@ -640,7 +640,7 @@ mod tests {
match builder.build(1, MockTxProver) {
Err(e) => assert_eq!(
e.kind(),
&ErrorKind::ChangeIsNegative(Amount::from_i64(-60000, true).unwrap())
&ErrorKind::ChangeIsNegative(Amount::from_i64(-60000).unwrap())
),
Ok(_) => panic!("Should have failed"),
}
@@ -683,7 +683,7 @@ mod tests {
match builder.build(1, MockTxProver) {
Err(e) => assert_eq!(
e.kind(),
&ErrorKind::ChangeIsNegative(Amount::from_i64(-1, true).unwrap())
&ErrorKind::ChangeIsNegative(Amount::from_i64(-1).unwrap())
),
Ok(_) => panic!("Should have failed"),
}

View File

@@ -79,7 +79,7 @@ impl TxOut {
let value = {
let mut tmp = [0; 8];
reader.read_exact(&mut tmp)?;
Amount::from_i64_le_bytes(tmp, false)
Amount::from_nonnegative_i64_le_bytes(tmp)
}
.map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "value out of range"))?;
let script_pubkey = Script::read(&mut reader)?;

View File

@@ -18,10 +18,19 @@ impl Amount {
/// Creates an Amount from an i64.
///
/// Returns an error if the amount is out of range.
pub fn from_i64(amount: i64, allow_negative: bool) -> Result<Self, ()> {
if 0 <= amount && amount <= MAX_MONEY {
pub fn from_i64(amount: i64) -> Result<Self, ()> {
if -MAX_MONEY <= amount && amount <= MAX_MONEY {
Ok(Amount(amount))
} else if allow_negative && -MAX_MONEY <= amount && amount < 0 {
} else {
Err(())
}
}
/// Creates a non-negative Amount from an i64.
///
/// Returns an error if the amount is out of range.
pub fn from_nonnegative_i64(amount: i64) -> Result<Self, ()> {
if 0 <= amount && amount <= MAX_MONEY {
Ok(Amount(amount))
} else {
Err(())
@@ -42,9 +51,17 @@ impl Amount {
/// Reads an Amount from a signed 64-bit little-endian integer.
///
/// Returns an error if the amount is out of range.
pub fn from_i64_le_bytes(bytes: [u8; 8], allow_negative: bool) -> Result<Self, ()> {
pub fn from_i64_le_bytes(bytes: [u8; 8]) -> Result<Self, ()> {
let amount = i64::from_le_bytes(bytes);
Amount::from_i64(amount, allow_negative)
Amount::from_i64(amount)
}
/// Reads a non-negative Amount from a signed 64-bit little-endian integer.
///
/// Returns an error if the amount is out of range.
pub fn from_nonnegative_i64_le_bytes(bytes: [u8; 8]) -> Result<Self, ()> {
let amount = i64::from_le_bytes(bytes);
Amount::from_nonnegative_i64(amount)
}
/// Reads an Amount from an unsigned 64-bit little-endian integer.
@@ -128,19 +145,16 @@ mod tests {
let zero = b"\x00\x00\x00\x00\x00\x00\x00\x00";
assert_eq!(Amount::from_u64_le_bytes(zero.clone()).unwrap(), Amount(0));
assert_eq!(
Amount::from_i64_le_bytes(zero.clone(), false).unwrap(),
Amount(0)
);
assert_eq!(
Amount::from_i64_le_bytes(zero.clone(), true).unwrap(),
Amount::from_nonnegative_i64_le_bytes(zero.clone()).unwrap(),
Amount(0)
);
assert_eq!(Amount::from_i64_le_bytes(zero.clone()).unwrap(), Amount(0));
let neg_one = b"\xff\xff\xff\xff\xff\xff\xff\xff";
assert!(Amount::from_u64_le_bytes(neg_one.clone()).is_err());
assert!(Amount::from_i64_le_bytes(neg_one.clone(), false).is_err());
assert!(Amount::from_nonnegative_i64_le_bytes(neg_one.clone()).is_err());
assert_eq!(
Amount::from_i64_le_bytes(neg_one.clone(), true).unwrap(),
Amount::from_i64_le_bytes(neg_one.clone()).unwrap(),
Amount(-1)
);
@@ -150,30 +164,30 @@ mod tests {
Amount(MAX_MONEY)
);
assert_eq!(
Amount::from_i64_le_bytes(max_money.clone(), false).unwrap(),
Amount::from_nonnegative_i64_le_bytes(max_money.clone()).unwrap(),
Amount(MAX_MONEY)
);
assert_eq!(
Amount::from_i64_le_bytes(max_money.clone(), true).unwrap(),
Amount::from_i64_le_bytes(max_money.clone()).unwrap(),
Amount(MAX_MONEY)
);
let max_money_p1 = b"\x01\x40\x07\x5a\xf0\x75\x07\x00";
assert!(Amount::from_u64_le_bytes(max_money_p1.clone()).is_err());
assert!(Amount::from_i64_le_bytes(max_money_p1.clone(), false).is_err());
assert!(Amount::from_i64_le_bytes(max_money_p1.clone(), true).is_err());
assert!(Amount::from_nonnegative_i64_le_bytes(max_money_p1.clone()).is_err());
assert!(Amount::from_i64_le_bytes(max_money_p1.clone()).is_err());
let neg_max_money = b"\x00\xc0\xf8\xa5\x0f\x8a\xf8\xff";
assert!(Amount::from_u64_le_bytes(neg_max_money.clone()).is_err());
assert!(Amount::from_i64_le_bytes(neg_max_money.clone(), false).is_err());
assert!(Amount::from_nonnegative_i64_le_bytes(neg_max_money.clone()).is_err());
assert_eq!(
Amount::from_i64_le_bytes(neg_max_money.clone(), true).unwrap(),
Amount::from_i64_le_bytes(neg_max_money.clone()).unwrap(),
Amount(-MAX_MONEY)
);
let neg_max_money_m1 = b"\xff\xbf\xf8\xa5\x0f\x8a\xf8\xff";
assert!(Amount::from_u64_le_bytes(neg_max_money_m1.clone()).is_err());
assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone(), false).is_err());
assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone(), true).is_err());
assert!(Amount::from_nonnegative_i64_le_bytes(neg_max_money_m1.clone()).is_err());
assert!(Amount::from_i64_le_bytes(neg_max_money_m1.clone()).is_err());
}
}

View File

@@ -188,7 +188,7 @@ impl Transaction {
let vb = {
let mut tmp = [0; 8];
reader.read_exact(&mut tmp)?;
Amount::from_i64_le_bytes(tmp, true)
Amount::from_i64_le_bytes(tmp)
}
.map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "valueBalance out of range"))?;
let ss = Vector::read(&mut reader, SpendDescription::read)?;

View File

@@ -1905,7 +1905,7 @@ fn zip_0143() {
Some((
n as usize,
Script(tv.script_code),
Amount::from_i64(tv.amount, false).unwrap(),
Amount::from_nonnegative_i64(tv.amount).unwrap(),
))
} else {
None
@@ -5400,7 +5400,7 @@ fn zip_0243() {
Some((
n as usize,
Script(tv.script_code),
Amount::from_i64(tv.amount, false).unwrap(),
Amount::from_nonnegative_i64(tv.amount).unwrap(),
))
} else {
None