Merge pull request #19 from radixdlt/factor_instances_provider_simpli…

…fied_by_sketch_in_pr_description

.vscode/settings.json

@@ -1,6 +1,7 @@
 {
 	"cSpell.words": [
 		"analyser",
+		"Appendable",
 		"Banksy",
 		"bdfs",
 		"Fulfillable",

src/factor_instances_provider/agnostic_paths/derivation_preset.rs

@@ -2,26 +2,30 @@ use crate::prelude::*;
 
 /// Derivation Presets are Network agnostic and Index agnostic
 /// "templates" for DerivationPaths.
-#[derive(Clone, Debug, Copy, Hash, PartialEq, Eq, enum_iterator::Sequence)]
+#[derive(Clone, Copy, Hash, PartialEq, Eq, enum_iterator::Sequence, derive_more::Debug)]
 pub enum DerivationPreset {
     /// Used to form DerivationPaths used to derive FactorInstances
     /// for "veci": Virtual Entity Creating (Factor)Instance for accounts.
     /// `(EntityKind::Account, KeySpace::Unsecurified, KeyKind::TransactionSigning)`
+    #[debug("A-VECI")]
     AccountVeci,
 
     /// Used to form DerivationPaths used to derive FactorInstances
     /// for "mfa" to securify accounts.
     /// `(EntityKind::Account, KeySpace::Securified, KeyKind::TransactionSigning)`
+    #[debug("A-MFA")]
     AccountMfa,
 
     /// Used to form DerivationPaths used to derive FactorInstances
     /// for "veci": Virtual Entity Creating (Factor)Instance for personas.
     /// `(EntityKind::Identity, KeySpace::Unsecurified, KeyKind::TransactionSigning)`
+    #[debug("I-VECI")]
     IdentityVeci,
 
     /// Used to form DerivationPaths used to derive FactorInstances
     /// for "mfa" to securify personas.
     /// `(EntityKind::Identity, KeySpace::Securified, KeyKind::TransactionSigning)`
+    #[debug("I-MFA")]
     IdentityMfa,
 }
 impl DerivationPreset {

src/factor_instances_provider/agnostic_paths/index_agnostic_path.rs

@@ -1,24 +1,42 @@
 use crate::prelude::*;
 
 /// A DerivationPath which is not indexed. On a specific network.
-#[derive(Clone, Debug, Copy, Hash, PartialEq, Eq)]
+#[derive(Clone, Copy, Hash, PartialEq, Eq, derive_more::Debug, derive_more::Display)]
+#[display("{}/{}/{}/?{}", network_id, entity_kind, key_kind, key_space.indicator())]
+#[debug("{:?}/{:?}/{:?}/?{}", network_id, entity_kind, key_kind, key_space.indicator())]
 pub struct IndexAgnosticPath {
     pub network_id: NetworkID,
     pub entity_kind: CAP26EntityKind,
     pub key_kind: CAP26KeyKind,
     pub key_space: KeySpace,
 }
 
-impl From<(NetworkID, DerivationPreset)> for IndexAgnosticPath {
-    fn from((network_id, agnostic_path): (NetworkID, DerivationPreset)) -> Self {
+impl IndexAgnosticPath {
+    pub fn new(
+        network_id: NetworkID,
+        entity_kind: CAP26EntityKind,
+        key_kind: CAP26KeyKind,
+        key_space: KeySpace,
+    ) -> Self {
         Self {
             network_id,
-            entity_kind: agnostic_path.entity_kind(),
-            key_kind: agnostic_path.key_kind(),
-            key_space: agnostic_path.key_space(),
+            entity_kind,
+            key_kind,
+            key_space,
         }
     }
 }
+
+impl From<(NetworkID, DerivationPreset)> for IndexAgnosticPath {
+    fn from((network_id, agnostic_path): (NetworkID, DerivationPreset)) -> Self {
+        Self::new(
+            network_id,
+            agnostic_path.entity_kind(),
+            agnostic_path.key_kind(),
+            agnostic_path.key_space(),
+        )
+    }
+}
 impl TryFrom<IndexAgnosticPath> for DerivationPreset {
     type Error = CommonError;
     fn try_from(value: IndexAgnosticPath) -> Result<DerivationPreset> {
@@ -44,26 +62,13 @@ impl TryFrom<IndexAgnosticPath> for DerivationPreset {
     }
 }
 
-#[derive(Clone, Copy, Hash, PartialEq, Eq)]
+#[derive(Clone, Copy, Hash, PartialEq, Eq, derive_more::Debug)]
+#[debug("🎯: {:?} #{}", self.derivation_preset, self.quantity)]
 pub struct QuantifiedDerivationPresets {
     pub derivation_preset: DerivationPreset,
     pub quantity: usize,
 }
 
-/// For `DerivationPreset` we keep track of
-/// the quantity of instances that are cached and
-#[derive(Clone, Debug, Hash, PartialEq, Eq)]
-pub struct QuantifiedToCacheToUseDerivationPresets {
-    pub derivation_preset: DerivationPreset,
-    pub quantity: QuantityToCacheToUseDirectly,
-}
-
-#[derive(Clone, Hash, PartialEq, Eq)]
-pub struct QuantifiedToCacheToUseIndexAgnosticPath {
-    pub agnostic_path: IndexAgnosticPath,
-    pub quantity: QuantityToCacheToUseDirectly,
-}
-
 impl From<(IndexAgnosticPath, HDPathComponent)> for DerivationPath {
     fn from((path, index): (IndexAgnosticPath, HDPathComponent)) -> Self {
         assert_eq!(index.key_space(), path.key_space);

src/factor_instances_provider/agnostic_paths/quantities.rs

@@ -1,57 +1,3 @@
 use crate::prelude::*;
 
 pub const CACHE_FILLING_QUANTITY: usize = 30;
-
-#[derive(Clone, Debug, Hash, PartialEq, Eq)]
-pub enum QuantityToCacheToUseDirectly {
-    OnlyCacheFilling {
-        /// `CACHE_FILLING_QUANTITY` - `FOUND_IN_CACHE`
-        fill_cache: usize,
-        /// We peeked into the cache and found FactorInstance with the max index, which we must used
-        /// when we calculate the next index path, we are gonna do `max(max_from_profile, max_from_cache)`
-        /// where `max_from_cache` is this `max_index` field.
-        instance_with_max_index: Option<HierarchicalDeterministicFactorInstance>,
-    },
-
-    /// We will derive `remaining + extra_to_fill_cache` more instances
-    ///
-    /// If:
-    /// CACHE_FILLING_QUANTITY: 30
-    /// FOUND_IN_CACHE: 12
-    /// REQUESTED: 14
-    ///
-    /// Then `remaining` below will be `2` and `extra_to_fill_cache` will be
-    /// `CACHE_FILLING_QUANTITY` (`30`) since all `FOUND_IN_CACHE` instances
-    /// will be used and method `total_quantity_to_derive` below will return
-    /// `2 + 30 = 32`
-    ToCacheToUseDirectly {
-        /// Remaining quantity to satisfy the request, `originally_requested - from_cache_instances.len()`
-        /// Used later to split the newly derived instances into two groups, to cache and to use directly,
-        /// can be zero.
-        remaining: usize,
-
-        /// Typically `CACHE_FILLING_QUANTITY` (always?)
-        extra_to_fill_cache: usize,
-    },
-}
-
-impl QuantityToCacheToUseDirectly {
-    pub fn max_index(&self) -> Option<HierarchicalDeterministicFactorInstance> {
-        match self {
-            Self::OnlyCacheFilling {
-                fill_cache: _,
-                instance_with_max_index,
-            } => instance_with_max_index.clone(),
-            Self::ToCacheToUseDirectly { .. } => None,
-        }
-    }
-    pub fn total_quantity_to_derive(&self) -> usize {
-        match self {
-            Self::OnlyCacheFilling { fill_cache, .. } => *fill_cache,
-            Self::ToCacheToUseDirectly {
-                remaining,
-                extra_to_fill_cache,
-            } => *remaining + *extra_to_fill_cache,
-        }
-    }
-}

src/factor_instances_provider/next_index_assigner/mod.rs

@@ -1,11 +1,11 @@
 mod next_derivation_entity_index_assigner;
+mod next_derivation_entity_index_cache_analyzing_assigner;
 mod next_derivation_entity_index_profile_analyzing_assigner;
 mod next_derivation_entity_index_with_ephemeral_offsets;
 mod next_derivation_entity_index_with_ephemeral_offsets_for_factor_source;
-mod offset_from_cache;
 
 pub use next_derivation_entity_index_assigner::*;
+pub use next_derivation_entity_index_cache_analyzing_assigner::*;
 pub use next_derivation_entity_index_profile_analyzing_assigner::*;
 pub use next_derivation_entity_index_with_ephemeral_offsets::*;
 pub use next_derivation_entity_index_with_ephemeral_offsets_for_factor_source::*;
-pub use offset_from_cache::*;

src/factor_instances_provider/next_index_assigner/next_derivation_entity_index_assigner.rs

@@ -5,7 +5,7 @@ use crate::prelude::*;
 /// some NetworkID.
 ///
 /// This assigner works with the:
-/// * cache (indirectly, via the `OffsetFromCache` parameter on `next` [should probably clean up])
+/// * cache
 /// * profile
 /// * local offsets
 ///
@@ -17,9 +17,8 @@ use crate::prelude::*;
 ///    &mut self,
 ///    fs_id: FactorSourceIDFromHash,
 ///    path: IndexAgnosticPath,
-///    cache_offset: OffsetFromCache,
 /// ) -> Result<HDPathComponent> {
-///     let next_from_cache = offset_from_cache.next(fs_id, path).unwrap_or(0);
+///     let next_from_cache = self.cache_analyzing.next(fs_id, path).unwrap_or(0);
 ///     let next_from_profile = self.profile_analyzing.next(fs_id, path).unwrap_or(0);
 ///     
 ///     let max_index = std::cmp::max(next_from_profile, next_from_cache);
@@ -31,58 +30,55 @@ pub struct NextDerivationEntityIndexAssigner {
     #[allow(dead_code)]
     network_id: NetworkID,
     profile_analyzing: NextDerivationEntityIndexProfileAnalyzingAssigner,
+    cache_analyzing: NextDerivationEntityIndexCacheAnalyzingAssigner,
     ephemeral_offsets: NextDerivationEntityIndexWithEphemeralOffsets,
 }
 
 impl NextDerivationEntityIndexAssigner {
-    pub fn new(network_id: NetworkID, profile: Option<Profile>) -> Self {
+    pub fn new(
+        network_id: NetworkID,
+        profile: Option<Profile>,
+        cache: FactorInstancesCache,
+    ) -> Self {
         let profile_analyzing =
             NextDerivationEntityIndexProfileAnalyzingAssigner::new(network_id, profile);
+        let cache_analyzing = NextDerivationEntityIndexCacheAnalyzingAssigner::new(cache);
+        let ephemeral_offsets = NextDerivationEntityIndexWithEphemeralOffsets::default();
         Self {
             network_id,
             profile_analyzing,
-            ephemeral_offsets: NextDerivationEntityIndexWithEphemeralOffsets::default(),
+            cache_analyzing,
+            ephemeral_offsets,
         }
     }
 
     pub fn next(
         &self,
         factor_source_id: FactorSourceIDFromHash,
         index_agnostic_path: IndexAgnosticPath,
-        cache_offset: OffsetFromCache,
     ) -> Result<HDPathComponent> {
         let default_index = HDPathComponent::new_with_key_space_and_base_index(
             index_agnostic_path.key_space,
             U30::new(0).unwrap(),
         );
 
-        // Must update local offset based on values found in cache.
-        // Imagine we are securifying 3 accounts with a single FactorSource
-        // `L` to keep things simple, profile already contains 28 securified
-        // accounts controlled by `L`, with the highest entity index is `27^`
-        // We look for keys in the cache for `L` and we find 2, with entity
-        // indices `[28^, 29^]`, so we need to derive 2 (+CACHE_FILLING_QUANTITY)
-        // more keys. The next index assigner will correctly use a profile based offset
-        // of 28^ for `L`, since it found the max value `28^` in Profile controlled by `L`.
-        // If we would use `next` now, the index would be `next = max + 1`, and
-        // `max = offset_from_profile + ephemeral_offset` = `28^ + 0^` = 28^.
-        // Which is wrong! Since the cache contains `28^` and `29^`, we should
-        // derive `2 (+CACHE_FILLING_QUANTITY)` starting at `30^`.
-        let maybe_next_from_cache = cache_offset.next(factor_source_id, index_agnostic_path)?;
+        let maybe_next_from_cache = self
+            .cache_analyzing
+            .next(factor_source_id, index_agnostic_path)?;
 
         let next_from_cache = maybe_next_from_cache.unwrap_or(default_index);
-        let local = self
+        let ephemeral = self
             .ephemeral_offsets
             .reserve(factor_source_id, index_agnostic_path);
 
         let maybe_next_from_profile = self
             .profile_analyzing
-            .next(index_agnostic_path, factor_source_id)?;
+            .next(factor_source_id, index_agnostic_path)?;
 
         let next_from_profile = maybe_next_from_profile.unwrap_or(default_index);
 
         let max_index = std::cmp::max(next_from_profile, next_from_cache);
 
-        max_index.add_n(local)
+        max_index.add_n(ephemeral)
     }
 }

src/factor_instances_provider/next_index_assigner/next_derivation_entity_index_cache_analyzing_assigner.rs

@@ -0,0 +1,22 @@
+use crate::prelude::*;
+
+pub struct NextDerivationEntityIndexCacheAnalyzingAssigner {
+    cache: FactorInstancesCache,
+}
+impl NextDerivationEntityIndexCacheAnalyzingAssigner {
+    pub fn new(cache: FactorInstancesCache) -> Self {
+        Self { cache }
+    }
+
+    pub fn next(
+        &self,
+        factor_source_id: FactorSourceIDFromHash,
+        index_agnostic_path: IndexAgnosticPath,
+    ) -> Result<Option<HDPathComponent>> {
+        let max = self
+            .cache
+            .max_index_for(factor_source_id, index_agnostic_path);
+        let Some(max) = max else { return Ok(None) };
+        max.add_one().map(Some)
+    }
+}