fix: mismatched tms db state#7131
Conversation
WalkthroughA new method was introduced to fetch completed transactions with mismatched mined status, filtering by cancellation and specific transaction statuses. The transaction detection logic was updated to include these transactions in its results, aggregating them with previously fetched unconfirmed transactions. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant SqliteDB
Caller->>SqliteDB: fetch_unconfirmed_detected_transactions()
SqliteDB->>SqliteDB: fetch_one_sided_unconfirmed_transactions()
SqliteDB->>SqliteDB: fetch_coinbase_unconfirmed_transactions()
SqliteDB->>SqliteDB: fetch_transactions_with_mismatched_mined_status(cancelled=false)
SqliteDB-->>Caller: Aggregate and return all results
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
base_layer/wallet/src/transaction_service/storage/sqlite_db.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test (mainnet, stagenet)
- GitHub Check: test (testnet, esmeralda)
- GitHub Check: test (nextnet, nextnet)
- GitHub Check: ci
- GitHub Check: cargo check with stable
- GitHub Check: Cucumber tests / Base Layer
- GitHub Check: Cucumber tests / FFI
🔇 Additional comments (1)
base_layer/wallet/src/transaction_service/storage/sqlite_db.rs (1)
1048-1056:⚠️ Potential issuePotential duplicate transactions in aggregated results.
The current implementation may include duplicate transactions in the final result. The new
mismatched_txquery includes transactions withOneSidedUnconfirmedandCoinbaseUnconfirmedstatuses that havemined_height = None, but these same statuses are already fetched in the earlier queries (lines 1027-1046). This could lead to the same transaction appearing multiple times in the returned vector.Consider modifying the approach to avoid duplicates:
let mut mismatched_tx = CompletedTransactionSql::fetch_transactions_with_mismatched_mined_status(false, &mut conn)? .into_iter() + .filter(|ct| { + // Exclude statuses already fetched above to prevent duplicates + let status = TransactionStatus::try_from(ct.status).unwrap_or(TransactionStatus::Pending); + !matches!(status, TransactionStatus::OneSidedUnconfirmed | TransactionStatus::CoinbaseUnconfirmed) + }) .map(|ct: CompletedTransactionSql| { CompletedTransaction::try_from(ct, &cipher).map_err(TransactionStorageError::from) }) .collect::<Result<Vec<CompletedTransaction>, TransactionStorageError>>()?;Alternatively, modify the
fetch_transactions_with_mismatched_mined_statusmethod to exclude these statuses when called from this context.Likely an incorrect or invalid review comment.
Test Results (CI)344 tests 343 ✅ 8m 0s ⏱️ For more details on these failures, see this check. Results for commit a6ddf7a. ♻️ This comment has been updated with latest results. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Description
Somehow the TMS db for completed transactions loses its mined height, but keeps its mined_confirmed status. The outputs are correctly marked with mined_heights in the OMS.
This tries to correct the fields and update them
Summary by CodeRabbit