Skip to content

feat: improve scanning feedback#7622

Merged
SWvheerden merged 1 commit intotari-project:developmentfrom
SWvheerden:sw_improve_scanning_feedback
Dec 4, 2025
Merged

feat: improve scanning feedback#7622
SWvheerden merged 1 commit intotari-project:developmentfrom
SWvheerden:sw_improve_scanning_feedback

Conversation

@SWvheerden
Copy link
Copy Markdown
Collaborator

@SWvheerden SWvheerden commented Dec 3, 2025

Description

Improve the scanning feddback

Summary by CodeRabbit

  • Chores
    • Enhanced state inspection and logging for UTXO processing in wallet server operations, including mined and deletion status tracking.
    • Added Display formatting for transaction types to support improved debugging and logging output.

✏️ Tip: You can customize this high-level summary in your review settings.

@SWvheerden SWvheerden requested a review from a team as a code owner December 3, 2025 13:15
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

These changes enhance UTXO processing by adding state inspection and logging in the gRPC wallet server, and introduce Display trait implementations for transaction models to support debugging and logging.

Changes

Cohort / File(s) Change Summary
gRPC Server UTXO State Inspection
applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs
Replaces direct database result access with db_output reference; adds logging for mined status (mined_in_block, mined_timestamp) and deletion status (marked_deleted_in_block, marked_deleted_at_height); fetches and logs transaction existence via Transaction Service; retains ScanFeedback push sourced from db_output.
Transaction Model Display Implementations
base_layer/wallet/src/transaction_service/storage/models.rs
Adds Display trait implementations for InboundTransaction, OutboundTransaction, CompletedTransaction, and WalletTransaction; formats key fields including tx_id, source/destination, amount, fee, status, timestamp, and cancelled status for debugging and logging.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • wallet_grpc_server.rs requires understanding UTXO state semantics, transaction service integration, and verification of logging correctness
  • models.rs Display implementations are straightforward but should verify formatting completeness across all transaction variants

Poem

🐰 In the realm of UTXOs, we spy and inspect,
Each state and each block, we log and detect,
Transactions now speak in Display's fine tongue,
The wallet grows wise, the audit songs sung!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: improve scanning feedback' directly relates to the main changes in the PR, which enhance logging and state inspection in the wallet scanning process.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9406e48 and 0019dfb.

📒 Files selected for processing (2)
  • applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs (1 hunks)
  • base_layer/wallet/src/transaction_service/storage/models.rs (4 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-07-15T12:23:14.650Z
Learnt from: hansieodendaal
Repo: tari-project/tari PR: 7284
File: applications/minotari_console_wallet/src/automation/commands.rs:0-0
Timestamp: 2025-07-15T12:23:14.650Z
Learning: In applications/minotari_console_wallet/src/automation/commands.rs, the consistent error handling pattern for command execution is to use match statements that: 1) On success: log with debug!, print user feedback, and push tx_id to tx_ids vector for monitoring, 2) On error: print error message with eprintln! using the format "{CommandName} error! {}", rather than using .unwrap() which would panic.

Applied to files:

  • applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs
📚 Learning: 2025-06-26T13:18:55.898Z
Learnt from: hansieodendaal
Repo: tari-project/tari PR: 7266
File: base_layer/core/src/chain_storage/blockchain_database.rs:457-461
Timestamp: 2025-06-26T13:18:55.898Z
Learning: The MinedInfo struct in the Tari blockchain database contains two optional fields: `input: Option<InputMinedInfo>` and `output: Option<OutputMinedInfo>`. This design allows methods returning MinedInfo to always return a valid struct rather than wrapping it in Option, with "not found" scenarios represented through the internal optional fields.

Applied to files:

  • applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs
📚 Learning: 2025-08-11T15:37:17.354Z
Learnt from: hansieodendaal
Repo: tari-project/tari PR: 7387
File: base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs:2896-2927
Timestamp: 2025-08-11T15:37:17.354Z
Learning: The `update_accumulated_difficulty` method in `base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs` is migration-specific code that only runs during a once-off migration when existing ChainHeader data is present in LMDB. The consistency guarantees are upheld by the calling function `process_accumulated_data_for_height`, so additional validation within this method is redundant.

Applied to files:

  • applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs
📚 Learning: 2025-08-26T06:28:13.374Z
Learnt from: hansieodendaal
Repo: tari-project/tari PR: 7432
File: applications/minotari_app_grpc/proto/wallet.proto:1933-1948
Timestamp: 2025-08-26T06:28:13.374Z
Learning: In PR #7432 "feat!: remove comms from wallet", the HttpPeer message in the wallet.proto should only include url, last_latency, and is_online fields. The base node public_key and node_id fields mentioned in the original PR summary were removed from the scope and should not be included.

Applied to files:

  • applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs
📚 Learning: 2025-08-22T10:22:27.310Z
Learnt from: hansieodendaal
Repo: tari-project/tari PR: 7432
File: integration_tests/src/ffi/ffi_import.rs:351-356
Timestamp: 2025-08-22T10:22:27.310Z
Learning: The PR "feat!: remove comms from wallet" is a breaking change PR where backward compatibility is intentionally not maintained. The FFI functions `comms_config_create` and `comms_config_destroy` were renamed to `wallet_db_config_create` and `wallet_db_config_destroy` as part of the intentional breaking changes.

Applied to files:

  • applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs
⏰ 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). (10)
  • GitHub Check: cargo check with msrv
  • GitHub Check: test (nextnet, nextnet)
  • GitHub Check: ci
  • GitHub Check: test (testnet, esmeralda)
  • GitHub Check: test (mainnet, stagenet)
  • GitHub Check: wasm build tests
  • GitHub Check: cargo check with stable
  • GitHub Check: ledger build tests
  • GitHub Check: Cucumber tests / FFI
  • GitHub Check: Cucumber tests / Base Layer
🔇 Additional comments (7)
base_layer/wallet/src/transaction_service/storage/models.rs (4)

92-100: LGTM! Clean Display implementation for debugging.

The Display implementation provides clear, concise formatting of key transaction fields for logging and debugging purposes.


179-187: LGTM! Consistent Display implementation.

The formatting is consistent with InboundTransaction and appropriately includes the fee field for outbound transactions.


522-551: LGTM! Proper handling of optional cancelled reason.

The Display implementation correctly handles the optional cancellation reason and provides comprehensive transaction information for debugging.


711-719: LGTM! Clean delegation to variant Display implementations.

The implementation appropriately delegates to each variant's Display implementation while adding a descriptive prefix to indicate the transaction state.

applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs (3)

3200-3202: LGTM! Safe extraction with appropriate guard.

The .expect() is properly guarded by the !db_result.is_empty() check on line 3200, making it safe.


3203-3210: LGTM! Enhanced mined status logging.

The logging provides valuable state information by checking both mined_in_block and mined_timestamp fields, improving scanning feedback.


3223-3247: LGTM! Comprehensive transaction inspection logging.

The transaction fetching and logging logic is well-structured:

  • Properly handles all result cases (found, not found, error)
  • Line 3227 effectively uses the new Display implementation from models.rs for clear logging
  • Line 3247's unwrap_or_default() is safe as a fallback for the transaction ID

These changes enhance scanning feedback without altering control flow, which is appropriate for debugging improvements.

Comment on lines +3211 to +3222
match (db_output.marked_deleted_in_block, db_output.marked_deleted_at_height) {
(Some(hash), Some(height)) => {
debug_info.push(format!(
"UTXO with hash {} is marked deleted at height: {} in block: {}",
hex, height, hash
));
},
(None, None) => {},
_ => {
debug_info.push(format!("UTXO with hash {} is has inconsistent mined data", hex));
},
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix grammar error in log message.

Line 3220 contains a grammar error: "is has inconsistent" should be "has inconsistent".

Apply this diff:

-                        debug_info.push(format!("UTXO with hash {} is has inconsistent mined data", hex));
+                        debug_info.push(format!("UTXO with hash {} has inconsistent deletion data", hex));

Note: Also clarified "mined data" to "deletion data" since this is checking deletion-related fields.

🤖 Prompt for AI Agents
In applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs around
lines 3211 to 3222, fix the log message that currently reads "UTXO with hash {}
is has inconsistent mined data" by correcting the grammar and clarifying the
field checked: replace it with a message like "UTXO with hash {} has
inconsistent deletion data" so it reads properly and accurately reflects that
deletion-related fields are inconsistent.

@SWvheerden SWvheerden merged commit 0868273 into tari-project:development Dec 4, 2025
17 of 18 checks passed
@SWvheerden SWvheerden deleted the sw_improve_scanning_feedback branch December 4, 2025 06:44
SWvheerden added a commit that referenced this pull request Dec 5, 2025
Description
---
Improve the scanning feddback

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
* Enhanced state inspection and logging for UTXO processing in wallet
server operations, including mined and deletion status tracking.
* Added Display formatting for transaction types to support improved
debugging and logging output.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants