Apply suggestions from code review

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

src/primitives/transaction.h

@@ -523,7 +523,7 @@ public:
     std::string ToString() const;
 };
 
-/** An outpoint - a combination of a transaction hash and an index n into its orchard
+/** An outpoint - a combination of a txid and an index n into its orchard
  * actions */
 class OrchardOutPoint : public BaseOutPoint
 {

src/rust/include/rust/orchard/keys.h

@@ -35,7 +35,7 @@ void orchard_address_free(OrchardRawAddressPtr* ptr);
 /**
  * Implements the "less than" operation `k0 < k1` for comparing two Orchard
  * addresses.  This is a comparison of the raw bytes, only useful for cases
- * where a semantically irrelevant ordering is needed (such as for map keys.)
+ * where a semantically irrelevant ordering is needed (such as for map keys).
  */
 bool orchard_address_lt(
     const OrchardRawAddressPtr* k0,
@@ -83,7 +83,7 @@ OrchardIncomingViewingKeyPtr* orchard_incoming_viewing_key_parse(
  * Serializes an Orchard incoming viewing key to the given stream.
  *
  * This will return `false` and leave the stream unmodified if
- * `incoming_viewing_key == nullptr`;
+ * `incoming_viewing_key == nullptr`.
  */
 bool orchard_incoming_viewing_key_serialize(
     const OrchardIncomingViewingKeyPtr* incoming_viewing_key,
@@ -138,7 +138,7 @@ OrchardFullViewingKeyPtr* orchard_full_viewing_key_parse(
  * Serializes an Orchard full viewing key to the given stream.
  *
  * This will return `false` and leave the stream unmodified if
- * `full_viewing_key == nullptr`;
+ * `full_viewing_key == nullptr`.
  */
 bool orchard_full_viewing_key_serialize(
     const OrchardFullViewingKeyPtr* full_viewing_key,
@@ -201,7 +201,8 @@ OrchardSpendingKeyPtr* orchard_spending_key_parse(
 /**
  * Serializes an Orchard spending key to the given stream.
  *
- * If `spending_key == nullptr`, this will return `false`
+ * This will return `false` and leave the stream unmodified if
+ * `spending_key == nullptr`.
  */
 bool orchard_spending_key_serialize(
     const OrchardSpendingKeyPtr* spending_key,
@@ -212,7 +213,7 @@ bool orchard_spending_key_serialize(
  * Returns the full viewing key for the specified spending key.
  *
  * The resulting pointer must be ultimately freed by the caller
- * using `orchard_full_viewing_key_free`
+ * using `orchard_full_viewing_key_free`.
  */
 OrchardFullViewingKeyPtr* orchard_spending_key_to_full_viewing_key(
     const OrchardSpendingKeyPtr* key);

src/rust/include/rust/orchard/wallet.h

@@ -19,7 +19,8 @@ typedef struct OrchardWalletPtr OrchardWalletPtr;
 
 /**
  * Constructs a new empty Orchard wallet and return a pointer to it.
- * Memory is allocated by Rust and must be manually freed.
+ * Memory is allocated by Rust and must be manually freed using
+ * `orchard_wallet_free`.
  */
 OrchardWalletPtr* orchard_wallet_new();
 
@@ -111,6 +112,9 @@ bool orchard_wallet_add_raw_address(
  * Returns a pointer to the Orchard incoming viewing key
  * corresponding to the specified raw address, if it is
  * known to the wallet, or `nullptr` otherwise.
+ *
+ * Memory is allocated by Rust and must be manually freed using
+ * `orchard_incoming_viewing_key_free`.
  */
 OrchardIncomingViewingKeyPtr* orchard_wallet_get_ivk_for_address(
         const OrchardWalletPtr* wallet,

src/rust/src/wallet.rs

@@ -45,7 +45,9 @@ struct TxNotes {
     decrypted_notes: BTreeMap<usize, DecryptedNote>,
 }
 
-/// A type used to pass note metadata across the FFI boundary
+/// A type used to pass note metadata across the FFI boundary.
+/// This must have the same representation as `struct RawOrchardNoteMetadata`
+/// in `rust/include/rust/orchard/wallet.h`.
 #[repr(C)]
 pub struct NoteMetadata {
     txid: [u8; 32],
@@ -410,17 +412,13 @@ pub extern "C" fn orchard_wallet_get_filtered_notes(
         wallet.get_filtered_notes(ivk, ignore_spent, ignore_locked, require_spending_key)
     {
         let recipient = Box::new(dnote.note.recipient());
-        unsafe {
-            (push_cb.unwrap())(
-                result,
-                NoteMetadata {
-                    txid: *outpoint.txid.as_ref(),
-                    action_idx: outpoint.action_idx as u32,
-                    recipient: Box::into_raw(recipient),
-                    note_value: dnote.note.value().inner() as i64,
-                    memo: dnote.memo,
-                },
-            )
+        let metadata = NoteMetadata {
+            txid: *outpoint.txid.as_ref(),
+            action_idx: outpoint.action_idx as u32,
+            recipient: Box::into_raw(recipient),
+            note_value: dnote.note.value().inner() as i64,
+            memo: dnote.memo,
         };
+        unsafe { (push_cb.unwrap())(result, metadata) };
     }
 }

src/wallet/orchard.h

@@ -143,8 +143,6 @@ public:
         uint256 txid;
         std::move(std::begin(rawNoteMeta.txid), std::end(rawNoteMeta.txid), txid.begin());
         OrchardOutPoint op(txid, rawNoteMeta.actionIdx);
-        // TODO: what's the efficient way to copy the memo in the OrchardNoteMetadata
-        // constructor?
         std::array<uint8_t, ZC_MEMO_SIZE> memo;
         std::move(std::begin(rawNoteMeta.memo), std::end(rawNoteMeta.memo), memo.begin());
         OrchardNoteMetadata noteMeta(