feat: mrh watcher #974
feat: mrh watcher #974
Conversation
WalkthroughThis change introduces a new asynchronous watcher, Changes
Sequence Diagram(s)sequenceDiagram
participant Blockchain as Blockchain Stream
participant MrhWatcher
participant Database
participant BroadcastChannel
participant Sidecar
Blockchain->>MrhWatcher: New Transaction
MrhWatcher->>Database: Query routing hints for outputs
Database-->>MrhWatcher: Matching routing hints
MrhWatcher->>BroadcastChannel: SwapStatus(TransactionDirect)
BroadcastChannel->>Sidecar: SwapStatus(TransactionDirect)
Sidecar->>Sidecar: handleSentSwapUpdate(TransactionDirect)
Sidecar->>gRPC Stream: Write SwapUpdateRequest
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ 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 (
|
jackstar12
force-pushed
the
feat/mrh-watcher
branch
2 times, most recently
from
779cea2 to
4d84988
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
boltzr/src/swap/status.rs (1)
24-25: LGTM! Consistent enum variant addition.The new
TransactionDirectvariant follows the existing pattern and matches the corresponding TypeScript enum value. The serialization string is consistent with the naming convention.Consider adding this variant to the test cases in the
test_swap_update_parsefunction to ensure it's properly covered.boltzr/src/chain/mrh_watcher.rs (1)
65-65: Define the status string as a constant.The status string "transaction.direct" is hardcoded. Consider defining it as a constant for better maintainability and consistency.
Add a constant at the module level:
const TRANSACTION_DIRECT_STATUS: &str = "transaction.direct";Then use it in the SwapStatus construction:
- status: "transaction.direct".to_string(), + status: TRANSACTION_DIRECT_STATUS.to_string(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
boltzr/src/chain/mod.rs(1 hunks)boltzr/src/chain/mrh_watcher.rs(1 hunks)boltzr/src/chain/utils.rs(1 hunks)boltzr/src/db/helpers/reverse_swap.rs(3 hunks)boltzr/src/swap/manager.rs(3 hunks)boltzr/src/swap/status.rs(1 hunks)lib/consts/Enums.ts(1 hunks)lib/sidecar/Sidecar.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (ubuntu-latest, 20, stable)
- GitHub Check: build-rust (ubuntu-latest, 20, stable)
- GitHub Check: build-rust (ubuntu-latest, 20, stable)
- GitHub Check: build (ubuntu-latest, 20, stable)
🔇 Additional comments (6)
boltzr/src/chain/utils.rs (1)
46-51: LGTM! Clean implementation of transaction ID extraction.The method correctly handles both Bitcoin and Elements transaction types, using the appropriate txid extraction methods for each. The hex conversion is consistent and the implementation follows Rust conventions.
boltzr/src/chain/mod.rs (1)
10-10: LGTM! Standard module declaration.The module declaration follows the existing pattern and is properly positioned with other module declarations.
lib/consts/Enums.ts (1)
51-51: LGTM! Consistent enum addition.The new
TransactionDirectenum member follows the existing naming convention and is properly positioned with other transaction-related events.lib/sidecar/Sidecar.ts (1)
484-492: LGTM! Proper handling of TransactionDirect updates.The implementation correctly:
- Logs the update for debugging purposes
- Creates and sends the appropriate swap update request
- Uses optional chaining to safely handle potential undefined
subscribeSwapUpdatesCall- Returns early to prevent fallthrough to the default case
The logic is consistent with the existing codebase patterns.
boltzr/src/db/helpers/reverse_swap.rs (1)
8-56: LGTM! Clean database helper implementation.The new
get_routing_hintsmethod and its implementation follow the established patterns in this module. The type alias, trait method, and Diesel query are all properly structured.boltzr/src/chain/mrh_watcher.rs (1)
1-161: Well-structured implementation with comprehensive test coverage.The MrhWatcher implementation is clean and follows good async patterns. The test effectively validates the core functionality using mocks.
jackstar12
force-pushed
the
feat/mrh-watcher
branch
from
4d84988 to
526dd32
Compare
jackstar12
force-pushed
the
feat/mrh-watcher
branch
from
526dd32 to
e87ba07
Compare
allows for better tests
jackstar12
force-pushed
the
feat/mrh-watcher
branch
2 times, most recently
from
a2abc9d to
501f233
Compare
jackstar12
force-pushed
the
feat/mrh-watcher
branch
2 times, most recently
from
123586d to
9f0738b
Compare
jackstar12
force-pushed
the
feat/mrh-watcher
branch
from
9f0738b to
a5cc029
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/integration/sidecar/Sidecar.spec.ts (1)
81-103: Good test coverage for TransactionDirect handling with minor suggestions.The test case follows existing patterns and correctly verifies the new TransactionDirect functionality. However, consider these improvements:
Address type casting: The
as anycast on line 88 suggests potential type safety issues. Consider using proper typing or creating a proper mock.Consider edge case testing: The test only covers the happy path. You might want to add tests for error scenarios, such as missing transaction info or invalid data.
- update.setTransactionInfo(transactionInfo as any); + update.setTransactionInfo(transactionInfo);If the type casting is necessary due to protobuf type constraints, consider adding a comment explaining why the cast is required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
boltzr/src/chain/mod.rs(1 hunks)boltzr/src/chain/mrh_watcher.rs(1 hunks)boltzr/src/chain/utils.rs(1 hunks)boltzr/src/db/helpers/reverse_swap.rs(3 hunks)boltzr/src/swap/manager.rs(3 hunks)boltzr/src/swap/status.rs(1 hunks)lib/consts/Enums.ts(1 hunks)lib/sidecar/Sidecar.ts(1 hunks)test/integration/sidecar/Sidecar.spec.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- boltzr/src/chain/mod.rs
- lib/consts/Enums.ts
- lib/sidecar/Sidecar.ts
- boltzr/src/swap/status.rs
- boltzr/src/swap/manager.rs
- boltzr/src/chain/utils.rs
- boltzr/src/db/helpers/reverse_swap.rs
- boltzr/src/chain/mrh_watcher.rs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-rust (ubuntu-latest, 20, stable)
- GitHub Check: build (ubuntu-latest, 20, stable)
- GitHub Check: build-rust (ubuntu-latest, 20, stable)
- GitHub Check: build (ubuntu-latest, 20, stable)
🔇 Additional comments (1)
test/integration/sidecar/Sidecar.spec.ts (1)
9-13: LGTM! Mock enhancement supports new test case.The addition of the
emitmethod to the eventHandler mock is consistent with the existing pattern and necessary for testing the new TransactionDirect functionality.
Summary by CodeRabbit