Apply suggestions from code review

Co-authored-by: Daira-Emma Hopwood <daira@jacaranda.org>
This commit is contained in:
Kris Nuttycombe 2024-03-05 19:44:29 -07:00 committed by Kris Nuttycombe
parent 8955cfc559
commit eaabc0f514
5 changed files with 32 additions and 30 deletions

View File

@ -29,7 +29,7 @@ pub const HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY: &str = "zxviews";
/// [Zcash Protocol Specification]: https://github.com/zcash/zips/blob/master/protocol/protocol.pdf
pub const HRP_SAPLING_PAYMENT_ADDRESS: &str = "zs";
/// The prefix for a Base58Check-encoded mainnet Sprout address
/// The prefix for a Base58Check-encoded mainnet Sprout address.
///
/// Defined in the [Zcash Protocol Specification section 5.6.3][sproutpaymentaddrencoding].
///

View File

@ -29,7 +29,7 @@ pub const HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY: &str = "zxviewtestsapling";
/// [Zcash Protocol Specification]: https://github.com/zcash/zips/blob/master/protocol/protocol.pdf
pub const HRP_SAPLING_PAYMENT_ADDRESS: &str = "ztestsapling";
/// The prefix for a Base58Check-encoded testnet Sprout addresses.
/// The prefix for a Base58Check-encoded testnet Sprout address.
///
/// Defined in the [Zcash Protocol Specification section 5.6.3][sproutpaymentaddrencoding].
///

View File

@ -155,7 +155,7 @@ impl TryFrom<ZatBalance> for u64 {
type Error = BalanceError;
fn try_from(value: ZatBalance) -> Result<Self, Self::Error> {
value.0.try_into().map_err(|_| BalanceError::Overflow)
value.0.try_into().map_err(|_| BalanceError::Underflow)
}
}
@ -261,11 +261,9 @@ impl Zatoshis {
///
/// Returns an error if the amount is outside the range `{0..MAX_MONEY}`.
pub fn from_nonnegative_i64(amount: i64) -> Result<Self, BalanceError> {
if amount >= 0 {
Self::from_u64(u64::try_from(amount).unwrap())
} else {
Err(BalanceError::Underflow)
}
u64::try_from(amount)
.map_err(|_| BalanceError::Underflow)
.and_then(Self::from_u64)
}
/// Reads an Zatoshis from an unsigned 64-bit little-endian integer.
@ -439,6 +437,12 @@ pub mod testing {
}
}
prop_compose! {
pub fn arb_nonnegative_zat_balance()(amt in 0i64..MAX_BALANCE) -> ZatBalance {
ZatBalance::from_i64(amt).unwrap()
}
}
prop_compose! {
pub fn arb_zatoshis()(amt in 0u64..MAX_MONEY) -> Zatoshis {
Zatoshis::from_u64(amt).unwrap()

View File

@ -57,9 +57,11 @@ and this library adheres to Rust's notion of
`NonNegativeAmount` rather than a signed `Amount` as its argument.
- `zcash_client_backend::zip321::parse::parse_amount` now parses a
`NonNegativeAmount` rather than a signed `Amount`.
- `zcash_client_backend::zip321::TransactionRequest::total` now
returns `Result<_, BalanceError>` instead of `Result<_, ()>`.
### Removed
- `zcash_client_backend::PoolType::is_receiver` use
- `zcash_client_backend::PoolType::is_receiver`: use
`zcash_keys::Address::has_receiver` instead.
## [0.11.0] - 2024-03-01

View File

@ -245,7 +245,7 @@ impl TransactionRequest {
payment_index: Option<usize>,
) -> impl IntoIterator<Item = String> + '_ {
std::iter::empty()
.chain(render::amount_param(payment.amount, payment_index))
.chain(Some(render::amount_param(payment.amount, payment_index)))
.chain(
payment
.memo
@ -414,28 +414,24 @@ mod render {
format!("address{}={}", param_index(idx), addr.encode(params))
}
/// Converts an [`NonNegativeAmount`] value to a correctly formatted decimal ZEC
/// value for inclusion in a ZIP 321 URI.
pub fn amount_str(amount: NonNegativeAmount) -> Option<String> {
if amount.is_positive() {
/// Converts a [`NonNegativeAmount`] value to a correctly formatted decimal ZEC
/// string for inclusion in a ZIP 321 URI.
pub fn amount_str(amount: NonNegativeAmount) -> String {
let coins = u64::from(amount) / COIN;
let zats = u64::from(amount) % COIN;
Some(if zats == 0 {
if zats == 0 {
format!("{}", coins)
} else {
format!("{}.{:0>8}", coins, zats)
.trim_end_matches('0')
.to_string()
})
} else {
None
}
}
/// Constructs an "amount" key/value pair containing the encoded ZEC amount
/// at the specified parameter index.
pub fn amount_param(amount: NonNegativeAmount, idx: Option<usize>) -> Option<String> {
amount_str(amount).map(|s| format!("amount{}={}", param_index(idx), s))
pub fn amount_param(amount: NonNegativeAmount, idx: Option<usize>) -> String {
format!("amount{}={}", param_index(idx), amount_str(amount))
}
/// Constructs a "memo" key/value pair containing the base64URI-encoded memo
@ -650,7 +646,7 @@ mod parse {
digit1,
opt(preceded(
char('.'),
map_opt(digit0, |s: &str| if s.len() > 8 { None } else { Some(s) }),
map_opt(digit1, |s: &str| if s.len() > 8 { None } else { Some(s) }),
)),
)),
|(whole_s, decimal_s): (&str, Option<&str>)| {
@ -671,7 +667,7 @@ mod parse {
.and_then(|coin_zats| coin_zats.checked_add(zats))
.ok_or(BalanceError::Overflow)
.and_then(NonNegativeAmount::from_u64)
.map_err(|_| format!("Not a valid zat amount: {}.{}", coins, zats))
.map_err(|_| format!("Not a valid amount: {}.{:0>8} ZEC", coins, zats))
},
)(input)
}
@ -854,7 +850,7 @@ mod tests {
for amt_u64 in amounts {
let amt = NonNegativeAmount::from_u64(amt_u64).unwrap();
let amt_str = amount_str(amt).unwrap();
let amt_str = amount_str(amt);
assert_eq!(amt, parse_amount(&amt_str).unwrap().1);
}
}
@ -1099,7 +1095,7 @@ mod tests {
#[test]
fn prop_zip321_roundtrip_amount(amt in arb_nonnegative_amount()) {
let amt_str = amount_str(amt).unwrap();
let amt_str = amount_str(amt);
assert_eq!(amt, parse_amount(&amt_str).unwrap().1);
}