refactor(dapi): rewrite dapi in Rust as rs-dapi #2716
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds a new Rust DAPI (rs-dapi) service and rs-dash-event-bus crate, integrates rs-dapi across build/CI/docker/dashmate, adds clients/services/streaming/logging/metrics/caching/configs/migrations/docs/tests, and performs many version bumps and workspace updates. Changes
Sequence Diagram(s)%%{init: {"themeVariables": {"primaryColor":"#8FB9A8", "secondaryColor":"#DCEFE0"}}}%%
sequenceDiagram
participant Client
participant JSONRPC as JSON‑RPC HTTP
participant Translator as JsonRpcTranslator
participant Platform as PlatformServiceImpl
participant Drive as DriveClient
participant Core as CoreClient
participant Tender as TenderdashClient
Client->>JSONRPC: POST JSON‑RPC request
JSONRPC->>Translator: parse & translate -> JsonRpcCall
Translator->>Platform: invoke JsonRpcCall
alt needs Drive
Platform->>Drive: Drive RPC (cached)
Drive-->>Platform: Drive response
end
alt needs Core
Platform->>Core: Core RPC (cached)
Core-->>Platform: Core response
end
alt needs Tenderdash
Platform->>Tender: Tenderdash RPC
Tender-->>Platform: Tenderdash response
end
Platform-->>Translator: assemble result
Translator->>JSONRPC: JSON‑RPC response
JSONRPC-->>Client: HTTP response
%%{init: {"themeVariables": {"primaryColor":"#F2C1C1", "noteBackground":"#FFFBE6"}}}%%
sequenceDiagram
participant CoreNode as Dash Core (ZMQ)
participant ZMQ as ZmqListener
participant Bus as SubscriberManager/EventBus
participant Stream as StreamingService
participant Client
CoreNode->>ZMQ: publish raw events (tx/block/locks)
ZMQ->>Bus: broadcast StreamingEvent
Client->>Stream: subscribe(filter)
Stream->>Bus: add_subscription(filter)
Bus-->>Stream: SubscriptionHandle
Bus-->>Client: deliver matching events
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
✅ gRPC Query Coverage Report |
lklimek
force-pushed
the
refactor/dapi-rewrite-to-rust
branch
from
d7a4c79 to
84dc6d6
Compare
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "c71ab82138b16feb0b42bdb3b1997a68263177dc2c66e801690a0c71a40885e2"
)Xcode manual integration:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/rs-dapi/src/clients/tenderdash_client.rs (3)
318-377: Consider per-request timeout for flexibility (optional).The
postmethod correctly uses.json(request)to avoid manual serialization and leverages the global timeout configured in the client builder (lines 390-391). Error handling is comprehensive with appropriate logging levels.However, if certain RPC calls need different timeout behavior, consider adding a per-request timeout parameter:
async fn post<T, R>(&self, request: &R) -> DAPIResult<T>If needed, you could add an optional timeout parameter:
async fn post_with_timeout<T, R>(&self, request: &R, timeout: Option<Duration>) -> DAPIResult<T> where T: serde::de::DeserializeOwned + Debug, R: Serialize + Debug, { let mut req = self.client.post(&self.base_url) .header("Content-Type", "application/json") .json(request); if let Some(t) = timeout { req = req.timeout(t); } // ... rest of the method }This is optional since the global timeout should handle most cases.
454-476: Consider the implications of error masking in net_info.The
net_infomethod returns default values when the RPC call fails, masking technical failures from the caller. While the error is logged, this could hide connectivity or configuration issues from upstream code that might want to handle them differently.Consider one of these approaches:
- Propagate the error and let callers decide whether to use defaults:
-pub async fn net_info(&self) -> DAPIResult<NetInfoResponse> { - match self.net_info_internal().await { - Ok(netinfo) => { - trace!("Successfully retrieved Tenderdash net_info"); - Ok(netinfo) - } - Err(e) => { - error!( - error = ?e, - "Failed to get Tenderdash net_info - technical failure, returning defaults" - ); - Ok(NetInfoResponse::default()) - } - } -} +pub async fn net_info(&self) -> DAPIResult<NetInfoResponse> { + self.net_info_internal().await +}
- Add a separate method for error-tolerant behavior:
pub async fn net_info_or_default(&self) -> NetInfoResponse { self.net_info().await.unwrap_or_default() }If the current behavior (returning defaults on failure) is intentional for specific operational reasons, consider documenting why this design choice was made in the method's doc comment.
379-419: Clarify WebSocket listener spawning location.
TenderdashClient::newonly constructs the client; the retrying WebSocket listener loop is started inPlatformServiceImpl::new(packages/rs-dapi/src/services/platform_service/mod.rs:123).
Optionally validate thatws_uribegins withws://orwss://before creating theTenderdashWebSocketClient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-dapi/src/clients/tenderdash_client.rs(1 hunks)packages/rs-dapi/src/services/platform_service/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-dapi/src/services/platform_service/mod.rs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/clients/tenderdash_client.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/clients/tenderdash_client.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.824Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
📚 Learning: 2025-10-09T15:59:33.162Z
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/rs-dapi/src/clients/tenderdash_client.rs:0-0
Timestamp: 2025-10-09T15:59:33.162Z
Learning: In the rs-dapi codebase, the `Workers::spawn` method (in `packages/rs-dapi/src/sync.rs`) accepts futures that return `Result<O, E>`, not `()`. Its signature is `fn spawn<F, O, E>(&self, fut: F) where F: Future<Output = Result<O, E>> + Send + 'static`. The method handles error logging internally, so spawned tasks should return Results directly without additional error handling wrappers.
Applied to files:
packages/rs-dapi/src/clients/tenderdash_client.rs
📚 Learning: 2025-10-09T15:59:41.943Z
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/rs-dapi/src/clients/tenderdash_client.rs:0-0
Timestamp: 2025-10-09T15:59:41.943Z
Learning: In packages/rs-dapi, the Workers::spawn method accepts futures that return Result<O, E> where E: Into<DapiError>. It handles error logging internally, so spawned tasks should return Result types rather than handling errors inside the task. The Workers implementation automatically logs worker failures.
Applied to files:
packages/rs-dapi/src/clients/tenderdash_client.rs
🧬 Code graph analysis (1)
packages/rs-dapi/src/clients/tenderdash_client.rs (5)
packages/rs-dapi/src/utils.rs (3)
deserialize_string_number_or_null(147-163)deserialize_string_or_number(23-86)generate_jsonrpc_id(12-21)packages/rs-dapi/src/config/mod.rs (7)
std(225-225)default(47-54)default(136-145)default(149-153)default(157-162)default(166-174)default(198-205)packages/rs-dapi/src/clients/tenderdash_websocket.rs (1)
new(78-88)packages/rs-dapi/src/server/mod.rs (1)
new(29-77)packages/rs-dapi/src/error.rs (2)
from_tenderdash_error(170-172)client(201-203)
⏰ 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). (79)
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
🔇 Additional comments (5)
packages/rs-dapi/src/clients/tenderdash_client.rs (5)
1-34: LGTM! Well-documented client structure.The imports are clean, and the
TenderdashClientstruct is properly documented with clear explanations of tracing behavior and error handling architecture. The use ofClientWithMiddlewarefor tracing andOption<Arc<TenderdashWebSocketClient>>for optional WebSocket support is appropriate.
36-85: LGTM! Clean JSON-RPC request/response types.The generic
TenderdashResponse<T>andJsonRpcRequest<T>structures correctly implement JSON-RPC 2.0 protocol. The use of lifetime parameters in parameter structs (e.g.,BroadcastTxParams<'a>) avoids unnecessary allocations.
87-316: LGTM! Comprehensive data models with proper deserialization.The data models correctly handle Tenderdash's varied JSON formats using custom deserializers (
deserialize_string_or_number,deserialize_string_number_or_null). The#[serde(default)]annotations ensure resilience to missing fields. TheExecTxResult::is_empty()helper method is a useful addition for detecting absent results.
478-512: LGTM! Consistent RPC wrapper implementations.All RPC methods (
broadcast_tx,check_tx,unconfirmed_tx,tx) follow a consistent pattern with proper parameter structures and logging. The implementations are clean and straightforward.
513-541: LGTM! Graceful WebSocket subscription handling.The subscription methods (
subscribe_to_transactions,subscribe_to_blocks) handle the absence of a WebSocket client gracefully by returning a receiver that will never receive messages. This allows callers to subscribe without explicitly checking WebSocket availability, and providesis_websocket_connected()for explicit state checking when needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/rs-dapi/src/main.rs (3)
45-50: Consider whether full README is appropriate for CLI help.Including the entire README as
long_aboutmay produce very long help text when users runrs-dapi --help. Typically, CLI tools provide a concise description in the help output and refer users to external documentation for details.Consider either:
- Extracting a shorter "About" section from the README for CLI help
- Creating a separate short description specifically for the CLI
- Keeping only the first paragraph of the README
139-157: Past review comment properly addressed; consider improving error pattern matching.The incorrect
--forceflag references have been successfully removed and replaced with appropriate guidance to check upstream services. Well done!However, Line 145 uses
msg.contains("Failed to connect")which is fragile as it depends on exact error message text. Consider having theDapiError::Clientvariant carry structured error types instead of strings, or add a specificDapiError::ConnectionFailedvariant for connection failures.Example refactor for more robust error handling:
// In error.rs, add a specific variant: ConnectionFailed(String), // Then match more precisely: DapiError::Client(msg) => { error!(error = %msg, "Client initialization failed. Check drive-abci and tenderdash and try again."); return Err(format!("Client error: {}", e)); } DapiError::ConnectionFailed(msg) => { error!(error = %msg, "Client connection failed. Check drive-abci and tenderdash and try again."); return Err(format!("Connection error: {}", e)); }
264-281: Consider making worker thread count configurable.The main function correctly initializes the Tokio runtime and handles CLI execution. However, the worker thread count is hardcoded to 4 (line 267). For a server application, you might want to make this configurable or use Tokio's default (which matches available CPU cores).
Consider one of these approaches:
- Use
.worker_threads(num_cpus::get())to match CPU count- Add a CLI flag or environment variable to configure thread count
- Remove the
.worker_threads(4)call to use Tokio's default behaviorOtherwise, the structure is clean and follows Rust CLI best practices.
Example to use CPU count:
fn main() -> Result<(), ExitCode> { let rt = tokio::runtime::Builder::new_multi_thread() - .worker_threads(4) + .worker_threads(num_cpus::get()) .enable_all() .build() .expect("Failed to create Tokio runtime");Or simply use Tokio's default:
fn main() -> Result<(), ExitCode> { let rt = tokio::runtime::Builder::new_multi_thread() - .worker_threads(4) .enable_all() .build() .expect("Failed to create Tokio runtime");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-dapi/src/main.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/main.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/main.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
🧬 Code graph analysis (1)
packages/rs-dapi/src/main.rs (5)
packages/rs-dapi/src/config/mod.rs (2)
std(225-225)load_from_dotenv(224-226)packages/rs-dapi/src/logging/mod.rs (1)
init_logging(18-43)packages/rs-dapi/src/error.rs (1)
server(236-238)packages/rs-dapi/src/server/mod.rs (2)
run(82-109)new(29-77)packages/rs-dapi/src/logging/access_log.rs (1)
new(261-272)
⏰ 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). (170)
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
🔇 Additional comments (5)
packages/rs-dapi/src/main.rs (5)
1-10: LGTM!Imports are well-organized and all necessary dependencies are present.
12-39: LGTM!The Commands enum is well-documented with clear descriptions. The warning about sensitive data in the Config subcommand is particularly important for security awareness.
176-199: LGTM!Both helper functions follow standard CLI patterns. Using
process::exit(1)for configuration loading failures is appropriate for command-line applications, and the logging configuration logic is clean.
201-238: LGTM!These helper functions are well-implemented:
run_servercorrectly wraps the config in an Arc for sharing across componentsdump_configappropriately warns about sensitive data before outputtingprint_versionprovides clear version information
240-262: LGTM!The
shutdown_signalfunction correctly implements cross-platform graceful shutdown handling using tokio signals. The platform-specific implementations for Unix (SIGTERM/SIGINT) and non-Unix (Ctrl+C) are standard best practices.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs (2)
42-54: Deserialization fallback issue persists.The fallback behavior flagged in a previous review has not been addressed. When transaction deserialization fails, the code falls back to
bloom.contains(raw_tx)(line 48), which:
- Silently degrades behavior and masks encoding/decoding bugs
- Uses semantically incorrect matching (raw bytes instead of structured transaction data)
The previous review recommended returning
falsefor malformed transactions or propagating errors. This issue remains unresolved.Apply this diff to return
falseon deserialization errors:Err(e) => { - debug!( + tracing::error!( error = %e, - "Failed to deserialize core transaction for bloom filter matching, falling back to contains()" + "Failed to deserialize core transaction for bloom filter matching" ); - match bloom.read() { - Ok(guard) => guard.contains(raw_tx), - Err(_) => { - debug!("Failed to acquire read lock for bloom filter"); - false - } - } + // Malformed transactions should not match + false }
246-325: Failing test will break CI.The test comment states "expected to FAIL with the current implementation" (lines 248-249), but the test lacks
#[ignore]or#[should_panic]attributes. This will cause CI failures.A previous review recommended adding
#[ignore]with a TODO comment. This issue remains unresolved.Apply this diff to prevent CI breakage:
#[tokio::test] +#[ignore] // TODO: Enable once filter persistence is implemented (track in issue) async fn test_bloom_update_persistence_across_messages() {packages/rs-dapi/src/cache.rs (1)
237-261: Fix compile error: must wrap return value inSome().At line 253, the function returns
value(typeT) but the signature specifiesOption<T>. This will not compile.Apply this diff:
if inserted_at.elapsed() <= ttl { metrics::cache_hit(self.label.as_ref(), &method_label); - return value; + return Some(value); }Note: This issue was previously flagged and marked as addressed in commits 7ec8fad to 11a8f1a, but appears to have been reintroduced or not fully applied.
🧹 Nitpick comments (5)
packages/platform-test-suite/lib/wait.js (1)
6-6: Consider conditional logging to reduce verbosity.The
console.debugcall executes unconditionally for every wait, even when no description is provided (where it just logs the duration). Whileconsole.debugis typically filtered in CI/production, you might consider only logging when a description is explicitly provided to reduce noise:- console.debug(`Waiting for ${details}`); + if (description) { + console.debug(`Waiting for ${details}`); + }This would make descriptive waits stand out more clearly in test logs.
packages/rs-dapi/src/server/metrics.rs (4)
166-173: Consider logging errors from the response builder.The
unwrap_or_elsefallback at line 172 silently returns an empty body if the response builder fails. For observability, consider logging the error before returning the fallback response.Apply this diff:
async fn handle_metrics() -> axum::response::Response { let (body, content_type) = crate::metrics::gather_prometheus(); axum::response::Response::builder() .status(200) .header(axum::http::header::CONTENT_TYPE, content_type) .body(axum::body::Body::from(body)) - .unwrap_or_else(|_| axum::response::Response::new(axum::body::Body::from(""))) + .unwrap_or_else(|e| { + tracing::error!("Failed to build metrics response: {}", e); + axum::response::Response::new(axum::body::Body::from("")) + }) }
221-252: Consider adding explicit handling forFailedPrecondition.The
DapiError::FailedPreconditionvariant exists (seeerror.rs:311in theFrom<dashcore_rpc::Error>implementation) but falls through to the catch-all"internal error"label. For more accurate health reporting, consider adding an explicit case.Apply this diff:
match err { Configuration(_) => "configuration error", StreamingService(_) => "streaming service error", Client(_) | ClientGone(_) => "client error", ServerUnavailable(_, _) | Unavailable(_) | ServiceUnavailable(_) => "service unavailable", Server(_) => "server error", Serialization(_) | InvalidData(_) | NoValidTxProof(_) => "invalid data", Transport(_) | Http(_) | WebSocket(_) | Request(_) => "transport error", TenderdashClientError(_) => "tenderdash error", Status(_) => "upstream returned error", TaskJoin(_) => "internal task error", Io(_) => "io error", UrlParse(_) => "invalid url", Base64Decode(_) => "invalid base64 data", TransactionHashNotFound => "transaction hash missing", NotFound(_) => "not found", AlreadyExists(_) => "already exists", InvalidRequest(_) => "invalid request", InvalidArgument(_) => "invalid argument", ResourceExhausted(_) => "resource exhausted", + FailedPrecondition(_) => "failed precondition", Aborted(_) => "aborted", Timeout(_) => "timeout", Internal(_) => "internal error", ConnectionClosed => "connection closed", MethodNotFound(_) => "method not found", ZmqConnection(_) => "zmq connection error", _ => "internal error", }
47-47: Optionally make the health check timeout configurable.The 3-second timeout is hardcoded. Depending on deployment environments (local dev vs. production with network latency), this duration might need adjustment. Consider adding a configuration option.
154-154: Optionally use ISO 8601 timestamp for better readability.Unix timestamps (i64) are compact but less human-readable in JSON responses. Consider using an ISO 8601 string (e.g.,
chrono::Utc::now().to_rfc3339()) for easier debugging and log correlation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/platform-test-suite/lib/test/waitForBalanceToChange.js(1 hunks)packages/platform-test-suite/lib/wait.js(1 hunks)packages/platform-test-suite/lib/waitForBlocks.js(1 hunks)packages/platform-test-suite/lib/waitForSTPropagated.js(1 hunks)packages/platform-test-suite/test/e2e/withdrawals.spec.js(2 hunks)packages/platform-test-suite/test/functional/core/getTransaction.spec.js(2 hunks)packages/platform-test-suite/test/functional/dapi/subscribeToBlockHeadersWithChainLocksHandlerFactory.spec.js(3 hunks)packages/rs-dapi/src/cache.rs(1 hunks)packages/rs-dapi/src/clients/drive_client.rs(1 hunks)packages/rs-dapi/src/server/metrics.rs(1 hunks)packages/rs-dapi/src/services/platform_service/get_status.rs(1 hunks)packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-dapi/src/services/platform_service/get_status.rs
🧰 Additional context used
📓 Path-based instructions (4)
packages/platform-test-suite/**
📄 CodeRabbit inference engine (AGENTS.md)
packages/platform-test-suite/**: Keep end-to-end tests and helpers in packages/platform-test-suite
Keep all E2E tests exclusively in packages/platform-test-suite
Files:
packages/platform-test-suite/lib/waitForBlocks.jspackages/platform-test-suite/lib/waitForSTPropagated.jspackages/platform-test-suite/test/functional/dapi/subscribeToBlockHeadersWithChainLocksHandlerFactory.spec.jspackages/platform-test-suite/test/e2e/withdrawals.spec.jspackages/platform-test-suite/test/functional/core/getTransaction.spec.jspackages/platform-test-suite/lib/wait.jspackages/platform-test-suite/lib/test/waitForBalanceToChange.js
packages/**/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/**/*.{js,ts,jsx,tsx}: Adhere to ESLint with Airbnb/TypeScript configs for JS/TS code
Use camelCase for JS/TS variables and functions
Use PascalCase for JS/TS classes
Prefer kebab-case filenames within JS packages
Files:
packages/platform-test-suite/lib/waitForBlocks.jspackages/platform-test-suite/lib/waitForSTPropagated.jspackages/platform-test-suite/test/functional/dapi/subscribeToBlockHeadersWithChainLocksHandlerFactory.spec.jspackages/platform-test-suite/test/e2e/withdrawals.spec.jspackages/platform-test-suite/test/functional/core/getTransaction.spec.jspackages/platform-test-suite/lib/wait.jspackages/platform-test-suite/lib/test/waitForBalanceToChange.js
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/server/metrics.rspackages/rs-dapi/src/services/streaming_service/subscriber_manager.rspackages/rs-dapi/src/cache.rspackages/rs-dapi/src/clients/drive_client.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/server/metrics.rspackages/rs-dapi/src/services/streaming_service/subscriber_manager.rspackages/rs-dapi/src/cache.rspackages/rs-dapi/src/clients/drive_client.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
🧬 Code graph analysis (7)
packages/platform-test-suite/test/functional/dapi/subscribeToBlockHeadersWithChainLocksHandlerFactory.spec.js (5)
packages/platform-test-suite/lib/test/waitForBalanceToChange.js (1)
wait(1-1)packages/platform-test-suite/lib/waitForBlocks.js (1)
wait(1-1)packages/platform-test-suite/lib/waitForSTPropagated.js (1)
wait(1-1)packages/platform-test-suite/test/e2e/withdrawals.spec.js (2)
wait(3-3)require(1-1)packages/platform-test-suite/test/functional/core/getTransaction.spec.js (1)
wait(3-3)
packages/rs-dapi/src/server/metrics.rs (6)
packages/rs-dapi/src/logging/middleware.rs (1)
new(27-29)packages/rs-dapi/src/metrics.rs (2)
new(400-402)gather_prometheus(383-392)packages/rs-dapi/src/error.rs (5)
timeout(251-253)from(128-130)from(135-137)from(142-144)from(302-333)packages/rs-dapi/src/server/mod.rs (1)
new(29-77)packages/rs-dapi/src/services/core_service.rs (1)
new(37-47)packages/rs-dapi/src/services/platform_service/get_status.rs (1)
is_healthy(42-44)
packages/platform-test-suite/test/e2e/withdrawals.spec.js (5)
packages/platform-test-suite/lib/test/waitForBalanceToChange.js (1)
wait(1-1)packages/platform-test-suite/lib/waitForBlocks.js (1)
wait(1-1)packages/platform-test-suite/lib/waitForSTPropagated.js (1)
wait(1-1)packages/platform-test-suite/test/functional/core/getTransaction.spec.js (1)
wait(3-3)packages/platform-test-suite/test/functional/dapi/subscribeToBlockHeadersWithChainLocksHandlerFactory.spec.js (1)
wait(16-16)
packages/platform-test-suite/lib/wait.js (6)
packages/platform-test-suite/lib/test/waitForBalanceToChange.js (1)
wait(1-1)packages/platform-test-suite/lib/waitForBlocks.js (1)
wait(1-1)packages/platform-test-suite/lib/waitForSTPropagated.js (1)
wait(1-1)packages/platform-test-suite/test/e2e/withdrawals.spec.js (1)
wait(3-3)packages/platform-test-suite/test/functional/core/getTransaction.spec.js (1)
wait(3-3)packages/platform-test-suite/test/functional/dapi/subscribeToBlockHeadersWithChainLocksHandlerFactory.spec.js (1)
wait(16-16)
packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs (3)
packages/rs-dapi/src/services/streaming_service/mod.rs (1)
summarize_streaming_event(87-129)packages/rs-dapi/src/services/streaming_service/bloom.rs (1)
matches_transaction(55-101)packages/rs-dash-event-bus/src/event_bus.rs (4)
id(234-236)matches(16-16)matches(429-431)subscription_count(185-187)
packages/rs-dapi/src/cache.rs (4)
packages/rs-dapi/src/sync.rs (3)
fmt(61-64)new(23-27)default(112-114)packages/rs-dapi/src/metrics.rs (7)
fmt(39-41)new(400-402)cache_hit(333-335)cache_miss(338-340)cache_memory_usage_bytes(348-352)cache_memory_capacity_bytes(355-359)cache_entries(362-366)packages/rs-dapi/src/services/platform_service/mod.rs (1)
new(110-166)packages/rs-dapi/src/clients/core_client.rs (6)
new(27-40)new(465-469)cache(135-137)cache(182-184)cache(245-247)cache(324-326)
packages/rs-dapi/src/clients/drive_client.rs (4)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetStatusRequest(5482-5497)Version(5599-5618)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (1)
PlatformClient(509-936)packages/rs-dapi/src/services/platform_service/mod.rs (1)
new(110-166)packages/rs-dapi/src/services/streaming_service/mod.rs (1)
new(168-187)
⏰ 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). (168)
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (dpns-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Unused dependencies
- GitHub Check: Rust packages (dpns-contract) / Formatting
- GitHub Check: Rust packages (dpns-contract) / Linting
- GitHub Check: Rust packages (wallet-utils-contract) / Tests
- GitHub Check: Rust packages (wallet-utils-contract) / Unused dependencies
- GitHub Check: Rust packages (wallet-utils-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
🔇 Additional comments (22)
packages/platform-test-suite/lib/test/waitForBalanceToChange.js (1)
18-23: LGTM!The enhanced wait message with attempt counter provides clear diagnostic context for balance change polling. The 1-indexed attempt counter is user-friendly.
packages/platform-test-suite/lib/waitForSTPropagated.js (1)
13-13: LGTM!The descriptive message clearly indicates the purpose of this wait period.
packages/platform-test-suite/test/functional/core/getTransaction.spec.js (1)
31-31: LGTM!The descriptive wait messages enhance test observability. Including the transaction ID in the propagation message is particularly helpful for debugging.
Also applies to: 40-40
packages/platform-test-suite/test/e2e/withdrawals.spec.js (1)
3-3: LGTM!The local import path is consistent with other test files, and the descriptive wait message provides clear context for the polling loop.
Also applies to: 258-258
packages/platform-test-suite/lib/waitForBlocks.js (1)
13-14: LGTM!The attempt counter and descriptive wait message provide excellent diagnostic context for block height polling, clearly showing progress toward the target height.
Also applies to: 19-23
packages/platform-test-suite/test/functional/dapi/subscribeToBlockHeadersWithChainLocksHandlerFactory.spec.js (3)
16-16: LGTM!The import path change aligns with the standardized wait module usage across the test suite.
129-129: LGTM!The iteration counter and descriptive wait message improve observability for the historical block header streaming test.
Also applies to: 135-140
222-222: LGTM!The iteration counter and descriptive wait message provide clear diagnostic context for tracking fresh block delivery and chain lock reception.
Also applies to: 228-233
packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs (2)
77-79: Verify intentional forwarding of all blocks/locks to bloom filter subscribers.Bloom filter subscribers receive all
CoreRawBlock,CoreInstantLock, andCoreChainLockevents without filtering (always returntrue), while onlyCoreRawTransactionevents are filtered viamatches_core_transaction.Is this intentional? If subscribers need full chain context (blocks, locks), this is correct. Otherwise, consider whether these events should be filtered or use separate subscription types.
204-244: Test validates problematic fallback behavior.This test explicitly validates the
contains(raw_tx)fallback that was flagged as semantically incorrect in previous reviews. The test name and implementation suggest this fallback is intentional, but it conflicts with the bloom filter's design (matching structured transaction data, not raw bytes).If the fallback behavior is removed per the previous review recommendation, this test should also be removed or updated to verify the new behavior (returning
falsefor non-deserializable data).packages/rs-dapi/src/server/metrics.rs (2)
16-42: LGTM! Metrics server startup is well-structured.The early return when metrics are disabled, clear logging, optional access-log layer, and clean Axum router setup all follow best practices. The past review concern about error leakage on
/healthhas been addressed by thehealth_error_labelredaction function used in the health handler.
46-163: Health check implementation is sound.The parallel execution with timeout, comprehensive error handling, and aggregated status logic (ok/degraded/error) are all correct. The use of
health_error_labelensures that internal error details are properly redacted before being exposed over HTTP, addressing the previous security concern.packages/rs-dapi/src/cache.rs (10)
1-14: LGTM!Imports and constants are appropriate for the LRU cache implementation.
16-23: LGTM!The LruResponseCache struct design is sound: wrapping the underlying cache in Arc enables cheap cloning, the label supports metrics, and the Workers handle background invalidation. The Debug impl provides useful diagnostics.
Also applies to: 27-45
47-75: LGTM!The CacheKey design elegantly handles serialization failures by making the digest optional, causing all lookups to miss when key serialization fails. This prevents panics while maintaining type safety.
76-117: LGTM!The CachedValue implementation properly handles both serialization and deserialization errors without panics. The switch from bincode to rmp_serde is well-documented and addresses a real bug encountered during testing.
119-129: LGTM!The weighter correctly accounts for both structural overhead and payload size, with a sensible minimum weight of 1 byte.
131-189: LGTM!The dual constructor design is appropriate:
with_capacityfor immutable data (e.g., blocks by hash) andnewwith background invalidation for mutable data. The background worker pattern is clean and the memory observation is consistently applied.
191-235: LGTM!The
get_and_parsehelper correctly treats deserialization failures as cache misses and removes corrupted entries. Thegetmethod properly records hit/miss metrics.
263-298: LGTM!Both
removeandputhandle edge cases gracefully:removereturns a clear success indicator andputskips caching when serialization fails, avoiding panics while logging the issue.
300-375: LGTM!The
get_or_try_insertimplementation correctly handles all error paths without panics: serialization failures return errors, deserialization failures remove corrupted entries and propagate errors, and all paths record appropriate metrics. The async design prevents the producer from executing on cache hits.
377-631: LGTM!The
observe_memoryhelper consistently tracks cache metrics, andmake_cache_keyproduces stable digests while handling serialization failures gracefully. The test suite is comprehensive, covering all cache methods, error handling, TTL expiration, and specifically validates the workaround for the bincode deserialization bug.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/rs-dapi/src/clients/drive_client.rs (1)
171-174: Clone the request instead of dereferencing itLine 173 dereferences
&GetStatusRequest, attempting to move a non-Copytype and breaking compilation. Clone the payload (or wrap it intonic::Request) before callingget_status.- let drive_response = self.get_client().get_status(*request).await?.into_inner(); + let drive_response = self + .get_client() + .get_status(request.clone()) + .await? + .into_inner();packages/rs-dapi/src/server/grpc.rs (3)
105-111: Hardcoded streaming methods list remains fragile.This was flagged in a previous review but not yet addressed. Maintaining a hardcoded list is error-prone—new streaming methods won't receive the correct timeout unless manually updated here.
Consider using a naming convention to detect streaming methods dynamically:
- // All known streaming methods in Core service (all use "stream" return type) - const STREAMING_METHODS: &[&str] = &[ - "/org.dash.platform.dapi.v0.Core/subscribeToBlockHeadersWithChainLocks", - "/org.dash.platform.dapi.v0.Core/subscribeToTransactionsWithProofs", - "/org.dash.platform.dapi.v0.Core/subscribeToMasternodeList", - "/org.dash.platform.dapi.v0.Platform/waitForStateTransitionResult", - "/org.dash.platform.dapi.v0.Platform/subscribePlatformEvents", - ]; - - // Check if this is a known streaming method - if STREAMING_METHODS.contains(&path) { + // Streaming methods follow naming convention: subscribe* or waitFor* + if path.contains("/subscribe") || path.contains("/waitFor") {
115-117: Fix tracing field to log the path value.
tracing::trace!(path, ...)records a boolean field namedpathrather than logging the actual path string.Apply this diff:
tracing::trace!( - path, + path = %path, "Detected streaming gRPC method, applying streaming timeout" );
171-182: Fix tracing fields to log path values.Lines 172 and 179 use bare
pathidentifiers, which record boolean fields instead of logging the actual path strings.Apply this diff:
if timeout_from_header.is_some() { trace!( - path, + path = %path, header_timeout = timeout_from_header.unwrap_or_default().as_secs_f32(), timeout = effective_timeout.as_secs_f32(), "Applying gRPC timeout from header" ); } else { tracing::trace!( - path, + path = %path, timeout = effective_timeout.as_secs_f32(), "Applying default gRPC timeout" );
🧹 Nitpick comments (2)
packages/rs-dapi/src/clients/tenderdash_client.rs (1)
328-337: Consider eliminating double serialization.The request is serialized twice: once at line 328 for tracing and again at line 337 for the actual HTTP request. Since
.json()performs serialization internally, the first serialization is redundant.You could eliminate the separate
request_valueand trace the request object directly, or serialize once and reuse:- let request_value = serde_json::to_value(request).map_err(|e| { - error!("Failed to serialize Tenderdash request: {}", e); - DapiError::Client(format!("Failed to serialize request: {}", e)) - })?; - let response_body = self .client .post(&self.base_url) .header("Content-Type", "application/json") .json(request) .send() .awaitThen update the tracing to use the request object directly:
tracing::trace!( elapsed = ?start.elapsed(), - request = ?request_value, + request = ?request, response = ?response, "tenderdash_client request executed");This avoids the overhead of serializing to
Valuejust for tracing.packages/rs-dapi/src/server/grpc.rs (1)
164-168: Log warning for client deadline shorter than safety marginThe
grpc-timeoutheader may be less thanGRPC_REQUEST_TIME_SAFETY_MARGIN, causingchecked_subto returnNoneand fall back to a much larger default timeout—potentially exceeding the client’s deadline. Add a warning when this occurs:let timeout_from_header = parse_grpc_timeout_header(req.headers()); +if let Some(raw_deadline) = timeout_from_header { + if raw_deadline < GRPC_REQUEST_TIME_SAFETY_MARGIN { + warn!( + method = %path, + client_deadline_ms = raw_deadline.as_millis(), + safety_margin_ms = GRPC_REQUEST_TIME_SAFETY_MARGIN.as_millis(), + "Client deadline shorter than safety margin, using default timeout" + ); + } +} let effective_timeout = timeout_from_header .and_then(|d| d.checked_sub(GRPC_REQUEST_TIME_SAFETY_MARGIN)) .unwrap_or(default_timeout) .min(default_timeout);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/rs-dapi/src/clients/drive_client.rs(1 hunks)packages/rs-dapi/src/clients/tenderdash_client.rs(1 hunks)packages/rs-dapi/src/server/grpc.rs(1 hunks)packages/rs-dapi/src/server/metrics.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/clients/drive_client.rspackages/rs-dapi/src/clients/tenderdash_client.rspackages/rs-dapi/src/server/metrics.rspackages/rs-dapi/src/server/grpc.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/clients/drive_client.rspackages/rs-dapi/src/clients/tenderdash_client.rspackages/rs-dapi/src/server/metrics.rspackages/rs-dapi/src/server/grpc.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
📚 Learning: 2025-10-09T15:59:33.162Z
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/rs-dapi/src/clients/tenderdash_client.rs:0-0
Timestamp: 2025-10-09T15:59:33.162Z
Learning: In the rs-dapi codebase, the `Workers::spawn` method (in `packages/rs-dapi/src/sync.rs`) accepts futures that return `Result<O, E>`, not `()`. Its signature is `fn spawn<F, O, E>(&self, fut: F) where F: Future<Output = Result<O, E>> + Send + 'static`. The method handles error logging internally, so spawned tasks should return Results directly without additional error handling wrappers.
Applied to files:
packages/rs-dapi/src/clients/tenderdash_client.rs
📚 Learning: 2025-10-09T15:59:41.943Z
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/rs-dapi/src/clients/tenderdash_client.rs:0-0
Timestamp: 2025-10-09T15:59:41.943Z
Learning: In packages/rs-dapi, the Workers::spawn method accepts futures that return Result<O, E> where E: Into<DapiError>. It handles error logging internally, so spawned tasks should return Result types rather than handling errors inside the task. The Workers implementation automatically logs worker failures.
Applied to files:
packages/rs-dapi/src/clients/tenderdash_client.rs
🧬 Code graph analysis (4)
packages/rs-dapi/src/clients/drive_client.rs (6)
packages/dapi-grpc/clients/drive/v0/web/drive_pb_service.d.ts (1)
DriveInternalClient(49-62)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetStatusRequest(5482-5497)Version(5599-5618)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (1)
PlatformClient(509-936)packages/rs-dapi/src/clients/tenderdash_client.rs (2)
new(53-60)new(383-422)packages/rs-dapi/src/services/platform_service/mod.rs (1)
new(110-166)packages/rs-dapi/src/services/streaming_service/mod.rs (1)
new(168-187)
packages/rs-dapi/src/clients/tenderdash_client.rs (4)
packages/rs-dapi/src/utils.rs (3)
deserialize_string_number_or_null(147-163)deserialize_string_or_number(23-86)generate_jsonrpc_id(12-21)packages/rs-dapi/src/clients/tenderdash_websocket.rs (2)
new(78-88)test_connection(106-125)packages/rs-dapi/src/server/mod.rs (1)
new(29-77)packages/rs-dapi/src/error.rs (2)
from_tenderdash_error(170-172)client(201-203)
packages/rs-dapi/src/server/metrics.rs (6)
packages/rs-dapi/src/logging/middleware.rs (1)
new(27-29)packages/rs-dapi/src/metrics.rs (2)
new(400-402)gather_prometheus(383-392)packages/rs-dapi/src/error.rs (4)
from(128-130)from(135-137)from(142-144)from(302-333)packages/rs-dapi/src/services/platform_service/mod.rs (1)
new(110-166)packages/rs-dapi/src/server/mod.rs (1)
new(29-77)packages/rs-dapi/src/services/platform_service/get_status.rs (1)
is_healthy(42-44)
packages/rs-dapi/src/server/grpc.rs (4)
packages/rs-dapi/src/logging/middleware.rs (7)
layer(35-41)new(27-29)poll_ready(62-65)call(68-189)req(292-292)req(296-296)as_str(309-309)packages/rs-dapi/src/metrics.rs (7)
layer(413-415)new(400-402)poll_ready(430-432)call(434-498)req(438-438)as_str(33-35)as_str(144-149)packages/rs-dapi/src/server/mod.rs (1)
new(29-77)packages/rs-dapi/src/logging/access_log.rs (1)
new(261-272)
⏰ 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). (167)
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
🔇 Additional comments (14)
packages/rs-dapi/src/clients/tenderdash_client.rs (6)
1-34: LGTM!The imports are well-organized, and the struct documentation clearly explains the tracing behavior and error handling architecture. The client design with optional WebSocket support is sound.
36-316: LGTM!The JSON-RPC request/response structures and data models are well-designed:
- Proper use of custom deserializers (
deserialize_string_or_number,deserialize_string_number_or_null) to handle JSON-RPC type variations- Comprehensive
#[serde(default)]attributes for resilience- The
ExecTxResult::is_empty()helper is a useful addition for detecting absent results- Clear type aliases improve readability
379-447: LGTM!The constructor properly addresses past review concerns:
- HTTP client timeouts are configured (connect_timeout and request timeout)
- TracingMiddleware is correctly wired into the client builder
- Both HTTP and WebSocket connections are validated before returning
- Comprehensive error handling and logging throughout
The defensive validation approach (testing both HTTP and WebSocket connectivity) ensures the client is fully operational before use.
481-515: LGTM!The RPC wrapper methods (
broadcast_tx,check_tx,unconfirmed_tx,tx) are cleanly implemented with appropriate parameter structures and proper delegation to the genericpostmethod.
537-544: LGTM!The
is_websocket_connectedhelper provides a clean way to check WebSocket connectivity status.
457-472: Fallback behavior documented; no changes needed Thenet_infodoc comment explicitly states it falls back to defaults on transport errors, confirming the error-masking is intentional.packages/rs-dapi/src/server/grpc.rs (3)
26-82: LGTM: Well-structured server initialization.The unified gRPC server setup correctly configures timeouts, layers, message limits, and service registration. The layer execution order is properly documented.
205-223: LGTM: Correct RFC 8681 timeout parsing.The header parsing correctly handles all gRPC timeout units with appropriate overflow protection via
saturating_mul.
252-275: LGTM: Comprehensive timeout test coverage.The test correctly validates that timeouts produce
DeadlineExceededstatus with the timeout duration in the message.packages/rs-dapi/src/server/metrics.rs (5)
16-42: LGTM: Clean metrics server initialization.The method properly handles disabled metrics with early return, sets up routes, and optionally applies access logging.
123-181: LGTM: Correct overall health status calculation.The core RPC health check properly redacts errors, and the overall status logic appropriately categorizes health based on failure count (0=ok, 1=degraded, 2+=error).
184-191: LGTM: Straightforward Prometheus metrics endpoint.The metrics handler correctly gathers and formats Prometheus metrics with appropriate content-type headers.
193-272: LGTM: Well-structured health response models and error redaction.The data structures properly use serde annotations for JSON formatting, and
health_error_labelprovides comprehensive redaction of internal error details.
274-301: LGTM: ComponentCheck helpers are correctly implemented.The
from_optionandfrom_boolhelpers appropriately convert status values to ComponentCheck structs. Note thatfrom_optionpasses error strings through unchanged, so callers must ensure proper redaction (see comment on lines 76-86).
Comment on lines
516
to
535
| /// Subscribe to streaming Tenderdash transaction events if WebSocket is available. | ||
| pub fn subscribe_to_transactions(&self) -> broadcast::Receiver<TransactionEvent> { | ||
| if let Some(ws_client) = &self.websocket_client { | ||
| ws_client.subscribe() | ||
| } else { | ||
| // Return a receiver that will never receive messages | ||
| let (_, rx) = broadcast::channel(1); | ||
| rx | ||
| } | ||
| } | ||
| /// Subscribe to block events from Tenderdash via WebSocket. | ||
| pub fn subscribe_to_blocks(&self) -> broadcast::Receiver<BlockEvent> { | ||
| if let Some(ws_client) = &self.websocket_client { | ||
| ws_client.subscribe_blocks() | ||
| } else { | ||
| // Return a receiver that will never receive messages | ||
| let (_, rx) = broadcast::channel(1); | ||
| rx | ||
| } | ||
| } |
There was a problem hiding this comment.
Document the fallback behavior for subscription methods.
Both subscription methods return a dead channel (via broadcast::channel(1)) when WebSocket is unavailable, which could be surprising to callers. While the is_websocket_connected helper exists, the subscription methods don't document this fallback behavior.
Add doc comments to clarify the behavior:
+ /// Subscribe to streaming Tenderdash transaction events if WebSocket is available.
+ ///
+ /// Returns a broadcast receiver for transaction events. If WebSocket is not connected,
+ /// returns a receiver that will never produce events. Check `is_websocket_connected()`
+ /// to verify connectivity before subscribing.
pub fn subscribe_to_transactions(&self) -> broadcast::Receiver<TransactionEvent> {+ /// Subscribe to block events from Tenderdash via WebSocket.
+ ///
+ /// Returns a broadcast receiver for block events. If WebSocket is not connected,
+ /// returns a receiver that will never produce events. Check `is_websocket_connected()`
+ /// to verify connectivity before subscribing.
pub fn subscribe_to_blocks(&self) -> broadcast::Receiver<BlockEvent> {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/rs-dapi/src/clients/tenderdash_client.rs around lines 516 to 535,
the two subscription methods return a dead broadcast receiver (created with
broadcast::channel(1)) when the websocket_client is None but do not document
this fallback; update the doc comments for subscribe_to_transactions and
subscribe_to_blocks to clearly state that when WebSocket is unavailable the
method returns a receiver that will never receive messages, mention the
existence of is_websocket_connected so callers can check connection state
beforehand, and add a brief usage note advising callers to either check
is_websocket_connected or handle the no-message case (e.g., timeouts or
alternative flows).
Comment on lines
+76
to
+86
| drive: Some(ComponentCheck::from_option(health.drive_error.clone())), | ||
| tenderdash_websocket: Some(ComponentCheck::from_bool( | ||
| websocket_connected, | ||
| "disconnected", | ||
| )), | ||
| tenderdash_status: Some(ComponentCheck::from_option( | ||
| health.tenderdash_status_error.clone(), | ||
| )), | ||
| tenderdash_net_info: Some(ComponentCheck::from_option( | ||
| health.tenderdash_netinfo_error.clone(), | ||
| )), |
There was a problem hiding this comment.
Redact component-level error messages to prevent info leakage.
While top-level errors are redacted via health_error_label (line 96), component-level errors (drive_error, tenderdash_status_error, tenderdash_netinfo_error) are passed directly to ComponentCheck::from_option without redaction. This could leak internal error details through the public health endpoint.
Apply health_error_label to component errors as well:
drive: Some(ComponentCheck::from_option(health.drive_error.clone())),
+ drive: Some(ComponentCheck::from_option(
+ health.drive_error.as_ref().map(|_| health_error_label(
+ &DapiError::Client(health.drive_error.clone().unwrap_or_default())
+ ).to_string())
+ )),
tenderdash_websocket: Some(ComponentCheck::from_bool(
websocket_connected,
"disconnected",
)),
- tenderdash_status: Some(ComponentCheck::from_option(
- health.tenderdash_status_error.clone(),
- )),
- tenderdash_net_info: Some(ComponentCheck::from_option(
- health.tenderdash_netinfo_error.clone(),
- )),
+ tenderdash_status: Some(ComponentCheck::from_option(
+ health.tenderdash_status_error.as_ref().map(|_| "tenderdash error".to_string())
+ )),
+ tenderdash_net_info: Some(ComponentCheck::from_option(
+ health.tenderdash_netinfo_error.as_ref().map(|_| "tenderdash error".to_string())
+ )),Alternatively, extend ComponentCheck::from_option to accept a redaction function or create a from_option_redacted variant.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/rs-dapi/src/server/metrics.rs around lines 76 to 86, component-level
error fields (drive_error, tenderdash_status_error, tenderdash_netinfo_error)
are passed into ComponentCheck::from_option unredacted and may leak internal
error details; update these calls to wrap the error option with the existing
health_error_label redaction before calling ComponentCheck::from_option (or, if
preferred, add a ComponentCheck::from_option_redacted that applies
health_error_label internally and use that) so the component error strings
exposed by the health endpoint are redacted.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/rs-dapi/src/sync.rs (1)
1-12: Consider adding module-level documentation.While the code is well-commented, a module-level doc comment explaining the worker pool architecture (command pattern, manager task, lazy AbortHandle resolution) would help future maintainers quickly understand the design.
Example:
+//! Async worker pool for managing background tasks. +//! +//! The pool uses a command pattern: `Workers` handles send spawn requests +//! to a `WorkerManager` task that owns a `JoinSet`. The manager continuously +//! drains completed tasks and returns `AbortHandle`s to callers via oneshot channels. + use std::fmt::Debug;packages/rs-dapi/src/clients/tenderdash_client.rs (2)
458-472: Consider documenting net_info's error-masking behavior.Unlike other RPC methods,
net_info()catches transport errors and returns default values instead of propagating them. While this may be intentional (preventing network peer stats from blocking other operations), it silently masks failures that callers might want to know about.Consider:
- Adding a doc comment explaining why errors are masked
- Or exposing both
net_info()(with masking) andnet_info_internal()(without masking) publiclyApply this diff to document the behavior:
+ /// Retrieve network peer statistics, falling back to defaults on transport errors. + /// + /// This method masks technical failures (network errors, timeouts) by returning + /// default values, ensuring peer stats don't block other operations. For strict + /// error handling, use `net_info_internal()`. pub async fn net_info(&self) -> DAPIResult<NetInfoResponse> {
516-523: Clarify subscription behavior when WebSocket is unavailable.The doc comments mention "if WebSocket is available" but don't explain what happens when it's not. Based on past review comments, these methods return a broadcast receiver that won't produce events when WebSocket is disconnected.
Consider enhancing the documentation:
Apply this diff:
- /// Subscribe to streaming Tenderdash transaction events if WebSocket is available. + /// Subscribe to streaming Tenderdash transaction events. + /// + /// Returns a broadcast receiver for transaction events. When WebSocket is not connected, + /// the receiver will not produce events. Use `is_websocket_connected()` to check connectivity. pub fn subscribe_to_transactions(&self) -> broadcast::Receiver<TransactionEvent> { self.websocket_client.subscribe() } - /// Subscribe to block events from Tenderdash via WebSocket. + /// Subscribe to block events from Tenderdash via WebSocket. + /// + /// Returns a broadcast receiver for block events. When WebSocket is not connected, + /// the receiver will not produce events. Use `is_websocket_connected()` to check connectivity. pub fn subscribe_to_blocks(&self) -> broadcast::Receiver<BlockEvent> { self.websocket_client.subscribe_blocks() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/rs-dapi/src/clients/tenderdash_client.rs(1 hunks)packages/rs-dapi/src/services/platform_service/mod.rs(1 hunks)packages/rs-dapi/src/sync.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/sync.rspackages/rs-dapi/src/services/platform_service/mod.rspackages/rs-dapi/src/clients/tenderdash_client.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/sync.rspackages/rs-dapi/src/services/platform_service/mod.rspackages/rs-dapi/src/clients/tenderdash_client.rs
🧠 Learnings (4)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
📚 Learning: 2025-10-09T15:59:33.162Z
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/rs-dapi/src/clients/tenderdash_client.rs:0-0
Timestamp: 2025-10-09T15:59:33.162Z
Learning: In the rs-dapi codebase, the `Workers::spawn` method (in `packages/rs-dapi/src/sync.rs`) accepts futures that return `Result<O, E>`, not `()`. Its signature is `fn spawn<F, O, E>(&self, fut: F) where F: Future<Output = Result<O, E>> + Send + 'static`. The method handles error logging internally, so spawned tasks should return Results directly without additional error handling wrappers.
Applied to files:
packages/rs-dapi/src/sync.rspackages/rs-dapi/src/clients/tenderdash_client.rs
📚 Learning: 2025-10-09T15:59:41.943Z
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/rs-dapi/src/clients/tenderdash_client.rs:0-0
Timestamp: 2025-10-09T15:59:41.943Z
Learning: In packages/rs-dapi, the Workers::spawn method accepts futures that return Result<O, E> where E: Into<DapiError>. It handles error logging internally, so spawned tasks should return Result types rather than handling errors inside the task. The Workers implementation automatically logs worker failures.
Applied to files:
packages/rs-dapi/src/sync.rspackages/rs-dapi/src/clients/tenderdash_client.rs
📚 Learning: 2024-10-10T10:30:24.653Z
Learnt from: lklimek
PR: dashpay/platform#2232
File: packages/rs-sdk/src/sync.rs:68-71
Timestamp: 2024-10-10T10:30:24.653Z
Learning: In synchronous code within a Tokio runtime, we cannot await spawned task handles (`JoinHandle`), so it's acceptable to check if the task is finished and abort it if necessary.
Applied to files:
packages/rs-dapi/src/sync.rs
🧬 Code graph analysis (3)
packages/rs-dapi/src/sync.rs (4)
packages/rs-dapi/src/cache.rs (3)
new(58-60)new(96-109)new(148-171)packages/rs-dapi/src/metrics.rs (3)
new(400-402)workers_active_inc(562-564)workers_active_dec(567-569)packages/rs-dapi/src/services/platform_service/mod.rs (1)
new(110-163)packages/rs-dapi/src/services/streaming_service/masternode_list_sync.rs (1)
new(32-41)
packages/rs-dapi/src/services/platform_service/mod.rs (5)
packages/rs-dapi/src/cache.rs (16)
cache(474-475)cache(484-485)cache(494-495)cache(518-519)cache(536-537)cache(542-543)cache(577-578)cache(603-604)cache(623-624)make_cache_key(388-401)method(63-65)get(220-235)new(58-60)new(96-109)new(148-171)put(281-298)packages/rs-dapi/src/clients/drive_client.rs (2)
get_client(230-232)new(95-128)packages/rs-dapi/src/clients/tenderdash_client.rs (4)
new(53-60)new(383-422)status(450-455)websocket_client(531-533)packages/rs-dapi/src/services/platform_service/error_mapping.rs (1)
new(20-35)packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs (1)
build_wait_for_state_transition_error_response(289-312)
packages/rs-dapi/src/clients/tenderdash_client.rs (5)
packages/rs-dapi/src/utils.rs (3)
deserialize_string_number_or_null(147-163)deserialize_string_or_number(23-86)generate_jsonrpc_id(12-21)packages/rs-dapi/src/services/platform_service/mod.rs (1)
new(110-163)packages/rs-dapi/src/clients/tenderdash_websocket.rs (2)
new(78-88)test_connection(106-125)packages/rs-dapi/src/server/mod.rs (1)
new(29-77)packages/rs-dapi/src/error.rs (3)
from_tenderdash_error(170-172)client(201-203)server_unavailable(231-233)
⏰ 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). (30)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
🔇 Additional comments (7)
packages/rs-dapi/src/sync.rs (2)
1-257: Excellent worker pool implementation; previous concerns fully addressed.The implementation is well-architected and correctly handles the issues raised in prior reviews:
Memory leak fixed: The WorkerManager now continuously drains the JoinSet via
join_next()in theselect!loop (lines 227-231) and performs a final drain on shutdown (lines 236-238), preventing accumulation of completed tasks.AbortHandle handling is correct: The previous review suggesting
futures::abortablewas mistaken—JoinSet::spawnreturnsAbortHandledirectly per Tokio 1.47.0 docs. The current design properly:
- Spawns tasks into the JoinSet and obtains AbortHandle (lines 211-212, 221-222)
- Returns AbortHandle to the caller via oneshot channel
- Provides async abort via
WorkerTaskHandle(lines 150-154)Additional strengths:
- Clean RAII-based metrics tracking via
WorkerMetricsGuard- Proper error logging in
handle_result(lines 242-255)- Lazy AbortHandle resolution using
OnceCell+Notifypattern- Comprehensive test coverage
Based on learnings and retrieved documentation.
258-357: Well-designed test suite.The tests comprehensively cover:
- Task execution and cleanup
- Abort cancellation with drop verification (using
DropGuard)- Error handling and metric cleanup
The
wait_for_active_counthelper appropriately handles async timing, and all tests verify the critical invariant thattask_countreturns to zero after task completion.packages/rs-dapi/src/services/platform_service/mod.rs (3)
58-59: Past issue resolved: Cache.get now uses type inference.The invalid cast
as Option<$response_type>has been corrected. The current code properly uses Rust's type inference withcache.get(&key), which will inferTas$response_typefrom the context.
102-104: Past issue resolved: Workers abstraction ensures proper cleanup.The background task cleanup concern has been addressed by using the
Workersabstraction. The#[allow(dead_code)]comment correctly notes that droppingWorkerswill cancel spawned tasks, ensuring proper resource cleanup whenPlatformServiceImplis dropped.
110-163: LGTM: PlatformServiceImpl initialization is well-structured.The constructor properly:
- Initializes the
Workersabstraction for background task management- Spawns the WebSocket listener with automatic retry logic (10s delay)
- Wires cache invalidation to block events via
PlatformAllBlockssubscription- Returns appropriate
Result<(), DapiError>from spawned tasks (as per learnings)The infinite loop with retry logic is appropriate for a persistent background connection, and the
#[allow(unreachable_code)]annotation correctly acknowledges the unreachableOk(()).packages/rs-dapi/src/clients/tenderdash_client.rs (2)
390-401: Past issue resolved: HTTP client now has proper timeouts and tracing.The HTTP client initialization has been updated to include:
- Connection timeout via
connect_timeout(CONNECT_TIMEOUT)- Request timeout via
timeout(REQUEST_TIMEOUT)- Tracing middleware via
TracingMiddleware::default()This addresses the previous concerns about missing timeouts and unattached tracing.
337-337: Past issue resolved: Using RequestBuilder::json() for clean serialization.The code now uses
.json(request)instead of manual JSON stringification, which is cleaner and handles serialization automatically. The request timeout is already configured at the client level (line 392), so no additional per-request timeout is needed here.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (1)
927-945: Do not fallback to raw-bytescontains()when tx deserialization failsRaw-byte checks misrepresent bloom semantics and hide encoding issues. Treat as non-match instead.
Based on learnings.
packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs (2)
37-58: Do not fallback to raw-bytescontains()when tx deserialization failsTreat malformed data as non-matching rather than falling back to raw-bytes check. This fallback misrepresents bloom filter semantics and can mask encoding bugs.
246-325: Mark test with#[ignore]to avoid CI failuresThis test documents desired behavior that is not yet implemented (filter persistence). It should be marked with
#[ignore]to prevent CI failures.
🧹 Nitpick comments (1)
packages/rs-dapi/src/services/platform_service/mod.rs (1)
52-73: Consider explicit type parameter for cache lookupFor clarity, specify the type parameter explicitly in the cache lookup.
- if let Some(decoded) = cache.get(&key) { + if let Some(decoded) = cache.get::<$response_type>(&key) { return Ok((Response::new(decoded), true)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/rs-dapi/src/services/platform_service/mod.rs(1 hunks)packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs(1 hunks)packages/rs-dapi/src/services/streaming_service/transaction_stream.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rspackages/rs-dapi/src/services/streaming_service/subscriber_manager.rspackages/rs-dapi/src/services/platform_service/mod.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rspackages/rs-dapi/src/services/streaming_service/subscriber_manager.rspackages/rs-dapi/src/services/platform_service/mod.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
🧬 Code graph analysis (3)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
packages/rs-dapi/src/services/streaming_service/bloom.rs (2)
bloom_flags_from_int(103-114)matches_transaction(55-101)packages/rs-dapi/src/services/streaming_service/mod.rs (8)
txid_bytes_from_bytes(57-65)txid_hex_from_bytes(48-54)block_hash_hex_from_block_bytes(68-74)summarize_streaming_event(87-129)deserialize(51-51)deserialize(62-62)deserialize(71-71)short_hex(77-84)
packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs (4)
packages/rs-dapi/src/services/streaming_service/mod.rs (2)
summarize_streaming_event(87-129)new(168-187)packages/rs-dapi/src/services/streaming_service/bloom.rs (1)
matches_transaction(55-101)packages/rs-dash-event-bus/src/event_bus.rs (4)
id(234-236)matches(16-16)matches(429-431)subscription_count(185-187)packages/rs-dapi/src/clients/tenderdash_websocket.rs (1)
new(78-88)
packages/rs-dapi/src/services/platform_service/mod.rs (5)
packages/rs-drive-abci/src/query/service.rs (4)
broadcast_state_transition(257-262)get_status(578-588)wait_for_state_transition_result(420-425)new(75-77)packages/rs-dapi/src/cache.rs (16)
cache(474-475)cache(484-485)cache(494-495)cache(518-519)cache(536-537)cache(542-543)cache(577-578)cache(603-604)cache(623-624)make_cache_key(388-401)method(63-65)get(220-235)new(58-60)new(96-109)new(148-171)put(281-298)packages/rs-dapi/src/clients/drive_client.rs (1)
get_client(230-232)packages/rs-dapi/src/clients/tenderdash_client.rs (4)
new(53-60)new(383-422)status(450-455)websocket_client(531-533)packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs (1)
build_wait_for_state_transition_error_response(289-312)
⏰ 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). (3)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Swift SDK and Example build (warnings as errors)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
929-947: Critical: Raw-bytes bloom fallback still present despite past fix.Lines 943-947 still contain the problematic raw-bytes
contains()fallback that was flagged and marked as addressed in previous reviews. This misrepresents bloom semantics and masks encoding errors. Treat deserialization failures as non-matches instead:Err(e) => { - debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed, checking raw-bytes contains()"); - let guard = bloom.read().unwrap(); - guard.contains(tx_bytes) + debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed; treating as non-match"); + false }Based on learnings.
943-947: Handle RwLock poisoning instead of panicking.Line 945's
.unwrap()will panic if the lock is poisoned (e.g., a thread panicked while holding the write lock). Returnfalseor log and continue instead:Err(e) => { - debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed, checking raw-bytes contains()"); - let guard = bloom.read().unwrap(); - guard.contains(tx_bytes) + debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed"); + match bloom.read() { + Ok(guard) => guard.contains(tx_bytes), + Err(e) => { + debug!(height, error = %e, "bloom_lock_poisoned"); + false + } + } }(Note: This suggestion assumes you keep the raw-bytes fallback; the previous comment recommends removing it entirely.)
🧹 Nitpick comments (2)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
477-494: Consider removing unnecessary response clone.At line 480,
resp.clone()is called before sending, but the originalrespis not used afterward. The clone can be removed:- if tx_sender.send(Ok(resp.clone())).await.is_err() { + if tx_sender.send(Ok(resp)).await.is_err() {This eliminates an allocation for every forwarded event.
515-584: Consider extracting common merkle-block logic.The
CoreAllTxs(lines 524-544) andCoreBloomFilter(lines 545-577) branches share significant structure—both deserialize the block, compute match flags, build merkle bytes, and fall back to raw block on error. Extract the shared path into a helper to reduce duplication:fn build_merkle_with_flags( raw_block: &[u8], match_flags: Vec<bool>, handle_id: &str, ) -> TransactionsWithProofsResponse { // deserialize, build, fallback logic }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
🧬 Code graph analysis (1)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
packages/rs-dapi/src/services/streaming_service/bloom.rs (2)
bloom_flags_from_int(103-114)matches_transaction(55-101)packages/rs-dapi/src/services/streaming_service/mod.rs (8)
txid_bytes_from_bytes(57-65)txid_hex_from_bytes(48-54)block_hash_hex_from_block_bytes(68-74)summarize_streaming_event(87-129)deserialize(51-51)deserialize(62-62)deserialize(71-71)short_hex(77-84)
⏰ 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). (158)
- GitHub Check: Rust packages (token-history-contract) / Detect immutable structure changes
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Detect immutable structure changes
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
🔇 Additional comments (6)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (6)
1-41: LGTM!Type aliases and constants are well-defined. The 3-minute gate timeout (
GATE_MAX_TIMEOUT) is generous but reasonable for slow historical replay scenarios.
43-148: LGTM!The
TransactionStreamStatedesign is sound. The gate mechanism correctly coordinates historical replay with live streaming, and the delivered-sets prevent duplicates across transaction and instant-lock sources.
150-193: LGTM!Clean entry point with appropriate mode routing. The bloom filter setup and error handling are correct.
586-784: LGTM!The orchestration of live and historical streams is well-structured. The gate mechanism correctly ensures historical events are delivered before live events, and error propagation to the client is handled properly.
786-868: LGTM!The mempool worker correctly filters, deduplicates, and batches matching transactions. The integration with the delivery-tracking state prevents duplicates.
1025-1096: LGTM!Helper functions are well-implemented with appropriate validation. The merkle tree construction validates inputs, bloom filter parsing checks for invalid parameters, and txid formatting correctly reverses byte order for display.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (1)
929-933: Raw-bytes bloom filter fallback still present despite past resolution.This code segment was previously flagged and marked as addressed, but the problematic fallback to
guard.contains(tx_bytes)when deserialization fails is still present. Raw-byte checks misrepresent bloom semantics and hide encoding issues.Additionally, line 931 uses
unwrap()on the RwLock which will panic if the lock is poisoned. Apply this diff to address both issues:Err(e) => { - debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed, checking raw-bytes contains()"); - let guard = bloom.read().unwrap(); - guard.contains(tx_bytes) + debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed; treating as non-match"); + false }Based on learnings and past review feedback.
🧹 Nitpick comments (3)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (3)
421-421: Simplify or document complex boolean condition.The condition combines three negations which can be hard to reason about. Consider refactoring for clarity:
- if !bloom_matched && !already_delivered && !matches!(filter, FilterType::CoreAllTxs) { + let should_skip = !bloom_matched + && !already_delivered + && !matches!(filter, FilterType::CoreAllTxs); + if should_skip {Or add a comment explaining the logic: "Skip if not matched by bloom, not previously delivered, and not in all-txs mode."
538-543: Extract duplicated "include delivered tx in merkle block" logic.The logic for checking if a transaction has been previously delivered and including it in the merkle block (regardless of bloom match) appears in both
build_transaction_merkle_responseandprocess_transactions_from_height.Consider extracting this pattern into a helper method:
async fn should_include_in_merkle( bloom_matched: bool, tx_bytes: &[u8], state: Option<&TransactionStreamState>, ) -> bool { if bloom_matched { return true; } if let Some(state) = state { if let Some(txid_bytes) = Self::txid_bytes_from_bytes(tx_bytes) { return state.has_transaction_been_delivered(&txid_bytes).await; } } false }Then use it in both locations to improve maintainability.
Also applies to: 940-946
1112-1142: Expand test coverage for critical streaming logic.The file has extensive business logic (bloom filtering, deduplication, gate mechanism, historical vs. live streaming) but only one unit test. Consider adding tests for:
- Bloom filter matching with various transaction types
- Deduplication across delivered sets (txs, blocks, instant locks)
- Gate mechanism timing and pending event flushing
- Merkle block construction with different match patterns
- Error handling for deserialization failures
This would improve confidence in the correctness of the streaming implementation, especially for edge cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
🧬 Code graph analysis (1)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
packages/rs-dapi/src/services/streaming_service/bloom.rs (2)
bloom_flags_from_int(103-114)matches_transaction(55-101)packages/rs-dapi/src/services/streaming_service/mod.rs (8)
txid_bytes_from_bytes(57-65)txid_hex_from_bytes(48-54)block_hash_hex_from_block_bytes(68-74)summarize_streaming_event(87-129)deserialize(51-51)deserialize(62-62)deserialize(71-71)short_hex(77-84)
⏰ 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). (127)
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
🔇 Additional comments (2)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
92-98: Verify forced gate opening doesn't cause data races.Forcibly opening the gate on timeout could allow live events to be processed while historical sync is still ongoing, potentially causing duplicates or ordering issues if the historical worker hasn't completed marking transactions as delivered.
Consider adding a check or warning if historical sync is still active when the timeout fires, or validate that the deduplication logic is robust enough to handle this scenario.
1031-1075: LGTM! Comprehensive bloom filter validation.The bloom filter parsing includes appropriate validation for empty data and zero hash functions, with clear error messages. The use of
bloom_flags_from_inthelper and proper error propagation is well-implemented.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (1)
915-933: Raw-bytes fallback still present despite past review.Lines 929-933 still perform raw-bytes
bloom.contains(tx_bytes)when transaction deserialization fails. This pattern was previously flagged and marked as addressed, but the problematic code remains. Treat deserialization failures as non-matches instead of falling back to semantically incorrect raw-bytes checks.Apply this diff:
match deserialize::<CoreTx>(tx_bytes.as_slice()) { Ok(tx) => { let matches = super::bloom::matches_transaction( Arc::clone(bloom), &tx, *flags, ); trace!(height,matches, txid = %tx.txid(), "transactions_with_proofs=bloom_match"); matches } Err(e) => { - debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed, checking raw-bytes contains()"); - let guard = bloom.read().unwrap(); - guard.contains(tx_bytes) + debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed; treating as non-match"); + false } }Based on learnings.
🧹 Nitpick comments (2)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
357-373: Handle Option directly instead of sentinel string.Using
"n/a"as a sentinel and then string-comparing is fragile. Refactor to handle theOption<String>result fromblock_hash_hex_from_block_bytesdirectly.Apply this diff:
StreamingEvent::CoreRawBlock { data } => { - let block_hash = - super::StreamingServiceImpl::block_hash_hex_from_block_bytes(&data) - .unwrap_or_else(|| "n/a".to_string()); - - if block_hash != "n/a" - && let Ok(hash_bytes) = hex::decode(&block_hash) - && !state.mark_block_delivered(&hash_bytes).await - { + let block_hash_opt = super::StreamingServiceImpl::block_hash_hex_from_block_bytes(&data); + + if let Some(block_hash) = &block_hash_opt { + if let Ok(hash_bytes) = hex::decode(block_hash) { + if !state.mark_block_delivered(&hash_bytes).await { - trace!( - subscriber_id, - handle_id, - block_hash = %block_hash, - "transactions_with_proofs=skip_duplicate_merkle_block" - ); - return true; + trace!( + subscriber_id, + handle_id, + block_hash = %block_hash, + "transactions_with_proofs=skip_duplicate_merkle_block" + ); + return true; + } + } } trace!( subscriber_id, handle_id, - block_hash = %block_hash, + block_hash = %block_hash_opt.unwrap_or_else(|| "unknown".to_string()), payload_size = data.len(), "transactions_with_proofs=forward_merkle_block" );
1088-1092: Use_prefix for unused error binding.Line 1091 binds
errbut never uses it. Rust convention is to prefix with_to indicate intentional non-use.Apply this diff:
Ok(tx) => match InstantLock::consensus_decode(&mut cursor) { Ok(instant_lock) => (Some(tx), Some(instant_lock)), - Err(err) => (Some(tx), None), + Err(_) => (Some(tx), None), },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
🧬 Code graph analysis (1)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (3)
packages/rs-dapi/src/services/streaming_service/bloom.rs (2)
bloom_flags_from_int(103-114)matches_transaction(55-101)packages/rs-dapi/src/services/streaming_service/mod.rs (8)
txid_bytes_from_bytes(57-65)txid_hex_from_bytes(48-54)block_hash_hex_from_block_bytes(68-74)summarize_streaming_event(87-129)deserialize(51-51)deserialize(62-62)deserialize(71-71)short_hex(77-84)packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs (2)
matches(97-99)deserialize(39-39)
⏰ 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). (158)
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
Comment on lines
+736
to
+738
| if start > best_height.saturating_add(1) { | ||
| return Err(Status::not_found(format!("Block {} not found", start))); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify height bounds check allows future block.
The check start > best_height.saturating_add(1) permits requesting best_height + 1, which is a block that doesn't exist yet. Confirm this is intentional behavior for handling edge cases where a new block might arrive between calls.
Run this verification:
🏁 Script executed:
#!/bin/bash
# Check if other parts of the codebase have similar height validation patterns
rg -nP --type=rust -C3 'best.*height.*saturating_add\(1\)' Length of output: 2896
Based on the script output, I've identified a critical inconsistency that confirms the review comment's concern.
Fix the height bounds check to use >= instead of > for consistency.
The script output reveals an inconsistency between two streaming services:
transaction_stream.rs:736:if start > best_height.saturating_add(1)(allowsbest_height + 1)block_header_stream.rs:491:if start_height >= best_height.saturating_add(1)(deniesbest_height + 1)
The > operator in transaction_stream.rs permits requesting best_height + 1, a block that doesn't yet exist. While this may compute available = 1 through saturating arithmetic, this differs from block_header_stream's stricter >= check. The two files should use the same bounds validation logic for consistency and correctness.
Change line 736 from > to >= to align with block_header_stream.rs and prevent requests beyond the known blockchain height.
🤖 Prompt for AI Agents
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs around
lines 736-738: the height bounds check currently uses `if start >
best_height.saturating_add(1)` which permits `best_height + 1` while
block_header_stream.rs denies it; change the operator to `>=` so the line reads
`if start >= best_height.saturating_add(1)` to align validation with
block_header_stream.rs and prevent requesting a non-existent block height.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs (1)
465-490: Health check mechanism may be unreliable and could cause resource issues.This is the same concern raised in the previous review. The health check subscribes to a "ping" topic every 10 seconds, which:
- May not be idempotent—repeated subscriptions could create internal state in the ZeroMQ socket.
- Tests a non-existent topic rather than verifying receipt of real Dash Core messages.
- Uses
subscribe()success/failure as a proxy for connection health, which may not catch all failure modes.The previous review suggested alternatives like tracking time since last message or using a lightweight socket health check API if available.
🧹 Nitpick comments (3)
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs (3)
34-34: Consider removing unusedrawtxlockfield.The
rawtxlockfield is defined but never included into_vec()(line 62 usesrawtxlocksiginstead), andparse_zmq_messageexplicitly ignoresrawtxlockmessages (lines 402-403). Ifrawtxlockis intentionally unused in favor ofrawtxlocksig, consider removing this field or adding a comment explaining why it's kept for future use.
309-346: Infinite retry loop lacks explicit cancellation check.The
loopon line 309 runs indefinitely with no explicit check forcancel_parent.is_cancelled(). While cancellation is handled implicitly throughwith_cancelwrapper andZmqConnection::newaccepting a cancel token, the loop will continue to retry even after cancellation is requested, only exiting when the task is aborted.Consider adding an explicit cancellation check:
loop { + if cancel_parent.is_cancelled() { + return Err(DapiError::ConnectionClosed); + } + // We don't want to cancel parent task by mistake let cancel = cancel_parent.child_token();This makes the cancellation behavior more explicit and avoids unnecessary work after cancellation.
507-523: Consider expanding test coverage.The current tests only cover basic initialization. Consider adding tests for:
- Message parsing (
parse_zmq_messagewith various frame combinations)- Topic vector generation (
to_vec()completeness)- Error handling (connection failures, malformed messages)
- Cancellation behavior
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
🧬 Code graph analysis (1)
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs (2)
packages/rs-dapi/src/config/mod.rs (7)
std(225-225)default(47-54)default(136-145)default(149-153)default(157-162)default(166-174)default(198-205)packages/rs-dapi/src/services/streaming_service/mod.rs (2)
new(168-187)summarize_zmq_event(132-166)
⏰ 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). (162)
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Unused dependencies
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
🔇 Additional comments (12)
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs (12)
1-23: LGTM!The imports are well-organized and include all necessary dependencies for async ZMQ handling, cancellation, and tracing.
70-83: LGTM!The
ZmqEventenum is well-structured with clear variants for each Dash Core ZMQ topic. The use ofVec<u8>for raw data is appropriate.
85-99: LGTM!The
ZmqConnectionstruct properly manages shared state withArcand atomic types, and theDropimplementation ensures cancellation on cleanup.
197-206: Acknowledged limitation: Disconnected event unreliable.The comment correctly notes that the
Disconnectedevent is never received, which is why the health check workaround intick_event_10sexists. This is a known limitation of the zeromq library.
225-241: LGTM!The
SocketRecvimplementation correctly handles channel closure and explicitly drops the lock before processing the result.
252-263: LGTM!The
new()constructor properly initializes the listener and establishes the initial connection. The broadcast channel size of 1000 is reasonable for buffering ZMQ events.
265-288: LGTM!The connection setup properly uses child tokens to allow connection retries within the task without cancelling the task itself. The error handling correctly cancels the parent task when the listener fails.
349-387: LGTM!The message processing loop correctly handles different error cases and gracefully continues when broadcast fails (which is expected when no subscribers are listening).
389-413: LGTM!The message parsing correctly handles all expected Dash Core ZMQ topics and gracefully ignores unknown topics with logging.
424-429: LGTM!The
spawnmethod correctly sets up the dispatcher worker with cancellation support.
433-463: LGTM!The dispatcher worker correctly handles both message reception and periodic health checks, with appropriate error handling and cancellation.
493-505: LGTM!The
with_cancelhelper is a clean abstraction for adding cancellation support to futures.
Comment on lines
+137
to
+142
| for topic in topics { | ||
| socket | ||
| .subscribe(topic) | ||
| .await | ||
| .map_err(DapiError::ZmqConnection)?; | ||
| } |
There was a problem hiding this comment.
Subscription failures may leave socket in inconsistent state.
If subscription fails for a topic after some topics have already been subscribed (e.g., topic 3 fails but topics 1-2 succeeded), the socket remains connected with partial subscriptions, and the error propagates up. This could lead to missing events for the failed topic while still receiving others.
Consider either:
- Collecting subscription errors and unsubscribing from already-subscribed topics before returning an error.
- Documenting that partial subscription failures are acceptable and will be retried on the next connection attempt.
// Subscribe to topics
+let mut subscribed_topics = Vec::new();
for topic in topics {
- socket
+ if let Err(e) = socket
.subscribe(topic)
.await
- .map_err(DapiError::ZmqConnection)?;
+ {
+ // Cleanup partial subscriptions
+ for sub in subscribed_topics {
+ let _ = socket.unsubscribe(&sub).await;
+ }
+ return Err(DapiError::ZmqConnection(e));
+ }
+ subscribed_topics.push(topic.clone());
}🤖 Prompt for AI Agents
In packages/rs-dapi/src/services/streaming_service/zmq_listener.rs around lines
137 to 142, the loop subscribes topics one-by-one but if a subscribe call fails
after some prior successes the socket is left in a partially-subscribed state;
change the logic to track successfully subscribed topics and on any subscription
error iterate over that list to unsubscribe them (awaiting each and logging or
swallowing unsubscribe errors) before returning the original
DapiError::ZmqConnection; ensure the unsubscribe calls use the same async socket
API and preserve the original error for return so callers see the root failure.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (11)
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs (3)
301-304: Rename applied correctly
is_running()now reflects cancellation state, not socket connectivity. Good.
142-149: Prevent partial-subscription state on failureIf one topic subscribe fails after earlier successes, the socket remains partially subscribed. Roll back successful subscriptions before returning the error.
- // Subscribe to topics - for topic in topics { - socket - .subscribe(topic) - .await - .map_err(DapiError::ZmqConnection)?; - } + // Subscribe to topics with rollback on failure + let mut subscribed: Vec<String> = Vec::new(); + for topic in topics { + if let Err(e) = socket.subscribe(topic).await { + // best-effort cleanup + for sub in &subscribed { + let _ = socket.unsubscribe(sub).await; + } + return Err(DapiError::ZmqConnection(e)); + } + subscribed.push(topic.clone()); + }
499-524: Health check via subscribe('ping') is unreliable; track last-message insteadRepeatedly subscribing to a dummy topic can be non‑idempotent and doesn’t prove data flow. Track last received message time and declare unhealthy after prolonged silence.
Sketch:
struct ZmqDispatcher { socket: SubSocket, zmq_tx: mpsc::Sender<ZmqMessage>, cancel: CancellationToken, connected: Arc<AtomicBool>, + last_msg_at: std::sync::Arc<std::sync::atomic::AtomicU64>, // unix_secs } @@ - Ok(msg) => if let Err(e) = self.zmq_tx.send(msg).await { + Ok(msg) => { + self.last_msg_at.store( + std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_secs() as u64, + Ordering::SeqCst + ); + if let Err(e) = self.zmq_tx.send(msg).await { ... - }, + } + } @@ - async fn tick_event_10s(&mut self) { - // Health check of zmq connection (hack) - let current_status = self.socket.subscribe("ping").await.is_ok(); - self.socket.unsubscribe("ping").await.inspect_err(|e| { ... }).ok(); - let previous_status = self.connected.swap(current_status, Ordering::SeqCst); + async fn tick_event_10s(&mut self) { + let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_secs() as u64; + let last = self.last_msg_at.load(Ordering::SeqCst); + let current_status = now.saturating_sub(last) <= 60; + let previous_status = self.connected.swap(current_status, Ordering::SeqCst); if current_status != previous_status { if current_status { debug!("ZMQ connection recovered"); } else { debug!("ZMQ connection is lost, connection will be restarted"); self.cancel.cancel(); } } }packages/rs-dapi/src/services/streaming_service/mod.rs (3)
446-448: Use structured field for lag count; lower severityRecord the numeric count and log at debug to avoid noisy error logs during pressure.
- Err(RecvError::Lagged(skipped)) => { - tracing::error!(skipped, "ZMQ event reader lagged, skipped events"); - } + Err(RecvError::Lagged(skipped)) => { + debug!(skipped = skipped, "ZMQ receiver lagged; skipping"); + continue; + }
284-289: Standardize lag logging (structured fields)Prefer structured fields over string formatting for
skipped.- debug!( - "Tenderdash event receiver lagged, skipped {} events", - skipped - ); + debug!(skipped = skipped, "Tenderdash tx receiver lagged");- debug!( - skipped, - "Tenderdash block event receiver lagged, skipped events", - ); + debug!(skipped = skipped, "Tenderdash block receiver lagged");Also applies to: 326-331
361-365: Good: backoff reset after successful processingBackoff resets to 1s after processing ends; aligns with earlier guidance.
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
941-960: Remove raw-bytes bloom fallback; avoid unwrap on lockFalling back to
contains(raw_bytes)misrepresents bloom semantics and masks encoding issues;.unwrap()can panic.- FilterType::CoreBloomFilter(bloom, flags) => { - match deserialize::<CoreTx>(tx_bytes.as_slice()) { - Ok(tx) => { - let matches = super::bloom::matches_transaction( - Arc::clone(bloom), - &tx, - *flags, - ); - trace!(height,matches, txid = %tx.txid(), "transactions_with_proofs=bloom_match"); - matches - } - Err(e) => { - debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed, checking raw-bytes contains()"); - let guard = bloom.read().unwrap(); - guard.contains(tx_bytes) - } - } - } + FilterType::CoreBloomFilter(bloom, flags) => match deserialize::<CoreTx>(tx_bytes.as_slice()) { + Ok(tx) => { + let matches = super::bloom::matches_transaction(Arc::clone(bloom), &tx, *flags); + trace!(height, matches, txid = %tx.txid(), "transactions_with_proofs=bloom_match"); + matches + } + Err(e) => { + debug!(height, error = %e, "transactions_with_proofs=tx_deserialize_failed; treating as non-match"); + false + } + }
756-766: Align height bounds check with other streams (deny best_height + 1)Use
>=to avoid allowing a non-existent future block and to keep consistency with block_header_stream.- if start > best_height.saturating_add(1) { + if start >= best_height.saturating_add(1) { return Err(Status::not_found(format!("Block {} not found", start))); }packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs (3)
38-59: Do not fallback to raw-bytes bloom.contains(); treat as non-matchFallback hides encoding bugs and breaks bloom semantics. Log and return false.
- FilterType::CoreBloomFilter(bloom, flags) => match deserialize::<CoreTx>(raw_tx) { - Ok(tx) => super::bloom::matches_transaction(Arc::clone(bloom), &tx, *flags), - - Err(e) => { - debug!( - error = %e, - "Failed to deserialize core transaction for bloom filter matching, falling back to contains()" - ); - match bloom.read() { - Ok(guard) => guard.contains(raw_tx), - Err(_) => { - debug!("Failed to acquire read lock for bloom filter"); - false - } - } - } - }, + FilterType::CoreBloomFilter(bloom, flags) => match deserialize::<CoreTx>(raw_tx) { + Ok(tx) => super::bloom::matches_transaction(Arc::clone(bloom), &tx, *flags), + Err(e) => { + debug!(error = %e, "deserialize failed; treating as non-match"); + false + } + },
219-259: Update test to reflect non-fallback semanticsAfter removing fallback, non‑TX bytes should not match. Assert no message is received within timeout.
- async fn test_non_tx_bytes_fallbacks_to_contains() { + async fn test_non_tx_bytes_do_not_match() { @@ - // We should receive one transaction message with the same bytes - let msg = timeout(Duration::from_millis(200), handle.recv()) - .await - .expect("timed out") - .expect("channel closed"); - - match msg { - StreamingEvent::CoreRawTransaction { data } => { - assert_eq!(data, payload); - } - other => panic!("unexpected message: {:?}", other), - } + // Expect no message within timeout + let res = timeout(Duration::from_millis(200), handle.recv()).await; + assert!(res.is_err(), "unexpected match for non-transaction bytes");
261-341: Ignore the known failing persistence test to keep CI greenMark the test with #[ignore] and reference a tracking issue.
- #[tokio::test] + #[tokio::test] + #[ignore] // TODO(dapi#2795): enable once bloom update persistence across messages is implemented async fn test_bloom_update_persistence_across_messages() {
🧹 Nitpick comments (1)
packages/rs-dapi/src/services/streaming_service/mod.rs (1)
382-389: Increment counter only on successful receiveAvoid counting lag/closed as processed events.
- loop { - let event = zmq_events.recv().await; - - processed_events = processed_events.saturating_add(1); - match event { + loop { + match zmq_events.recv().await { Ok(ZmqEvent::RawTransaction { data }) => { + processed_events = processed_events.saturating_add(1); ... } Ok(ZmqEvent::RawBlock { data }) => { + processed_events = processed_events.saturating_add(1); ... } Ok(ZmqEvent::RawTransactionLock { tx_bytes, lock_bytes }) => { + processed_events = processed_events.saturating_add(1); ... } Ok(ZmqEvent::RawChainLock { data }) => { + processed_events = processed_events.saturating_add(1); ... } Ok(ZmqEvent::HashBlock { hash }) => { + processed_events = processed_events.saturating_add(1); ... } Err(RecvError::Closed) => break, Err(RecvError::Lagged(skipped)) => { ... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/rs-dapi/src/services/streaming_service/mod.rs(1 hunks)packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs(1 hunks)packages/rs-dapi/src/services/streaming_service/transaction_stream.rs(1 hunks)packages/rs-dapi/src/services/streaming_service/zmq_listener.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rspackages/rs-dapi/src/services/streaming_service/mod.rspackages/rs-dapi/src/services/streaming_service/subscriber_manager.rspackages/rs-dapi/src/services/streaming_service/zmq_listener.rs
packages/rs-*/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
packages/rs-*/**/*.rs: Use rustfmt defaults for Rust code (format via cargo fmt --all)
Keep Rust code clippy-clean (cargo clippy)
Name Rust modules in snake_case
Use PascalCase for Rust types
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rspackages/rs-dapi/src/services/streaming_service/mod.rspackages/rs-dapi/src/services/streaming_service/subscriber_manager.rspackages/rs-dapi/src/services/streaming_service/zmq_listener.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: lklimek
PR: dashpay/platform#2716
File: packages/dashmate/src/test/constants/services.js:4-4
Timestamp: 2025-10-15T14:45:30.856Z
Learning: In the dashmate codebase (packages/dashmate), during the DAPI Rust migration (rs-dapi), the old service keys `dapi_api` and `dapi_core_streams` are intentionally kept in `generateEnvsFactory.js` for backward compatibility even though the test constants in `src/test/constants/services.js` have been updated to use `rs_dapi`. These deprecated keys will be removed in a future PR after the transition is complete.
Learnt from: shumkov
PR: dashpay/platform#2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:69-125
Timestamp: 2024-10-24T05:07:35.892Z
Learning: Future development plans involve rewriting DAPI into Rust, as the rest of the project has already migrated.
🧬 Code graph analysis (4)
packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (2)
packages/rs-dapi/src/services/streaming_service/bloom.rs (2)
bloom_flags_from_int(103-114)matches_transaction(55-101)packages/rs-dapi/src/services/streaming_service/mod.rs (8)
txid_bytes_from_bytes(57-65)txid_hex_from_bytes(48-54)block_hash_hex_from_block_bytes(68-74)summarize_streaming_event(87-133)deserialize(51-51)deserialize(62-62)deserialize(71-71)short_hex(77-84)
packages/rs-dapi/src/services/streaming_service/mod.rs (3)
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs (2)
new(111-152)new(258-269)packages/rs-dapi/src/services/streaming_service/masternode_list_sync.rs (1)
new(32-41)packages/rs-dapi/src/clients/tenderdash_websocket.rs (1)
new(78-88)
packages/rs-dapi/src/services/streaming_service/subscriber_manager.rs (3)
packages/rs-dapi/src/services/streaming_service/mod.rs (1)
summarize_streaming_event(87-133)packages/rs-dapi/src/services/streaming_service/bloom.rs (1)
matches_transaction(55-101)packages/rs-dash-event-bus/src/event_bus.rs (6)
clone(36-43)clone(218-225)id(234-236)matches(16-16)matches(429-431)subscription_count(185-187)
packages/rs-dapi/src/services/streaming_service/zmq_listener.rs (2)
packages/rs-dapi/src/config/mod.rs (7)
std(225-225)default(47-54)default(136-145)default(149-153)default(157-162)default(166-174)default(198-205)packages/rs-dapi/src/services/streaming_service/mod.rs (2)
new(176-195)summarize_zmq_event(136-174)
⏰ 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). (147)
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (dashpay-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Linting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Tests
- GitHub Check: Rust packages (json-schema-compatibility-validator) / Formatting
- GitHub Check: Rust packages (masternode-reward-shares-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Linting
- GitHub Check: Rust packages (dashpay-contract) / Formatting
- GitHub Check: Rust packages (dashpay-contract) / Tests
- GitHub Check: Rust packages (withdrawals-contract) / Unused dependencies
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (withdrawals-contract) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Tests
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
Comment on lines
+358
to
+373
| let block_hash = | ||
| super::StreamingServiceImpl::block_hash_hex_from_block_bytes(&data) | ||
| .unwrap_or_else(|| "n/a".to_string()); | ||
|
|
||
| if block_hash != "n/a" | ||
| && let Ok(hash_bytes) = hex::decode(&block_hash) | ||
| && !state.mark_block_delivered(&hash_bytes).await | ||
| { | ||
| trace!( | ||
| subscriber_id, | ||
| handle_id, | ||
| block_hash = %block_hash, | ||
| "transactions_with_proofs=skip_duplicate_merkle_block" | ||
| ); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Fix block-hash dedupe: byte-order inconsistency causes missed duplicates
Here you decode the displayed hex (big‑endian) to bytes, but elsewhere you store BlockHash bytes via AsRef/to_byte_array() (little‑endian). Use the same representation in both paths.
- let block_hash =
- super::StreamingServiceImpl::block_hash_hex_from_block_bytes(&data)
- .unwrap_or_else(|| "n/a".to_string());
-
- if block_hash != "n/a"
- && let Ok(hash_bytes) = hex::decode(&block_hash)
- && !state.mark_block_delivered(&hash_bytes).await
- {
+ let block_hash =
+ super::StreamingServiceImpl::block_hash_hex_from_block_bytes(&data)
+ .unwrap_or_else(|| "n/a".to_string());
+
+ // Normalize to internal byte order for dedupe
+ if let Ok(block) = dashcore_rpc::dashcore::consensus::encode::deserialize::<Block>(&data) {
+ let hash_bytes = block.block_hash().to_byte_array().to_vec();
+ if !state.mark_block_delivered(&hash_bytes).await {
trace!(
subscriber_id,
handle_id,
- block_hash = %block_hash,
+ block_hash = %block_hash,
"transactions_with_proofs=skip_duplicate_merkle_block"
);
return true;
- }
+ }
+ }🤖 Prompt for AI Agents
In packages/rs-dapi/src/services/streaming_service/transaction_stream.rs around
lines 358 to 373, the code decodes the displayed hex (big-endian) to bytes but
compares/stores BlockHash elsewhere as little-endian, causing missed duplicates;
after decoding hex::decode(&block_hash) convert the decoded byte vector to the
same little-endian representation used by mark_block_delivered (e.g., reverse
the byte order or call the same to_byte_array()/AsRef conversion used when
storing) before calling state.mark_block_delivered(&hash_bytes). Ensure the
comparison uses the identical byte-ordering path as the rest of the codebase so
deduplication works correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Infrastructure
Documentation
Tests
Chores