diff --git a/librustzcash/src/rustzcash.rs b/librustzcash/src/rustzcash.rs index 0fad706..99c3187 100644 --- a/librustzcash/src/rustzcash.rs +++ b/librustzcash/src/rustzcash.rs @@ -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, }; diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 36490e4..ba07f34 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -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"), } diff --git a/zcash_primitives/src/transaction/components.rs b/zcash_primitives/src/transaction/components.rs index c7dcebd..d50b4fb 100644 --- a/zcash_primitives/src/transaction/components.rs +++ b/zcash_primitives/src/transaction/components.rs @@ -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)?; diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index dca7056..d244876 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -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 { - if 0 <= amount && amount <= MAX_MONEY { + pub fn from_i64(amount: i64) -> Result { + 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 { + 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 { + pub fn from_i64_le_bytes(bytes: [u8; 8]) -> Result { 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 { + 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()); } } diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs index e5b83e5..aac0d35 100644 --- a/zcash_primitives/src/transaction/mod.rs +++ b/zcash_primitives/src/transaction/mod.rs @@ -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)?; diff --git a/zcash_primitives/src/transaction/tests.rs b/zcash_primitives/src/transaction/tests.rs index 7d84100..4cd5d72 100644 --- a/zcash_primitives/src/transaction/tests.rs +++ b/zcash_primitives/src/transaction/tests.rs @@ -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