-
Notifications
You must be signed in to change notification settings - Fork 49
refactor(platform-wallet): extract shared contact request logic and use tracing #3206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
thepastaclaw
wants to merge
3
commits into
dashpay:v3.1-dev
Choose a base branch
from
thepastaclaw:feat/identity-discovery-scan
base: v3.1-dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
154 changes: 154 additions & 0 deletions
154
packages/rs-platform-wallet/src/platform_wallet_info/identity_discovery.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| //! Gap-limit identity discovery for wallet sync | ||
| //! | ||
| //! This module implements DashSync-style gap-limit scanning for identities | ||
| //! during wallet sync. It derives consecutive authentication keys from the | ||
| //! wallet's BIP32 tree and queries Platform to find registered identities. | ||
|
|
||
| use super::key_derivation::derive_identity_auth_key_hash; | ||
| use super::PlatformWalletInfo; | ||
| use crate::error::PlatformWalletError; | ||
| use dpp::identity::accessors::IdentityGettersV0; | ||
| use dpp::prelude::Identifier; | ||
| use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; | ||
|
|
||
| impl PlatformWalletInfo { | ||
| /// Discover identities by scanning consecutive identity indices with a gap limit | ||
| /// | ||
| /// Starting from `start_index`, derives ECDSA authentication keys for consecutive | ||
| /// identity indices and queries Platform for registered identities. Scanning stops | ||
| /// when `gap_limit` consecutive indices yield no identity. | ||
| /// | ||
| /// This mirrors the DashSync gap-limit approach: keep scanning until N consecutive | ||
| /// misses, then stop. | ||
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `wallet` - The wallet to derive authentication keys from | ||
| /// * `start_index` - The first identity index to check | ||
| /// * `gap_limit` - Number of consecutive misses before stopping (typically 5) | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// Returns the list of newly discovered identity IDs | ||
| pub async fn discover_identities( | ||
| &mut self, | ||
| wallet: &key_wallet::Wallet, | ||
| start_index: u32, | ||
| gap_limit: u32, | ||
| ) -> Result<Vec<Identifier>, PlatformWalletError> { | ||
| use dash_sdk::platform::types::identity::PublicKeyHash; | ||
| use dash_sdk::platform::Fetch; | ||
|
|
||
| let sdk = self | ||
| .identity_manager() | ||
| .sdk | ||
| .as_ref() | ||
| .ok_or_else(|| { | ||
| PlatformWalletError::InvalidIdentityData( | ||
| "SDK not configured in identity manager".to_string(), | ||
| ) | ||
| })? | ||
| .clone(); | ||
|
|
||
| let network = self.network(); | ||
| let mut discovered = Vec::new(); | ||
| let mut consecutive_misses = 0u32; | ||
| let mut identity_index = start_index; | ||
|
|
||
| while consecutive_misses < gap_limit { | ||
| // Derive the authentication key hash for this identity index (key_index 0) | ||
| let key_hash_array = derive_identity_auth_key_hash(wallet, network, identity_index, 0)?; | ||
|
|
||
| // Query Platform for an identity registered with this key hash | ||
| match dpp::identity::Identity::fetch(&sdk, PublicKeyHash(key_hash_array)).await { | ||
| Ok(Some(identity)) => { | ||
| let identity_id = identity.id(); | ||
|
|
||
| // Add to manager if not already present | ||
| if self.identity_manager().identity(&identity_id).is_none() { | ||
| self.identity_manager_mut().add_identity(identity)?; | ||
| discovered.push(identity_id); | ||
| } | ||
| consecutive_misses = 0; | ||
| } | ||
| Ok(None) => { | ||
| consecutive_misses += 1; | ||
| } | ||
| Err(e) => { | ||
| tracing::warn!( | ||
| identity_index, | ||
| "Failed to query identity by public key hash: {}", | ||
| e | ||
| ); | ||
| // Don't increment consecutive_misses: a query failure is not | ||
| // a confirmed empty slot and should not consume the gap budget. | ||
| } | ||
| } | ||
|
|
||
| identity_index += 1; | ||
| } | ||
|
|
||
| Ok(discovered) | ||
| } | ||
|
|
||
| /// Discover identities and fetch their DashPay contact requests | ||
| /// | ||
| /// Calls [`discover_identities`] then fetches sent and received contact requests | ||
| /// for each discovered identity, storing them in the identity manager. | ||
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `wallet` - The wallet to derive authentication keys from | ||
| /// * `start_index` - The first identity index to check | ||
| /// * `gap_limit` - Number of consecutive misses before stopping (typically 5) | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// Returns the list of newly discovered identity IDs | ||
| pub async fn discover_identities_with_contacts( | ||
| &mut self, | ||
| wallet: &key_wallet::Wallet, | ||
| start_index: u32, | ||
| gap_limit: u32, | ||
| ) -> Result<Vec<Identifier>, PlatformWalletError> { | ||
| let discovered = self | ||
| .discover_identities(wallet, start_index, gap_limit) | ||
| .await?; | ||
|
|
||
| if discovered.is_empty() { | ||
| return Ok(discovered); | ||
| } | ||
|
|
||
| let sdk = self | ||
| .identity_manager() | ||
| .sdk | ||
| .as_ref() | ||
| .ok_or_else(|| { | ||
| PlatformWalletError::InvalidIdentityData( | ||
| "SDK not configured in identity manager".to_string(), | ||
| ) | ||
| })? | ||
| .clone(); | ||
|
|
||
| for identity_id in &discovered { | ||
| // Get the identity from the manager to pass to the SDK | ||
| let identity = match self.identity_manager().identity(identity_id) { | ||
| Some(id) => id.clone(), | ||
| None => continue, | ||
| }; | ||
|
|
||
| if let Err(e) = self | ||
| .fetch_and_store_contact_requests(&sdk, &identity, identity_id) | ||
| .await | ||
| { | ||
| tracing::warn!( | ||
| %identity_id, | ||
| "Failed to fetch contact requests during discovery: {}", | ||
| e | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| Ok(discovered) | ||
| } | ||
| } | ||
76 changes: 76 additions & 0 deletions
76
packages/rs-platform-wallet/src/platform_wallet_info/key_derivation.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| //! Shared key derivation utilities for identity authentication keys | ||
| //! | ||
| //! This module provides helper functions used by both the matured transactions | ||
| //! processor and the identity discovery scanner. | ||
|
|
||
| use crate::error::PlatformWalletError; | ||
| use key_wallet::Network; | ||
|
|
||
| /// Derive the 20-byte RIPEMD160(SHA256) hash of the public key at the given | ||
| /// identity authentication path. | ||
| /// | ||
| /// Path format: `base_path / identity_index' / key_index'` | ||
| /// where `base_path` is `m/9'/COIN_TYPE'/5'/0'` (mainnet or testnet). | ||
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `wallet` - The wallet to derive keys from | ||
| /// * `network` - Network to select the correct coin type | ||
| /// * `identity_index` - The identity index (hardened) | ||
| /// * `key_index` - The key index within that identity (hardened) | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// Returns the 20-byte public key hash suitable for Platform identity lookup. | ||
| pub(crate) fn derive_identity_auth_key_hash( | ||
| wallet: &key_wallet::Wallet, | ||
| network: Network, | ||
| identity_index: u32, | ||
| key_index: u32, | ||
| ) -> Result<[u8; 20], PlatformWalletError> { | ||
| use dashcore::secp256k1::Secp256k1; | ||
| use dpp::util::hash::ripemd160_sha256; | ||
| use key_wallet::bip32::{ChildNumber, DerivationPath, ExtendedPubKey}; | ||
| use key_wallet::dip9::{ | ||
| IDENTITY_AUTHENTICATION_PATH_MAINNET, IDENTITY_AUTHENTICATION_PATH_TESTNET, | ||
| }; | ||
|
|
||
| let base_path = match network { | ||
| Network::Dash => IDENTITY_AUTHENTICATION_PATH_MAINNET, | ||
| Network::Testnet => IDENTITY_AUTHENTICATION_PATH_TESTNET, | ||
| _ => { | ||
| return Err(PlatformWalletError::InvalidIdentityData( | ||
| "Unsupported network for identity derivation".to_string(), | ||
| )); | ||
| } | ||
| }; | ||
|
|
||
| let mut full_path = DerivationPath::from(base_path); | ||
| full_path = full_path.extend([ | ||
| ChildNumber::from_hardened_idx(identity_index).map_err(|e| { | ||
| PlatformWalletError::InvalidIdentityData(format!("Invalid identity index: {}", e)) | ||
| })?, | ||
| ChildNumber::from_hardened_idx(key_index).map_err(|e| { | ||
| PlatformWalletError::InvalidIdentityData(format!("Invalid key index: {}", e)) | ||
| })?, | ||
| ]); | ||
|
|
||
| let auth_key = wallet | ||
| .derive_extended_private_key(&full_path) | ||
| .map_err(|e| { | ||
| PlatformWalletError::InvalidIdentityData(format!( | ||
| "Failed to derive authentication key: {}", | ||
| e | ||
| )) | ||
| })?; | ||
|
|
||
| let secp = Secp256k1::new(); | ||
| let public_key = ExtendedPubKey::from_priv(&secp, &auth_key); | ||
| let public_key_bytes = public_key.public_key.serialize(); | ||
| let key_hash = ripemd160_sha256(&public_key_bytes); | ||
|
|
||
| let mut key_hash_array = [0u8; 20]; | ||
| key_hash_array.copy_from_slice(&key_hash); | ||
|
|
||
| Ok(key_hash_array) | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't spend the gap budget on query failures.
A Platform/SDK error is not a confirmed empty slot. Incrementing
consecutive_misseshere can stop the scan early and skip identities later in the range. Either bubble the error up, or log/retry without counting it as a miss.🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 33b3da2 — removed the
consecutive_misses += 1from the error branch. Query failures now just log a warning without consuming gap budget.