doc: improve/fix doc-comments.

* updates mdbook page on utxo notifications
* fixes broken comment links to SpendingKeyType, ReceiverAddressType
* documents receiver_preimage field in ExpectedUtxo

docs/src/neptune-core/keys.md

@@ -9,8 +9,8 @@ Three `enum` are provided for working with keys and addresses:
 | enum                   | description                                      |
 |------------------------| -------------------------------------------------|
 | `KeyType`              | enumerates available key/address implementations |
-| `SpendingKeyType`      | enumerates key types and provides methods        |
-| `ReceivingAddressType` | enumerates address types and provides methods    |
+| `SpendingKey`      | enumerates key types and provides methods        |
+| `ReceivingAddress` | enumerates address types and provides methods    |
 
 note: It was decided to use `enum` rather than traits because the enums can be
 used within our RPC layer while traits cannnot.
@@ -59,4 +59,4 @@ Data encrypted with `Symmetric` keys is smaller than data encrypted with asymmet
 For this reason change output notifications are encrypted with a `Symmetric` key by default and it is desirable to do the same for all outputs destined for the
 originating wallet.
 
-Note that the `Symmetric` variant of abstract types `SpendingKeyType` and `ReceivingAddressType` both use the same underlying `SymmetricKey`.  So they differ only in the methods available.  For this reason, it is important never to give an "address" of the `Symmetric` type to an untrusted third party, because it is also the spending key.
+Note that the `Symmetric` variant of abstract types `SpendingKey` and `ReceivingAddress` both use the same underlying `SymmetricKey`.  So they differ only in the methods available.  For this reason, it is important never to give an "address" of the `Symmetric` type to an untrusted third party, because it is also the spending key.

docs/src/neptune-core/utxo_notification.md

@@ -38,7 +38,7 @@ This is where the `key-type` and `receiver_identifier` of the `PublicAnnouncemen
 
 Since these fields are plaintext we can use them to identify notifications intended for our wallet prior to attempting decryption.
 
-Each `SpendingKeyType` has a `receiver_identifier` field that is derived from the secret-key.  This uniquely identifies the key without giving away the secret.  As such, it can be shared in the public-announcement.
+Each `SpendingKey` has a `receiver_identifier` field that is derived from the secret-key.  This uniquely identifies the key without giving away the secret.  As such, it can be shared in the public-announcement.
 
 The algorithm looks like:
 
@@ -62,9 +62,9 @@ It is planned to address this privacy concern but it may not happen until after
 
 ## OffChain Utxo transfers
 
-Many types of OffChain transfers are possible.  `neptune-core` aims to support the following types at launch:
+Many types of OffChain transfers are possible.  examples:
 
-1. Local state (never leaves source machine/wallet)
+1. Local state (never leaves source machine/wallet).
 
 2. Neptune p2p network
 
@@ -81,12 +81,16 @@ For example Bob performs an OffChain utxo transfer to Sally.  Everything goes fi
 
 For this reason, it becomes crucial to maintain ongoing backups/redundancy of wallet data when receiving payments via OffChain notification.  And/or to ensure that the OffChain mechanism can reasonably provide data storage indefinitely into the future.
 
-Wallet authors should have strategies in mind to help prevent funds loss for recipients if providing off-chain send functionality.  Using decentralized storage for encrypted wallet files might be one such strategy.
+Wallet authors should have strategies in mind to help prevent funds loss for
+recipients if providing off-chain send functionality.  Using decentralized cloud
+storage for encrypted wallet files might be one such strategy.
 
 With the scary stuff out of the way, let's look at some `OffChain` notification methods.
 
 ### Local state.
 
+note: `neptune-core` already supports `OffChain` notifications via local state.
+
 Local state transfers are useful when a wallet makes a payment to itself.
 Self-payments occur for almost every transaction when a change output is
 created.  Let's say that Bob has a single `Utxo` in his wallet worth 5 tokens.
@@ -104,22 +108,24 @@ claimed, and add it to the list of wallet-owned `Utxo` called `monitored_utxos`.
 
 ### Neptune p2p network
 
+note: concept only. not yet supported in `neptune-core`.
+
 `Utxo` secrets that are destined for 3rd party wallets can be distributed via the neptune P2P network. This would use the same p2p protocol that distributes transactions and blocks however the secrets would be stored in a separate `UtxoNotificationPool` inside each neptune-core node.
 
-|alan or sword-smith, please flesh this out.|
+There are challenges with keeping the data around in perpetuity as this would place a great storage burden on p2p nodes. A solution outside the p2p network might be required for that.
 
 ### External / Serialized
 
 note: this is a proposed mechanism.  It does not exist at time of writing.
 
-The idea here is that the transfer takes place completely outside of `neptune-core`.
+The idea here is that the transfer and ongoing storage takes place completely outside of `neptune-core`.
 
-1. When a transaction is sent `neptune-core` would provide a serialized `PublicAnnouncement` for each `OffChain` output.
+1. When a transaction is sent `neptune-core` would provide a serialized data structure, eg `OffchainUtxoNotification` containing fields: key_type, receiver_identifier, ciphertext(utxo, sender_randomness) for each `OffChain` output.   Note that these are the exact fields stored in `PublicAnnouncement` for notifications.
 
-2. Some external process then transfers the `PublicAnnouncement` to the intended recipient.
+2. Some external process then transfers the serialized data to the intended recipient.
 
-3. The recipient then invokes the `claim_utxos()` RPC api and passes in a list of serialized `PublicAnnouncement`.  `neptune-core` then attempts to recognize and claim each `PublicAnnouncement`, just as if it had been found on the blockchain.
+3. The recipient then invokes the `claim_utxos()` RPC api and passes in a list of serialized `OffchainUtxoNotification`.  `neptune-core` then attempts to recognize and claim each one, just as if it had been found on the blockchain.
 
-4. Optionally the recipient could pass a flag to `claim_utxos()` that would cause it to initiate a new OnChain payment into the recipient's wallet.  This could serve a couple purposes:
-    * using OnChain notification minimizes future data-loss risk for recipient.
+4. Optionally the recipient could pass a flag to `claim_utxos()` that would cause it to initiate a new `OnChain` payment into the recipient's wallet.  This could serve a couple purposes:
+    * using `OnChain` notification minimizes future data-loss risk for recipient.
     * if the funds were sent with a symmetric-key this prevents the sender from spending (stealing) the funds later.

src/models/state/mod.rs

@@ -493,7 +493,7 @@ impl GlobalState {
     /// The `change_utxo_notify_method` parameter should normally be
     /// [UtxoNotifyMethod::OnChain] for safest transfer.
     ///
-    /// The change_key should normally be a [SpendingKeyType::Symmetric] in
+    /// The change_key should normally be a [SpendingKey::Symmetric] in
     /// order to save blockchain space compared to a regular address.
     ///
     /// Note that `create_transaction()` does not modify any state and does not
@@ -624,9 +624,8 @@ impl GlobalState {
     /// `OnChain` or `OffChain`.
     ///
     /// After this call returns it is the caller's responsibility to inform the
-    /// wallet of any returned [ExpectedUtxo], ie `OffChain` secret
-    /// notifications, for utxos that match wallet keys.  Failure to do so can
-    /// result in loss of funds!
+    /// wallet of any returned [ExpectedUtxo] for utxos that match wallet keys.
+    /// Failure to do so can result in loss of funds!
     ///
     /// Note that `create_raw_transaction()` does not modify any state and does
     /// not require acquiring write lock.  This is important becauce internally

src/models/state/wallet/address.rs

@@ -4,11 +4,11 @@ mod common;
 pub mod generation_address;
 pub mod symmetric_key;
 
-/// ReceivingAddressType abstracts over any address type and should be used
+/// ReceivingAddress abstracts over any address type and should be used
 /// wherever possible.
 pub use address_type::ReceivingAddress;
 
-/// SpendingKeyType abstracts over any spending key type and should be used
+/// SpendingKey abstracts over any spending key type and should be used
 /// wherever possible.
 pub use address_type::SpendingKey;
 

src/models/state/wallet/address/address_type.rs

@@ -84,7 +84,7 @@ impl KeyType {
 /// Represents any type of Neptune receiving Address.
 ///
 /// This enum provides an abstraction API for Address types, so that
-/// a method or struct may simply accept a `ReceivingAddressType` and be
+/// a method or struct may simply accept a `ReceivingAddress` and be
 /// forward-compatible with new types of Address as they are implemented.
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
 pub enum ReceivingAddress {
@@ -170,7 +170,7 @@ impl ReceivingAddress {
     }
 
     /// returns a privacy digest which corresponds to the privacy_preimage
-    /// of the matching [SpendingKeyType]
+    /// of the matching [SpendingKey]
     pub fn privacy_digest(&self) -> Digest {
         match self {
             Self::Generation(a) => a.privacy_digest,
@@ -233,7 +233,7 @@ impl ReceivingAddress {
 /// Represents any type of Neptune spending key.
 ///
 /// This enum provides an abstraction API for spending key types, so that a
-/// method or struct may simply accept a `SpendingKeyType` and be
+/// method or struct may simply accept a `SpendingKey` and be
 /// forward-compatible with new types of spending key as they are implemented.
 #[derive(Debug, Clone, Copy)]
 pub enum SpendingKey {
@@ -442,7 +442,7 @@ mod test {
         use super::*;
 
         /// this tests the generate_public_announcement() and
-        /// scan_for_announced_utxos() methods with a [SpendingKeyType]
+        /// scan_for_announced_utxos() methods with a [SpendingKey]
         ///
         /// a PublicAnnouncement is created with generate_public_announcement() and
         /// added to a Tx.  It is then found by scanning for announced_utoxs.  Then
@@ -499,7 +499,7 @@ mod test {
             assert_eq!(key.privacy_preimage(), announced_utxo.receiver_preimage);
         }
 
-        /// This tests encrypting and decrypting with a [SpendingKeyType]
+        /// This tests encrypting and decrypting with a [SpendingKey]
         pub fn test_encrypt_decrypt(key: SpendingKey) {
             let mut rng = thread_rng();
 
@@ -522,7 +522,7 @@ mod test {
             assert_eq!(sender_randomness, sender_randomness_again);
         }
 
-        /// tests key generation, signing, and decrypting with a [SpendingKeyType]
+        /// tests key generation, signing, and decrypting with a [SpendingKey]
         ///
         /// note: key generation is performed by the caller. Both the
         /// spending_key and receiving_address must be independently derived from
@@ -549,7 +549,7 @@ mod test {
             assert_eq!(receiving_address, receiving_address_again);
         }
 
-        /// tests bech32m serialize, deserialize for [ReceivingAddressType]
+        /// tests bech32m serialize, deserialize for [ReceivingAddress]
         pub fn test_bech32m_conversion(receiving_address: ReceivingAddress) {
             // 1. serialize address to bech32m
             let encoded = receiving_address.to_bech32m(Network::Testnet).unwrap();

src/models/state/wallet/utxo_notification_pool.rs

@@ -48,6 +48,49 @@ pub const RECEIVED_UTXO_NOTIFICATION_THRESHOLD_AGE_IN_SECS: u64 = 3 * 24 * 60 *
 /// blockchain space and may leak some privacy if a key is ever used more than
 /// once.
 ///
+/// ### about `receiver_preimage`
+///
+/// The `receiver_preimage` field in expected_utxo is not strictly necessary.
+/// It is an optimization and a compromise.
+///
+/// #### optimization
+///
+/// An `ExpectedUtxo` really only needs `utxo`, `sender_randomness` and
+/// `receiver_identifier`.  These match the fields used in `PublicAnnouncement`.
+///
+/// However it then becomes necessary to scan all known wallet keys (of all key
+/// types) in order to find the matching key, in order to obtain the preimage.
+///
+/// improvement: To avoid scanning a map of \[`receiver_identifier`\] -->
+/// `derivation_index` could be stored in local wallet state for all known keys.
+///
+/// However no such map presently exists, and so the most efficient and easy
+/// thing is to include the preimage in [ExpectedUtxo] instead of a
+/// `receiver_identifier`.  This buys us an equivalent optimization for little
+/// effort.
+///
+/// #### compromise
+///
+/// Because the preimage is necessary to create an [ExpectedUtxo]
+/// `create_transaction()` must accept a `change_key: SpendingKey` parameter
+/// rather than `change_address: ReceivingAddress`.  One would normally expect
+/// an output to require only an address.
+///
+/// Further if [ExpectedUtxo] and `PublicAnnouncement` use the same fields then
+/// they can share much of the same codepath when claiming.  At present, we have
+/// separate codepaths that perform largely the same function.
+///
+/// We may revisit this area in the future, as it seems ripe for improvement.
+/// In particular wallet receiver_identifier map idea indicates it is possible
+/// to do this efficiently.  As such it may be best to implement at least the
+/// scanning based approach before mainnet.
+///
+/// A branch with an implementation of the scanning approach exists:
+/// danda/symmetric_keys_and_expected_utxos_without_receiver_preimage
+///
+/// At time of writing many tests are not passing and need updating with the new
+/// field.
+///
 /// see [UtxoNotificationPool], [AnnouncedUtxo], [UtxoNotification](crate::models::blockchain::transaction::UtxoNotification)
 #[derive(Clone, Debug, PartialEq, Eq, Hash, GetSize)]
 pub struct ExpectedUtxo {

src/models/state/wallet/wallet_state.rs

@@ -288,7 +288,7 @@ impl WalletState {
         self.find_spending_key_for_utxo(utxo).is_some()
     }
 
-    // returns Some(SpendingKeyType) if the utxo can be unlocked by one of the known
+    // returns Some(SpendingKey) if the utxo can be unlocked by one of the known
     // wallet keys.
     pub fn find_spending_key_for_utxo(&self, utxo: &Utxo) -> Option<SpendingKey> {
         self.get_all_known_spending_keys()

src/rpc_server.rs

@@ -163,7 +163,7 @@ pub trait RPC {
     ///
     /// `outputs` is a list of transaction outputs in the format
     /// `[(address:amount)]`.  The address may be any type supported by
-    /// [ReceivingAddressType].
+    /// [ReceivingAddress].
     ///
     /// `owned_utxo_notify_method` specifies how our wallet will be notified of
     /// any outputs destined for it. This includes the change output if one is
@@ -175,7 +175,7 @@ pub trait RPC {
     /// losing funds should the wallet files become corrupted or lost.
     ///
     /// tip: if using `OnChain` notification use a
-    /// [ReceivingAddressType::Symmetric] as the receiving address for any
+    /// [ReceivingAddress::Symmetric] as the receiving address for any
     /// outputs destined for your own wallet.  This happens automatically for
     /// the Change output only.
     ///