Apply suggestions from code review

Co-authored-by: str4d <jack@electriccoin.co>
Co-authored-by: Daira Hopwood <daira@jacaranda.org>

src/gtest/test_keys.cpp

@@ -183,7 +183,7 @@ TEST(Keys, EncodeAndDecodeUnifiedFullViewingKeys)
         if (test.size() == 1) continue; // comment
 
         libzcash::UnifiedFullViewingKeyBuilder builder;
-        // ["p2pkh_key_bytes, sapling_key_bytes, orchard_key_bytes, unknown_key_bytes, unified_key"]
+        // ["t_key_bytes, sapling_fvk_bytes, orchard_fvk_bytes, unknown_fvk_typecode, unknown_fvk_bytes, unified_fvk"]
         // These were added to the UA in preference order by the Python test vectors.
         if (!test[0].isNull()) {
             auto data = ParseHex(test[0].get_str());

src/rust/include/librustzcash.h

@@ -314,8 +314,8 @@ extern "C" {
      * Arguments:
      * - fvk: [c_uchar; 96] the serialized form of a Sapling full viewing key
      * - dk: [c_uchar; 32] the byte representation of a Sapling diversifier key
-     * - j: [c_uchar; 11] the 88-bit diversifier address at which to start
-     *   searching, encoded in little-endian order
+     * - j: [c_uchar; 11] the 88-bit diversifier index, encoded in little-endian
+     *   order
      * - addr_ret: [c_uchar; 43] array to which the returned address will be
      *   written, if the specified diversifier index `j` produces a valid
      *   address.
@@ -338,7 +338,7 @@ extern "C" {
      * Arguments:
      * - fvk: [c_uchar; 96] the serialized form of a Sapling full viewing key
      * - dk: [c_uchar; 32] the byte representation of a Sapling diversifier key
-     * - j: [c_uchar; 11] the 88-bit diversifier address at which to start
+     * - j: [c_uchar; 11] the 88-bit diversifier index at which to start
      *   searching, encoded in little-endian order
      * - j_ret: [c_uchar; 11] array that will store the diversifier index at
      *   which the returned address was found

src/rust/include/rust/unified_keys.h

@@ -56,7 +56,7 @@ char* unified_full_viewing_key_serialize(
     const UnifiedFullViewingKeyPtr* full_viewing_key);
 
 /**
- * Reads the transparent component of a unified viewing key.
+ * Reads the transparent component of a unified full viewing key.
  *
  * `tkeyout` must be of length 65.
  *
@@ -70,7 +70,7 @@ bool unified_full_viewing_key_read_transparent(
     unsigned char* tkeyout);
 
 /**
- * Reads the Sapling component of a unified viewing key.
+ * Reads the Sapling component of a unified full viewing key.
  *
  * `skeyout` must be of length 128.
  *
@@ -86,15 +86,14 @@ bool unified_full_viewing_key_read_sapling(
 
 /**
  * Constructs a unified full viewing key from the binary encodings
- * of its constituent parts
+ * of its constituent parts.
  *
- * `t_key` must be of length 65 and must be the concatenated
+ * If `t_key` is not `null`, it must be of length 65 and must be the concatenated
  * bytes of the serialized `(ChainCode, CPubKey)` pair.
  *
- * `sapling_key` must be of length 128 and must be the concatenated
- * bytes of the serialized `(SaplingFullViewingKey, DiversifierKey)`
- * pair in the encoding given by `EncodeExtFVKParts` defined in
- * ZIP 32.
+ * If `sapling_key` is not `null`, it must be of length 128 and must be the concatenated
+ * bytes of the `(ak, nk, ovk, dk)` fields in the encoding given by
+ * `EncodeExtFVKParts` defined in ZIP 32.
  *
  * Returns a pointer to newly allocated UFVK if the operation succeeds,
  * or the null pointer otherwise. The pointer returned by this function

src/rust/src/unified_keys_ffi.rs

@@ -34,17 +34,13 @@ pub extern "C" fn unified_full_viewing_key_parse(
 
     match unsafe { CStr::from_ptr(encoded) }.to_str() {
         Ok(encoded) => match Ufvk::decode(encoded) {
-            Ok((parsed_network, fvk)) => {
-                if parsed_network == network {
-                    Box::into_raw(Box::new(fvk))
-                } else {
-                    error!(
-                        "Key was encoded for a different network ({:?}) than what was requested ({:?})",
-                        parsed_network,
-                        network,
-                    );
-                    std::ptr::null_mut()
-                }
+            Ok((parsed_network, fvk)) if parsed_network == network => Box::into_raw(Box::new(fvk)),
+            Ok((parsed_network, _)) => {
+                error!(
+                    "Key was encoded for a different network ({:?}) than what was requested ({:?})",
+                    parsed_network, network,
+                );
+                std::ptr::null_mut()
             }
             Err(e) => {
                 error!("Failure decoding unified full viewing key: {}", e);
@@ -125,10 +121,6 @@ pub extern "C" fn unified_full_viewing_key_from_components(
     match Ufvk::try_from_items(items) {
         Ok(ufvk) => Box::into_raw(Box::new(ufvk)),
         Err(e) => {
-            println!(
-                "An error occurred constructing the unified full viewing key: {:?}",
-                e
-            );
             error!(
                 "An error occurred constructing the unified full viewing key: {:?}",
                 e