feat: change wallet birthday offset #6992
Conversation
WalkthroughA new configurable field, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Wallet
participant UtxoScannerServiceInitializer
participant UtxoScannerServiceBuilder
participant UtxoScannerResources
participant UtxoScannerTask
User->>Wallet: start(config)
Wallet->>UtxoScannerServiceInitializer: new(..., config.birthday_offset)
UtxoScannerServiceInitializer->>UtxoScannerServiceBuilder: build_with_resources(..., birthday_offset)
UtxoScannerServiceBuilder->>UtxoScannerResources: construct(..., birthday_offset)
UtxoScannerResources->>UtxoScannerTask: create_task(..., birthday_offset)
UtxoScannerTask-->>UtxoScannerResources: use birthday_offset for scanning start
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
common/config/presets/d_console_wallet.toml (1)
102-103: Configuration option added with clear documentation.The PR adds a well-documented configuration option for the birthday offset with a default value of 2 days. This provides users the flexibility to customize scanning behavior.
Note that there's a minor typo "defaul" (missing 't') in the comment text, but this won't affect functionality.
-#The defaul offset for scanning from the birthday, it will start at the birthday - the offset +#The default offset for scanning from the birthday, it will start at the birthday - the offset
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
base_layer/wallet/src/config.rs(2 hunks)base_layer/wallet/src/utxo_scanner_service/initializer.rs(3 hunks)base_layer/wallet/src/utxo_scanner_service/service.rs(2 hunks)base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs(2 hunks)base_layer/wallet/src/utxo_scanner_service/uxto_scanner_service_builder.rs(3 hunks)base_layer/wallet/src/wallet.rs(6 hunks)base_layer/wallet/tests/transaction_service_tests/service.rs(1 hunks)base_layer/wallet/tests/utxo_scanner/mod.rs(1 hunks)common/config/presets/d_console_wallet.toml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs (2)
base_layer/key_manager/src/lib.rs (1)
get_birthday_from_unix_epoch_in_seconds(41-43)base_layer/key_manager/src/cipher_seed.rs (1)
birthday(330-332)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test (testnet, esmeralda)
- GitHub Check: cargo check with stable
- GitHub Check: ci
- GitHub Check: test (nextnet, nextnet)
- GitHub Check: test (mainnet, stagenet)
- GitHub Check: Cucumber tests / Base Layer
- GitHub Check: Cucumber tests / FFI
🔇 Additional comments (24)
base_layer/wallet/src/config.rs (1)
124-125: Well-implemented birthday offset configuration with sensible defaultThe addition of the
birthday_offsetfield to theWalletConfigstruct provides users with the flexibility to configure how many days the wallet should start scanning before the actual birthday. The default value of 2 days (reduced from the previous hardcoded 14 days) seems reasonable for most use cases.The implementation is clean, with a descriptive comment and appropriate type (
u16).Also applies to: 165-165
base_layer/wallet/tests/transaction_service_tests/service.rs (1)
281-286: Ensure test initializer reflects new configurable birthday_offset
TheUtxoScannerServiceInitializer::newcall now accepts thebirthday_offsetparameter. Passing14here maintains the original 2-week offset for tests, isolating them from the updated default of 2 days.base_layer/wallet/tests/utxo_scanner/mod.rs (6)
203-218: Birthday offset parameter properly added to test setup.The PR correctly adds the explicit birthday offset parameter (14 days) when calling
build_with_resources. This ensures test compatibility with the new configurable offset feature.
307-308: Verify consistency between test value and actual offset parameter.The hardcoded
14u16value here matches the explicitly passed parameter on line 216, maintaining consistency within the test. This is important as the wallet scanned block detection logic depends on both values matching.
398-399: Confirm consistent offset value across test cases.The hardcoded value
14in test_utxo_scanner_recovery_with_restart is consistent with the previously reviewed test cases, ensuring uniform test behavior.
543-543: Consistent offset value in chain restart test case.The hardcoded value
14in test_utxo_scanner_recovery_with_restart_and_reorg properly aligns with the other test methods, ensuring tests remain consistent after refactoring.
741-741: Consistent offset value in cache clearing test case.The cache clearing test case uses the same offset value (14) as the other test methods, maintaining consistency and ensuring accurate test behavior.
851-852: Birthday offset type annotated for clarity.The use of
14u16here explicitly indicates the type expected by the birthday offset calculation function, providing clarity for future maintainers.base_layer/wallet/src/utxo_scanner_service/service.rs (2)
95-107: Birthday offset properly propagated to scanner task.The birthday offset is correctly added to the
UtxoScannerTaskwhen creating a new task. This ensures the configurable parameter is passed through to the component that actually uses it for UTXO scanning calculations.
189-202: Field added to resource struct for birthday offset storage.The
birthday_offsetfield of typeu16is appropriately added to theUtxoScannerResourcesstruct. This is a clean and backward-compatible way to extend the structure.base_layer/wallet/src/utxo_scanner_service/utxo_scanner_task.rs (3)
75-85: Field added to task struct for birthday offset.The
birthday_offsetfield of typeu16is appropriately added to theUtxoScannerTaskstruct, completing the chain of propagation from config through to the actual scanning logic.
746-766: Updated scanning logic to use configurable offset.The scanning logic has been successfully updated to use the configurable
birthday_offsetinstead of the previously hardcoded value. The comment has also been updated to reflect the new default of 2 days, aligning with the configuration change.This change improves flexibility by allowing users to set their preferred scanning range while maintaining the same functionality to avoid potential reorg issues.
777-786: Comprehensive logging message maintained.The PR preserves the detailed logging message that explains the birthday-to-scanning time conversion, which is important for troubleshooting and understanding wallet recovery behavior.
base_layer/wallet/src/utxo_scanner_service/initializer.rs (4)
50-56: Field addition properly encapsulates the new configuration parameter.The addition of the
birthday_offsetfield to theUtxoScannerServiceInitializerstruct is well-placed and uses an appropriate type (u16) for storing a day-based offset.
61-69: Constructor updated correctly to handle the new parameter.The constructor has been properly updated to accept the new
birthday_offsetparameter and initialize it in the struct construction. The parameter is placed at the end of the parameter list, which is a good practice for backward compatibility if this was an external API.
100-100: Parameter extraction looks good.The birthday_offset is correctly extracted from self to be used in the service initialization.
142-142: Parameter correctly passed to the builder.The birthday_offset is properly passed to the build_with_resources method of the UtxoScannerService builder, ensuring the configurable value is used throughout the scanning process.
base_layer/wallet/src/utxo_scanner_service/uxto_scanner_service_builder.rs (3)
121-121: Config value correctly propagated to resources.The wallet config's birthday_offset is properly accessed and passed to the UtxoScannerResources, ensuring the user-defined configuration is respected during UTXO scanning.
157-157: Parameter addition to method signature looks good.The builder's build_with_resources method has been correctly updated to accept the birthday_offset parameter. Adding it at the end of the parameter list is a good practice for backward compatibility.
170-170: Assignment to resources is correctly implemented.The birthday_offset parameter is properly assigned to the UtxoScannerResources struct, ensuring the configurable value is available during the UTXO scanning process.
base_layer/wallet/src/wallet.rs (4)
163-163: Good addition of config field to Wallet struct.Adding the
configfield to the Wallet struct is a good design choice that allows the wallet instance to reference configuration values after initialization. Making it public allows external components to access these settings if needed.
217-218: Proper cloning of config values.Cloning the config values when passing them to initializers is a good practice that avoids ownership issues. This ensures the config remains intact for use by subsequent initializers and for storing in the final Wallet struct.
Also applies to: 229-230, 259-260
265-265: Config value correctly passed to scanner initializer.The birthday_offset value from config is properly passed to the UtxoScannerServiceInitializer, ensuring the configurable offset is used during UTXO scanning.
383-383: Wallet struct properly initialized with config.The Wallet struct returned by the start method now includes the config field, making the configuration values accessible to consumers of the wallet instance.
Test Results (CI) 3 files 129 suites 57m 21s ⏱️ Results for commit e2851ac. |
Test Results (Integration tests) 2 files 11 suites 1h 18m 29s ⏱️ For more details on these failures, see this check. Results for commit e2851ac. |
Description
changes the wallet birthday offset from 2 weeks to 2 days. Allows users to set it to a custom number if so desired
Summary by CodeRabbit
New Features
Documentation
Bug Fixes