feat: add block height to query#7033
Conversation
Added block height to the grpc query to get completed transactions
WalkthroughThis change extends the completed transactions query functionality across the wallet and its interfaces by adding support for filtering by an optional block height. The new parameter is integrated throughout the gRPC API, backend storage, service logic, and all relevant method signatures, as well as updated in tests and FFI interfaces. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant gRPC_Server
participant TransactionServiceHandle
participant TransactionService
participant Database
Client->>gRPC_Server: GetCompletedTransactionsRequest(payment_id, block_hash, block_height)
gRPC_Server->>TransactionServiceHandle: get_completed_transactions(payment_id, block_hash, block_height)
TransactionServiceHandle->>TransactionService: GetCompletedTransactions(payment_id, block_hash, block_height)
TransactionService->>Database: get_completed_transactions(payment_id, block_hash, block_height)
Database-->>TransactionService: CompletedTransactions
TransactionService-->>TransactionServiceHandle: CompletedTransactions
TransactionServiceHandle-->>gRPC_Server: CompletedTransactions
gRPC_Server-->>Client: CompletedTransactions
Possibly related PRs
Suggested reviewers
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:
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 (2)
base_layer/wallet/tests/transaction_service_tests/service.rs (1)
2307-6335: Consider adding specific tests for the block_height filter functionalityWhile all existing calls have been correctly updated to include the new parameter, there are no tests that specifically verify the block height filtering works as expected. Consider adding test cases that use non-None values for this parameter to verify the filtering functionality.
applications/minotari_app_grpc/proto/wallet.proto (1)
343-349: Update RPC documentation to referenceblock_heightin summary and examples
The high-level description on line 319 still mentions filtering bypayment_idand/orblock_hashonly. Please update it to includeblock_height, and extend the sample JavaScript usage to show how to setblock_height(e.g.,block_height: 12345).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
applications/minotari_app_grpc/proto/wallet.proto(2 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(2 hunks)base_layer/wallet/src/transaction_service/service.rs(1 hunks)base_layer/wallet/src/transaction_service/storage/database.rs(5 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 (1)
base_layer/wallet/tests/transaction_service_tests/storage.rs (1)
base_layer/wallet/src/transaction_service/storage/database.rs (2)
get_cancelled_completed_transactions(614-616)get_cancelled_completed_transaction(425-430)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test (mainnet, stagenet)
- GitHub Check: test (nextnet, nextnet)
- GitHub Check: ci
- GitHub Check: test (testnet, esmeralda)
- GitHub Check: cargo check with stable
- GitHub Check: Cucumber tests / FFI
- GitHub Check: Cucumber tests / Base Layer
🔇 Additional comments (26)
applications/minotari_console_wallet/src/ui/state/app_state.rs (1)
757-757: Updated method call to include new block_height parameter.The call to
get_completed_transactionshas been updated to include a thirdNoneparameter for the new optional block height filter, matching the extended method signature.applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs (1)
941-945: Properly implemented block height filtering for completed transactions.The implementation correctly extracts the optional block height from the incoming gRPC request message and passes it to the transaction service method, enabling the new filtering capability as intended by the PR.
base_layer/wallet/tests/transaction_service_tests/transaction_protocols.rs (1)
858-858: Updated test calls to match new method signature.All calls to
get_completed_transactionshave been properly updated to include the newNoneparameter for block_height, ensuring the tests continue to fetch all transactions without any block height filtering.Also applies to: 893-893, 946-946, 1084-1085, 1105-1105, 1275-1275, 1392-1392
base_layer/wallet/tests/transaction_service_tests/service.rs (7)
2307-2307: Updated method call to include new block_height parameterThe call to
get_completed_transactionshas been updated to include the newblock_heightparameter with a value ofNone, maintaining backward compatibility.
2412-2412: Updated method call to include new block_height parameterThe call has been correctly updated to match the new method signature which includes the optional block height filter.
2416-2416: Updated method call to include new block_height parameterThe bob_ts instance's call to get_completed_transactions has been properly updated with the third parameter.
2421-2421: Updated method call to include new block_height parameterThe carol_ts instance's call has been correctly updated to match the new method signature.
5591-5591: Updated method call to include new block_height parameterThe call in the transaction_service_tx_broadcast test has been correctly updated.
5699-5699: Updated method call to include new block_height parameterAnother instance in the same test correctly updated to the new signature.
6335-6335: Updated method call to include new block_height parameterThe call in test_completed_transactions_ordering has been properly updated.
base_layer/wallet/tests/transaction_service_tests/storage.rs (1)
361-361: Updates to match new signature with block_height parameterThese changes update the test calls to
db.get_completed_transactionswith an additionalNoneparameter to match the new method signature that includes the optionalblock_heightparameter.Also applies to: 419-419, 427-427
integration_tests/tests/steps/wallet_steps.rs (1)
165-165: Updates to all GetCompletedTransactionsRequest instancesAll instances of
GetCompletedTransactionsRequestconstruction have been updated to include the newblock_height: Nonefield, ensuring the integration tests properly handle the new parameter for filtering transactions by block height.Also applies to: 503-503, 556-556, 977-977, 1037-1037, 1256-1256, 2672-2672, 2827-2827, 2854-2854
applications/minotari_app_grpc/proto/wallet.proto (1)
1185-1185: Verify that theBlockHeighttype is defined and imported
The new field uses aBlockHeightmessage. Ensure this type is declared in one of the imported proto files (e.g.,sidechain_types.protoortypes.proto). If it’s missing, add the proper import or defineBlockHeight(e.g.,message BlockHeight { uint64 height = 1; }).base_layer/wallet_ffi/src/lib.rs (1)
8300-8300: Consistently updated method calls with new block_height parameterAll calls to
get_completed_transactionshave been correctly updated to include the new optionalblock_heightparameter (passed asNonein all instances). This is consistent with the described feature enhancement to support filtering transactions by block height.Also applies to: 8371-8371, 8443-8443, 8600-8600, 8664-8664, 8740-8740
base_layer/wallet/src/transaction_service/handle.rs (3)
83-86: LGTM: Block height filter addition to GetCompletedTransactions enum variantThe addition of the
block_heightfield to theGetCompletedTransactionsenum variant is well-structured as an optional parameter, consistent with the existing filter fields.
967-972: Function signature correctly updated with block height parameterThe
get_completed_transactionsmethod signature has been properly extended to include the new optional block height parameter, maintaining API compatibility.
973-979: Implementation correctly passes block height parameter to requestThe implementation properly includes the block height parameter in the request structure, ensuring it will be passed to the underlying service layer.
base_layer/wallet/src/transaction_service/service.rs (1)
894-901: Implementation looks good for block height filtering parameter.The addition of the
block_heightparameter to theGetCompletedTransactionsrequest variant properly extends the transaction querying functionality to filter by block height. This change is consistent with the existing optional parameters (payment_idandblock_hash), allowing for flexible filtering of completed transactions.base_layer/wallet/src/transaction_service/storage/sqlite_db.rs (2)
1051-1056: Implementation looks good - adding block height filtering capability.The signature change adds an optional parameter to filter transactions by block height, which aligns well with the existing filtering capabilities for payment ID and block hash.
1067-1069: Implementation correctly adds block height filtering.The implementation follows the same pattern as the existing filters, properly handling the optional nature of the parameter and adding the SQL filter condition only when a block height is provided.
base_layer/wallet/src/transaction_service/storage/database.rs (6)
159-164: Proper trait extension for block height filtering.This change correctly extends the
TransactionBackendtrait to include the new block height parameter, ensuring all implementations will provide this filtering capability.
487-496: Method correctly propagates new block height parameter.The method has been updated to include and pass along the block height parameter to the underlying database implementation.
605-612: Public API correctly exposes block height filtering.The change properly extends the public-facing API to include block height filtering, passing the parameter down to the implementation method.
614-616: Explicitly passes None for unused filters in cancelled transactions method.This change makes the code more explicit, clearly showing that cancelled transactions aren't filtered by any of the available filter parameters.
637-643: Internal implementation properly handles block height parameter.The method has been correctly updated to accept and handle the new block height parameter.
649-668: Filter checking logic properly includes block height.The conditional logic has been updated to check all three filter parameters (payment_id, block_hash, and block_height) before deciding which database method to call. This follows the existing pattern and works correctly.
Test Results (CI)1 074 tests 1 069 ✅ 11m 40s ⏱️ For more details on these failures, see this check. Results for commit cd92578. |
2 participants
Description
Added block height to the grpc query to get completed transactions.
Motivation and Context
Some clients need to query transactions in a block by height.
How Has This Been Tested?
System-level testing [TBD]
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
Bug Fixes