diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 8962bb0ee..cb2c2e0b2 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -18,7 +18,7 @@ and this library adheres to Rust's notion of - `ScannedBlock::orchard` - `ScannedBlockCommitments::orchard` - `zcash_client_backend::fees::orchard` -- `zcash_client_backend::fees::ChangeValue::orchard` +- `zcash_client_backend::fees::ChangeValue::orchard` - `zcash_client_backend::wallet`: - `Note::Orchard` @@ -74,6 +74,8 @@ and this library adheres to Rust's notion of wallet::{ReceivedSaplingNote, WalletTransparentOutput}, wallet::input_selection::{Proposal, SaplingInputs}, }` +- `zcash_client_backend::zip321::to_uri` now returns a `String` rather than an + `Option` and provides canonical serialization for the empty proposal. ### Moved - `zcash_client_backend::data_api::{PoolType, ShieldedProtocol}` have @@ -102,7 +104,7 @@ and this library adheres to Rust's notion of as the previously-public fields. - `WalletSummary::new` now takes an additional `next_sapling_subtree_index` argument. - - `WalletWrite::get_next_available_address` now takes an additional + - `WalletWrite::get_next_available_address` now takes an additional `UnifiedAddressRequest` argument. - `chain::scan_cached_blocks` now returns a `ScanSummary` containing metadata about the scanned blocks on success. diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index b11ed1530..027f0ee90 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -350,8 +350,8 @@ impl proposal::Proposal { pub fn from_standard_proposal( params: &P, value: &Proposal, - ) -> Option { - let transaction_request = value.transaction_request().to_uri(params)?; + ) -> Self { + let transaction_request = value.transaction_request().to_uri(params); let anchor_height = value .shielded_inputs() @@ -393,7 +393,7 @@ impl proposal::Proposal { }); #[allow(deprecated)] - Some(proposal::Proposal { + proposal::Proposal { proto_version: PROPOSAL_SER_V1, transaction_request, anchor_height, @@ -407,7 +407,7 @@ impl proposal::Proposal { .into(), min_target_height: value.min_target_height().into(), is_shielding: value.is_shielding(), - }) + } } /// Attempts to parse a [`Proposal`] based upon a supported [`StandardFeeRule`] from its diff --git a/zcash_client_backend/src/zip321.rs b/zcash_client_backend/src/zip321.rs index 5be66e5c8..841557815 100644 --- a/zcash_client_backend/src/zip321.rs +++ b/zcash_client_backend/src/zip321.rs @@ -189,7 +189,7 @@ impl TransactionRequest { // It doesn't matter what params we use here, as none of the validity // requirements depend on them. let params = consensus::MAIN_NETWORK; - TransactionRequest::from_uri(¶ms, &request.to_uri(¶ms).unwrap())?; + TransactionRequest::from_uri(¶ms, &request.to_uri(¶ms))?; } Ok(request) @@ -242,7 +242,7 @@ impl TransactionRequest { /// Convert this request to a URI string. /// /// Returns None if the payment request is empty. - pub fn to_uri(&self, params: &P) -> Option { + pub fn to_uri(&self, params: &P) -> String { fn payment_params( payment: &Payment, payment_index: Option, @@ -276,17 +276,18 @@ impl TransactionRequest { } match &self.payments[..] { - [] => None, + [] => "zcash:".to_string(), [payment] => { let query_params = payment_params(payment, None) .into_iter() .collect::>(); - Some(format!( - "zcash:{}?{}", + format!( + "zcash:{}{}{}", payment.recipient_address.encode(params), + if query_params.is_empty() { "" } else { "?" }, query_params.join("&") - )) + ) } _ => { let query_params = self @@ -301,7 +302,7 @@ impl TransactionRequest { }) .collect::>(); - Some(format!("zcash:?{}", query_params.join("&"))) + format!("zcash:?{}", query_params.join("&")) } } } @@ -797,7 +798,7 @@ pub mod testing { prop_compose! { pub fn arb_zip321_uri()(req in arb_zip321_request()) -> String { - req.to_uri(&TEST_NETWORK).unwrap() + req.to_uri(&TEST_NETWORK) } } @@ -846,12 +847,9 @@ mod tests { }; fn check_roundtrip(req: TransactionRequest) { - if let Some(req_uri) = req.to_uri(&TEST_NETWORK) { - let parsed = TransactionRequest::from_uri(&TEST_NETWORK, &req_uri).unwrap(); - assert_eq!(parsed, req); - } else { - panic!("Generated invalid payment request: {:?}", req); - } + let req_uri = req.to_uri(&TEST_NETWORK); + let parsed = TransactionRequest::from_uri(&TEST_NETWORK, &req_uri).unwrap(); + assert_eq!(parsed, req); } #[test] @@ -954,6 +952,14 @@ mod tests { #[test] fn test_zip321_spec_valid_examples() { + let valid_0 = "zcash:"; + let v0r = TransactionRequest::from_uri(&TEST_NETWORK, valid_0).unwrap(); + assert!(v0r.payments.is_empty()); + + let valid_0 = "zcash:?"; + let v0r = TransactionRequest::from_uri(&TEST_NETWORK, valid_0).unwrap(); + assert!(v0r.payments.is_empty()); + let valid_1 = "zcash:ztestsapling10yy2ex5dcqkclhc7z7yrnjq2z6feyjad56ptwlfgmy77dmaqqrl9gyhprdx59qgmsnyfska2kez?amount=1&memo=VGhpcyBpcyBhIHNpbXBsZSBtZW1vLg&message=Thank%20you%20for%20your%20purchase"; let v1r = TransactionRequest::from_uri(&TEST_NETWORK, valid_1).unwrap(); assert_eq!( @@ -1015,6 +1021,11 @@ mod tests { #[test] fn test_zip321_spec_invalid_examples() { + // invalid; empty string + let invalid_0 = ""; + let i0r = TransactionRequest::from_uri(&TEST_NETWORK, invalid_0); + assert!(i0r.is_err()); + // invalid; missing `address=` let invalid_1 = "zcash:?amount=3491405.05201255&address.1=ztestsapling10yy2ex5dcqkclhc7z7yrnjq2z6feyjad56ptwlfgmy77dmaqqrl9gyhprdx59qgmsnyfska2kez&amount.1=5740296.87793245"; let i1r = TransactionRequest::from_uri(&TEST_NETWORK, invalid_1); @@ -1120,12 +1131,9 @@ mod tests { #[test] fn prop_zip321_roundtrip_request(mut req in arb_zip321_request()) { - if let Some(req_uri) = req.to_uri(&TEST_NETWORK) { - let mut parsed = TransactionRequest::from_uri(&TEST_NETWORK, &req_uri).unwrap(); - assert!(TransactionRequest::normalize_and_eq(&TEST_NETWORK, &mut parsed, &mut req)); - } else { - panic!("Generated invalid payment request: {:?}", req); - } + let req_uri = req.to_uri(&TEST_NETWORK); + let mut parsed = TransactionRequest::from_uri(&TEST_NETWORK, &req_uri).unwrap(); + assert!(TransactionRequest::normalize_and_eq(&TEST_NETWORK, &mut parsed, &mut req)); } #[test] @@ -1133,7 +1141,7 @@ mod tests { let mut parsed = TransactionRequest::from_uri(&TEST_NETWORK, &uri).unwrap(); parsed.normalize(&TEST_NETWORK); let serialized = parsed.to_uri(&TEST_NETWORK); - assert_eq!(serialized, Some(uri)) + assert_eq!(serialized, uri) } } } diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 40e743cf1..31ca460d3 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -1069,9 +1069,6 @@ pub(crate) fn check_proposal_serialization_roundtrip( proposal: &Proposal, ) { let proposal_proto = proposal::Proposal::from_standard_proposal(&db_data.params, proposal); - assert_matches!(proposal_proto, Some(_)); - let deserialized_proposal = proposal_proto - .unwrap() - .try_into_standard_proposal(&db_data.params, db_data); + let deserialized_proposal = proposal_proto.try_into_standard_proposal(&db_data.params, db_data); assert_matches!(deserialized_proposal, Ok(r) if &r == proposal); }