-
Notifications
You must be signed in to change notification settings - Fork 8
feat: track synced_height per ManagedWalletInfo
#305
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
base: v0.41-dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughRemoved the FFI height-update API, added per-wallet synced-height tracking and a new FFI getter. WalletManager.update_height now propagates heights to each ManagedWalletInfo; WalletMetadata gained a synced_height field and WalletInfoInterface exposes synced_height. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WalletManager
participant WalletInfo
participant Metadata
rect rgb(245,245,255)
Note over Client,WalletManager: Old public flow (removed FFI)
Client->>WalletManager: wallet_manager_update_height(height)
WalletManager->>WalletManager: set current_height
Note over WalletManager: No per-wallet propagation
end
rect rgb(245,255,245)
Note over Client,Metadata: New flow (propagated)
Client->>WalletManager: update_height(height)
WalletManager->>WalletManager: set current_height
loop For each wallet
WalletManager->>WalletInfo: update_synced_height(height)
WalletInfo->>Metadata: metadata.synced_height = height
end
Note over Client,WalletInfo: Query per-wallet height via FFI
Client->>WalletInfo: managed_wallet_synced_height()
WalletInfo->>Metadata: read synced_height
Metadata-->>WalletInfo: synced_height
WalletInfo-->>Client: synced_height value
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used📓 Path-based instructions (6)key-wallet/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/{dash-network,dash-spv,key-wallet}/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
key-wallet/**/transaction_checking/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
**/tests/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (14)📓 Common learnings📚 Learning: 2025-08-21T05:01:58.949ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-16T09:03:55.811ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-22T17:59:37.849ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
📚 Learning: 2025-12-19T00:07:22.904ZApplied to files:
🧬 Code graph analysis (3)key-wallet-ffi/include/key_wallet_ffi.h (3)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (1)
key-wallet-manager/tests/integration_test.rs (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
key-wallet/src/wallet/metadata.rs (1)
19-20: Consider serde default for newsynced_heightto keep old data readableIf
WalletMetadatais ever deserialized from older serialized payloads (missing this new field), serde will fail unless you either bump the format or provide a default. Adding a field-levelserde(default)keeps deserialization backward compatible while still exposing the new field.Proposed serde-default tweak
- /// Synced to block height - pub synced_height: CoreBlockHeight, + /// Synced to block height + #[cfg_attr(feature = "serde", serde(default))] + pub synced_height: CoreBlockHeight,key-wallet-ffi/src/wallet_manager_tests.rs (1)
441-471: Test correctly adapted to API changes.The test properly verifies height retrieval via
wallet_manager_current_height. The direct access to internalFFIWalletManagerfields (runtime,manager) is acceptable here since the publicwallet_manager_update_heightFFI was intentionally removed—this simulates how height updates propagate internally.Consider adding a complementary test for
managed_wallet_synced_heightto verify per-wallet synced height propagation (the new API exposed at FFI boundary). The integration tests inkey-wallet-manager/tests/integration_test.rscover this, but FFI-level coverage would strengthen confidence.key-wallet-ffi/src/managed_wallet.rs (1)
589-612: Implementation follows established FFI patterns.The function correctly:
- Validates the input pointer for null and returns 0 with an error
- Uses the
WalletInfoInterfacetrait to accesssynced_height()- Sets success before returning the value
Minor observation: Line 609 uses an
unsafeblock inside anunsafe fn, which is explicit but redundant. This is stylistically acceptable and matches some other functions in the codebase.Missing unit test coverage: The tests module (lines 647-1146) doesn't include a test for
managed_wallet_synced_height. Consider adding a test similar totest_managed_wallet_get_balancethat validates null pointer handling and successful height retrieval.🔎 Suggested test skeleton
#[test] fn test_managed_wallet_synced_height() { let mut error = FFIError::success(); // Test with null managed wallet let height = unsafe { managed_wallet_synced_height(ptr::null(), &mut error) }; assert_eq!(height, 0); assert_eq!(error.code, FFIErrorCode::InvalidInput); // Create a valid managed wallet and test synced_height retrieval // ... similar setup to test_managed_wallet_get_balance ... }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
key-wallet-ffi/FFI_API.mdkey-wallet-ffi/include/key_wallet_ffi.hkey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/src/wallet_manager.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-manager/tests/integration_test.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/wallet/metadata.rs
💤 Files with no reviewable changes (1)
- key-wallet-ffi/src/wallet_manager.rs
🧰 Additional context used
📓 Path-based instructions (6)
key-wallet/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/*.rs: Separate immutable structures (Account,Wallet) containing only identity information from mutable wrappers (ManagedAccount,ManagedWalletInfo) with state management
Never serialize or log private keys in production; use public keys or key fingerprints for identification instead
Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
UseBTreeMapfor ordered data (accounts, transactions) andHashMapfor lookups (address mappings); apply memory management strategies for old transaction data
Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Use the?operator for error propagation, provide context in error messages, never panic in library code, and returnResult<T>for all fallible operations
Files:
key-wallet/src/wallet/metadata.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
key-wallet/src/wallet/metadata.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet-manager/tests/integration_test.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-manager/src/wallet_manager/mod.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings
**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with#[cfg(test)]annotation; integration tests use thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor asynchronous operations
Files:
key-wallet/src/wallet/metadata.rskey-wallet-ffi/src/wallet_manager_tests.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet-manager/tests/integration_test.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-manager/src/wallet_manager/mod.rs
**/{dash-network,dash-spv,key-wallet}/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use async/await for async operations in network and wallet modules
Files:
key-wallet/src/wallet/metadata.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful handling at FFI boundaries for memory safety
Be careful with FFI memory management
Files:
key-wallet-ffi/src/wallet_manager_tests.rskey-wallet-ffi/src/managed_wallet.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/tests/**/*.rs: Create integration tests for network operations
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate
**/tests/**/*.rs: Use descriptive test names (e.g.,test_parse_address_mainnet)
Mark network-dependent or long-running tests with#[ignore]and run with-- --ignored
Files:
key-wallet-manager/tests/integration_test.rs
🧠 Learnings (19)
📓 Common learnings
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize unit tests by functionality: separate test files for BIP32, mnemonics, addresses, derivation paths, and PSBT operations
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rskey-wallet-manager/tests/integration_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates when processing transactions: update transactions, UTXOs, balances, and address usage state together
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet-manager/tests/integration_test.rskey-wallet-manager/src/wallet_manager/mod.rskey-wallet-ffi/FFI_API.md
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Separate immutable structures (`Account`, `Wallet`) containing only identity information from mutable wrappers (`ManagedAccount`, `ManagedWalletInfo`) with state management
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet-manager/tests/integration_test.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Apply atomic state updates when managing watch-only wallets: validate that external signatures match expected pubkeys and never attempt signing operations
Applied to files:
key-wallet-ffi/src/wallet_manager_tests.rskey-wallet-manager/tests/integration_test.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet-manager/tests/integration_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use property-based testing for complex invariants such as gap limit constraints
Applied to files:
key-wallet-manager/tests/integration_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Always validate network consistency when deriving or validating addresses; never mix mainnet and testnet operations
Applied to files:
key-wallet-manager/tests/integration_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Use staged gap limit management with `GapLimitStage` tracking `last_used_index` and `used_indices` to enable efficient address discovery without loading entire chains into memory
Applied to files:
key-wallet-manager/tests/integration_test.rskey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/FFI_API.md
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/tests/**/*.rs : Test both mainnet and testnet configurations
Applied to files:
key-wallet-manager/tests/integration_test.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) to enable both full wallets and watch-only wallets with the same interface
Applied to files:
key-wallet-manager/tests/integration_test.rskey-wallet-ffi/src/managed_wallet.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/account/**/*.rs : Use enum-based type system for `AccountType` with specific variants (Standard, IdentityAuthentication, IdentityEncryption, MasternodeOperator, etc.) to provide compile-time safety and clear semantics
Applied to files:
key-wallet-manager/tests/integration_test.rskey-wallet-ffi/src/managed_wallet.rs
📚 Learning: 2025-08-21T05:01:58.949Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 108
File: key-wallet-ffi/src/wallet_manager.rs:270-318
Timestamp: 2025-08-21T05:01:58.949Z
Learning: In the key-wallet-ffi design, wallets retrieved from the wallet manager via lookup functions should return const pointers (*const FFIWallet) to enforce read-only access and prevent unintended modifications. The wallet manager should control wallet lifecycle and mutations through specific APIs rather than allowing external mutation of retrieved wallet references.
Applied to files:
key-wallet-ffi/include/key_wallet_ffi.hkey-wallet-ffi/src/managed_wallet.rskey-wallet-ffi/FFI_API.md
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)
Applied to files:
key-wallet-ffi/src/managed_wallet.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use the `?` operator for error propagation, provide context in error messages, never panic in library code, and return `Result<T>` for all fallible operations
Applied to files:
key-wallet-ffi/src/managed_wallet.rs
📚 Learning: 2025-06-26T15:48:36.342Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:48:36.342Z
Learning: In Rust FFI code, unwrap() must not be used on CString::new, especially for error messages or untrusted input, as it can panic if the string contains null bytes. Instead, use unwrap_or_else with a fallback to a known-safe, hardcoded string to prevent panics across FFI boundaries.
Applied to files:
key-wallet-ffi/src/managed_wallet.rs
📚 Learning: 2025-12-19T00:07:22.904Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Applies to key-wallet/**/*.rs : Use `BTreeMap` for ordered data (accounts, transactions) and `HashMap` for lookups (address mappings); apply memory management strategies for old transaction data
Applied to files:
key-wallet-ffi/src/managed_wallet.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/*-ffi/**/*.rs : Be careful with FFI memory management
Applied to files:
key-wallet-ffi/src/managed_wallet.rs
🧬 Code graph analysis (5)
key-wallet-ffi/src/wallet_manager_tests.rs (1)
key-wallet-ffi/src/wallet_manager.rs (3)
wallet_manager_create(105-126)wallet_manager_current_height(786-805)manager_guard(514-514)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (1)
key-wallet-manager/src/wallet_manager/mod.rs (1)
current_height(929-931)
key-wallet-manager/tests/integration_test.rs (4)
dash-spv/src/client/core.rs (1)
wallet(158-160)key-wallet-manager/src/wallet_manager/mod.rs (2)
current_height(929-931)wallet_count(318-320)key-wallet/src/mnemonic.rs (2)
generate(106-129)generate(133-144)key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
synced_height(65-65)synced_height(162-164)
key-wallet-ffi/include/key_wallet_ffi.h (4)
key-wallet-ffi/src/managed_wallet.rs (1)
managed_wallet_synced_height(597-612)key-wallet-ffi/src/managed_account.rs (1)
error(64-74)key-wallet-ffi/src/types.rs (1)
error(121-132)key-wallet-ffi/src/error.rs (1)
error(44-49)
key-wallet-ffi/src/managed_wallet.rs (2)
key-wallet-ffi/src/types.rs (1)
error(121-132)key-wallet-ffi/src/error.rs (2)
error(44-49)set_error(53-59)
🔇 Additional comments (8)
key-wallet-manager/src/wallet_manager/mod.rs (1)
933-938: Height propagation throughupdate_heightlooks correctUpdating
current_heightand then callingupdate_chain_heighton each wallet info keeps manager and per-wallet state consistent with the newsynced_heightsemantics.key-wallet-ffi/FFI_API.md (1)
45-46: FFI docs for synced height are consistent with the new API shapeThe updated function counts and the new
managed_wallet_synced_heightentry/section look consistent with the removal ofwallet_manager_update_heightand the move to per‑wallet synced height queries. Safety text and signature formatting match the existing conventions.Please double‑check that:
managed_wallet_synced_heightis declared identically inkey-wallet-ffi/include/key_wallet_ffi.h, and- Its Rust implementation in
key-wallet-ffi/src/managed_wallet.rsuses the same signature and safety contract as documented here.Also applies to: 70-71, 101-101, 1107-1121
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
64-66: Trait and implementation ofsynced_heightare wired correctlyExposing
synced_height()onWalletInfoInterfaceand backing it withmetadata.synced_heightinManagedWalletInfocleanly surfaces the per‑wallet synced height needed by the manager, tests, and FFI.Also applies to: 162-164
332-334: Updatingmetadata.synced_heightinsideupdate_chain_heightis appropriateTying
metadata.synced_heightdirectly toupdate_chain_heightensures all callers (manager, tests, FFI) see a coherent synced height whenever chain height advances.key-wallet-manager/tests/integration_test.rs (2)
8-8: ImportingWalletInfoInterfaceis necessary and minimalThe new import is exactly what’s needed to call
synced_height()on wallet infos in the test, with no extra surface added.
159-229: Expandedtest_block_height_trackinggives good coverage of the new semanticsThis test now validates:
- Manager height changes without wallets.
- Initial per‑wallet
synced_heightstate for newly created wallets.- Propagation of
update_heightto all wallets.- Independent per‑wallet height overrides via
update_chain_height.- That a later manager
update_heightcall re‑synchronizes all wallets.The flow accurately captures the intended behavior of the new
synced_heightfield and update logic.key-wallet-ffi/include/key_wallet_ffi.h (1)
3092-3104: LGTM!The new
managed_wallet_synced_heightfunction declaration is correctly structured with proper safety documentation. The signature correctly usesconstfor the managed wallet pointer, aligning with the FFI design philosophy of returning const pointers for read-only access. Based on learnings, this matches the pattern where wallet manager controls lifecycle and mutations through specific APIs.key-wallet-ffi/src/managed_wallet.rs (1)
7-8: LGTM on import changes.The new imports are correctly organized—
c_uintis grouped withc_charin the raw types import, andWalletInfoInterfaceis appropriately imported to enable thesynced_height()method call.Also applies to: 14-14
7316f44 to
0505ed2
Compare
This adds
synced_heightto the metadata of theManagedWalletInfowhich allows to keep track of the synced height of each individual wallet and also allows to leverage the synced height within wallet info for things like confirmation calculations.Few kind of related things included in this PR:
wallet_manager_update_heightfrom the wallet FFI since it's internal and i think we shouldn't be able to modify it from the FFI.Summary by CodeRabbit
New Features
Improvements
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.