refactor: extract BlockInfo from TransactionContext#578
Conversation
The `InBlock` and `InChainLockedBlock` variants of `TransactionContext` used `Option<BlockHash>` and `Option<u32>` for block hash and timestamp, but these were never `None` in practice. Extract them into a dedicated `BlockInfo` struct with required fields, replacing the per-field accessor methods (`block_height()`, `block_hash()`, `timestamp()`) with a single `block_info()` that returns the struct. Move `BlockInfo` and `TransactionContext` from `wallet_checker.rs` into their own `transaction_context.rs` module since they are general-purpose types, not specific to wallet checking logic. Also move the FFI helper `block_info_from_ffi` from `lib.rs` to `types.rs` where it belongs with other FFI type conversions.
📝 WalkthroughWalkthroughThis PR refactors block metadata handling across the transaction checking layer by introducing a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #578 +/- ##
=============================================
+ Coverage 66.33% 66.56% +0.23%
=============================================
Files 311 312 +1
Lines 64976 64909 -67
=============================================
+ Hits 43102 43209 +107
+ Misses 21874 21700 -174
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@key-wallet-ffi/src/types.rs`:
- Around line 14-28: The FFI wrapper block_info_from_ffi silently truncates a
u64 timestamp to u32 when calling BlockInfo::new; add a safety check and
documentation: update the function's safety docs to state the timestamp must fit
in u32 and add a debug_assert!(timestamp <= u32::MAX, "timestamp out of range")
(or similar runtime check) before casting, so unexpected large values are caught
during development while still calling BlockInfo::new(height, block_hash,
timestamp as u32).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4236dadb-ebb5-4903-8aae-7d48a45257b1
📒 Files selected for processing (13)
key-wallet-ffi/src/transaction.rskey-wallet-ffi/src/transaction_checking.rskey-wallet-ffi/src/types.rskey-wallet/src/managed_account/mod.rskey-wallet/src/manager/process_block.rskey-wallet/src/transaction_checking/mod.rskey-wallet/src/transaction_checking/transaction_context.rskey-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rskey-wallet/src/transaction_checking/transaction_router/tests/coinbase.rskey-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rskey-wallet/src/transaction_checking/transaction_router/tests/provider.rskey-wallet/src/transaction_checking/transaction_router/tests/routing.rskey-wallet/src/transaction_checking/wallet_checker.rs
The
InBlockandInChainLockedBlockvariants ofTransactionContextusedOption<BlockHash>andOption<u32>for block hash and timestamp, but these were neverNonein practice. Extract them into a dedicatedBlockInfostruct with required fields, replacing the per-field accessor methods (block_height(),block_hash(),timestamp()) with a singleblock_info()that returns the struct.Move
BlockInfoandTransactionContextfromwallet_checker.rsinto their owntransaction_context.rsmodule since they are general-purpose types, not specific to wallet checking logic. Also move the FFI helperblock_info_from_ffifromlib.rstotypes.rswhere it belongs with other FFI type conversions.Summary by CodeRabbit