Merge pull request #1009 from tw0po1nt/fix_txn_builder_panic

Gracefully handle when given an Orchard-only UA

zcash_client_backend/CHANGELOG.md

@@ -9,6 +9,11 @@ and this library adheres to Rust's notion of
 
 ### Added
 
+- `zcash_client_backend::data_api::ShieldedProtocol` has a new variant for `Orchard`,
+  allowing for better reporting to callers trying to perform actions using `Orchard`
+  before it is fully supported.
+- `zcash_client_backend::data_api::error::Error` has new error variant:
+  - `Error::UnsupportedPoolType(zcash_client_backend::data_api::PoolType)`
 - Added methods to `zcash_client_backend::wallet::ReceivedSaplingNote`:
   `{from_parts, txid, output_index, diversifier, rseed, note_commitment_tree_position}`.
 

zcash_client_backend/src/data_api.rs

@@ -2,7 +2,7 @@
 
 use std::{
     collections::{BTreeMap, HashMap},
-    fmt::Debug,
+    fmt::{self, Debug},
     io,
     num::{NonZeroU32, TryFromIntError},
 };
@@ -556,7 +556,8 @@ pub struct SentTransaction<'a> {
 pub enum ShieldedProtocol {
     /// The Sapling protocol
     Sapling,
-    // TODO: Orchard
+    /// The Orchard protocol
+    Orchard,
 }
 
 /// A unique identifier for a shielded transaction output
@@ -603,6 +604,16 @@ pub enum PoolType {
     Shielded(ShieldedProtocol),
 }
 
+impl fmt::Display for PoolType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            PoolType::Transparent => f.write_str("Transparent"),
+            PoolType::Shielded(ShieldedProtocol::Sapling) => f.write_str("Sapling"),
+            PoolType::Shielded(ShieldedProtocol::Orchard) => f.write_str("Orchard"),
+        }
+    }
+}
+
 /// A type that represents the recipient of a transaction output; a recipient address (and, for
 /// unified addresses, the pool to which the payment is sent) in the case of outgoing output, or an
 /// internal account ID and the pool to which funds were sent in the case of a wallet-internal

zcash_client_backend/src/data_api/error.rs

@@ -16,6 +16,7 @@ use zcash_primitives::{
 };
 
 use crate::data_api::wallet::input_selection::InputSelectorError;
+use crate::data_api::PoolType;
 
 #[cfg(feature = "transparent-inputs")]
 use zcash_primitives::{legacy::TransparentAddress, zip32::DiversifierIndex};
@@ -56,6 +57,9 @@ pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
     /// It is forbidden to provide a memo when constructing a transparent output.
     MemoForbidden,
 
+    /// Attempted to create a spend to an unsupported pool type (currently, Orchard).
+    UnsupportedPoolType(PoolType),
+
     /// A note being spent does not correspond to either the internal or external
     /// full viewing key for an account.
     NoteMismatch(NoteId),
@@ -112,6 +116,7 @@ where
             Error::ScanRequired => write!(f, "Must scan blocks first"),
             Error::Builder(e) => write!(f, "An error occurred building the transaction: {}", e),
             Error::MemoForbidden => write!(f, "It is not possible to send a memo to a transparent address."),
+            Error::UnsupportedPoolType(t) => write!(f, "Attempted to create spend to an unsupported pool type: {}", t),
             Error::NoteMismatch(n) => write!(f, "A note being spent ({:?}) does not correspond to either the internal or external full viewing key for the provided spending key.", n),
 
             #[cfg(feature = "transparent-inputs")]

zcash_client_backend/src/data_api/wallet.rs

@@ -410,6 +410,9 @@ where
 ///
 /// Returns the database identifier for the newly constructed transaction, or an error if
 /// an error occurs in transaction construction, proving, or signing.
+///
+/// Note: If the payment includes a recipient with an Orchard-only UA, this will attempt
+/// to fall back to the transparent receiver until full Orchard support is implemented.
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)]
 pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
@@ -534,17 +537,33 @@ where
                     .memo
                     .as_ref()
                     .map_or_else(MemoBytes::empty, |m| m.clone());
-                builder.add_sapling_output(
-                    external_ovk,
-                    *ua.sapling().expect("TODO: Add Orchard support to builder"),
-                    payment.amount,
-                    memo.clone(),
-                )?;
-                sapling_output_meta.push((
-                    Recipient::Unified(ua.clone(), PoolType::Shielded(ShieldedProtocol::Sapling)),
-                    payment.amount,
-                    Some(memo),
-                ));
+
+                if let Some(sapling_receiver) = ua.sapling() {
+                    builder.add_sapling_output(
+                        external_ovk,
+                        *sapling_receiver,
+                        payment.amount,
+                        memo.clone(),
+                    )?;
+                    sapling_output_meta.push((
+                        Recipient::Unified(
+                            ua.clone(),
+                            PoolType::Shielded(ShieldedProtocol::Sapling),
+                        ),
+                        payment.amount,
+                        Some(memo),
+                    ));
+                } else if let Some(taddr) = ua.transparent() {
+                    if payment.memo.is_some() {
+                        return Err(Error::MemoForbidden);
+                    } else {
+                        builder.add_transparent_output(taddr, payment.amount)?;
+                    }
+                } else {
+                    return Err(Error::UnsupportedPoolType(PoolType::Shielded(
+                        ShieldedProtocol::Orchard,
+                    )));
+                }
             }
             RecipientAddress::Shielded(addr) => {
                 let memo = payment

zcash_client_sqlite/CHANGELOG.md

@@ -7,6 +7,10 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 
+### Added
+- `zcash_client_sqlite::error::SqliteClientError` has new error variant:
+  - `SqliteClientError::UnsupportedPoolType(zcash_client_backend::data_api::PoolType)`
+
 ## [0.8.0] - 2023-09-25
 
 ### Notable Changes

zcash_client_sqlite/src/error.rs

@@ -4,6 +4,7 @@ use std::error;
 use std::fmt;
 
 use shardtree::error::ShardTreeError;
+use zcash_client_backend::data_api::PoolType;
 use zcash_client_backend::encoding::{Bech32DecodeError, TransparentCodecError};
 use zcash_primitives::{consensus::BlockHeight, zip32::AccountId};
 
@@ -98,6 +99,9 @@ pub enum SqliteClientError {
     /// [`WalletWrite::update_chain_tip`]:
     /// zcash_client_backend::data_api::WalletWrite::update_chain_tip
     ChainHeightUnknown,
+
+    /// Unsupported pool type
+    UnsupportedPoolType(PoolType),
 }
 
 impl error::Error for SqliteClientError {
@@ -144,7 +148,8 @@ impl fmt::Display for SqliteClientError {
             SqliteClientError::AddressNotRecognized(_) => write!(f, "The address associated with a received txo is not identifiable as belonging to the wallet."),
             SqliteClientError::CommitmentTree(err) => write!(f, "An error occurred accessing or updating note commitment tree data: {}.", err),
             SqliteClientError::CacheMiss(height) => write!(f, "Requested height {} does not exist in the block cache.", height),
-            SqliteClientError::ChainHeightUnknown => write!(f, "Chain height unknown; please call `update_chain_tip`")
+            SqliteClientError::ChainHeightUnknown => write!(f, "Chain height unknown; please call `update_chain_tip`"),
+            SqliteClientError::UnsupportedPoolType(t) => write!(f, "Pool type is not currently supported: {}", t)
         }
     }
 }

zcash_client_sqlite/src/wallet.rs

@@ -136,6 +136,7 @@ pub(crate) fn pool_code(pool_type: PoolType) -> i64 {
     match pool_type {
         PoolType::Transparent => 0i64,
         PoolType::Shielded(ShieldedProtocol::Sapling) => 2i64,
+        PoolType::Shielded(ShieldedProtocol::Orchard) => 3i64,
     }
 }
 
@@ -773,6 +774,11 @@ pub(crate) fn get_received_memo(
             )
             .optional()?
             .flatten(),
+        _ => {
+            return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded(
+                note_id.protocol(),
+            )))
+        }
     };
 
     memo_bytes