feat: add block hash to grpc method#7025
feat: add block hash to grpc method#7025SWvheerden merged 5 commits intotari-project:developmentfrom
Conversation
Added optional block hash to grpc method GetCompletedTransactionsRequest
WalkthroughThis change extends the wallet's completed transactions query functionality by introducing an optional block hash filter alongside the existing payment ID filter. The gRPC API, backend storage, service layer, and all relevant method signatures are updated to accept and process this new parameter. Supporting logic in the SQLite backend, FFI, UI state, and tests is adjusted to accommodate the additional argument. The gRPC request message and Rust service interfaces now support querying completed transactions filtered by either or both payment ID and block hash. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant gRPC_Server
participant WalletService
participant StorageBackend
Client->>gRPC_Server: GetCompletedTransactionsRequest(payment_id, block_hash)
gRPC_Server->>WalletService: get_completed_transactions(payment_id, block_hash)
WalletService->>StorageBackend: get_completed_transactions(payment_id, block_hash)
StorageBackend-->>WalletService: [CompletedTransaction...]
WalletService-->>gRPC_Server: [CompletedTransaction...]
gRPC_Server-->>Client: [CompletedTransaction...]
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ 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
🧹 Nitpick comments (3)
integration_tests/tests/steps/wallet_steps.rs (1)
551-554: Consistent import usage for request type.
Here you’re usinggrpc::GetCompletedTransactionsRequestinstead of the directly importedGetCompletedTransactionsRequest. Consider unifying to the same imported type across all calls for clarity and consistency.base_layer/wallet_ffi/src/lib.rs (1)
8596-8601: The call to get_completed_transactions has been properly updated.This change correctly implements the new method signature. Consider adding a comment to document what the parameters represent for improved readability.
let completed_transactions = (*wallet).runtime.block_on( (*wallet) .wallet .transaction_service - .get_completed_transactions(None, None), + .get_completed_transactions(None, None), // (payment_id, block_hash) );applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs (1)
933-940: Implementation correctly handles optional block hash parameter.The code properly extracts and parses the optional block hash from the gRPC request, converting it from hex string to a
BlockHashtype with appropriate error handling.However, the error message refers to an "Output hash" when it should refer to a "Block hash".
- BlockHash::from_hex(&hash.hash) - .map_err(|_| Status::internal("Output hash is malformed".to_string()))?, + BlockHash::from_hex(&hash.hash) + .map_err(|_| Status::internal("Block hash is malformed".to_string()))?,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
applications/minotari_app_grpc/proto/wallet.proto(22 hunks)applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs(1 hunks)applications/minotari_console_wallet/src/ui/state/app_state.rs(1 hunks)base_layer/wallet/src/transaction_service/handle.rs(3 hunks)base_layer/wallet/src/transaction_service/service.rs(1 hunks)base_layer/wallet/src/transaction_service/storage/database.rs(6 hunks)base_layer/wallet/src/transaction_service/storage/sqlite_db.rs(2 hunks)base_layer/wallet/tests/transaction_service_tests/service.rs(5 hunks)base_layer/wallet/tests/transaction_service_tests/storage.rs(2 hunks)base_layer/wallet/tests/transaction_service_tests/transaction_protocols.rs(8 hunks)base_layer/wallet_ffi/src/lib.rs(6 hunks)integration_tests/tests/steps/wallet_steps.rs(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
base_layer/wallet_ffi/src/lib.rs (2)
applications/minotari_console_wallet/src/ui/state/app_state.rs (3)
completed_transactions(773-779)wallet(725-731)wallet(734-740)base_layer/wallet/src/transaction_service/storage/sqlite_db.rs (10)
completed_transactions(845-851)completed_transactions(902-910)completed_transactions(941-941)completed_transactions(942-942)completed_transactions(943-943)completed_transactions(1746-1746)completed_transactions(1802-1804)completed_transactions(1851-1854)completed_transactions(1939-1942)completed_transactions(2189-2218)
base_layer/wallet/tests/transaction_service_tests/storage.rs (1)
base_layer/wallet/src/transaction_service/storage/database.rs (2)
get_cancelled_completed_transactions(611-613)get_cancelled_completed_transaction(424-429)
base_layer/wallet/src/transaction_service/storage/sqlite_db.rs (3)
base_layer/wallet/src/transaction_service/storage/database.rs (1)
find_completed_transactions_filter_payment_id_block_hash(159-163)base_layer/core/src/transactions/transaction_components/transaction_output.rs (1)
hash(207-224)base_layer/core/src/blocks/block_header.rs (1)
hash(148-155)
applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs (1)
base_layer/core/src/transactions/transaction_components/encrypted_data.rs (1)
from_hex(756-759)
base_layer/wallet/src/transaction_service/storage/database.rs (1)
base_layer/wallet/src/transaction_service/storage/sqlite_db.rs (1)
find_completed_transactions_filter_payment_id_block_hash(1051-1081)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test (testnet, esmeralda)
- GitHub Check: test (nextnet, nextnet)
- GitHub Check: test (mainnet, stagenet)
- GitHub Check: ci
- GitHub Check: cargo check with stable
- GitHub Check: Cucumber tests / Base Layer
- GitHub Check: Cucumber tests / FFI
🔇 Additional comments (32)
base_layer/wallet/tests/transaction_service_tests/storage.rs (2)
361-361: Updated method call with new block_hash parameter.The
get_completed_transactionsmethod call has been updated to include a secondNoneparameter for the new optional block hash filter, aligning with the PR's feature to allow filtering completed transactions by block hash.
419-419: Method calls consistently updated with new parameter.All calls to
get_completed_transactionsthroughout the test file have been properly updated to include the new optional block hash parameter, maintaining consistency with the updated method signature.Also applies to: 427-427
applications/minotari_console_wallet/src/ui/state/app_state.rs (1)
754-759: Method call updated to support block hash filtering.The UI state's call to
get_completed_transactionshas been correctly updated to include the new optional block hash parameter (None). This ensures the application state layer properly integrates with the updated transaction service interface.base_layer/wallet/tests/transaction_service_tests/transaction_protocols.rs (1)
858-858: Method calls consistently updated with block hash parameter.All calls to
get_completed_transactionsthroughout the transaction protocols test file have been properly updated to include the new optional block hash parameter. This comprehensive update ensures test compatibility with the modified method signature.Also applies to: 893-893, 946-946, 1039-1039, 1084-1084, 1105-1105, 1275-1275, 1392-1392
integration_tests/tests/steps/wallet_steps.rs (8)
162-165: Approve defaultblock_hashparameter.
The addition ofblock_hash: Nonealongsidepayment_id: Nonehere correctly preserves existing behavior by not filtering on block hash.
499-502: Approve defaultblock_hashin request builder.
Updating theGetCompletedTransactionsRequestto includeblock_hash: Noneis necessary to match the new method signature and keeps the filter inactive by default.
971-974: Approve defaultblock_hashparameter.
This insertion ofblock_hash: Nonecorrectly aligns with the updated method signature and maintains the prior behavior.
1030-1033: Approve defaultblock_hashparameter.
Adding the newblock_hashfield withNoneensures backward compatibility and satisfies the extended API.
1249-1252: Approve defaultblock_hashparameter.
Includingblock_hash: Nonehere properly updates the call to the new signature without altering existing semantics.
2663-2666: Approve defaultblock_hashparameter.
The request now matches the extended signature by settingblock_hash: None, preserving previous behavior.
2817-2820: Approve defaultblock_hashparameter.
This update correctly passesblock_hash: Noneto the new gRPC method, keeping the original filter semantics.
2843-2846: Approve defaultblock_hashparameter.
Addingblock_hash: Nonealigns with the updated signature and ensures no unintended filtering.base_layer/wallet/tests/transaction_service_tests/service.rs (1)
2307-2307: Updated method calls correctly match new API signatureAll calls to
get_completed_transactions()have been properly updated to match the new method signature that now requires two optional parameters: payment_id and block_hash. The changes maintain the original behavior by passingNonefor both parameters, effectively requesting all completed transactions without any filtering.Also applies to: 2412-2412, 2416-2416, 2421-2421, 5591-5591, 5699-5699, 6335-6335
base_layer/wallet_ffi/src/lib.rs (5)
8296-8301: The call to get_completed_transactions has been properly updated.The method signature has been correctly updated to include the new optional block_hash parameter, aligning with the PR's goal of allowing filtering by block hash in the transaction service.
8367-8372: The call to get_completed_transactions has been properly updated.This change correctly implements the second None parameter for the block_hash, consistent with the updated method signature across the codebase.
8439-8444: The call to get_completed_transactions has been properly updated.The method call has been correctly updated to include the block_hash parameter, maintaining consistency with other similar calls in this file.
8660-8665: The call to get_completed_transactions has been properly updated.This change correctly adds the second None parameter for the block_hash, maintaining consistency with the updated method signature.
8736-8741: The call to get_completed_transactions has been properly updated.The method call has been properly updated to align with the new signature that accepts an optional block_hash parameter, consistent with the PR objectives.
applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs (1)
944-944: Implementation correctly passes the block hash to the service layer.The updated method call passes both the existing
payment_idparameter and the newblock_hashparameter to the transaction service, which aligns with the PR objective to enable filtering by block hash.base_layer/wallet/src/transaction_service/service.rs (1)
894-898: Implementation correctly adds block hash filtering capability.The transaction service request handler now properly accepts and forwards the
block_hashparameter to the database layer, enabling filtering completed transactions by block hash alongside the existing payment_id filter.base_layer/wallet/src/transaction_service/handle.rs (3)
82-85: LGTM! Good implementation of the optional block hash filter.The change from a tuple variant to a struct variant with named fields improves readability and makes it clear what each parameter represents.
214-214: LGTM on the simplified format string.The display implementation has been properly updated to handle the new struct variant.
969-974: LGTM! Method signature and implementation properly updated.The method signature now includes the new optional block_hash parameter, and the call to the service correctly passes both parameters in the new struct variant.
base_layer/wallet/src/transaction_service/storage/sqlite_db.rs (3)
43-43: LGTM! Correctly imported FixedHash.The necessary FixedHash type has been imported to support the new block hash filtering functionality.
1051-1055: LGTM! Method renamed and signature updated appropriately.The method has been clearly renamed to reflect its new purpose and the signature has been updated to accept the additional block_hash parameter.
1059-1074: LGTM! Well-implemented query building based on provided filters.The implementation correctly handles all four possible combinations of the optional parameters:
- Both payment_id and block_hash provided
- Only payment_id provided
- Only block_hash provided
- Neither provided
The dynamic query building follows good practices and is consistent with the rest of the codebase.
base_layer/wallet/src/transaction_service/storage/database.rs (6)
34-34: Import update for new functionalityThe import was correctly updated to include
BlockHashandFixedHashtypes, which are necessary for the new block hash filtering capability.
159-163: Properly renamed and expanded trait methodThe trait method has been appropriately renamed from
find_completed_transactions_for_payment_idtofind_completed_transactions_filter_payment_id_block_hashand now accepts two optional parameters:payment_idandblock_hash. This clearly indicates the expanded filtering capability and maintains a consistent API design.
486-494: Updated implementation matches trait methodThe implementation method has been updated to match the expanded trait method, correctly passing both optional parameters to the backend method. This ensures proper propagation of the new filtering capability.
603-609: Public method now supports block hash filteringThe public method
get_completed_transactionshas been properly updated to accept an optionalblock_hashparameter and passes it to the underlying implementation, which enables clients to filter completed transactions by block hash as specified in the PR objectives.
611-613: Maintained backward compatibilityThe
get_cancelled_completed_transactionsmethod was updated to explicitly passNonefor the newblock_hashparameter, preserving its existing behavior while maintaining API consistency with the other changes.
634-663: Correctly implemented filtering logicThe
get_completed_transactions_by_cancelledmethod was properly updated to:
- Accept an optional
block_hashparameter- Match on both
payment_id.is_none()andblock_hash.is_none()to determine the appropriate query path- Call the backend method with both filter parameters when either is provided
The implementation correctly handles all combinations of the optional parameters, maintaining existing behavior when no filters are provided and enabling more specific queries when filters are specified.
Test Results (CI) 3 files 90 suites 40m 14s ⏱️ For more details on these failures, see this check. Results for commit b4688f5. ♻️ This comment has been updated with latest results. |
Test Results (Integration tests) 2 files 11 suites 1h 15m 35s ⏱️ For more details on these failures, see this check. Results for commit b4688f5. ♻️ This comment has been updated with latest results. |
2 participants
Description
Added optional block hash to grpc method GetCompletedTransactionsRequest
Motivation and Context
Some clients maw want to filter completed transactions by block hash.
How Has This Been Tested?
System-level testing running thre grpc call.
What process can a PR reviewer use to test or verify this change?
Code review
System-level testing
Breaking Changes
Summary by CodeRabbit
New Features
Tests
Chores
No changes to user interface or existing transaction retrieval behavior unless the new filter is applied.