feat(drive-abci): preliminary support for event subscriptions in Platform #2795
Conversation
WalkthroughPlatform event streaming support added through new Protobuf messages, a gRPC RPC method, event bus infrastructure for publishing events, subscription forwarding in rs-dapi, client library implementations, and associated configuration, tests, and documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant rsDapi as rs-dapi<br/>PlatformService
participant EventBus as Event Bus<br/>(in-process)
participant Drive as Drive<br/>(upstream)
participant ABCI as rs-drive-abci<br/>ConsensusApp
Client->>rsDapi: PlatformSubscriptionRequest<br/>(filter, keepalive)
activate rsDapi
rsDapi->>EventBus: subscribe(filter)
activate EventBus
rsDapi->>Drive: subscribe_platform_events(filter)
activate Drive
ABCI->>EventBus: publish BlockCommitted
EventBus-->>rsDapi: event via receiver
rsDapi-->>Client: PlatformSubscriptionResponse<br/>(subscription_id, BlockCommitted)
ABCI->>EventBus: publish StateTransitionFinalized
EventBus-->>rsDapi: event via receiver
rsDapi-->>Client: PlatformSubscriptionResponse<br/>(subscription_id, StateTransitionFinalized)
Drive->>rsDapi: upstream stream event
rsDapi->>EventBus: forward event
EventBus-->>rsDapi: relayed via channel
rsDapi-->>Client: PlatformSubscriptionResponse
note over rsDapi: Keepalive timer
rsDapi-->>Client: PlatformSubscriptionResponse<br/>(subscription_id, Keepalive)
Client->>rsDapi: cancel/stream ends
deactivate rsDapi
deactivate EventBus
deactivate Drive
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Key areas requiring attention:
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
|
@coderabbitai review all |
|
@lklimek: I'll perform a comprehensive review of all the changes in this PR. ✅ Actions performedFull review triggered. |
|
Client streaming and bi-directional streams are not available via GRPC Web, it is not supported by the Javascript libraries and most likely will not be supported, because all of them uses Fetch as underlying layer for transport. Server streaming is supported though, it would be great to have a subscription RPC that we could use from Web, and I feel like it is rarely when you want to change your subscription conditions, it is much easier to just cancel an create new subscription, and passing a list of params for subscription should work too. I recently received a question, how do one subscribe on new documents or transactions to his data contracts, because people would like to build applications like chats, or integrations in their backend already, for example for accepting tokens in payment gateways, so I'm looking forward to it! |
Thanks for that info! I'll refactor to unidirectional grpc-web. |
|
✅ 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: "c9201581129c144406ed01884ba79abfeac528e52fc6d457be057843a6651760"
)Xcode manual integration:
|
|
@coderabbitai review all |
|
@lklimek: I'll perform a comprehensive review of all the changes in this PR. ✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (14)
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs (1)
164-166: Nice refactor using let-chains.The consolidated conditional using
&& let Ok(tx_data) =is more concise and idiomatic than nested if-let blocks, with no change in behavior.packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs (2)
11-27: Good session tracking pattern with one consideration.The
ActiveSessionGuardwithArcandDropensures proper metric cleanup. The buffer size of 512 is used but lacks justification in comments.Consider documenting why 512 was chosen for
PLATFORM_EVENTS_STREAM_BUFFER(e.g., based on expected event rate, memory constraints, or latency requirements).
53-79: Forwarding loop is sound, but final return value appears unused.The message forwarding logic correctly handles both success and error cases with proper metrics. However, the
Err::<(), DapiError>(DapiError::ConnectionClosed)at line 78 serves no purpose since the spawned task's result is never awaited or checked.Consider either:
- Removing the return value entirely (just let the task complete without returning anything)
- Or documenting why it's needed if there's a future plan to track task results
} tracing::debug!("Platform events uplink response stream closed"); - Err::<(), DapiError>(DapiError::ConnectionClosed) });packages/rs-sdk/tests/fetch/platform_events.rs (1)
48-53: Make the “stream stays open” check non-flakyRelying on “no message within 250ms” can intermittently fail if an event arrives quickly. Treat both “timeout” and “got a message” as success, and only fail on early close or error.
Apply:
- let wait_result = timeout(Duration::from_millis(250), stream.message()).await; - assert!( - wait_result.is_err(), - "expected to time out waiting for initial platform event" - ); + match timeout(Duration::from_millis(250), stream.message()).await { + // No message within window → stream is open; good. + Err(_elapsed) => {} + // Message arrived quickly → also fine; stream is open. + Ok(Ok(Some(_))) => {} + // Stream closed unexpectedly. + Ok(Ok(None)) => panic!("stream closed unexpectedly"), + // Transport error. + Ok(Err(status)) => panic!("stream errored early: {status}"), + }packages/rs-drive-abci/src/abci/app/mod.rs (1)
21-27: Reduce cross-crate coupling of EventBusApplicationThe trait hard-codes dapi_grpc::platform::v0 types, coupling ABCI app to gRPC schema. Consider a type alias exported from this crate (or make the trait generic) to decouple and ease future proto evolution.
Example:
pub type PlatformEvent = dapi_grpc::platform::v0::PlatformEventV0; pub trait EventBusApplication { fn event_bus(&self) -> &EventBus<PlatformEvent, PlatformFilterAdapter>; }packages/rs-drive-abci/src/server.rs (1)
23-30: EventBus initialized and injected correctly; consider tunable capacityDefault per-subscription channel capacity comes from EventBus::new() (256). If high fan‑out or bursty publishers are expected, consider making capacity configurable via PlatformConfig and using with_capacity().
Also applies to: 79-80
packages/rs-dapi/doc/DESIGN.md (1)
267-291: Clarify gRPC‑Web limitation and client lifecycleExplicitly note gRPC‑Web doesn’t support client/bidi streaming; only server streaming is exposed. Recommend “cancel and recreate” pattern when changing subscription parameters, as it’s simpler than dynamic updates.
packages/rs-sdk/src/sdk.rs (1)
607-625: Avoid panic on contested lock; use async lock for workerstry_lock() can spuriously panic under contention. Use lock().await and drop yield_now(). Also consider honoring Sdk::cancel_token inside spawned tasks to ensure graceful shutdown.
[Based on learnings]
Apply:- pub(crate) async fn spawn( + pub(crate) async fn spawn( &self, task: impl std::future::Future<Output = ()> + Send + 'static, ) -> tokio::sync::oneshot::Receiver<()> { let (done_tx, done_rx) = tokio::sync::oneshot::channel(); - let mut workers = self - .workers - .try_lock() - .expect("workers lock is poisoned or in use"); - workers.spawn(async move { + let mut workers = self.workers.lock().await; + let cancel = self.cancel_token.clone(); + workers.spawn(async move { + tokio::select! { + _ = cancel.cancelled() => {}, + _ = task => {}, + } task.await; let _ = done_tx.send(()); }); - tokio::task::yield_now().await; done_rx }packages/rs-sdk/src/platform/events.rs (2)
79-86: Streaming deadline: consider no overall timeout.Setting
timeout: Some(Duration::from_secs(3600))will terminate long‑lived streams after 1 hour. For subscriptions, “no deadline” is usually preferable; rely on client cancel.let settings = self .dapi_client_settings .override_by(RequestSettings { connect_timeout: Some(Duration::from_secs(5)), - timeout: Some(Duration::from_secs(3600)), + // Streaming subscriptions typically shouldn't have a fixed deadline + timeout: None, ..Default::default() }) .finalize();
16-28: Comment/typo fixes to avoid confusion.
- “siginificant” → “significant”.
- “ofprocess” → “of process”.
- Display comment says “timepid” but code prints “pid_time”.
-/// ID generator with most siginificant bits based on local process info. +/// ID generator with most significant bits based on local process info. @@ - // 32..47 bits: lower 16 bits ofprocess start time in seconds + // 32..47 bits: lower 16 bits of process start time in seconds @@ - // Write in format: timepid:counter + // Write in format: pid_time:counterAlso applies to: 40-46
packages/rs-drive-abci/src/abci/handler/finalize_block.rs (1)
115-146: DRY: duplicate BlockMetadata computation.Factor out metadata creation to a helper to reduce duplication and keep event payloads consistent.
@@ -fn publish_block_committed_event( +fn block_metadata_from( + request_finalize_block: &FinalizeBlockCleanedRequest, +) -> platform_event_v0::BlockMetadata { + let header_time = request_finalize_block.block.header.time; + let seconds = header_time.seconds as i128; + let nanos = header_time.nanos as i128; + let time_ms = (seconds * 1000) + (nanos / 1_000_000); + platform_event_v0::BlockMetadata { + height: request_finalize_block.height, + time_ms: time_ms as u64, + block_id_hash: request_finalize_block.block_id.hash.to_vec(), + } +} + +fn publish_block_committed_event( event_bus: &dash_event_bus::event_bus::EventBus<PlatformEventV0, PlatformFilterAdapter>, request_finalize_block: &FinalizeBlockCleanedRequest, ) -> Result<(), Error> { - // Publish BlockCommitted platform event to the global event bus (best-effort) - let header_time = request_finalize_block.block.header.time; - let seconds = header_time.seconds as i128; - let nanos = header_time.nanos as i128; - let time_ms = (seconds * 1000) + (nanos / 1_000_000); - - let meta = platform_event_v0::BlockMetadata { - height: request_finalize_block.height, - time_ms: time_ms as u64, - block_id_hash: request_finalize_block.block_id.hash.to_vec(), - }; + // Publish BlockCommitted platform event to the global event bus (best-effort) + let meta = block_metadata_from(request_finalize_block); @@ -fn publish_state_transition_result_events( +fn publish_state_transition_result_events( event_bus: &dash_event_bus::event_bus::EventBus<PlatformEventV0, PlatformFilterAdapter>, request_finalize_block: &FinalizeBlockCleanedRequest, ) -> Result<(), Error> { - // Prepare BlockMetadata once - let header_time = request_finalize_block.block.header.time; - let seconds = header_time.seconds as i128; - let nanos = header_time.nanos as i128; - let time_ms = (seconds * 1000) + (nanos / 1_000_000); - - let meta = platform_event_v0::BlockMetadata { - height: request_finalize_block.height, - time_ms: time_ms as u64, - block_id_hash: request_finalize_block.block_id.hash.to_vec(), - }; + // Prepare BlockMetadata once + let meta = block_metadata_from(request_finalize_block);Also applies to: 148-179
packages/rs-sdk/examples/platform_events.rs (1)
87-96: Fail fast on invalid hex to avoid unintended “match all”.
.ok()swallows decode errors, turning an invalid hex filter into no filter (streaming all STRs).- let tx_hash_bytes = config - .state_transition_tx_hash_hex - .as_deref() - .and_then(|s| hex::decode(s).ok()); + let tx_hash_bytes = config + .state_transition_tx_hash_hex + .as_deref() + .map(|s| hex::decode(s).expect("invalid DASH_SDK_STATE_TRANSITION_TX_HASH_HEX"));packages/rs-drive-abci/src/query/service.rs (1)
271-310: Filter adapter logic looks correct and minimal.Matching All/BlockCommitted/StateTransitionResult with optional tx_hash is clear and efficient.
Consider logging the filter summary once per subscription for observability.
packages/dapi-grpc/protos/platform/v0/platform.proto (1)
165-167: Clarify RPC streaming semantics in comment.The comment states "Bi-directional stream" but the RPC signature is actually server-streaming only (request is unary, response is streamed). This is intentional for gRPC-Web compatibility, but the comment should reflect that. Update it to clarify the streaming direction and rationale.
// Bi-directional stream for multiplexed platform events subscriptions - rpc SubscribePlatformEvents(PlatformSubscriptionRequest) + // Server-streaming RPC for multiplexed platform events subscriptions + // (Unidirectional; gRPC-Web compatible per RFC on client-streaming limitations) + rpc SubscribePlatformEvents(PlatformSubscriptionRequest) returns (stream PlatformSubscriptionResponse);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/rs-dapi/doc/DESIGN.md(2 hunks)packages/rs-dapi/src/main.rs(2 hunks)packages/rs-dapi/src/metrics.rs(0 hunks)packages/rs-dapi/src/services/platform_service/mod.rs(2 hunks)packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs(1 hunks)packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs(1 hunks)packages/rs-dapi/src/services/streaming_service/bloom.rs(1 hunks)packages/rs-dash-event-bus/Cargo.toml(1 hunks)packages/rs-dash-event-bus/src/event_bus.rs(1 hunks)packages/rs-dash-event-bus/src/lib.rs(1 hunks)packages/rs-drive-abci/Cargo.toml(2 hunks)packages/rs-drive-abci/src/abci/app/consensus.rs(3 hunks)packages/rs-drive-abci/src/abci/app/full.rs(4 hunks)packages/rs-drive-abci/src/abci/app/mod.rs(1 hunks)packages/rs-drive-abci/src/abci/handler/finalize_block.rs(4 hunks)packages/rs-drive-abci/src/query/mod.rs(1 hunks)packages/rs-drive-abci/src/query/service.rs(4 hunks)packages/rs-drive-abci/src/server.rs(2 hunks)packages/rs-drive-abci/tests/strategy_tests/main.rs(1 hunks)packages/rs-sdk/Cargo.toml(4 hunks)packages/rs-sdk/examples/platform_events.rs(1 hunks)packages/rs-sdk/src/error.rs(1 hunks)packages/rs-sdk/src/platform.rs(1 hunks)packages/rs-sdk/src/platform/events.rs(1 hunks)packages/rs-sdk/src/sdk.rs(7 hunks)packages/rs-sdk/tests/fetch/mod.rs(1 hunks)packages/rs-sdk/tests/fetch/platform_events.rs(1 hunks)packages/wasm-sdk/src/error.rs(1 hunks)
💤 Files with no reviewable changes (1)
- packages/rs-dapi/src/metrics.rs
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-10-22T10:51:16.910Z
Learnt from: shumkov
PR: dashpay/platform#2259
File: packages/rs-dapi-client/src/transport.rs:51-51
Timestamp: 2024-10-22T10:51:16.910Z
Learning: In implementations of `TransportClient`, only `DapiClientError` is used as the `Error` type, and it implements `std::error::Error`.
Applied to files:
packages/rs-sdk/src/error.rs
📚 Learning: 2025-09-03T16:37:11.605Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2756
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:11-11
Timestamp: 2025-09-03T16:37:11.605Z
Learning: In packages/rs-dpp/Cargo.toml, the abci feature already includes core_rpc_client, and core_rpc_client is defined as ["dep:dashcore-rpc"]. This means rs-drive-abci can access dashcore-rpc types through dpp when using the abci feature.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-12-05T09:29:38.918Z
Learnt from: shumkov
PR: dashpay/platform#2375
File: packages/rs-drive-abci/Cargo.toml:61-63
Timestamp: 2024-12-05T09:29:38.918Z
Learning: In the `drive-abci` package, avoid adding unused dependencies like `hashbrown` to `Cargo.toml`. The team relies on CI to detect dependency version issues.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-10-10T10:30:25.283Z
Learnt from: lklimek
PR: dashpay/platform#2232
File: packages/rs-sdk/src/mock/sdk.rs:129-130
Timestamp: 2024-10-10T10:30:25.283Z
Learning: In the codebase `packages/rs-sdk/src/mock/sdk.rs`, `tokio::sync::Mutex` is used instead of `std::sync::Mutex` because `std::sync::MutexGuard` is not `Send` and cannot be used in async contexts.
Applied to files:
packages/rs-sdk/src/sdk.rs
📚 Learning: 2024-10-29T10:42:00.521Z
Learnt from: lklimek
PR: dashpay/platform#2277
File: packages/rs-dapi-client/Cargo.toml:22-22
Timestamp: 2024-10-29T10:42:00.521Z
Learning: In `packages/rs-dapi-client/Cargo.toml`, `backon` will not work without the `tokio-sleep` feature in our setup, so it's unnecessary to declare `tokio-sleep` as a separate feature in the `[features]` section.
Applied to files:
packages/rs-sdk/Cargo.toml
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: lklimek
PR: dashpay/platform#2139
File: packages/rs-sdk/Cargo.toml:0-0
Timestamp: 2024-10-09T00:22:57.778Z
Learning: `bincode` is used in the `mocks` feature of the `rs-sdk`.
Applied to files:
packages/rs-sdk/Cargo.toml
🔇 Additional comments (27)
packages/rs-dapi/src/services/streaming_service/bloom.rs (1)
14-17: LGTM: Clean refactor using let-chains.This simplification reduces nesting and is more idiomatic Rust. The logic remains functionally equivalent while improving readability.
packages/rs-drive-abci/src/query/mod.rs (1)
18-18: LGTM!The public re-export of
PlatformFilterAdapterfollows the existing pattern and cleanly exposes the adapter for external use in platform event subscriptions.packages/rs-dash-event-bus/src/lib.rs (1)
1-1: Documentation update aligns with architectural direction.The simplified description removing the bi-directional gRPC multiplexer reference aligns with the PR discussion about refactoring to unidirectional gRPC-Web streaming for better web client compatibility.
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs (1)
206-206: Consistent use of let-chains.The same clean refactoring pattern applied here with
let Somemaintains consistency throughout the file.packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs (2)
33-48: Upstream subscription and channel setup looks solid.The method correctly establishes the upstream subscription before creating the forwarding channel, and errors from the drive client are properly propagated to the caller.
82-83: Correct gRPC streaming response pattern.The
ReceiverStreamwrapper properly bridges the mpsc receiver to tonic's streaming response requirements.packages/rs-sdk/Cargo.toml (3)
77-77: Consider whether preliminary subscriptions feature should be default.The
subscriptionsfeature is added to the default feature set for a draft PR with "preliminary support." For an incomplete feature, consider requiring opt-in (default = []without subscriptions) until the feature is production-ready, to avoid exposing unstable APIs by default.Is the subscriptions feature stable enough to be enabled by default, or should users explicitly opt-in until DAPI integration (PR #2716) is complete?
Also applies to: 102-102
72-73: Note: AI summary inconsistency.The AI summary claims mocks feature was removed from defaults, but
mocksis still present in the default features list (line 73). This appears to be a pre-existing TODO item not addressed in this PR.
55-55: Thesignalfeature is correctly used and necessary.The examples in
packages/rs-sdk/examples/platform_events.rsusetokio::signal::ctrl_c()on line 128, which requires the "signal" feature. The addition is justified.packages/rs-drive-abci/tests/strategy_tests/main.rs (1)
201-247: LGTM! Well-structured event bus test.The test effectively validates the in-process event bus subscription and delivery mechanism. The 1-second timeout is appropriate, and the test clearly demonstrates the subscribe-publish-receive flow.
packages/rs-sdk/src/error.rs (1)
41-43: LGTM! Clear error variant addition.The new
SubscriptionErrorvariant follows the existing error pattern conventions and provides a clear error message for subscription-related failures.packages/rs-sdk/src/platform.rs (1)
21-22: LGTM! Appropriate feature gating.The
eventsmodule is correctly exposed only when thesubscriptionsfeature is enabled, following the pattern established elsewhere in the codebase.packages/wasm-sdk/src/error.rs (1)
172-177: LGTM! Consistent error translation.The
SubscriptionErrormapping toWasmSdkErroris consistent with other error translations. UsingDapiClientErroras the kind is reasonable since subscriptions are DAPI-related functionality.packages/rs-drive-abci/Cargo.toml (1)
65-82: LGTM! Dependencies align with event bus functionality.The dependency additions and updates are appropriate for the platform event streaming infrastructure:
tokio-streamfor async streaming constructsrs-dash-event-busfor in-process event bussha2for cryptographic hashing in event metadatametrics-exporter-prometheusupdate is a minor version bumppackages/rs-sdk/tests/fetch/mod.rs (1)
27-28: LGTM! Test module properly feature-gated.The
platform_eventstest module is correctly included only when thesubscriptionsfeature is enabled, consistent with the feature gating pattern used throughout the test suite.packages/rs-dapi/src/main.rs (2)
133-163: LGTM! Cleaner conditional flow.The merged conditional logic using
if let Some(result) = outcome && let Err(e) = resultis more concise and readable than the previous nested approach.
114-134: Theserver_futurehandling is correct as implemented.The code follows the proper pattern when using
tokio::pin!():
- The binding doesn't need
mutbecausetokio::pin!internally manages the pinning and mutability- Polling with
&mut server_futurein theselect!macro is the standard, correct approach for pinned futures- This combination is not an inconsistency but the idiomatic Rust pattern for this scenario
The original concern appears to have been based on a misunderstanding of how
tokio::pin!()works with mutability.packages/rs-dash-event-bus/Cargo.toml (1)
6-6: LGTM! Documentation refinement.The simplified description accurately reflects the package's purpose while being more concise.
packages/rs-dapi/src/services/platform_service/mod.rs (1)
171-184: Server‑streaming RPC wiring looks correctAssociated stream type and subscribe_platform_events delegate are aligned with gRPC‑Web constraints (server streaming). LGTM.
Please confirm subscribe_platform_events_impl returns ReceiverStream<Result<PlatformSubscriptionResponse, Status>> and sets appropriate compression/limits for large events.
packages/rs-drive-abci/src/abci/app/consensus.rs (1)
31-46: LGTM: EventBus integration is cleanField injection and EventBusApplication impl expose the bus without extra cloning; fits finalize_block publishers.
Also applies to: 62-66
packages/rs-dash-event-bus/src/event_bus.rs (1)
277-282: Compile blocker: invalidif letcondition chaining
if let Ok(mut subs) = bus.subs.try_write() && subs.remove(&id).is_some()is not valid Rust and won’t compile. Split into nested conditionals.Apply:
- if let Ok(mut subs) = bus.subs.try_write() - && subs.remove(&id).is_some() - { - metrics_unsubscribe_inc(); - metrics_active_gauge_set(subs.len()); - } + if let Ok(mut subs) = bus.subs.try_write() { + if subs.remove(&id).is_some() { + metrics_unsubscribe_inc(); + metrics_active_gauge_set(subs.len()); + tracing::debug!("event_bus: removed subscription id={} (no runtime)", id); + } + }Likely an incorrect or invalid review comment.
packages/rs-drive-abci/src/abci/app/full.rs (1)
31-33: EventBus wiring looks good; confirm Send/Sync and clone behavior.If
EventBusclone is cheap (Arc underneath) and Send/Sync, this is fine. If not, consider wrapping inArc<EventBus<...>>to avoid accidental heavy clones across threads.Also applies to: 42-43, 59-63
packages/rs-drive-abci/src/query/service.rs (1)
894-901: Confirm unsubscribe semantics on drop.We rely on dropping
subscriptionto remove it from the bus when the client disconnects. Please confirmEventBus::add_subscriptionreturns a handle withDropthat unsubscribes; otherwise we need an explicit removal path.packages/rs-drive-abci/src/abci/handler/finalize_block.rs (2)
107-111: Review comment is based on incorrect assumptions about the event bus implementation.The
notify_sync()method is already designed as non-blocking fire-and-forget. It spawns a background Tokio task that invokes the asyncnotify()method, returning immediately without blocking the ABCI handler path. No changes are required—the current implementation already provides the non-blocking behavior the review suggests pursuing.Likely an incorrect or invalid review comment.
165-176: SHA-256 of raw tx bytes is the canonical state transition hash used consistently throughout the codebase.Verification confirms this is correct. The code at lines 165-176 uses
Sha256::digest(tx)on raw transaction bytes, which matches the canonical approach used elsewhere:
- DAPI broadcast layer (
rs-dapi/broadcast_state_transition.rs:49) computes the same hash identically- Query/filtering service (
rs-drive-abci/query/service.rs:299-301) matches filters directly against this hash- Client filtering (SDK examples, JS DAPI tests) all use the same hash for StateTransitionResult queries
- Protocol definition confirms
bytes tx_hashis the standard field for both events and filtersNo alternative hash function or helper exists; the raw tx bytes → SHA-256 pattern is uniform across all layers (ABCI, DAPI, query, clients). No divergence between platform components or risk of filter mismatch.
packages/dapi-grpc/protos/platform/v0/platform.proto (2)
9-24: Versioned message structure is clean and consistent.The nested V0 + oneof version pattern aligns well with the rest of the proto file. The
client_subscription_idfield enables multiplexing multiple subscriptions over a single RPC stream, which is sound. The design will scale well if V1/V2 variants are introduced later.
26-64: Event and filter messages are well-structured; verify alignment with SDK implementations.The filter design uses oneof (all, block_committed, state_transition_result) for flexibility, and PlatformEventV0 pairs BlockMetadata consistently across event types. A few items to confirm:
- Optional tx_hash in StateTransitionResultFilter (line 30): Allows filtering by specific state transitions; good for subscribers interested in particular transactions.
- BlockMetadata nesting (lines 44–48): Shared across BlockCommitted and StateTransitionFinalized, reducing duplication.
- Extensibility: The oneof pattern in PlatformFilterV0 and PlatformEventV0 supports future event types (e.g., IdentityCreated, DocumentDeleted) without breaking existing subscriptions.
Verify that:
- Downstream Rust (rs-drive-abci) and WASM (rs-sdk) implementations correctly adapt these message definitions.
- Field semantics (e.g., block_id_hash, time_ms precision) are documented and match consensus layer definitions.
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rs-dapi/src/server/grpc.rs (1)
115-118: Fix tracing fields:pathisn’t recorded; log field names + valuesCurrent calls treat
pathas a boolean field. Use keyed fields with%to record values; also avoidunwrap_or_default()inside the “is_some()” branch.- tracing::trace!( - path, - "Detected streaming gRPC method, applying streaming timeout" - ); + tracing::trace!(path = %path, "Detected long‑lived gRPC method; using streaming timeout");- trace!( - path, - header_timeout = timeout_from_header.unwrap_or_default().as_secs_f32(), - timeout = effective_timeout.as_secs_f32(), - "Applying gRPC timeout from header" - ); + trace!( + path = %path, + header_timeout_secs = { timeout_from_header.unwrap().as_secs_f32() }, + timeout_secs = effective_timeout.as_secs_f32(), + "Applying gRPC timeout from header" + );- tracing::trace!( - path, - timeout = effective_timeout.as_secs_f32(), - "Applying default gRPC timeout" - ); + tracing::trace!( + path = %path, + timeout_secs = effective_timeout.as_secs_f32(), + "Applying default gRPC timeout" + );Also applies to: 171-176, 178-183
♻️ Duplicate comments (1)
packages/rs-drive-abci/src/query/service.rs (1)
71-74: Resource leak: Vec<JoinHandle<()>> accumulates finished tasks.The
workersVec accumulatesJoinHandles but never drains completed tasks during normal operation. In a long-running service, this causes unbounded memory growth as subscription workers finish.Apply one of these solutions:
Solution 1 (simplest): Spawn detached tasks
- let worker = tokio::task::spawn(async move { + tokio::task::spawn(async move { // ... worker logic ... }); - self.workers.lock().unwrap().push(worker);Then remove the
workersfield and its Drop implementation since tasks will be automatically cleaned up.Solution 2: Add a background reaper
Keep the Vec but spawn a separate task that periodically callsretainto remove finished handles:// In QueryService::new or similar tokio::spawn({ let workers = Arc::clone(&self.workers); async move { loop { tokio::time::sleep(Duration::from_secs(60)).await; workers.lock().unwrap().retain(|h| !h.is_finished()); } } });
🧹 Nitpick comments (18)
.github/scripts/check-grpc-coverage.py (1)
17-17: LGTM! Exclusion is appropriate for the streaming RPC.The addition correctly excludes
SubscribePlatformEventsfrom the SDK implementation check, which is appropriate for a server-streaming RPC that may follow different implementation patterns than standard request-response queries.Consider enhancing the comment to explain why streaming RPCs are excluded:
- 'SubscribePlatformEvents', # Streaming RPC, excluded + 'SubscribePlatformEvents', # Streaming RPC - excluded because server-streaming RPCs follow different implementation patternsAlternatively, if multiple streaming RPCs will be added in the future, consider documenting this exclusion pattern at the module or section level for better maintainability.
packages/rs-drive-abci/src/query/service.rs (2)
294-323: Consider removing the Keepalive early-return in matches().The filter returns
truefor Keepalive events (lines 298-300), but Keepalive events are generated locally by the worker task (lines 992-998) and never flow through the event bus subscription. This check will never match any events from the bus.If this is intentional (e.g., future-proofing for bus-generated keepalives), consider adding a comment explaining the rationale. Otherwise, simplify:
impl EventBusFilter<PlatformEvent> for PlatformFilterAdapter { fn matches(&self, event: &PlatformEvent) -> bool { use dapi_grpc::platform::v0::platform_event_v0::Event as Evt; use dapi_grpc::platform::v0::platform_filter_v0::Kind; - if matches!(event.event, Some(Evt::Keepalive(_))) { - return true; - } match self.inner.kind.as_ref() { None => false,
904-916: Consider documenting the keepalive bounds rationale.The keepalive interval validation enforces 25-300 seconds but lacks explanation for these specific bounds.
Add a brief comment:
+ // Keepalive bounds: minimum 25s to avoid excessive pings, + // maximum 300s to ensure reasonable connection liveness detection let keepalive_duration = if keepalive == 0 { None } else if keepalive < 25 || keepalive > 300 {packages/rs-sdk/examples/platform_events.rs (1)
145-193: Use consistent logging approach.The worker function mixes
info!()(lines 157, 178) andprintln!()(lines 168, 192) for logging events. For consistency and to leverage the configured tracing subscriber, useinfo!()throughout or explicitly useprintln!()for all user-facing output.Apply this diff for consistency:
PlatformEvent::StateTransitionFinalized(r) => { if let Some(meta) = r.meta { - println!( + info!( "{label}: id={sub_id} height={} tx_hash=0x{} block_id_hash=0x{}", meta.height, hex::encode(r.tx_hash), hex::encode(meta.block_id_hash) ); } }packages/rs-dapi/src/server/grpc.rs (1)
104-112: List name/comment are misleading (contains unary long‑poll); rename to LONG_TIMEOUT_METHODS
waitForStateTransitionResultis unary, yet intentionally long‑lived. Rename the list and fix the comment to avoid confusion for future edits.- /// Determine the appropriate timeout for a given gRPC method path. + /// Determine the appropriate timeout for a given gRPC method path. fn timeout_for_method(&self, path: &str) -> Duration { - // All known streaming methods in Core service (all use "stream" return type) - const STREAMING_METHODS: &[&str] = &[ + // Methods that require long timeouts (true streaming or long‑poll unary) + const LONG_TIMEOUT_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) { + if LONG_TIMEOUT_METHODS.contains(&path) {packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js (1)
1909-1947: Harden stream handler: null‑guard and validate event typeAvoid TypeErrors if callbacks fire after cancel/end, and fail fast on invalid event types.
var client = grpc.invoke(Platform.SubscribePlatformEvents, { @@ - onMessage: function (responseMessage) { - listeners.data.forEach(function (handler) { + onMessage: function (responseMessage) { + if (!listeners) return; + listeners.data.forEach(function (handler) { handler(responseMessage); }); }, - onEnd: function (status, statusMessage, trailers) { - listeners.status.forEach(function (handler) { + onEnd: function (status, statusMessage, trailers) { + if (!listeners) return; + listeners.status.forEach(function (handler) { handler({ code: status, details: statusMessage, metadata: trailers }); }); listeners.end.forEach(function (handler) { handler({ code: status, details: statusMessage, metadata: trailers }); }); listeners = null; } }); return { - on: function (type, handler) { - listeners[type].push(handler); + on: function (type, handler) { + if (!listeners || !listeners[type]) { + throw new Error("Unknown or closed listener type: " + type); + } + listeners[type].push(handler); return this; }, cancel: function () { listeners = null; client.close(); } };packages/dapi-grpc/protos/platform/v0/platform.proto (1)
13-16: Keepalive field: clarify units and enforce bounds server‑sideRename to
keepalive_secs(explicit units) or document clearly; ensure Drive/DAPI clamp to [25, 300] and handle 0 (disabled).- // Interval in seconds between keepalive events (min 25, max 300, 0 disables - // keepalive). - uint32 keepalive = 2; + // Keepalive interval in seconds (min 25, max 300; 0 disables keepalive). + uint32 keepalive_secs = 2;Please confirm server validation is implemented alongside this proto.
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (4)
31-39: Keepalive field needs unit clarity; prefer Duration or explicit suffix.A bare number for keepalive is ambiguous for web clients. Recommend:
- Use google.protobuf.Duration, or
- Rename to keepaliveMs (or KeepAliveSeconds) and document min/max.
Example proto tweak:
-message PlatformSubscriptionRequestV0 { - PlatformFilterV0 filter = 1; - uint32 keepalive = 2; // ambiguous units -} +import "google/protobuf/duration.proto"; +message PlatformSubscriptionRequestV0 { + PlatformFilterV0 filter = 1; + google.protobuf.Duration keepalive = 2; // precise, cross-language friendly +}
85-102: Subscription ID semantics: rename or add a client correlation id.Response exposes clientSubscriptionId, but the request has no matching field. If the server assigns the id, suggest:
- Rename to subscriptionId, or
- Keep as is and add optional clientCorrelationId to the request so the server echoes it back.
Proto sketch:
message PlatformSubscriptionRequestV0 { PlatformFilterV0 filter = 1; - uint32 keepalive = 2; + uint32 keepalive = 2; + string client_correlation_id = 3; // optional, echoed back } message PlatformSubscriptionResponseV0 { - string client_subscription_id = 1; + string subscription_id = 1; // server-assigned PlatformEventV0 event = 2; }
141-166: Oneof of booleans is awkward; align filter/event names.
- Using bools inside a oneof (all, blockCommitted) yields confusing AsObject defaults (false even when unset) and clunky client UX. Prefer:
- oneof with message wrappers (google.protobuf.Empty) or
- an explicit enum kind plus an optional message payload per kind.
- Name mismatch: filter uses stateTransitionResult but the event is StateTransitionFinalized. Pick one term platform‑wide (finalized is clearer for events).
Proto sketch:
message PlatformFilterV0 { oneof kind { google.protobuf.Empty all = 1; google.protobuf.Empty block_committed = 2; StateTransitionResultFilter state_transition_finalized = 3; // rename for consistency } }Also applies to: 168-181
183-208: Event payloads: minor schema polish for web streaming.
- Consider including a simple outcome/status on StateTransitionFinalized (accepted/rejected) to save a follow‑up call.
- BlockMetadata.timeMs is string (int64) which is fine; if you want stronger typing, google.protobuf.Timestamp is an option.
- Keepalive is good; ensure its cadence honors the requested keepalive and document server min/max caps.
If outcome is already decided elsewhere, point to the API consumers that should be used to resolve it.
Also applies to: 217-237, 247-264, 273-292, 301-323
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (2)
7960-7995: Avoid bools in oneof discriminators; use empty messages to make the variant explicit.
PlatformFilterV0.kindusesbool all = 1; bool block_committed = 2;in a oneof. Settingall=falsestill selects theALLvariant (presence), whiletoObject()showsfalse, which is misleading and error‑prone.Recommend replacing bools with empty messages so the discriminator is unambiguous:
message PlatformFilterV0 { oneof kind { - bool all = 1; - bool block_committed = 2; + message All {} + message BlockCommittedFilter {} + All all = 1; + BlockCommittedFilter block_committed = 2; StateTransitionResultFilter state_transition_result = 3; } }Also document that clients must not rely on
toObject()booleans for presence; usegetKindCase()/has*(). Regenerate JS after proto change.Also applies to: 8089-8113, 8120-8170
7591-7602: Consider omitting client_subscription_id on every event to reduce payload.
PlatformSubscriptionResponseV0includesclient_subscription_idwith each streamed message. If streams are one‑subscription‑per‑stream (common for server streaming), the ID is redundant. Options:
- Send the ID only in the first “ack” message, omit thereafter.
- Make the field optional behind a flag if future multiplexing is planned.
Small bandwidth win; behavior unchanged for clients that don’t need per‑event IDs.
If multiplexing multiple subscriptions per stream is planned, please confirm so we don’t remove a necessary field.
Also applies to: 7667-7684
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m (1)
597-614: Bound clientSubscriptionId size and format.Consider documenting/validating a max length and allowed charset for clientSubscriptionId to prevent oversized frames or ambiguous IDs across transports.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (4)
527-540: Subscription ID placement/namingclientSubscriptionId is only on responses; request has no matching field. If the server assigns it, rename to subscriptionId. If clients provide it, add it to request v0 to support correlation.
Proto options:
message PlatformSubscriptionResponseV0 { - string client_subscription_id = 1; + string subscription_id = 1; PlatformEventV0 event = 2; } // OR, if client-supplied correlation is intended: message PlatformSubscriptionRequestV0 { PlatformFilterV0 filter = 1; uint32 keepalive = 2; + string client_subscription_id = 3; // echoed back in responses }
487-498: Keepalive bounds: clarify server behavior and defaultDocs say “min 25, max 300, 0 disables”. Specify server behavior when out-of-range and the default when 0 (e.g., clamp to [25,300] or reject with INVALID_ARGUMENT; default to 60s if 0).
Proto comment tweak:
// Interval in seconds between keepalive events. // Valid: 25..300. 0 = use server default (e.g., 60s). // Out-of-range values are rejected with INVALID_ARGUMENT. uint32 keepalive = 2;
543-559: txHash vs stateTransitionHash namingNew types use txHash while existing WaitForStateTransitionResult* uses stateTransitionHash. Prefer one term platform-wide (stateTransitionHash is clearer).
Proto sketch (if adopting stateTransitionHash):
-message StateTransitionResultFilter { bytes tx_hash = 1; } +message StateTransitionResultFilter { bytes state_transition_hash = 1; } -message StateTransitionResult { BlockMetadata meta = 1; bytes tx_hash = 2; } +message StateTransitionResult { BlockMetadata meta = 1; bytes state_transition_hash = 2; }Also applies to: 665-678
576-585: Oneof usage with BOOL fields: add note to avoid ObjC confusionBecause all, block_committed are BOOL in a oneof, toggling them can silently replace the active case. Add a brief proto comment advising clients to use the 'kind' oneof semantics and the generated ClearKindOneOfCase() when switching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpackages/dashmate/templates/platform/gateway/envoy.yaml.dotis excluded by!**/*.dot
📒 Files selected for processing (22)
.github/scripts/check-grpc-coverage.py(1 hunks)packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java(8 hunks)packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h(3 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m(1 hunks)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py(4 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts(1 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.js(2 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts(3 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js(2 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/rs-dapi/src/server/grpc.rs(1 hunks)packages/rs-dash-event-bus/Cargo.toml(1 hunks)packages/rs-drive-abci/Cargo.toml(2 hunks)packages/rs-drive-abci/src/query/service.rs(5 hunks)packages/rs-sdk/Cargo.toml(4 hunks)packages/rs-sdk/examples/platform_events.rs(1 hunks)packages/rs-sdk/src/platform.rs(1 hunks)packages/rs-sdk/src/platform/events.rs(1 hunks)packages/rs-sdk/tests/fetch/platform_events.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rs-drive-abci/Cargo.toml
- packages/rs-dash-event-bus/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/grpc.rspackages/rs-sdk/src/platform/events.rspackages/rs-sdk/src/platform.rspackages/rs-sdk/tests/fetch/platform_events.rspackages/rs-drive-abci/src/query/service.rspackages/rs-sdk/examples/platform_events.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/grpc.rspackages/rs-sdk/src/platform/events.rspackages/rs-sdk/src/platform.rspackages/rs-sdk/tests/fetch/platform_events.rspackages/rs-drive-abci/src/query/service.rspackages/rs-sdk/examples/platform_events.rs
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/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.tspackages/dapi-grpc/clients/platform/v0/web/platform_pb_service.jspackages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
packages/**/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests alongside each package in packages//tests
Files:
packages/rs-sdk/tests/fetch/platform_events.rs
packages/**/tests/**/*.{js,ts,jsx,tsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/tests/**/*.{js,ts,jsx,tsx,rs}: Name tests descriptively, starting with “should …”
Unit/integration tests must not perform network calls; mock dependencies instead
Files:
packages/rs-sdk/tests/fetch/platform_events.rs
🧠 Learnings (2)
📚 Learning: 2024-10-29T10:42:00.521Z
Learnt from: lklimek
PR: dashpay/platform#2277
File: packages/rs-dapi-client/Cargo.toml:22-22
Timestamp: 2024-10-29T10:42:00.521Z
Learning: In `packages/rs-dapi-client/Cargo.toml`, `backon` will not work without the `tokio-sleep` feature in our setup, so it's unnecessary to declare `tokio-sleep` as a separate feature in the `[features]` section.
Applied to files:
packages/rs-sdk/Cargo.toml
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: lklimek
PR: dashpay/platform#2139
File: packages/rs-sdk/Cargo.toml:0-0
Timestamp: 2024-10-09T00:22:57.778Z
Learning: `bincode` is used in the `mocks` feature of the `rs-sdk`.
Applied to files:
packages/rs-sdk/Cargo.toml
🪛 Ruff (0.14.2)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py
549-549: Unused method argument: request
(ARG002)
⏰ 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). (2)
- 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)
🔇 Additional comments (45)
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h (1)
108-109: LGTM: Forward declarations properly added.The forward declarations for
PlatformSubscriptionRequestandPlatformSubscriptionResponsefollow the established pattern and are correctly positioned in alphabetical order.packages/rs-drive-abci/src/query/service.rs (5)
76-83: LGTM!The Drop implementation correctly aborts all worker tasks on service shutdown.
94-103: LGTM!Constructor correctly initializes the new event_bus and workers fields.
899-902: Good: filter.kind validation prevents silent no-op subscriptions.This validation correctly addresses the past review concern by ensuring clients get an explicit error rather than silently receiving no events.
918-951: LGTM!Subscription setup correctly generates a unique ID, establishes the event bus subscription, creates a buffered channel, and sends a handshake response before streaming events.
953-1023: LGTM: Worker task logic is correct.The worker properly:
- Receives events from the subscription handle with optional keepalive timeout
- Forwards events wrapped in PlatformSubscriptionResponse
- Generates keepalive events on timeout
- Detects client disconnection and exits cleanly
Note: The handle storage at line 1024 contributes to the resource leak flagged separately.
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (4)
2240-2245: bindService uses asyncServerStreamingCall appropriately.Registration and handler binding look correct.
3490-3490: Method id added without collision.Increment to 47 is consistent with prior ids.
3697-3700: Switch-case wiring added for the new RPC.Routing to subscribePlatformEvents is correct.
3809-3810: ServiceDescriptor includes the new method.Descriptor update is correct.
packages/rs-sdk/src/platform.rs (1)
21-22: LGTM!The feature-gated module declaration follows the existing pattern and properly exposes the events module when the subscriptions feature is enabled.
packages/rs-sdk/tests/fetch/platform_events.rs (4)
13-16: LGTM!The test configuration with multi-threaded runtime and appropriate feature gating aligns with the existing test patterns in the codebase.
18-32: LGTM!The gRPC client setup is correct, with appropriate timeouts and error handling for the test environment.
34-47: LGTM!The subscription request is properly constructed with appropriate filter and keepalive settings, and error handling is in place.
75-81: LGTM!The stream verification correctly validates that the subscription remains open without receiving immediate events, using an appropriate timeout.
packages/rs-sdk/Cargo.toml (3)
55-55: LGTM!Adding the
signalfeature to the tokio dev-dependency is appropriate for supporting Ctrl+C handling in the platform_events example.
77-77: Consider whether preliminary feature should be enabled by default.The
subscriptionsfeature is added to the default feature set. Since this PR provides "preliminary support" (per PR objectives) and is still a draft, verify whether enabling this by default is intentional. For preliminary/experimental features, it's common to require explicit opt-in until the API stabilizes.
102-103: LGTM!The empty
subscriptionsfeature declaration is correct for a feature flag that gates code without pulling additional dependencies.packages/rs-sdk/src/platform/events.rs (3)
13-13: LGTM!The
EventSubscriptionIdtype alias simplifies the API. Note that the past review comment about Default derive is no longer applicable as the implementation changed from a counter-based struct to a String type alias.
65-71: LGTM!The
Subscriptiontrait provides a clean abstraction for extracting subscription IDs from responses, with clear documentation.
73-80: LGTM!The
Subscriptionimplementation correctly extracts the subscription ID from V0 responses and handles other versions gracefully by returning an empty string.packages/rs-sdk/examples/platform_events.rs (4)
1-8: LGTM!The feature-gated main function provides a clear user experience whether the subscriptions feature is enabled or not.
31-67: LGTM!The Config struct is well-designed with appropriate types, and the loading logic correctly handles both .env file and environment variables. Using
Zeroizingfor the password is a good security practice.
89-143: LGTM!The main function correctly sets up multiple concurrent subscriptions with proper error handling and Ctrl+C signal handling. The use of abort handles for forceful shutdown is appropriate for an example.
195-219: LGTM!The SDK setup function correctly handles platform and core configuration, with appropriate conditional CA certificate loading for non-WASM targets.
packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js (1)
436-443: Service descriptor for SubscribePlatformEvents — LGTMServer‑streaming shape matches proto; no issues.
packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (1)
430-438: Typings align with server‑streaming API — LGTM
Platform.SubscribePlatformEventsdescriptor and clientsubscribePlatformEvents(...)returningResponseStreamlook correct.Also applies to: 488-489, 946-947
packages/dapi-grpc/protos/platform/v0/platform.proto (1)
170-173: Naming inconsistency confirmed:SubscribePlatformEventsis the only PascalCase RPC in platform.protoVerification shows all other RPCs (50+) use lowerCamel throughout the service (e.g.,
waitForStateTransitionResult,getConsensusParams). Core.proto follows the same lowerCamel pattern (subscribeToBlockHeadersWithChainLocks, etc.). Changing this would require updates across generated clients in Python, Java, Node.js, Objective-C, and Rust.packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (3)
252-256: LGTM: Client stub correctly implements server streaming.The client stub properly uses
channel.unary_streamfor the SubscribePlatformEvents endpoint, enabling server streaming of platform events to clients.
794-798: LGTM: Handler registration correctly configured for server streaming.The handler registration properly uses
grpc.unary_stream_rpc_method_handlerwith correct serialization/deserialization for the SubscribePlatformEvents RPC.
1608-1623: LGTM: Experimental API correctly implements server streaming.The experimental static method properly uses
grpc.experimental.unary_streamwith appropriate request/response serialization and standard gRPC options.packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (1)
9-24: Subscription API properly generated and wired.Verified: The
SubscribePlatformEventsserver-streaming RPC is correctly generated in bothplatform_pb_service.d.tsandplatform_pb_service.js. The method signature matches the d.ts declarations, and all required message classes (PlatformSubscriptionRequest,PlatformSubscriptionResponse,PlatformEventV0) with their nested types and oneof patterns (VersionCase,EventCase) are present in the runtime. Type definitions align with the grpc-web implementation.packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (1)
463-9203: LGTM - Generated protobuf code appears correct.This file is auto-generated by JsPbCodeGenerator from the platform.proto definitions. The generated code follows standard protobuf JavaScript patterns consistently throughout, including proper constructors, serialization/deserialization methods, getters/setters, and oneof field handling.
Any modifications to these message structures should be made in the source
.protofiles and regenerated, not edited manually here.packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (1)
7235-7382: Clarify keepalive semantics; document that server enforces 25–300 second range.
PlatformSubscriptionRequestV0.keepalive(uint32, seconds) already has server-side validation clamping the interval to [25, 300], which is enforced by the DAPI query service. However, the naming creates confusion: the field is namedkeepaliveand there is also a separateKeepaliveevent type.Recommendations:
- Document in generated code: Add JSDoc/comment in the generated JS/TS bindings clarifying that the field is in seconds and the valid range is 25–300, with 0 disabling keepalive.
- Consider renaming for clarity (optional, for next proto revision): Rename the field to
keepalive_interval_secsto avoid overlap with theKeepaliveevent type and make units explicit.- Verify downstream: Ensure client libraries and examples correctly interpret the uint32 as seconds, not milliseconds or opaque units.
No changes required now if the proto comment and server validation are sufficient. Recommend adding explicit unit documentation when updating client SDKs.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m (10)
320-337: New message declarations wired correctly — LGTM.The added GPBObjCClassDeclaration entries align with the new subscription/events schema. No issues spotted.
421-455: Versioned request envelope is future‑proof — LGTM.Oneof "version" with v0 matches the established pattern and keeps wire compatibility.
533-569: Versioned response envelope — LGTM.Oneof "version" mirrors request; descriptor setup is consistent.
639-660: Constrain txHash length for correctness.txHash should be a fixed length (likely 32 bytes). Document/enforce this in server‑side validation and proto comments to prevent ambiguous matching.
776-827: Event envelope oneof is consistent — LGTM.blockCommitted/stateTransitionFinalized/keepalive are correctly grouped under "event".
867-874: Document timeMs epoch and units.Confirm this is Unix time in milliseconds; add a comment in the proto for cross‑language consistency (JS 53‑bit note, Rust/Go 64‑bit ok).
933-940: txCount semantics.If zero is meaningful (e.g., empty block), keep optional; otherwise consider non‑optional with explicit default in docs to avoid presence ambiguity.
1029-1047: Keepalive message — LGTM.Empty marker message is appropriate for heartbeat events.
700-727: Confirmed: Avoid booleans inside oneof "kind" in PlatformFilterV0 proto definition.The proto definition at
packages/dapi-grpc/protos/platform/v0/platform.proto(lines 35–43) contains two boolean fields (allandblock_committed) insideoneof kind. Setting a bool to false still "selects" that oneof branch, creating semantically awkward behavior and potentially blocking other filters.Recommended refactor: Replace boolean fields with empty marker messages (e.g.,
message All {}andmessage BlockCommitted {}) or use an enum discriminator.
471-501: Proto documentation for keepalive is already comprehensive.The proto file at packages/dapi-grpc/protos/platform/v0/platform.proto lines 13-15 already documents: "Interval in seconds between keepalive events (min 25, max 300, 0 disables keepalive)." This explicitly specifies units (seconds), bounds (25–300), and behavior (0 disables).
Server-side validation in rs-drive-abci enforces these constraints, rejecting out-of-range values with the error message "keepalive interval must be between 25 and 300 seconds." Rust SDK and Objective-C client implementations both treat keepalive as a uint32 field, consistent with the proto definition. No client divergence is evident.
The only enhancement that could be made is to explicitly document the default value (0) in the proto comment if not already implied.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
231-246: Forward declarations look consistent with new proto surfaceTypes align with later interfaces. No issues.
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java
Show resolved
Hide resolved
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java
Show resolved
Hide resolved
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/dashmate/configs/defaults/getBaseConfigFactory.js (1)
242-258: RemovewaitForStResultTimeoutfrom defaults; it's handled by migration logic and shouldn't coexist withtimeouts.waitForStateTransitionResultin new configs.The
waitForStResultTimeoutfield at line 258 is the legacy field being migrated away. The migration logic (getConfigFileMigrationsFactory.js lines 1378-1382) reads this field from old configs and copies its value to the newtimeouts.waitForStateTransitionResultlocation, then deletes the old field (lines 1430-1435).Including
waitForStResultTimeoutin the base defaults creates unnecessary duplication in new configurations. The field should only exist:
- In old/migrating configs (handled by the migration)
- Not in the defaults used for fresh installations
Move
waitForStResultTimeout: 120000from the base defaults into the migration logic where it already exists as a fallback value (line 1389), or document it explicitly as deprecated with a timeline for removal.packages/dashmate/configs/getConfigFileMigrationsFactory.js (1)
1380-1425: Remove legacywaitForStResultTimeoutafter buildingtimeoutsAfter we copy the old value into the new
rsDapi.timeouts, the legacyrsDapi.waitForStResultTimeoutfield remains on the config. The updated JSON schema now forbids extra properties onrsDapi, so existing nodes will fail validation post-migration unless we drop that key. Please delete it once the new object is populated.options.platform.dapi.rsDapi.timeouts = { waitForStateTransitionResult, subscribePlatformEvents, coreStreams, }; + + delete options.platform.dapi.rsDapi.waitForStResultTimeout;
♻️ Duplicate comments (7)
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (1)
2658-2670: Same Javadoc typo; fix via proto comments and regenerate.Repeat of “subscribint” typo; apply the same proto‑doc fix as above.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m (1)
1115-1121: Remove unary convenience for a server‑streaming RPC.Method returns GRPCUnaryProtoCall for a streaming RPC; this is misleading and can cause misuse. Prefer the eventHandler variant that already returns GRPCProtoCall.
-/** - * Server-streaming subscription for platform events. - * - * Allows subscribing to various Platform events. - * - * Once connected, it sends handshake response with empty event. - */ -- (GRPCUnaryProtoCall *)subscribePlatformEventsWithMessage:(PlatformSubscriptionRequest *)message responseHandler:(id<GRPCProtoResponseHandler>)handler callOptions:(GRPCCallOptions *_Nullable)callOptions { - return [self RPCToMethod:@"subscribePlatformEvents" - message:message - responseHandler:handler - callOptions:callOptions - responseClass:[PlatformSubscriptionResponse class]]; -} +/* Server‑streaming RPC: use -subscribePlatformEventsWithRequest:eventHandler: or RPCTosubscribePlatformEventsWithRequest:... */packages/rs-drive-abci/src/query/service.rs (1)
70-74: Do not retain JoinHandles; this leaks over time.JoinHandles are pushed into a Vec and never removed; completed tasks accumulate until process exit. Either:
- Don’t store the handle at all (spawn and drop), or
- Use a TaskTracker/reaper to drain finished tasks.
Minimal fix: remove the workers field and the push.
@@ pub struct QueryService { - event_bus: EventBus<PlatformEvent, PlatformFilterAdapter>, - /// background worker tasks - workers: Mutex<Vec<tokio::task::JoinHandle<()>>>, + event_bus: EventBus<PlatformEvent, PlatformFilterAdapter>, } @@ -impl Drop for QueryService { - fn drop(&mut self) { - let mut workers = self.workers.lock().unwrap(); - for handle in workers.drain(..) { - handle.abort(); - } - } -} +// No Drop impl needed when not retaining join handles. @@ pub fn new( - event_bus, - workers: Mutex::new(Vec::new()), + event_bus, } @@ spawn worker ... - let worker = tokio::task::spawn(async move { + tokio::task::spawn(async move { @@ - }); - self.workers.lock().unwrap().push(worker); + });If you prefer to keep tracking, replace Vec<JoinHandle<()>> with a
tokio_util::task::TaskTrackerand calltracker.close()in Drop.Also applies to: 76-83, 92-103, 950-1024
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (1)
117-124: Align naming: use stateTransitionHash consistently (not txHash).Existing API (WaitForStateTransitionResultRequestV0) uses stateTransitionHash. New types introduce txHash; keep one term.
-message StateTransitionResultFilter { - bytes tx_hash = 1; -} +message StateTransitionResultFilter { + bytes state_transition_hash = 1; +} -message StateTransitionFinalized { - PlatformEventV0.BlockMetadata meta = 1; - bytes tx_hash = 2; -} +message StateTransitionFinalized { + PlatformEventV0.BlockMetadata meta = 1; + bytes state_transition_hash = 2; +}Reduces client confusion and code churn.
#!/bin/bash # Verify inconsistent hash field names across proto/clients rg -n --hidden -S -g '!**/node_modules/**' -C2 \ -e 'stateTransitionHash' \ -e 'state_transition_hash' \ -e '\btxHash\b' \ -e 'tx_hash' \ packagesAlso applies to: 279-283
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (2)
664-679: Unify naming: StateTransitionResult vs StateTransitionFinalized; txHash vs stateTransitionHashEvent type uses PlatformEventV0_StateTransitionFinalized with txHash, while filters and WaitForStateTransitionResult* use “StateTransitionResult” and stateTransitionHash. Please standardize the proto before locking the API and regenerate Objective‑C:
Proto-level sketch:
message PlatformEventV0 { oneof event { BlockCommitted block_committed = 1; - StateTransitionFinalized state_transition_finalized = 2; + StateTransitionResult state_transition_result = 2; Keepalive keepalive = 3; } } -message StateTransitionFinalized { +message StateTransitionResult { BlockMetadata meta = 1; - bytes tx_hash = 2; + bytes state_transition_hash = 2; }And align the filter field:
message StateTransitionResultFilter { - bytes tx_hash = 1; + bytes state_transition_hash = 1; }
587-589: Fix truncated doc for StateTransitionResult filter selectorComment ends mid‑sentence. Replace with a complete note and state oneof semantics.
Suggested wording for the proto (then regenerate):
// Subscribe to StateTransitionResult events (optionally filtered by // StateTransitionResultFilter). Setting this clears other 'kind' variants. StateTransitionResultFilter state_transition_result = 3;packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (1)
8504-8527: Generated file: fix parseInt() at source and regenerate.The previous review identified that
parseInt(f, 10) !== 0on lines 8507 and 8514 is semantically incorrect for uint64 fields (which are handled as strings elsewhere, see lines 8466-8467, 8426-8427, 8534-8535, 8552-8553). This issue remains unaddressed.Since this is a generated protobuf file, do not manually edit it. Instead:
- Fix the generator configuration or proto definitions to use string comparison:
if (f !== "0")- Regenerate this file using the corrected tooling
🧹 Nitpick comments (10)
packages/dashmate/docs/services/gateway.md (1)
109-111: Fix bullet indentation to satisfy markdownlintThe leading space before each dash makes the list fail MD005/MD007 and breaks our doc lint. Please align these bullets with the rest of the list so the docs and CI stay clean.
- - Core streaming endpoints: derived from `platform.dapi.rsDapi.timeouts.coreStreams` (default 600 000 ms) - - waitForStateTransitionResult endpoint: derived from `platform.dapi.rsDapi.timeouts.waitForStateTransitionResult` (default 120 000 ms) - - subscribePlatformEvents endpoint: derived from `platform.dapi.rsDapi.timeouts.subscribePlatformEvents` (default 600 000 ms) +- Core streaming endpoints: derived from `platform.dapi.rsDapi.timeouts.coreStreams` (default 600 000 ms) +- waitForStateTransitionResult endpoint: derived from `platform.dapi.rsDapi.timeouts.waitForStateTransitionResult` (default 120 000 ms) +- subscribePlatformEvents endpoint: derived from `platform.dapi.rsDapi.timeouts.subscribePlatformEvents` (default 600 000 ms)packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (1)
3031-3043: Improve blocking‑stub docs; mention Iterator return.Doc is generic; add that the blocking stub returns an Iterator. Also fix the typo.
Update proto comments so generated Javadoc reflects it:
- // Server-streaming subscription for Platform events. + // Server‑streaming subscription for Platform events. // Allows subscribing to various Platform events. + // Client notes: blocking stub returns an Iterator over PlatformSubscriptionResponse; + // async stub emits responses via StreamObserver..github/scripts/check-grpc-coverage.py (1)
17-18: Exclude streaming RPCs automatically (reduce manual list churn).Adding subscribePlatformEvents to EXCLUDED_QUERIES is fine. Consider detecting server-streaming from the proto and auto-excluding, so future streaming RPCs don’t require list updates.
Here’s a minimal adjustment to extract and skip server-streaming RPCs:
- # Extract all rpc methods - rpc_pattern = r'rpc\s+(\w+)\s*\(' - for match in re.finditer(rpc_pattern, service_content): - query_name = match.group(1) - if query_name not in EXCLUDED_QUERIES: - queries.append(query_name) + # Extract all rpc methods and detect server-streaming + rpc_pattern = r'rpc\s+(\w+)\s*\([^\)]*\)\s*returns\s*\(\s*(stream\s+)?([A-Za-z_]\w*)\s*\)' + for match in re.finditer(rpc_pattern, service_content): + query_name = match.group(1) + is_streaming = match.group(2) is not None + if is_streaming: + # auto-exclude server-streaming RPCs from coverage + continue + if query_name not in EXCLUDED_QUERIES: + queries.append(query_name)packages/rs-sdk/tests/fetch/config.rs (1)
48-52: Gate the allow(unused) with the feature flag (match core_host).Keep warnings active under network-testing and silence only when unused in offline runs.
- #[allow(unused)] + #[cfg_attr(not(feature = "network-testing"), allow(unused))] pub platform_ca_cert_path: Option<PathBuf>,packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (3)
31-39: Clarify keepalive units and intent.Name implies “interval” but units aren’t clear. Prefer keepaliveMs (uint32) and document behavior (0 disables, min clamp).
-message PlatformSubscriptionRequestV0 { - PlatformFilterV0 filter = 1; - uint32 keepalive = 2; -} +message PlatformSubscriptionRequestV0 { + PlatformFilterV0 filter = 1; + uint32 keepalive_ms = 2; // interval in milliseconds; 0 disables +}This prevents client/server mismatches, especially for gRPC‑Web.
141-181: Prefer oneof with empty messages over boolean flags for filter kind.Boolean members in a oneof serialize as false when unset, which is ambiguous in JS/TS. Use empty message markers for clarity.
-message PlatformFilterV0 { - oneof kind { - bool all = 1; - bool block_committed = 2; - StateTransitionResultFilter state_transition_result = 3; - } -} +message PlatformFilterV0 { + message All {} + message BlockCommitted {} + oneof kind { + All all = 1; + BlockCommitted block_committed = 2; + StateTransitionResultFilter state_transition_result = 3; + } +}Improves client ergonomics and reduces accidental misuse.
183-208: Add a cursor/sequence to support ordering and resumption.For server‑streaming over gRPC‑Web, include a monotonic cursor in each message (response‑level is fine) so clients can resume after disconnects and detect gaps.
message PlatformSubscriptionResponseV0 { - string subscription_id = 1; - PlatformEventV0 event = 2; + string subscription_id = 1; + PlatformEventV0 event = 2; + uint64 cursor = 3; // increases by 1 per event on this stream }Optionally add eventIndexInBlock to BlockCommitted/StateTransitionFinalized.
Also applies to: 94-104
packages/rs-sdk/tests/fetch/platform_events.rs (1)
14-17: Drop the secondsetup_logs()call.The double invocation doesn’t break anything, but it’s redundant. Please remove the extra call to keep the test lean.
Apply this diff:
- setup_logs(); - setup_logs();packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (2)
549-553: Remove “placeholder” phrasing; clarify filter semanticsPublic headers shouldn’t say “Initial placeholder… to be refined”. Tighten the proto docs and regenerate.
Proposed proto comment:
// Filter for StateTransitionResult events. // When state_transition_hash is set, only events for that hash are delivered. message StateTransitionResultFilter { ... }
564-575: Prefer Empty over bool in oneof to avoid ambiguous presenceUsing bool inside oneof (all, block_committed) is easy to mis-set to false yet still occupy the variant. Use google.protobuf.Empty for presence-only selectors.
Proto sketch:
+import "google/protobuf/empty.proto"; message PlatformFilterV0 { oneof kind { - bool all = 1; - bool block_committed = 2; + google.protobuf.Empty all = 1; + google.protobuf.Empty block_committed = 2; StateTransitionResultFilter state_transition_result = 3; } }Also applies to: 581-586
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpackages/dashmate/templates/platform/gateway/envoy.yaml.dotis excluded by!**/*.dot
📒 Files selected for processing (35)
.github/scripts/check-grpc-coverage.py(1 hunks)packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java(8 hunks)packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h(3 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m(1 hunks)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py(4 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts(1 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.js(2 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts(3 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js(2 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/dapi-grpc/src/lib.rs(1 hunks)packages/dashmate/configs/defaults/getBaseConfigFactory.js(1 hunks)packages/dashmate/configs/getConfigFileMigrationsFactory.js(5 hunks)packages/dashmate/docker-compose.yml(1 hunks)packages/dashmate/docs/config/gateway.md(1 hunks)packages/dashmate/docs/services/gateway.md(1 hunks)packages/dashmate/src/config/configJsonSchema.js(3 hunks)packages/rs-dapi/README.md(1 hunks)packages/rs-dapi/src/config/mod.rs(2 hunks)packages/rs-dapi/src/config/tests.rs(4 hunks)packages/rs-dapi/src/services/platform_service/mod.rs(2 hunks)packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs(1 hunks)packages/rs-dapi/src/services/streaming_service/block_header_stream.rs(3 hunks)packages/rs-dapi/src/services/streaming_service/masternode_list_stream.rs(2 hunks)packages/rs-dapi/src/services/streaming_service/mod.rs(2 hunks)packages/rs-dapi/src/services/streaming_service/transaction_stream.rs(1 hunks)packages/rs-drive-abci/src/query/service.rs(5 hunks)packages/rs-drive-abci/tests/strategy_tests/main.rs(1 hunks)packages/rs-sdk/examples/platform_events.rs(1 hunks)packages/rs-sdk/src/sdk.rs(6 hunks)packages/rs-sdk/tests/fetch/config.rs(1 hunks)packages/rs-sdk/tests/fetch/platform_events.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/rs-drive-abci/tests/strategy_tests/main.rs
- packages/rs-dapi/src/services/platform_service/mod.rs
- packages/rs-sdk/src/sdk.rs
- packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/block_header_stream.rspackages/rs-dapi/src/config/mod.rspackages/rs-dapi/src/services/streaming_service/masternode_list_stream.rspackages/rs-dapi/src/services/streaming_service/mod.rspackages/dapi-grpc/src/lib.rspackages/rs-dapi/src/services/platform_service/subscribe_platform_events.rspackages/rs-dapi/src/services/streaming_service/transaction_stream.rspackages/rs-sdk/tests/fetch/platform_events.rspackages/rs-dapi/src/config/tests.rspackages/rs-sdk/examples/platform_events.rspackages/rs-drive-abci/src/query/service.rspackages/rs-sdk/tests/fetch/config.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/block_header_stream.rspackages/rs-dapi/src/config/mod.rspackages/rs-dapi/src/services/streaming_service/masternode_list_stream.rspackages/rs-dapi/src/services/streaming_service/mod.rspackages/rs-dapi/src/services/platform_service/subscribe_platform_events.rspackages/rs-dapi/src/services/streaming_service/transaction_stream.rspackages/rs-sdk/tests/fetch/platform_events.rspackages/rs-dapi/src/config/tests.rspackages/rs-sdk/examples/platform_events.rspackages/rs-drive-abci/src/query/service.rspackages/rs-sdk/tests/fetch/config.rs
packages/**/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests alongside each package in packages//tests
Files:
packages/rs-sdk/tests/fetch/platform_events.rspackages/rs-sdk/tests/fetch/config.rs
packages/**/tests/**/*.{js,ts,jsx,tsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/tests/**/*.{js,ts,jsx,tsx,rs}: Name tests descriptively, starting with “should …”
Unit/integration tests must not perform network calls; mock dependencies instead
Files:
packages/rs-sdk/tests/fetch/platform_events.rspackages/rs-sdk/tests/fetch/config.rs
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/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.tspackages/dashmate/configs/defaults/getBaseConfigFactory.jspackages/dashmate/src/config/configJsonSchema.jspackages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb.d.tspackages/dapi-grpc/clients/platform/v0/web/platform_pb.jspackages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb_service.js
🧠 Learnings (39)
📓 Common learnings
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
📚 Learning: 2025-10-15T14:45:30.856Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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.
Applied to files:
packages/dashmate/docker-compose.ymlpackages/rs-dapi/README.mdpackages/dashmate/docs/config/gateway.mdpackages/dashmate/docs/services/gateway.mdpackages/dashmate/configs/defaults/getBaseConfigFactory.jspackages/rs-dapi/src/config/tests.rspackages/dashmate/src/config/configJsonSchema.jspackages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2025-10-29T10:31:02.449Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2827
File: packages/dashmate/configs/defaults/getBaseConfigFactory.js:254-255
Timestamp: 2025-10-29T10:31:02.449Z
Learning: In dashmate configs (packages/dashmate/configs/defaults/getBaseConfigFactory.js), the `waitForStResultTimeout` for rs-dapi (120000ms / 120s) is intentionally set lower than the gateway listener timeout (125s). This design allows rs-dapi to execute its own timeout handling and return a proper response before the gateway times out, ensuring better error handling.
Applied to files:
packages/dashmate/docker-compose.ymlpackages/rs-dapi/README.mdpackages/dashmate/docs/config/gateway.mdpackages/dashmate/docs/services/gateway.mdpackages/dashmate/configs/defaults/getBaseConfigFactory.jspackages/dashmate/src/config/configJsonSchema.jspackages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-10-04T09:08:48.470Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-platform-version/src/version/protocol_version.rs:155-157
Timestamp: 2024-10-04T09:08:48.470Z
Learning: In the dashmate default configurations, the default protocol version for local networks has been removed, and an update mechanism for the protocol version in the consensus parameters has been implemented.
Applied to files:
packages/dashmate/docker-compose.ymlpackages/dashmate/docs/config/gateway.mdpackages/dashmate/src/config/configJsonSchema.js
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:105-105
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In `run_block_proposal` in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, when retrieving `last_block_time_ms`, it's acceptable to use `platform_state` instead of `block_platform_state`, even after updating the protocol version.
Applied to files:
packages/rs-dapi/src/services/streaming_service/block_header_stream.rs
📚 Learning: 2024-10-22T10:53:12.111Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2259
File: packages/rs-dapi-client/src/dapi_client.rs:137-139
Timestamp: 2024-10-22T10:53:12.111Z
Learning: In `packages/rs-dapi-client/src/dapi_client.rs`, when passing data into asynchronous code, ensure that data structures are `Send + Sync`. Using `Arc<AtomicUsize>` is necessary for the retry counter.
Applied to files:
packages/rs-dapi/src/services/streaming_service/block_header_stream.rspackages/rs-dapi/src/services/streaming_service/masternode_list_stream.rspackages/rs-dapi/src/services/streaming_service/mod.rs
📚 Learning: 2025-01-23T09:23:32.740Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/rs-sdk/src/sync.rs:88-95
Timestamp: 2025-01-23T09:23:32.740Z
Learning: The `block_on` function in `packages/rs-sdk/src/sync.rs` is currently only used in tests, and its WebAssembly implementation is deferred until there's a user request for it.
Applied to files:
packages/rs-dapi/src/services/streaming_service/block_header_stream.rspackages/rs-sdk/tests/fetch/platform_events.rs
📚 Learning: 2024-10-19T06:10:00.808Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2255
File: packages/dashmate/src/status/scopes/platform.js:112-116
Timestamp: 2024-10-19T06:10:00.808Z
Learning: In the Dashmate project, when fetching from Tenderdash RPC endpoints like `/status`, `/net_info`, and `/abci_info`, if one request fails, it's likely that the others will fail as well, so individual error handling for each fetch may not be necessary.
Applied to files:
packages/rs-dapi/src/services/streaming_service/mod.rspackages/dashmate/docs/services/gateway.md
📚 Learning: 2025-09-03T16:37:11.605Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2756
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:11-11
Timestamp: 2025-09-03T16:37:11.605Z
Learning: In packages/rs-dpp/Cargo.toml, the abci feature already includes core_rpc_client, and core_rpc_client is defined as ["dep:dashcore-rpc"]. This means rs-drive-abci can access dashcore-rpc types through dpp when using the abci feature.
Applied to files:
packages/rs-dapi/src/services/streaming_service/mod.rs
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Use PascalCase for Rust types
Applied to files:
packages/dapi-grpc/src/lib.rs
📚 Learning: 2024-10-18T15:39:51.172Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2254
File: packages/rs-sdk/src/sdk.rs:585-585
Timestamp: 2024-10-18T15:39:51.172Z
Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.
Applied to files:
packages/dapi-grpc/src/lib.rspackages/rs-sdk/tests/fetch/platform_events.rspackages/rs-sdk/examples/platform_events.rspackages/rs-sdk/tests/fetch/config.rs
📚 Learning: 2024-10-08T13:28:03.529Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2227
File: packages/rs-drive-abci/src/platform_types/platform_state/mod.rs:141-141
Timestamp: 2024-10-08T13:28:03.529Z
Learning: When converting `PlatformStateV0` to `PlatformStateForSavingV1` in `packages/rs-drive-abci/src/platform_types/platform_state/mod.rs`, only version `0` needs to be handled in the match on `platform_state_for_saving_structure_default` because the changes are retroactive.
Applied to files:
packages/dapi-grpc/src/lib.rspackages/rs-drive-abci/src/query/service.rspackages/dapi-grpc/protos/platform/v0/platform.proto
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Keep Rust code clippy-clean (cargo clippy)
Applied to files:
packages/dapi-grpc/src/lib.rspackages/rs-sdk/tests/fetch/config.rs
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Use SCREAMING_SNAKE_CASE for Rust constants
Applied to files:
packages/dapi-grpc/src/lib.rs
📚 Learning: 2024-10-29T10:42:00.521Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/Cargo.toml:22-22
Timestamp: 2024-10-29T10:42:00.521Z
Learning: In `packages/rs-dapi-client/Cargo.toml`, `backon` will not work without the `tokio-sleep` feature in our setup, so it's unnecessary to declare `tokio-sleep` as a separate feature in the `[features]` section.
Applied to files:
packages/dapi-grpc/src/lib.rs
📚 Learning: 2024-11-20T10:01:50.837Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs:94-94
Timestamp: 2024-11-20T10:01:50.837Z
Learning: In `packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs`, when converting with `PublicKey::try_from`, it's acceptable to use `.expect()` to handle potential conversion errors.
Applied to files:
packages/dapi-grpc/src/lib.rspackages/rs-sdk/tests/fetch/platform_events.rs
📚 Learning: 2024-10-10T10:30:19.883Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:90-95
Timestamp: 2024-10-10T10:30:19.883Z
Learning: In `packages/rs-sdk/src/mock/sdk.rs`, the `load_expectations` method in `MockDashPlatformSdk` remains asynchronous (`async`) for backward compatibility, even though it now delegates to the synchronous `load_expectations_sync` method.
Applied to files:
packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rspackages/rs-sdk/tests/fetch/platform_events.rspackages/rs-sdk/examples/platform_events.rs
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-dapi/src/services/streaming_service/transaction_stream.rspackages/rs-sdk/tests/fetch/platform_events.rspackages/rs-sdk/examples/platform_events.rspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2024-10-30T11:19:59.163Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
Applied to files:
packages/rs-sdk/tests/fetch/platform_events.rspackages/rs-sdk/examples/platform_events.rspackages/rs-sdk/tests/fetch/config.rs
📚 Learning: 2024-11-25T01:17:02.001Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2347
File: packages/rs-drive/tests/query_tests.rs:438-460
Timestamp: 2024-11-25T01:17:02.001Z
Learning: In Rust test files (`packages/rs-drive/tests/query_tests.rs`), when code is used only in tests, defining explicit enums for fields (like the `status` field in the `Withdrawal` struct) may not be necessary; using primitive types is acceptable.
Applied to files:
packages/rs-sdk/tests/fetch/platform_events.rs
📚 Learning: 2024-11-20T16:05:40.200Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs:148-151
Timestamp: 2024-11-20T16:05:40.200Z
Learning: In `packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs`, when converting public keys from `QuorumForSavingV0` to `VerificationQuorum`, it's acceptable to use `.expect()` for public key conversion, as the conversion has been verified and panics are acceptable in this context.
Applied to files:
packages/rs-sdk/tests/fetch/platform_events.rs
📚 Learning: 2024-11-20T20:43:41.185Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/tests/strategy_tests/masternodes.rs:212-220
Timestamp: 2024-11-20T20:43:41.185Z
Learning: In `packages/rs-drive-abci/tests/strategy_tests/masternodes.rs`, the pattern of generating a `PrivateKey`, converting it to bytes, and reconstructing a `BlsPrivateKey` from those bytes is intentional. Avoid suggesting to simplify this code in future reviews.
Applied to files:
packages/rs-sdk/tests/fetch/platform_events.rs
📚 Learning: 2024-10-29T14:13:35.584Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/dapi-grpc/src/mock.rs:95-109
Timestamp: 2024-10-29T14:13:35.584Z
Learning: In test code (e.g., `packages/dapi-grpc/src/mock.rs`), panicking on deserialization errors is acceptable behavior.
Applied to files:
packages/rs-sdk/tests/fetch/platform_events.rs
📚 Learning: 2024-11-03T10:39:11.242Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2305
File: packages/rs-drive-abci/src/abci/handler/finalize_block.rs:81-0
Timestamp: 2024-11-03T10:39:11.242Z
Learning: In `packages/rs-drive-abci/src/abci/handler/finalize_block.rs`, the use of `.expect("commit transaction")` after `app.commit_transaction(platform_version)` is intentional due to the provided comment explaining its necessity. Do not flag this usage in future reviews.
Applied to files:
packages/rs-sdk/tests/fetch/platform_events.rs
📚 Learning: 2024-09-26T14:58:26.560Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2162
File: packages/dashmate/src/status/providers.js:87-96
Timestamp: 2024-09-26T14:58:26.560Z
Learning: In the `checkPortStatus` function, it's better to use `options.timeout` to set the socket timeout for both request and response.
Applied to files:
packages/dashmate/configs/defaults/getBaseConfigFactory.js
📚 Learning: 2024-10-06T16:18:07.994Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
Applied to files:
packages/rs-drive-abci/src/query/service.rspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2025-09-05T06:00:56.491Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2761
File: packages/rs-drive/src/query/filter.rs:268-272
Timestamp: 2025-09-05T06:00:56.491Z
Learning: For document subscription filters in rs-drive, validation should not require index availability checks like queries do. Subscription filtering has different validation requirements than query optimization.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-10T10:30:24.653Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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-drive-abci/src/query/service.rs
📚 Learning: 2025-10-09T15:59:33.162Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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-drive-abci/src/query/service.rs
📚 Learning: 2024-10-29T14:40:54.727Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/core/transaction.rs:0-0
Timestamp: 2024-10-29T14:40:54.727Z
Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-30T11:04:33.634Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/platform/fetch_unproved.rs:0-0
Timestamp: 2024-10-30T11:04:33.634Z
Learning: In `packages/rs-sdk/src/platform/fetch_unproved.rs`, the `execute()` method consumes the request object, so cloning the request is necessary before passing it to `execute()` and `maybe_from_unproved_with_metadata`.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2025-05-28T16:22:26.334Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2644
File: packages/rs-drive/src/cache/system_contracts.rs:18-19
Timestamp: 2025-05-28T16:22:26.334Z
Learning: In packages/rs-drive/src/cache/system_contracts.rs, the `active_since_protocol_version` field in `ActiveSystemDataContract` struct is intentionally added for future use, not current implementation. QuantumExplorer confirmed this is "meant for later" when questioned about the `#[allow(unused)]` attribute.
Applied to files:
packages/rs-sdk/tests/fetch/config.rs
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/rs-sdk/tests/fetch/config.rspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2024-12-03T15:00:50.385Z
Learnt from: lklimek
Repo: dashpay/platform PR: 1924
File: packages/rs-sdk/src/sdk.rs:855-870
Timestamp: 2024-12-03T15:00:50.385Z
Learning: In `packages/rs-sdk/src/sdk.rs`, remember that `rustls_pemfile::certs` returns an iterator over `Result`, not a `Result` directly. Handle it accordingly in future code reviews.
Applied to files:
packages/rs-sdk/tests/fetch/config.rs
📚 Learning: 2024-10-24T05:00:56.657Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:83-85
Timestamp: 2024-10-24T05:00:56.657Z
Learning: An upcoming filter will be introduced to the `unconfirmed_txs` endpoint in `broadcastStateTransitionHandlerFactory.js`, as noted in the TODO comment. Therefore, optimizing transaction existence checks may not be necessary at this time.
Applied to files:
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.tspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2024-10-24T04:59:20.436Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:75-77
Timestamp: 2024-10-24T04:59:20.436Z
Learning: Tenderdash's `tx` RPC method accepts transaction hashes in base64 encoding, or in hex encoding if prefixed with `0x`. Therefore, in `packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js`, it's acceptable to use `stHash.toString('base64')` when calling `requestTenderRpc('tx', { hash: stHash.toString('base64') })`.
Applied to files:
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
📚 Learning: 2025-02-03T23:39:10.579Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-02-03T23:39:10.579Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
📚 Learning: 2024-10-25T09:33:50.383Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2274
File: packages/js-dapi-client/lib/transport/GrpcTransport/createGrpcTransportError.js:127-127
Timestamp: 2024-10-25T09:33:50.383Z
Learning: In `createGrpcTransportError.js`, the size of consensus error data is already validated by gRPC transport, so adding an additional size check before base64 decoding in the `createGrpcTransportError` function is unnecessary.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
📚 Learning: 2025-10-09T15:59:18.338Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsReader.js:325-334
Timestamp: 2025-10-09T15:59:18.338Z
Learning: In `packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsReader.js`, the continuous sync restart intentionally uses `Math.max(1, lastSyncedBlockHeight)` rather than `lastSyncedBlockHeight + 1` because the last block's processing status is uncertain at restart time. This conservative approach ensures no blocks are missed by reprocessing the last synced block, accepting potential duplicate processing for data completeness.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
🪛 markdownlint-cli2 (0.18.1)
packages/dashmate/docs/services/gateway.md
109-109: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
109-109: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
110-110: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
110-110: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
111-111: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
111-111: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
🪛 Ruff (0.14.2)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py
549-549: Unused method argument: request
(ARG002)
⏰ 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). (17)
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- 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: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
🔇 Additional comments (30)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (3)
252-256: Client stub correctly implements server streaming.The
unary_streampattern is appropriate for this subscription use case, allowing the client to send one request and receive multiple event responses.
798-802: Handler registration correctly configured.The
unary_stream_rpc_method_handlerproperly wires the server-streaming RPC with appropriate serialization.
1612-1627: Experimental API correctly mirrors main implementation.The experimental
subscribePlatformEventsmethod properly implements server streaming with the same endpoint and payload types.packages/rs-dapi/src/services/streaming_service/transaction_stream.rs (1)
173-174: LGTM!The timeout is correctly applied to both historical and combined streaming modes, as it's scheduled before the mode check. The 600-second default (10 minutes) provides reasonable protection against hanging streams.
packages/rs-dapi/src/config/mod.rs (1)
88-99: LGTM!The new timeout configuration fields are well-structured and follow existing patterns. The 10-minute default is appropriate for long-running streaming subscriptions.
packages/rs-dapi/src/services/streaming_service/block_header_stream.rs (3)
3-3: LGTM!Import added for timeout functionality.
84-85: LGTM!Timeout correctly applied to historical streaming mode.
105-106: LGTM!Timeout correctly applied to combined streaming mode, consistent with the historical mode implementation.
packages/rs-dapi/README.md (1)
25-26: LGTM!The documentation clearly describes the new timeout environment variables with accurate defaults.
packages/dashmate/docs/config/gateway.md (1)
25-25: LGTM!The documentation update correctly reflects the new timeout configuration location and helpfully notes the Envoy grace period behavior.
packages/rs-dapi/src/services/streaming_service/masternode_list_stream.rs (2)
6-6: LGTM!Import added for timeout functionality.
24-25: LGTM!Timeout correctly applied to masternode list streaming, consistent with other streaming service implementations.
packages/dashmate/docker-compose.yml (1)
153-155: LGTM!The environment variable mappings correctly reflect the config structure migration, with the old flat
waitForStResultTimeoutnow nested undertimeouts.waitForStateTransitionResult, and new timeout variables added for platform events and core streams.packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (5)
2242-2248: Service binding for server‑streaming is correct.asyncServerStreamingCall with the new method ID is properly wired.
3499-3499: Method ID assignment looks consistent.METHODID_SUBSCRIBE_PLATFORM_EVENTS = 47 follows sequential numbering.
3706-3709: MethodHandlers case correctly routes to subscribePlatformEvents.Switch wiring matches SERVER_STREAMING path.
3818-3818: ServiceDescriptor includes subscribePlatformEvents.Descriptor registration completed; no issues.
1475-1504: Now let me verify the Javadoc typos mentioned in the original review:Now I need to search for and display the Javadoc areas to verify the typo claims:
Based on the verification results, the naming consistency check passed—no stale PascalCase references were found, and proto comments in protobuf 2.5+ generate Javadoc in the Java code, meaning any typos originate from the
.protofile. The verification script confirmed the proto file uses correct lowerCamel casing at line 181.However, the original review mentions Javadoc typos ("subscribint") at specific line ranges (1898–1909, 2242–2248, 2658–2670), but only the descriptor snippet (lines 1475–1504) was provided. To finalize the review assessment, those Javadoc areas must be inspected directly.
RPC descriptor: casing and streaming type are correct.
Naming verification passed—no stale PascalCase found; proto file uses
subscribePlatformEvents(lowerCamel) at line 181. MethodDescriptor wiring usesSERVER_STREAMINGconsistently as expected.Verify the Javadoc areas mentioned in the original review (ImplBase method around line 1898–1909 and stub implementations) to confirm reported typos and recommend regenerating from the proto file to fix them, if found.
packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (2)
430-437: Typings for server‑streaming look correct.
requestStream=false, responseStream=true with proper request/response types. LGTM.
946-947: Client API shape matches grpc-web streaming conventions.
Returning ResponseStream with optional metadata is consistent.packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs (1)
34-47: Proxy stream implementation looks solid. Confirm error metric semantics.Flow, backpressure, and lifecycle are sane; duplicate timeout/error emissions won’t surface due to channel close. Please confirm that counting a locally generated deadline_exceeded as a forwarded_error matches your metrics intent.
Also applies to: 54-67, 70-101
packages/rs-drive-abci/src/query/service.rs (2)
281-320: FilterAdapter semantics look correct. Keepalives bypass filtering.
Matches All, BlockCommitted, and STR with optional tx_hash; keepalives always pass. LGTM.
894-914: Good validation of filter.kind and keepalive bounds.
Returning InvalidArgument for missing kind and out-of-range keepalive aligns with API docs.packages/dapi-grpc/protos/platform/v0/platform.proto (1)
29-49: Solid presence‑based filter design.
Using empty messages in oneof avoids the bool-in-oneof ambiguity. Event envelope and metadata look fine.Also applies to: 51-75
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m (2)
320-337: New message declarations align with the proto changes.All Objective-C class declarations for the new event, filter, and subscription message types are present and match the schema additions.
409-1047: Descriptor wiring for subscriptions/events looks correct.The descriptor setups cover the expected oneofs, field flags, and containment relations for the new subscription request/response messages, filters, and event payloads. No issues spotted.
packages/dapi-grpc/src/lib.rs (1)
24-35: Allowing generated naming is appropriate here.Good call adding the non-CamelCase allowance so the generated platform bindings stay warning-free without touching the proto output.
packages/rs-sdk/examples/platform_events.rs (1)
89-143: Example flow looks solid.Appreciate the comprehensive sample—covering specific filters plus Ctrl+C teardown makes it easy for devs to reuse.
packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (1)
463-9203: LGTM! Auto-generated protobuf code follows standard patterns.This is correctly generated JavaScript protobuf code from protocol buffer definitions. The implementation follows all standard Google Protocol Buffer patterns for JavaScript:
- Proper message constructors with
jspb.Message.initialize- Correct oneof group handling for versioned messages
- Standard serialization/deserialization methods
- Appropriate getter/setter/clear/has accessors for all fields
- Proper JSDoc type annotations
The new platform event subscription API surface (PlatformSubscriptionRequest, PlatformSubscriptionResponse, PlatformEventV0, filters, etc.) is correctly generated. Any API design concerns should be addressed in the source
.protofiles rather than in this generated output.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
493-498: Server-side keepalive validation is already implemented correctly; range enforcement is in placeThe codebase already handles the concerns raised:
- Range validation: Server rejects values outside the 25–300 range with a clear error message
- Semantic interpretation of 0: Server treats keepalive == 0 as disabled (None), distinct from invalid out-of-range values
- Client-side safeguard: SDK clamps computed keepalive to the valid range [25, 300]
- Implicit optional semantics: The generated code uses optional field semantics despite proto3's lack of explicit presence
The Objective-C file under review is generated from the .proto definition, so manual edits would be overwritten. Switching to
google.protobuf.UInt32Valuewould require a protocol-breaking change without additional benefit, given that the current implementation semantically distinguishes "0 = disabled" from invalid values through server-side validation.
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (1)
1898-1909: Polish Javadoc handshake phrasing via proto comments, then regenerate.Wording “it sends handshake response with empty event” is slightly awkward; tweak in platform.proto and regen so all languages inherit it. For example:
- // Once connected, it sends handshake response with empty event. + // On connect, the server sends an initial handshake response (empty event) before real events.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (2)
603-616: Unify naming: StateTransitionResult (filter) vs StateTransitionFinalized (event); also rename txHash → stateTransitionHashFilter uses StateTransitionResult while event uses StateTransitionFinalized, and fields use txHash vs stateTransitionHash elsewhere (e.g., WaitForStateTransitionResult*). This cross-language/public API inconsistency will confuse SDKs and users. Prefer “StateTransitionResult” and “state_transition_hash” across proto and regenerate all bindings.
Proto-level sketch (edit platform.proto, then regenerate):
message PlatformEventV0 { oneof event { BlockCommitted block_committed = 1; - StateTransitionFinalized state_transition_finalized = 2; + StateTransitionResult state_transition_result = 2; Keepalive keepalive = 3; } } -message StateTransitionFinalized { +message StateTransitionResult { BlockMetadata meta = 1; - bytes tx_hash = 2; + bytes state_transition_hash = 2; } message PlatformFilterV0 { oneof kind { AllEvents all = 1; BlockCommitted block_committed = 2; StateTransitionResultFilter state_transition_result = 3; } } message StateTransitionResultFilter { - bytes tx_hash = 1; + bytes state_transition_hash = 1; }Also update generated names in ObjC/TS/Java, and references in rs-* and clients.
Also applies to: 621-633, 688-701
573-575: Fix truncated doc comment for stateTransitionResult filterThe comment ends mid‑sentence. Replace with a complete note that references the filter type and oneof semantics.
Proto comment suggestion (in platform.proto):
// Subscribe to StateTransitionResult events (optionally filtered by // StateTransitionResultFilter). Setting this clears other 'kind' variants. StateTransitionResultFilter state_transition_result = 3;packages/dapi-grpc/protos/platform/v0/platform.proto (1)
177-183: Fix grammar in subscription RPC doc.The RPC comment still omits the article that was requested previously (“handshake response with empty event”). Please update the proto comment so it reads “Once connected, it sends a handshake response with an empty event.” and drop the extra-spaced blank line. Regenerate the client bindings afterwards so the corrected wording propagates to Python, Objective-C, etc.
- // Allows subscribing to various Platform events. - // - // Once connected, it sends handshake response with empty event. + // Allows subscribing to various Platform events. + // + // Once connected, it sends a handshake response with an empty event.packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (1)
85-108: Resolve the clientSubscriptionId asymmetry before freezing the API.Response V0 still exposes
clientSubscriptionId, butPlatformSubscriptionRequestV0gives clients no field to set such an ID. That leaves the server inventing a “client” identifier, which is misleading for consumers that need to correlate streams. Either add an optionalclient_subscription_idto the request and echo it back, or rename the response field to a server-generatedsubscription_id. Please address this before the contract ships—this was flagged previously.message PlatformSubscriptionRequestV0 { PlatformFilterV0 filter = 1; - uint32 keepalive = 2; + uint32 keepalive = 2; + string client_subscription_id = 3; // optional: echo in responses } message PlatformSubscriptionResponseV0 { - string client_subscription_id = 1; + string subscription_id = 1; // echo client's ID or server-generated PlatformEventV0 event = 2; }packages/rs-drive-abci/src/query/service.rs (1)
69-83: Completed subscription tasks still accumulate in the workers Vec.The
Dropimplementation ensures cleanup on service shutdown, which is good. However, when a subscription completes normally (e.g., client disconnects), itsJoinHandleremains in theVecindefinitely. Over the service's lifetime, this causes unbounded memory growth as subscription handles accumulate.Consider one of these approaches:
Option 1 (simplest): Use
tokio::spawndirectly at line 1017 and drop the handle immediately, removing theworkersfield entirely:- let worker = tokio::task::spawn(async move { + tokio::spawn(async move { ... }); - self.workers.lock().unwrap().push(worker);Then remove the
workersfield andDropimpl.Option 2: Implement a background reaper task that periodically drains finished tasks from the
JoinSet/Vec.
🧹 Nitpick comments (8)
packages/rs-sdk/examples/platform_events.rs (3)
98-101: Log hex decode failures for better user experience.Silent failure on invalid hex input can confuse users. Consider logging a warning when decode fails.
Apply this diff to warn on decode failures:
- let tx_hash_bytes = config - .state_transition_tx_hash_hex - .as_deref() - .and_then(|s| hex::decode(s).ok()); + let tx_hash_bytes = config + .state_transition_tx_hash_hex + .as_deref() + .and_then(|s| { + hex::decode(s).map_err(|e| { + tracing::warn!("Failed to decode tx hash hex: {}", e); + e + }).ok() + });
142-142: Consider handling worker task results.Ignoring join results means panics or errors in worker tasks are silently lost, making debugging harder.
For better observability:
- let _ = tokio::join!(block_worker, str_worker, all_worker); + let (r1, r2, r3) = tokio::join!(block_worker, str_worker, all_worker); + for result in [r1, r2, r3] { + if let Err(e) = result { + tracing::error!("Worker task failed: {}", e); + } + }
145-193: Use consistent logging throughout the worker function.The function mixes
println!/eprintln!(lines 168, 187, 192) withtracing::info!(lines 157, 178), which is inconsistent and bypasses the configured logging framework.Apply this diff for consistency:
PlatformEvent::StateTransitionFinalized(r) => { if let Some(meta) = r.meta { - println!( + info!( "{label}: id={sub_id} height={} tx_hash=0x{} block_id_hash=0x{}", meta.height, hex::encode(r.tx_hash), hex::encode(meta.block_id_hash) ); } } Err(status) => { - eprintln!("{label}: stream error {status}"); + tracing::error!("{label}: stream error {status}"); break; } } } - println!("{label}: stream closed"); + info!("{label}: stream closed"); }packages/dashmate/configs/getConfigFileMigrationsFactory.js (2)
1167-1175: Remove redundant initialization ofdeprecatedconfig.The migration initializes
options.platform.dapi.deprecatedfrom defaults (lines 1167-1175) but then deletes it shortly after in the same migration (line 1338). This initialization is unnecessary since the value is immediately discarded.Consider removing the initialization block (lines 1167-1175) entirely, as the deletion at line 1338 is the intended final state:
- if (!options.platform.dapi.deprecated) { - if (defaultConfig.has('platform.dapi.deprecated')) { - options.platform.dapi.deprecated = defaultConfig.get('platform.dapi.deprecated'); - } - } else if (typeof options.platform.dapi.deprecated.enabled === 'undefined') { - if (defaultConfig.has('platform.dapi.deprecated.enabled')) { - options.platform.dapi.deprecated.enabled = defaultConfig.get('platform.dapi.deprecated.enabled'); - } - } -Also applies to: 1337-1339
1416-1444: Refactor timeout normalization to improve clarity.The timeout normalization logic accesses
options.platform.dapi.api.timeoutsandoptions.platform.dapi.api.waitForStResultTimeout(lines 1417, 1419) afteroptions.platform.dapi.apiwas already deleted at line 1389. While the optional chaining prevents runtime errors, these accesses will always yieldundefined, making the fallback chains misleading. Additionally, the subsequent deletions at lines 1434-1440 attempt to remove fields from the already-deletedapiobject, making them no-ops.The timeout migration at lines 1380-1390 already handles copying
waitForStResultTimeoutfromapitorsDapibefore deletingapi. The later access at line 1419 is redundant.Consider restructuring to make the logic flow clearer:
// Normalize rsDapi timeout structure and remove redundant legacy fields const existingTimeouts = options.platform.dapi.rsDapi.timeouts - ?? options.platform.dapi.api?.timeouts; + ?? {}; const waitForStateTransitionResult = existingTimeouts?.waitForStateTransitionResult - ?? options.platform?.dapi?.api?.waitForStResultTimeout ?? 120000; const subscribePlatformEvents = existingTimeouts?.subscribePlatformEvents ?? 600000; const coreStreams = existingTimeouts?.coreStreams ?? 600000; options.platform.dapi.rsDapi.timeouts = { waitForStateTransitionResult, subscribePlatformEvents, coreStreams, }; if (typeof rsDapi.waitForStResultTimeout !== 'undefined') { delete rsDapi.waitForStResultTimeout; } - - if (options.platform?.dapi?.api?.timeouts) { - delete options.platform.dapi.api.timeouts; - } - - if (typeof options.platform?.dapi?.api?.waitForStResultTimeout !== 'undefined') { - delete options.platform.dapi.api.waitForStResultTimeout; - } -This removes the confusing attempts to access and delete from the already-deleted
apiobject, since those values were already migrated earlier at lines 1380-1390.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (2)
682-683: Clarify what txCount countsSpecify that txCount is the number of state transitions included in the committed block (not Core txs). Update proto comment.
495-500: Keepalive: document default and server-side validation/clampingDocs provide range (25–300s, 0 disables) but not default or enforcement. Please document the default interval and confirm server clamps out-of-range values (and whether per-connection overrides apply). Edit proto comments accordingly.
packages/rs-drive-abci/src/query/service.rs (1)
970-970: Remove unnecessary.clone()on Copy type.
subscription_idisu64, which implementsCopy. The.clone()call is redundant.Apply this diff:
- subscription_id: subscription_id.clone(), + subscription_id,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java(8 hunks)packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js(3 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h(3 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m(1 hunks)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py(4 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts(1 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.js(3 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/dashmate/configs/defaults/getBaseConfigFactory.js(1 hunks)packages/dashmate/configs/getConfigFileMigrationsFactory.js(5 hunks)packages/rs-drive-abci/Cargo.toml(4 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_state_transition_identity_signed/v0/mod.rs(0 hunks)packages/rs-drive-abci/src/query/service.rs(5 hunks)packages/rs-sdk/examples/platform_events.rs(1 hunks)packages/rs-sdk/src/platform/events.rs(1 hunks)packages/wasm-drive-verify/tests/fuzz_tests.rs(1 hunks)
💤 Files with no reviewable changes (1)
- packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_state_transition_identity_signed/v0/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rs-sdk/src/platform/events.rs
- packages/dashmate/configs/defaults/getBaseConfigFactory.js
- packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.m
🧰 Additional context used
📓 Path-based instructions (5)
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/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb.jspackages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-sdk/examples/platform_events.rspackages/rs-drive-abci/src/query/service.rspackages/wasm-drive-verify/tests/fuzz_tests.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-sdk/examples/platform_events.rspackages/rs-drive-abci/src/query/service.rs
packages/**/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests alongside each package in packages//tests
Files:
packages/wasm-drive-verify/tests/fuzz_tests.rs
packages/**/tests/**/*.{js,ts,jsx,tsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/tests/**/*.{js,ts,jsx,tsx,rs}: Name tests descriptively, starting with “should …”
Unit/integration tests must not perform network calls; mock dependencies instead
Files:
packages/wasm-drive-verify/tests/fuzz_tests.rs
🧠 Learnings (37)
📚 Learning: 2025-09-03T16:37:11.605Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2756
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:11-11
Timestamp: 2025-09-03T16:37:11.605Z
Learning: In packages/rs-dpp/Cargo.toml, the abci feature already includes core_rpc_client, and core_rpc_client is defined as ["dep:dashcore-rpc"]. This means rs-drive-abci can access dashcore-rpc types through dpp when using the abci feature.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-12-05T09:29:38.918Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2375
File: packages/rs-drive-abci/Cargo.toml:61-63
Timestamp: 2024-12-05T09:29:38.918Z
Learning: In the `drive-abci` package, avoid adding unused dependencies like `hashbrown` to `Cargo.toml`. The team relies on CI to detect dependency version issues.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2025-01-19T07:36:46.042Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2431
File: packages/rs-drive/Cargo.toml:55-60
Timestamp: 2025-01-19T07:36:46.042Z
Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-10-29T10:42:00.521Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/Cargo.toml:22-22
Timestamp: 2024-10-29T10:42:00.521Z
Learning: In `packages/rs-dapi-client/Cargo.toml`, `backon` will not work without the `tokio-sleep` feature in our setup, so it's unnecessary to declare `tokio-sleep` as a separate feature in the `[features]` section.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-10-18T15:39:51.172Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2254
File: packages/rs-sdk/src/sdk.rs:585-585
Timestamp: 2024-10-18T15:39:51.172Z
Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.
Applied to files:
packages/rs-drive-abci/Cargo.tomlpackages/rs-sdk/examples/platform_events.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-drive-abci/tests/strategy_tests/main.rs:1162-1162
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the Rust test file `packages/rs-drive-abci/tests/strategy_tests/main.rs`, specific protocol versions like `PROTOCOL_VERSION_1` are intentionally used in tests instead of `PROTOCOL_VERSION_LATEST`.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2025-03-11T09:39:23.071Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2489
File: packages/rs-dpp/Cargo.toml:32-32
Timestamp: 2025-03-11T09:39:23.071Z
Learning: In the Dash Platform project, dependencies are currently managed using Git repository references with tags (repo+tag format in Cargo.toml) rather than published crates, as the team is not currently publishing crates to crates.io.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-10-10T10:30:25.283Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:129-130
Timestamp: 2024-10-10T10:30:25.283Z
Learning: In the codebase `packages/rs-sdk/src/mock/sdk.rs`, `tokio::sync::Mutex` is used instead of `std::sync::Mutex` because `std::sync::MutexGuard` is not `Send` and cannot be used in async contexts.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-10-10T10:30:19.883Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:90-95
Timestamp: 2024-10-10T10:30:19.883Z
Learning: In `packages/rs-sdk/src/mock/sdk.rs`, the `load_expectations` method in `MockDashPlatformSdk` remains asynchronous (`async`) for backward compatibility, even though it now delegates to the synchronous `load_expectations_sync` method.
Applied to files:
packages/rs-drive-abci/Cargo.tomlpackages/rs-sdk/examples/platform_events.rs
📚 Learning: 2024-10-29T14:16:00.141Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/dapi-grpc/src/mock.rs:125-145
Timestamp: 2024-10-29T14:16:00.141Z
Learning: Using `serde_json` encoding produces deterministic output required to identify requests and responses by hash, therefore `serde_json` is used as the basic serialization algorithm for mocking.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-09-27T08:23:18.028Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2139
File: packages/rs-sdk/Cargo.toml:0-0
Timestamp: 2024-09-27T08:23:18.028Z
Learning: `bincode` is used in the `mocks` feature of the `rs-sdk`.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2025-01-24T07:54:55.723Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2440
File: packages/rs-drive-abci/Cargo.toml:76-76
Timestamp: 2025-01-24T07:54:55.723Z
Learning: In the drive-abci package, bls-signatures is intentionally declared twice:
1. As an optional dependency in [dependencies] to make it an optional feature in production
2. As a non-optional dependency in [dev-dependencies] to ensure it's always available for tests
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-10-29T14:44:01.184Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/src/executor.rs:38-38
Timestamp: 2024-10-29T14:44:01.184Z
Learning: In `packages/rs-dapi-client/src/executor.rs`, for the structs `ExecutionError<E>` and `ExecutionResponse<R>`, only the serde derives (`Serialize`, `Deserialize`) should be conditional based on the "mocks" feature; the other derives (`Debug`, `Clone`, `Eq`, `PartialEq`) should always be present.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Keep Rust code clippy-clean (cargo clippy)
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-10-17T08:52:54.300Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2248
File: packages/rs-drive-abci/src/main.rs:106-111
Timestamp: 2024-10-17T08:52:54.300Z
Learning: In this project, the environment variable `CARGO_PKG_RUST_VERSION` is defined and can be used safely with the `env!` macro.
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-sdk-ffi/** : Keep iOS/FFI Rust SDK artifacts in packages/rs-sdk-ffi
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Use rustfmt defaults for Rust code (format via cargo fmt --all)
Applied to files:
packages/rs-drive-abci/Cargo.toml
📚 Learning: 2024-10-30T11:19:59.163Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
Applied to files:
packages/rs-drive-abci/Cargo.tomlpackages/rs-sdk/examples/platform_events.rspackages/wasm-drive-verify/tests/fuzz_tests.rs
📚 Learning: 2025-10-15T14:45:30.856Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb.js
📚 Learning: 2025-10-29T10:31:02.449Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2827
File: packages/dashmate/configs/defaults/getBaseConfigFactory.js:254-255
Timestamp: 2025-10-29T10:31:02.449Z
Learning: In dashmate configs (packages/dashmate/configs/defaults/getBaseConfigFactory.js), the `waitForStResultTimeout` for rs-dapi (120000ms / 120s) is intentionally set lower than the gateway listener timeout (125s). This design allows rs-dapi to execute its own timeout handling and return a proper response before the gateway times out, ensuring better error handling.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-10-04T09:08:48.470Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-platform-version/src/version/protocol_version.rs:155-157
Timestamp: 2024-10-04T09:08:48.470Z
Learning: In the dashmate default configurations, the default protocol version for local networks has been removed, and an update mechanism for the protocol version in the consensus parameters has been implemented.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-sdk/examples/platform_events.rspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2024-10-06T16:18:07.994Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
Applied to files:
packages/rs-sdk/examples/platform_events.rspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.hpackages/dapi-grpc/protos/platform/v0/platform.protopackages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-24T05:00:56.657Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:83-85
Timestamp: 2024-10-24T05:00:56.657Z
Learning: An upcoming filter will be introduced to the `unconfirmed_txs` endpoint in `broadcastStateTransitionHandlerFactory.js`, as noted in the TODO comment. Therefore, optimizing transaction existence checks may not be necessary at this time.
Applied to files:
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.hpackages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2024-10-08T13:28:03.529Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2227
File: packages/rs-drive-abci/src/platform_types/platform_state/mod.rs:141-141
Timestamp: 2024-10-08T13:28:03.529Z
Learning: When converting `PlatformStateV0` to `PlatformStateForSavingV1` in `packages/rs-drive-abci/src/platform_types/platform_state/mod.rs`, only version `0` needs to be handled in the match on `platform_state_for_saving_structure_default` because the changes are retroactive.
Applied to files:
packages/dapi-grpc/protos/platform/v0/platform.protopackages/rs-drive-abci/src/query/service.rs
📚 Learning: 2025-09-05T06:00:56.491Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2761
File: packages/rs-drive/src/query/filter.rs:268-272
Timestamp: 2025-09-05T06:00:56.491Z
Learning: For document subscription filters in rs-drive, validation should not require index availability checks like queries do. Subscription filtering has different validation requirements than query optimization.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-10T10:30:24.653Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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-drive-abci/src/query/service.rs
📚 Learning: 2025-10-09T15:59:33.162Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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-drive-abci/src/query/service.rs
📚 Learning: 2024-10-29T14:40:54.727Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/core/transaction.rs:0-0
Timestamp: 2024-10-29T14:40:54.727Z
Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.
Applied to files:
packages/rs-drive-abci/src/query/service.rspackages/wasm-drive-verify/tests/fuzz_tests.rs
📚 Learning: 2024-10-30T11:04:33.634Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/platform/fetch_unproved.rs:0-0
Timestamp: 2024-10-30T11:04:33.634Z
Learning: In `packages/rs-sdk/src/platform/fetch_unproved.rs`, the `execute()` method consumes the request object, so cloning the request is necessary before passing it to `execute()` and `maybe_from_unproved_with_metadata`.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-11-25T01:17:02.001Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2347
File: packages/rs-drive/tests/query_tests.rs:438-460
Timestamp: 2024-11-25T01:17:02.001Z
Learning: In Rust test files (`packages/rs-drive/tests/query_tests.rs`), when code is used only in tests, defining explicit enums for fields (like the `status` field in the `Withdrawal` struct) may not be necessary; using primitive types is acceptable.
Applied to files:
packages/wasm-drive-verify/tests/fuzz_tests.rs
📚 Learning: 2025-09-03T14:41:16.196Z
Learnt from: thephez
Repo: dashpay/platform PR: 2754
File: packages/wasm-sdk/AI_REFERENCE.md:766-766
Timestamp: 2025-09-03T14:41:16.196Z
Learning: In packages/wasm-sdk/, the AI_REFERENCE.md file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in AI_REFERENCE.md, as manual changes to AI_REFERENCE.md would be overwritten during regeneration.
Applied to files:
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h
📚 Learning: 2025-02-03T23:39:10.579Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-02-03T23:39:10.579Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
📚 Learning: 2024-10-25T09:33:50.383Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2274
File: packages/js-dapi-client/lib/transport/GrpcTransport/createGrpcTransportError.js:127-127
Timestamp: 2024-10-25T09:33:50.383Z
Learning: In `createGrpcTransportError.js`, the size of consensus error data is already validated by gRPC transport, so adding an additional size check before base64 decoding in the `createGrpcTransportError` function is unnecessary.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
📚 Learning: 2025-10-09T15:59:18.338Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsReader.js:325-334
Timestamp: 2025-10-09T15:59:18.338Z
Learning: In `packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsReader.js`, the continuous sync restart intentionally uses `Math.max(1, lastSyncedBlockHeight)` rather than `lastSyncedBlockHeight + 1` because the last block's processing status is uncertain at restart time. This conservative approach ensures no blocks are missed by reprocessing the last synced block, accepting potential duplicate processing for data completeness.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
📚 Learning: 2024-10-24T04:59:20.436Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:75-77
Timestamp: 2024-10-24T04:59:20.436Z
Learning: Tenderdash's `tx` RPC method accepts transaction hashes in base64 encoding, or in hex encoding if prefixed with `0x`. Therefore, in `packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js`, it's acceptable to use `stHash.toString('base64')` when calling `requestTenderRpc('tx', { hash: stHash.toString('base64') })`.
Applied to files:
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
🧬 Code graph analysis (8)
packages/rs-sdk/examples/platform_events.rs (2)
packages/rs-sdk/src/sdk.rs (3)
fmt(173-187)new(847-852)default(797-834)packages/rs-dapi-client/src/address_list.rs (1)
uri(65-67)
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
PlatformEventV0(215-240)PlatformFilterV0(117-142)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
PlatformSubscriptionRequest(9-24)PlatformSubscriptionResponse(63-78)
packages/rs-drive-abci/src/query/service.rs (5)
packages/rs-drive-abci/src/abci/app/full.rs (3)
platform(48-50)event_bus(60-62)new(37-44)packages/rs-drive-abci/src/abci/app/consensus.rs (3)
platform(51-53)event_bus(63-65)new(37-47)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (10)
Keepalive(333-342)PlatformEventV0(215-240)PlatformSubscriptionRequest(9-24)PlatformSubscriptionResponse(63-78)PlatformFilterV0(117-142)BlockCommitted(167-176)BlockCommitted(279-296)StateTransitionFinalized(305-324)Version(5947-5966)PlatformSubscriptionResponseV0(85-102)packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs (3)
mpsc(50-52)drop(25-27)new(18-21)packages/rs-sdk/src/platform/events.rs (1)
subscribe_platform_events(17-61)
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (2)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
PlatformSubscriptionRequest(9-24)PlatformSubscriptionResponse(63-78)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (2)
subscribePlatformEvents(549-558)subscribePlatformEvents(1613-1627)
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbrpc.h (2)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
PlatformSubscriptionRequest(9-24)PlatformSubscriptionResponse(63-78)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (2)
subscribePlatformEvents(549-558)subscribePlatformEvents(1613-1627)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (3)
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (1)
proto(16-16)packages/dapi-grpc/clients/drive/v0/nodejs/drive_protoc.js (1)
proto(16-16)packages/dapi-grpc/clients/drive/v0/web/drive_pb.js (1)
proto(16-16)
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (1)
proto(16-16)
🪛 Ruff (0.14.2)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py
549-549: Unused method argument: request
(ARG002)
⏰ 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). (20)
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- 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 JS packages / Build JS
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: Rust crates security audit
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
🔇 Additional comments (24)
packages/rs-drive-abci/Cargo.toml (3)
112-112: Reconsider including "mocks" and "console" in default features.Including test/debugging features (
mocks,console) in thedefaultfeature set means the production binary will include mock infrastructure and console debugging by default, which is unusual and potentially problematic (bloated binary, exposed internals, unintended debug behavior).Typically, these should be opt-in features. Verify this is intentional.
128-130: Cargo-machete ignore list is reasonable.Adding
rs-dash-event-busto the cargo-machete ignore list is appropriate if the crate is used indirectly (e.g., through trait bounds or type references) rather than direct imports.
76-82: All three dependencies are actively used.
- tokio-stream: Used in
packages/rs-drive-abci/src/query/service.rs:63to wrap MPSC receivers as streams.- sha2: Used in
packages/rs-drive-abci/src/abci/handler/finalize_block.rs:166to hash transactions (Sha256::digest(tx)).- rs-dash-event-bus: Extensively used across the codebase for event publishing and subscription (server initialization, query service, and finalize_block handlers).
packages/wasm-drive-verify/tests/fuzz_tests.rs (1)
73-73: LGTM! Correctly removes unnecessarymutkeyword.The binding doesn't need to be mutable since the object is mutated through JavaScript's interior mutability via
Reflect::setat line 84. This change makes the code more idiomatic for wasm-bindgen FFI.packages/rs-sdk/examples/platform_events.rs (3)
1-8: LGTM: Clean feature-gated entry point.The interaction between the sync main and the async
subscribe::main()works correctly because#[tokio::main]transforms the async function into a regular function with runtime setup.
31-67: LGTM: Well-structured configuration with secure password handling.Good use of
Zeroizing<String>for the password field, and the fallback to environment variables when the .env file is missing is appropriate for an example.
195-220: LGTM: SDK setup demonstrates proper configuration.The function correctly handles URI construction, core connection setup, and conditional CA certificate loading for non-WASM targets. The use of
.expect()with descriptive messages is acceptable for example code.packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (7)
2242-2248: bindService wiring for server stream is correct.Registered with asyncServerStreamingCall and correct METHODID.
2658-2670: Async stub Javadoc and call match server‑streaming.Uses asyncServerStreamingCall; docs now aligned.
3031-3043: Blocking stub correctly exposes Iterator for server stream.Javadoc and blockingServerStreamingCall usage are consistent.
3499-3499: Method ID added without collision.New METHODID_SUBSCRIBE_PLATFORM_EVENTS = 47 fits sequence.
3706-3709: MethodHandlers routing is correct.Routes to subscribePlatformEvents with proper types.
3818-3819: Service descriptor includes the new RPC.addMethod(getSubscribePlatformEventsMethod()) present.
1475-1504: Server-streaming descriptor wiring verified as correct; naming is consistent throughout.MethodType SERVER_STREAMING, lowerCamel full name ("subscribePlatformEvents"), marshallers, and schema descriptor all verified correct. Proto definition (platform.proto:182) matches Java code generation. No stale PascalCase references found.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m (2)
320-332: LGTM! Auto-generated forward declarations.The forward class declarations for the new subscription-related protobuf message types are correctly formatted and consistent with the existing code patterns in this file.
409-1121: LGTM! Auto-generated protobuf implementations.The implementations for all 13 new subscription-related message classes are correctly structured and follow standard Google Protocol Buffer Objective-C conventions:
- Thread-safe lazy descriptor initialization with proper recursion guards
- Correct storage structures with properly calculated field offsets
- Appropriate field flags and data types for all message, scalar, and bytes fields
- Proper oneof group setup for discriminated unions (version, kind, event)
- Nested message classes correctly reference their containing classes
Since this is auto-generated code from protobuf definitions, any modifications should be made to the source
.protofiles rather than this implementation file.packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (1)
463-9456: LGTM! Auto-generated protobuf bindings are correctly structured.This file contains auto-generated JavaScript protobuf client bindings for platform event subscription messages. The code follows standard protobuf JS patterns consistently:
- Proper export declarations for Google Closure Compiler
- Correct constructor definitions with inheritance from jspb.Message
- Complete serialization/deserialization implementation
- Appropriate handling of oneof groups, nested messages, and type conversions
- Consistent with other generated files in the codebase
Since this is auto-generated code, ensure the underlying protobuf definitions (platform.proto) are correct, as that's the true source of truth.
packages/rs-drive-abci/src/query/service.rs (7)
13-13: LGTM!The new imports and buffer constant properly support the event subscription infrastructure.
Also applies to: 51-51, 55-55, 59-59, 62-63, 66-66
94-102: LGTM!The constructor properly initializes the new
event_busandworkersfields, aligning with the event subscription infrastructure.
281-320: LGTM!The
PlatformFilterAdaptercorrectly implements filter matching semantics: keepalive events always pass through, and subscription filters are properly applied based on event type and optional transaction hash.
896-899: LGTM!The filter kind validation now correctly rejects requests with missing
filter.kind, addressing the previous review concern and preventing silent no-op subscriptions.
901-913: LGTM!Keepalive interval validation is sound: the 25–300 second range is sensible and clearly communicated to clients.
930-947: LGTM!The handshake message ensures the client receives the
subscription_idbefore events flow, enabling proper association of responses with the subscription request.
952-1010: LGTM!The event forwarding loop correctly handles:
- Real events from the subscription
- Keepalive timeout to emit keepalive messages
- Client disconnection via channel closure
The timeout-based keepalive mechanism is elegant and ensures the stream remains active per the client's requirements.
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
Outdated
Show resolved
Hide resolved
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs (1)
178-207: Fix move oftransaction_event.txafter matchingtransaction_event.result.Line 178 matches
transaction_event.result, which moves that field out of the struct. Later in the same branch (Line 186) we try to movetransaction_event.tx, but the struct is already partially moved, so this will not compile. Please destructure bothresultandtxup front (or match on a tuple) before moving either field.Apply this diff to keep ownership straight:
- match transaction_event.result { + let crate::clients::TransactionEvent { result, tx, .. } = transaction_event; + match result { crate::clients::TransactionResult::Success => { - if prove && let Some(tx_bytes) = transaction_event.tx { - match self.fetch_proof_for_state_transition(tx_bytes).await { + if prove { + if let Some(tx_bytes) = tx { + match self.fetch_proof_for_state_transition(tx_bytes).await { Ok((proof, metadata)) => { response_v0.result = Some( wait_for_state_transition_result_response::wait_for_state_transition_result_response_v0::Result::Proof(proof), ); response_v0.metadata = Some(metadata); } Err(e) => { debug!("Failed to fetch proof: {}", e); // Continue without proof } } + } }
♻️ Duplicate comments (2)
packages/rs-dapi/src/server/grpc.rs (2)
126-192: Emit the actual gRPC path in tracing fields.
Lines [128], [183], and [189] still calltracing::trace!(path, …), which recordspath=trueinstead of the URI. This was flagged previously and remains unresolved—please emit the real value using the%formatter (and reuse it in both branches) so we keep observability when debugging deadline issues.Apply this diff to log the real path everywhere:
- tracing::trace!( - path, + tracing::trace!( + path = %path, timeout = timeout.as_secs_f32(), "Applying timeout for gRPC method" ); ... - tracing::trace!( - path, - header_timeout = timeout_from_header.unwrap_or_default().as_secs_f32(), + tracing::trace!( + path = %path, + header_timeout = timeout_from_header.unwrap_or_default().as_secs_f32(), timeout = effective_timeout.as_secs_f32(), "Applying gRPC timeout from header" ); ... - tracing::trace!( - path, + tracing::trace!( + path = %path, timeout = effective_timeout.as_secs_f32(), "Applying default gRPC timeout" );
262-289: Add a timeout test for the real streaming URI.
Line [272] still uses"/test"; if this test exercised/org.dash.platform.dapi.v0.Platform/SubscribePlatformEvents, it would have caught the casing bug above. Please expand the test (or add a new one) that hits the actual streaming path so per-method timeouts stay covered.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/rs-dapi/src/server/grpc.rs(5 hunks)packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs(1 hunks)packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs(2 hunks)packages/rs-dapi/src/services/streaming_service/block_header_stream.rs(0 hunks)packages/rs-dapi/src/services/streaming_service/mod.rs(2 hunks)
💤 Files with no reviewable changes (1)
- packages/rs-dapi/src/services/streaming_service/block_header_stream.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-dapi/src/services/streaming_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/services/platform_service/wait_for_state_transition_result.rspackages/rs-dapi/src/server/grpc.rspackages/rs-dapi/src/services/platform_service/subscribe_platform_events.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/platform_service/wait_for_state_transition_result.rspackages/rs-dapi/src/server/grpc.rspackages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs
🧠 Learnings (9)
📓 Common learnings
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rspackages/rs-dapi/src/server/grpc.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:105-105
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In `run_block_proposal` in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, when retrieving `last_block_time_ms`, it's acceptable to use `platform_state` instead of `block_platform_state`, even after updating the protocol version.
Applied to files:
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs
📚 Learning: 2024-10-06T16:18:07.994Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
Applied to files:
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rspackages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs
📚 Learning: 2024-10-08T13:28:03.529Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2227
File: packages/rs-drive-abci/src/platform_types/platform_state/mod.rs:141-141
Timestamp: 2024-10-08T13:28:03.529Z
Learning: When converting `PlatformStateV0` to `PlatformStateForSavingV1` in `packages/rs-drive-abci/src/platform_types/platform_state/mod.rs`, only version `0` needs to be handled in the match on `platform_state_for_saving_structure_default` because the changes are retroactive.
Applied to files:
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs
📚 Learning: 2024-11-20T10:01:50.837Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs:94-94
Timestamp: 2024-11-20T10:01:50.837Z
Learning: In `packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs`, when converting with `PublicKey::try_from`, it's acceptable to use `.expect()` to handle potential conversion errors.
Applied to files:
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/rs-dapi/src/services/platform_service/wait_for_state_transition_result.rs
📚 Learning: 2024-10-17T08:52:46.847Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2248
File: packages/rs-drive-abci/src/main.rs:311-331
Timestamp: 2024-10-17T08:52:46.847Z
Learning: In our codebase, connections to the gRPC server happen within the local network, so timeouts are not expected, and adding timeouts is unnecessary.
Applied to files:
packages/rs-dapi/src/server/grpc.rs
📚 Learning: 2024-10-10T10:30:19.883Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:90-95
Timestamp: 2024-10-10T10:30:19.883Z
Learning: In `packages/rs-sdk/src/mock/sdk.rs`, the `load_expectations` method in `MockDashPlatformSdk` remains asynchronous (`async`) for backward compatibility, even though it now delegates to the synchronous `load_expectations_sync` method.
Applied to files:
packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs
🧬 Code graph analysis (1)
packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs (3)
packages/rs-drive-abci/src/query/service.rs (4)
mpsc(926-926)new(94-103)new(289-291)drop(77-82)packages/rs-dapi/src/services/platform_service/mod.rs (1)
new(111-164)packages/rs-dapi/src/metrics.rs (6)
new(373-375)platform_events_active_sessions_inc(500-502)platform_events_active_sessions_dec(505-507)platform_events_upstream_stream_started(520-522)platform_events_forwarded_event(510-512)platform_events_forwarded_error(515-517)
⏰ 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). (21)
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / 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 (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
🔇 Additional comments (1)
packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs (1)
52-77: Nice session guard to tie metrics to the stream lifetime.Keeping the guard inside the forwarder ensures active-session metrics stay accurate until the uplink loop actually exits. Clean and robust.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
packages/rs-drive-abci/src/query/service.rs (1)
915-915: Add missingrandimport.
rand::random::<u64>()is called without importing therandcrate, which will cause a compilation error.Add this import at the top of the file with other use statements:
+use rand::random;Then update line 915:
- let subscription_id = rand::random::<u64>(); + let subscription_id = random::<u64>();packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (2)
622-633: Unify event/filter naming and hash field names across APIFilter uses StateTransitionResult while event is StateTransitionFinalized, and fields use txHash vs stateTransitionHash. Align on “StateTransitionResult” and “state_transition_hash” to match WaitForStateTransitionResult*. Regenerate bindings after editing platform.proto.
Proto-level sketch:
message PlatformEventV0 { oneof event { BlockCommitted block_committed = 1; - StateTransitionFinalized state_transition_finalized = 2; + StateTransitionResult state_transition_result = 2; Keepalive keepalive = 3; } } -message StateTransitionFinalized { +message StateTransitionResult { BlockMetadata meta = 1; - bytes tx_hash = 2; + bytes state_transition_hash = 2; } message PlatformFilterV0 { oneof kind { AllEvents all = 1; BlockCommitted block_committed = 2; - StateTransitionResultFilter state_transition_result = 3; + StateTransitionResultFilter state_transition_result = 3; // unchanged name } } message PlatformFilterV0_StateTransitionResultFilter { - bytes tx_hash = 1; + bytes state_transition_hash = 1; }Also applies to: 687-702, 546-559
574-576: Complete the truncated doc comment for stateTransitionResult filterComment ends mid‑sentence. Replace with a full sentence and presence semantics.
Suggested proto comment (then regenerate):
// Subscribe to StateTransitionResult events (optionally filtered by // StateTransitionResultFilter). Setting this clears other 'kind' variants. StateTransitionResultFilter state_transition_result = 3;packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
183-205: Inconsistent naming: txHash vs stateTransitionHash; align with existing API.Elsewhere you use stateTransitionHash (e.g., WaitForStateTransitionResultRequestV0). Rename here for consistency and client ergonomics.
Proto-level suggestion:
-message StateTransitionResultFilter { - bytes tx_hash = 1; -} +message StateTransitionResultFilter { + bytes state_transition_hash = 1; +}
305-331: Event field naming: use stateTransitionHash instead of txHash.Stay consistent with request/filters to avoid confusion between Tenderdash tx vs Platform state transition terms.
Proto-level suggestion:
-message StateTransitionFinalized { - BlockMetadata meta = 1; - bytes tx_hash = 2; -} +message StateTransitionFinalized { + BlockMetadata meta = 1; + bytes state_transition_hash = 2; +}packages/dapi-grpc/protos/platform/v0/platform.proto (1)
30-50: Standardize ongoogle.protobuf.Emptyfor presence-only oneof fields.The code correctly uses local empty message types (
AllEvents,BlockCommitted) to avoid the bool-in-oneof ambiguity issue flagged in prior reviews. However, prefer the standard library pattern for clarity and consistency with proto3 idioms.Apply this diff:
+import "google/protobuf/empty.proto"; + // Criteria that must be met by platform event to be propagated to the user. message PlatformFilterV0 { - // Include all events generated by the Platform; experimental - message AllEvents{} - // Notify about every Platform (Tenderdash) block that gets committed (mined) - message BlockCommitted{} - // Filter for StateTransitionResult events, by state transition hash message StateTransitionResultFilter { // When set, only match StateTransitionResult events for this tx hash. optional bytes tx_hash = 1; } oneof kind { - AllEvents all = 1; // subscribe to all platform events - BlockCommitted block_committed = 2; // subscribe to BlockCommitted events only + google.protobuf.Empty all = 1; // subscribe to all platform events + google.protobuf.Empty block_committed = 2; // subscribe to BlockCommitted events only StateTransitionResultFilter state_transition_result = 3; // subscribe to StateTransitionResult events (optionally filtered by // tx_hash) } }packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (1)
8759-8766: uint64 zero-checks use parseInt(); regenerate from proto definitions.The
parseInt(f, 10) !== 0checks for uint64 string fields (height,timeMs) contradict proper uint64 handling as strings/BigInt used elsewhere (lines 8718, 8722, 8786-8787, 8804-8805). While the string value is ultimately serialized correctly, the zero-check should compare strings:if (f !== "0").Since this is generated code from
JsPbCodeGenerator, fix the proto generator configuration or proto files and regenerate rather than editing manually.
🧹 Nitpick comments (5)
packages/rs-drive-abci/src/query/service.rs (1)
901-913: LGTM! Subscription logic is well-implemented.The keepalive validation, handshake flow, and event forwarding logic are correct. The worker task properly handles events, timeouts, and client disconnections.
Optional: Consider periodically removing completed JoinHandles from
self.workersto prevent unbounded Vec growth, though the memory overhead is minimal since JoinHandles are lightweight:// Before pushing new worker at line 1017: let mut workers = self.workers.lock().unwrap(); workers.retain(|h| !h.is_finished()); workers.push(worker);This is optional and can be deferred if subscription churn is low.
Also applies to: 921-1020
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
495-501: Document keepalive validation and filtering behavior in proto fileThe code correctly validates keepalive bounds (rejects values outside [25, 300] and accepts 0) and always delivers keepalive events regardless of filter kind. However, the proto documentation should be enhanced for clarity.
Suggested proto comment refinement (lines ~9-10 in platform.proto):
// keepalive: interval in seconds (min 25, max 300; 0 disables). The server // rejects invalid intervals with INVALID_ARGUMENT. Keepalive events are always // delivered regardless of filter kind.Note: The review comment proposes "clamps to bounds," but the implementation actually rejects out-of-range values (see
rs-drive-abci/src/query/service.rs:908-910), so the documentation should reflect rejection, not clamping.packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (3)
31-39: keepalive lacks time unit; prefer explicit keepaliveMs.Ambiguous units create client bugs, especially on the web. Rename to keepalive_ms in proto (surfacing as keepaliveMs here).
Proto-level suggestion:
-message PlatformSubscriptionRequestV0 { - PlatformFilterV0 filter = 1; - uint32 keepalive = 2; -} +message PlatformSubscriptionRequestV0 { + PlatformFilterV0 filter = 1; + uint32 keepalive_ms = 2; // client ping/expectation interval in milliseconds +}
85-102: subscriptionId as number risks precision; use string/bytes and consider a cursor.JS numbers are unsafe > 2^53−1. If this ID can grow or be non-trivial, represent it as string (or bytes). Also consider adding a monotonically increasing cursor to aid reconnection/resume.
Proto-level suggestion:
-message PlatformSubscriptionResponseV0 { - uint32 subscription_id = 1; - PlatformEventV0 event = 2; -} +message PlatformSubscriptionResponseV0 { + string subscription_id = 1; // or bytes; stable across cluster shards + PlatformEventV0 event = 2; + uint64 cursor = 3; // optional: per-stream monotonic sequence for resume +}
215-231: Optional: include metadata on Keepalive (server interval, deadline).Empty Keepalive works, but adding fields like interval_ms or deadline_ms can help clients tune heartbeats/timeouts without out-of-band config.
Proto-level idea:
message Keepalive { uint32 interval_ms = 1; // server-intended cadence uint32 timeout_ms = 2; // suggest client-side timeout }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/dashmate/templates/platform/gateway/envoy.yaml.dotis excluded by!**/*.dot
📒 Files selected for processing (9)
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js(3 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h(2 hunks)packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m(2 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts(1 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.js(3 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/dashmate/configs/getConfigFileMigrationsFactory.js(4 hunks)packages/dashmate/docs/config/dapi.md(1 hunks)packages/rs-drive-abci/src/query/service.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb.jspackages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.jspackages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-drive-abci/src/query/service.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-drive-abci/src/query/service.rs
🧠 Learnings (29)
📚 Learning: 2025-10-15T14:45:30.856Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dashmate/docs/config/dapi.mdpackages/dapi-grpc/clients/platform/v0/web/platform_pb.js
📚 Learning: 2025-10-29T10:31:02.449Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2827
File: packages/dashmate/configs/defaults/getBaseConfigFactory.js:254-255
Timestamp: 2025-10-29T10:31:02.449Z
Learning: In dashmate configs (packages/dashmate/configs/defaults/getBaseConfigFactory.js), the `waitForStResultTimeout` for rs-dapi (120000ms / 120s) is intentionally set lower than the gateway listener timeout (125s). This design allows rs-dapi to execute its own timeout handling and return a proper response before the gateway times out, ensuring better error handling.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dashmate/docs/config/dapi.md
📚 Learning: 2024-10-04T09:08:48.470Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-platform-version/src/version/protocol_version.rs:155-157
Timestamp: 2024-10-04T09:08:48.470Z
Learning: In the dashmate default configurations, the default protocol version for local networks has been removed, and an update mechanism for the protocol version in the consensus parameters has been implemented.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-11-22T10:28:33.588Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2107
File: packages/dashmate/src/listr/tasks/setup/local/configureCoreTaskFactory.js:116-124
Timestamp: 2024-11-22T10:28:33.588Z
Learning: In `configureCoreTaskFactory` in `packages/dashmate/src/listr/tasks/setup/local/configureCoreTaskFactory.js`, the `local_seed` configuration is already established earlier in the code, so adding error handling for its absence in the 'Generating funds to use as a collateral for masternodes' task is unnecessary.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-10-18T15:37:36.387Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2249
File: packages/dashmate/test/unit/status/scopes/platform.spec.js:77-78
Timestamp: 2024-10-18T15:37:36.387Z
Learning: In `packages/dashmate/test/unit/status/scopes/platform.spec.js`, prefer to keep mocks explicitly in tests to increase readability, rather than refactoring them into shared `beforeEach` blocks or helper functions.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-12-10T12:39:38.182Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2381
File: packages/rs-sdk/scripts/connect_to_remote.sh:0-0
Timestamp: 2024-12-10T12:39:38.182Z
Learning: When reviewing scripts in `packages/rs-sdk/scripts/`, avoid suggesting additional error handling and timeout management that complicate the script without adding significant value.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-10-18T15:37:21.329Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2249
File: packages/dashmate/test/unit/status/scopes/platform.spec.js:466-467
Timestamp: 2024-10-18T15:37:21.329Z
Learning: In test files for the dashmate project, such as `packages/dashmate/test/unit/status/scopes/platform.spec.js`, prefer to keep mocks explicitly in tests rather than consolidating them, to increase readability.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-10-06T16:18:07.994Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
Applied to files:
packages/rs-drive-abci/src/query/service.rspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.hpackages/dapi-grpc/protos/platform/v0/platform.proto
📚 Learning: 2024-10-08T13:28:03.529Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2227
File: packages/rs-drive-abci/src/platform_types/platform_state/mod.rs:141-141
Timestamp: 2024-10-08T13:28:03.529Z
Learning: When converting `PlatformStateV0` to `PlatformStateForSavingV1` in `packages/rs-drive-abci/src/platform_types/platform_state/mod.rs`, only version `0` needs to be handled in the match on `platform_state_for_saving_structure_default` because the changes are retroactive.
Applied to files:
packages/rs-drive-abci/src/query/service.rspackages/dapi-grpc/protos/platform/v0/platform.proto
📚 Learning: 2025-09-05T06:00:56.491Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2761
File: packages/rs-drive/src/query/filter.rs:268-272
Timestamp: 2025-09-05T06:00:56.491Z
Learning: For document subscription filters in rs-drive, validation should not require index availability checks like queries do. Subscription filtering has different validation requirements than query optimization.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-10T10:30:24.653Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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-drive-abci/src/query/service.rs
📚 Learning: 2025-10-09T15:59:33.162Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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-drive-abci/src/query/service.rs
📚 Learning: 2024-10-29T14:40:54.727Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/core/transaction.rs:0-0
Timestamp: 2024-10-29T14:40:54.727Z
Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Use PascalCase for Rust types
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-11-03T10:39:11.242Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2305
File: packages/rs-drive-abci/src/abci/handler/finalize_block.rs:81-0
Timestamp: 2024-11-03T10:39:11.242Z
Learning: In `packages/rs-drive-abci/src/abci/handler/finalize_block.rs`, the use of `.expect("commit transaction")` after `app.commit_transaction(platform_version)` is intentional due to the provided comment explaining its necessity. Do not flag this usage in future reviews.
Applied to files:
packages/rs-drive-abci/src/query/service.rspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Use SCREAMING_SNAKE_CASE for Rust constants
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-11-20T20:43:41.185Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/tests/strategy_tests/masternodes.rs:212-220
Timestamp: 2024-11-20T20:43:41.185Z
Learning: In `packages/rs-drive-abci/tests/strategy_tests/masternodes.rs`, the pattern of generating a `PrivateKey`, converting it to bytes, and reconstructing a `BlsPrivateKey` from those bytes is intentional. Avoid suggesting to simplify this code in future reviews.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-22T10:53:12.111Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2259
File: packages/rs-dapi-client/src/dapi_client.rs:137-139
Timestamp: 2024-10-22T10:53:12.111Z
Learning: In `packages/rs-dapi-client/src/dapi_client.rs`, when passing data into asynchronous code, ensure that data structures are `Send + Sync`. Using `Arc<AtomicUsize>` is necessary for the retry counter.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-12-05T09:29:38.918Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2375
File: packages/rs-drive-abci/Cargo.toml:61-63
Timestamp: 2024-12-05T09:29:38.918Z
Learning: In the `drive-abci` package, avoid adding unused dependencies like `hashbrown` to `Cargo.toml`. The team relies on CI to detect dependency version issues.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-12-03T15:00:50.385Z
Learnt from: lklimek
Repo: dashpay/platform PR: 1924
File: packages/rs-sdk/src/sdk.rs:855-870
Timestamp: 2024-12-03T15:00:50.385Z
Learning: In `packages/rs-sdk/src/sdk.rs`, remember that `rustls_pemfile::certs` returns an iterator over `Result`, not a `Result` directly. Handle it accordingly in future code reviews.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-drive-abci/src/query/service.rspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:105-105
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In `run_block_proposal` in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, when retrieving `last_block_time_ms`, it's acceptable to use `platform_state` instead of `block_platform_state`, even after updating the protocol version.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-30T11:04:33.634Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/platform/fetch_unproved.rs:0-0
Timestamp: 2024-10-30T11:04:33.634Z
Learning: In `packages/rs-sdk/src/platform/fetch_unproved.rs`, the `execute()` method consumes the request object, so cloning the request is necessary before passing it to `execute()` and `maybe_from_unproved_with_metadata`.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-24T05:00:56.657Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:83-85
Timestamp: 2024-10-24T05:00:56.657Z
Learning: An upcoming filter will be introduced to the `unconfirmed_txs` endpoint in `broadcastStateTransitionHandlerFactory.js`, as noted in the TODO comment. Therefore, optimizing transaction existence checks may not be necessary at this time.
Applied to files:
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.hpackages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h
📚 Learning: 2025-02-03T23:39:10.579Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-02-03T23:39:10.579Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
📚 Learning: 2024-10-25T09:33:50.383Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2274
File: packages/js-dapi-client/lib/transport/GrpcTransport/createGrpcTransportError.js:127-127
Timestamp: 2024-10-25T09:33:50.383Z
Learning: In `createGrpcTransportError.js`, the size of consensus error data is already validated by gRPC transport, so adding an additional size check before base64 decoding in the `createGrpcTransportError` function is unnecessary.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
📚 Learning: 2025-10-09T15:59:18.338Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsReader.js:325-334
Timestamp: 2025-10-09T15:59:18.338Z
Learning: In `packages/wallet-lib/src/plugins/Workers/TransactionsSyncWorker/TransactionsReader.js`, the continuous sync restart intentionally uses `Math.max(1, lastSyncedBlockHeight)` rather than `lastSyncedBlockHeight + 1` because the last block's processing status is uncertain at restart time. This conservative approach ensures no blocks are missed by reprocessing the last synced block, accepting potential duplicate processing for data completeness.
Applied to files:
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js
📚 Learning: 2024-10-24T04:59:20.436Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:75-77
Timestamp: 2024-10-24T04:59:20.436Z
Learning: Tenderdash's `tx` RPC method accepts transaction hashes in base64 encoding, or in hex encoding if prefixed with `0x`. Therefore, in `packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js`, it's acceptable to use `stHash.toString('base64')` when calling `requestTenderRpc('tx', { hash: stHash.toString('base64') })`.
Applied to files:
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts
🧬 Code graph analysis (5)
packages/dashmate/configs/getConfigFileMigrationsFactory.js (1)
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (4)
waitForStateTransitionResult(364-368)waitForStateTransitionResult(1103-1117)subscribePlatformEvents(549-558)subscribePlatformEvents(1613-1627)
packages/rs-drive-abci/src/query/service.rs (4)
packages/rs-drive-abci/src/abci/app/full.rs (3)
platform(48-50)event_bus(60-62)new(37-44)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (10)
Keepalive(333-342)PlatformEventV0(215-240)PlatformSubscriptionRequest(9-24)PlatformSubscriptionResponse(63-78)PlatformFilterV0(117-142)BlockCommitted(167-176)BlockCommitted(279-296)StateTransitionFinalized(305-324)Version(5947-5966)PlatformSubscriptionResponseV0(85-102)packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs (3)
mpsc(46-48)drop(24-26)new(17-20)packages/rs-dash-event-bus/src/event_bus.rs (5)
mpsc(96-96)drop(261-286)new(76-78)matches(16-16)matches(430-432)
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
PlatformEventV0(215-240)PlatformFilterV0(117-142)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (4)
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (1)
proto(16-16)packages/js-dapi-client/test/unit/methods/platform/getStatus/GetStatusResponse.spec.js (1)
proto(18-18)packages/dapi-grpc/clients/drive/v0/nodejs/drive_protoc.js (1)
proto(16-16)packages/dapi-grpc/clients/drive/v0/web/drive_pb.js (1)
proto(16-16)
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (3)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (1)
proto(16-16)packages/dapi-grpc/clients/drive/v0/nodejs/drive_protoc.js (1)
proto(16-16)packages/dapi-grpc/clients/drive/v0/web/drive_pb.js (1)
proto(16-16)
⏰ 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). (11)
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Linting
- 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 (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 JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (18)
packages/dashmate/configs/getConfigFileMigrationsFactory.js (5)
1167-1175: LGTM: Deprecated config initialization.This correctly initializes the
deprecatedconfiguration structure from defaults in preparation for migration. The logic properly handles both missing structure and missingenabledflag scenarios.
1336-1367: LGTM: Clean migration of legacy configuration.The logic correctly:
- Removes the deprecated marker added in earlier migrations
- Derives
rsDapi.maxRequestsfrom the maximum of legacydapiApianddapiCoreStreamsvalues- Falls back to defaults when legacy values are unavailable
- Cleans up obsolete upstream keys
1373-1389: LGTM: Proper legacy configuration extraction.The code correctly:
- Ensures
rsDapiexists by cloning from defaults when missing- Extracts legacy timeout values before deleting the old structure
- Removes the deprecated
options.platform.dapi.apiconfiguration
1391-1421: LGTM: Docker alignment and timeout structure normalization.The migration properly:
- Aligns docker images with current defaults
- Consolidates timeouts into a unified structure with proper fallback chain (existing → legacy → defaults)
- Introduces three distinct timeout settings for different operations
- Cleans up the legacy
waitForStResultTimeoutfield fromrsDapi
1431-1433: LGTM: Gateway listener timeout cleanup.Correctly removes the legacy
waitForStResultTimeoutfrom the gateway listener configuration.packages/dashmate/docs/config/dapi.md (1)
42-44: LGTM: Clear and accurate timeout documentation.The documentation correctly describes the three new timeout options introduced in this PR:
waitForStateTransitionResult(renamed from the oldwaitForStResultTimeout)subscribePlatformEvents(new feature for platform event subscriptions)coreStreams(for Core streaming subscriptions)The default values match those specified in the migration code, and the descriptions clearly explain the purpose of each timeout.
packages/rs-drive-abci/src/query/service.rs (5)
66-66: LGTM! Constant naming and struct fields are correct.The constant follows SCREAMING_SNAKE_CASE convention, and the new fields properly support event subscription infrastructure.
Also applies to: 71-74
76-83: LGTM! Proper cleanup of background workers.The Drop implementation correctly aborts all worker tasks when the service is dropped, preventing resource leaks.
94-103: LGTM! Constructor properly initializes new fields.The updated signature and initialization correctly set up the event bus and worker tracking.
281-320: LGTM! PlatformFilterAdapter implementation is correct.The filter matching logic properly handles all filter types (All, BlockCommitted, StateTransitionResult) and ensures keepalive events are always delivered to all subscriptions.
894-899: LGTM! Filter validation prevents silent no-op subscriptions.The validation ensures
filter.kindis specified before creating a subscription, providing clear error feedback to clients. This properly addresses the earlier concern about silent failures.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
595-598: Grammar fix acknowledged“gets committed” is corrected. Thanks for addressing.
packages/dapi-grpc/protos/platform/v0/platform.proto (3)
9-28: Message structure and versioning look good. Request/response wrappers follow the established versioning pattern in the file, with clear field documentation. The keepalive interval bounds (min 25s, max 300s, 0 disables) are properly documented.
52-76: Event message structure is well-designed. Nested messages provide clear separation of concerns, andjstype = JS_STRINGannotations properly handle 64-bit integer serialization for JavaScript clients. The oneof discriminator correctly selects among the three event types (BlockCommitted, StateTransitionFinalized, Keepalive).
177-183: RPC comment and signature are aligned. The comment correctly describes this as a server-streaming RPC (not bi-directional), which aligns with thereturns (stream ...)signature and resolves the prior mismatch. The handshake semantics ("Once connected, it sends handshake response with empty event") are clear. Prior typo and formatting issues have also been resolved.packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (1)
463-9456: Generated protobuf code looks correct.This file contains auto-generated JavaScript code from protobuf definitions. The new message types (PlatformSubscriptionRequest, PlatformSubscriptionResponse, PlatformFilterV0, PlatformEventV0) and their implementations follow standard protobuf JS patterns:
- Google Closure exports and inheritance setup
- Standard oneof group handling
- Consistent (de)serialization logic
- Proper getter/setter/clear/has accessor methods
Any modifications to these message structures should be made in the source
.protofiles (likelypackages/dapi-grpc/protos/platform/v0/platform.proto) and then regenerated, not edited directly in this file.packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.m (2)
320-332: LGTM - Generated protobuf class declarations.The forward declarations for the new platform event subscription classes follow the standard Google Protocol Buffers pattern for Objective-C and are correctly positioned in the file.
409-1121: LGTM - Well-formed protobuf descriptor implementations.The descriptor implementations for the platform event subscription infrastructure are correctly generated:
- Thread-safe lazy initialization with static descriptors
- Proper field descriptions with correct types, offsets, and flags
- Oneof groups correctly configured with clear helper functions
- Nested message relationships properly established
- Storage structs appropriately sized and aligned
All patterns are consistent with Google Protocol Buffers conventions for Objective-C.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/dapi-grpc/protos/platform/v0/platform.proto (1)
30-50: Consider using standardgoogle.protobuf.Emptyfor presence-only variants.The code defines custom empty message types locally (
AllEvents,BlockCommitted,Keepalive). While functionally equivalent and correct, using the standard librarygoogle.protobuf.Emptywould be more idiomatic and reduce boilerplate. The file already imports othergoogle.protobuftypes.Proposed change:
+import "google/protobuf/empty.proto"; message PlatformFilterV0 { - message AllEvents{} - message BlockCommitted{} + // Remove local empty messages; use google.protobuf.Empty instead message StateTransitionResultFilter { optional bytes tx_hash = 1; } oneof kind { - AllEvents all = 1; - BlockCommitted block_committed = 2; + google.protobuf.Empty all = 1; + google.protobuf.Empty block_committed = 2; StateTransitionResultFilter state_transition_result = 3; } } message PlatformEventV0 { // ... existing messages ... - message Keepalive {} + // Use google.protobuf.Empty for keepalive oneof event { BlockCommitted block_committed = 1; StateTransitionFinalized state_transition_finalized = 2; - Keepalive keepalive = 3; + google.protobuf.Empty keepalive = 3; } }packages/rs-drive-abci/src/query/service.rs (1)
915-915: [CRITICAL] Add missing rand import.Line 915 uses
rand::random::<u64>()butrandis not imported in this file. Whilerandis declared in Cargo.toml, Rust requires an explicit import to use it.Add the import at the top of the file:
+use rand::random;Then update line 915:
- let subscription_id = rand::random::<u64>(); + let subscription_id = random::<u64>();Or alternatively, import the crate and keep the full path:
+use rand;(The existing line 915 will then compile as-is)
🧹 Nitpick comments (1)
packages/rs-drive-abci/src/query/service.rs (1)
1018-1026: LGTM: Worker lifecycle management.The worker management strategy is sound:
- Spawned tasks are tracked in the workers vector
retain()removes finished tasks on each new subscription (prevents memory leak)- The Drop impl ensures all workers are aborted when QueryService is dropped
Optional: Consider running cleanup more frequently (e.g., via a periodic background task) if you expect many short-lived subscriptions. However, the current approach is acceptable since cleanup happens on each new subscription and at Drop time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/dashmate/templates/platform/gateway/envoy.yaml.dotis excluded by!**/*.dot
📒 Files selected for processing (5)
packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/dashmate/configs/getConfigFileMigrationsFactory.js(4 hunks)packages/dashmate/docs/services/gateway.md(1 hunks)packages/rs-drive-abci/src/query/service.rs(5 hunks)packages/rs-sdk/src/platform/events.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-sdk/src/platform/events.rs
🧰 Additional context used
📓 Path-based instructions (3)
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/dashmate/configs/getConfigFileMigrationsFactory.js
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-drive-abci/src/query/service.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-drive-abci/src/query/service.rs
🧠 Learnings (24)
📚 Learning: 2025-10-15T14:45:30.856Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dashmate/docs/services/gateway.md
📚 Learning: 2025-10-29T10:31:02.449Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2827
File: packages/dashmate/configs/defaults/getBaseConfigFactory.js:254-255
Timestamp: 2025-10-29T10:31:02.449Z
Learning: In dashmate configs (packages/dashmate/configs/defaults/getBaseConfigFactory.js), the `waitForStResultTimeout` for rs-dapi (120000ms / 120s) is intentionally set lower than the gateway listener timeout (125s). This design allows rs-dapi to execute its own timeout handling and return a proper response before the gateway times out, ensuring better error handling.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dashmate/docs/services/gateway.md
📚 Learning: 2024-10-04T09:08:48.470Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-platform-version/src/version/protocol_version.rs:155-157
Timestamp: 2024-10-04T09:08:48.470Z
Learning: In the dashmate default configurations, the default protocol version for local networks has been removed, and an update mechanism for the protocol version in the consensus parameters has been implemented.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-11-22T10:28:33.588Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2107
File: packages/dashmate/src/listr/tasks/setup/local/configureCoreTaskFactory.js:116-124
Timestamp: 2024-11-22T10:28:33.588Z
Learning: In `configureCoreTaskFactory` in `packages/dashmate/src/listr/tasks/setup/local/configureCoreTaskFactory.js`, the `local_seed` configuration is already established earlier in the code, so adding error handling for its absence in the 'Generating funds to use as a collateral for masternodes' task is unnecessary.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-10-18T15:37:36.387Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2249
File: packages/dashmate/test/unit/status/scopes/platform.spec.js:77-78
Timestamp: 2024-10-18T15:37:36.387Z
Learning: In `packages/dashmate/test/unit/status/scopes/platform.spec.js`, prefer to keep mocks explicitly in tests to increase readability, rather than refactoring them into shared `beforeEach` blocks or helper functions.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-12-10T12:39:38.182Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2381
File: packages/rs-sdk/scripts/connect_to_remote.sh:0-0
Timestamp: 2024-12-10T12:39:38.182Z
Learning: When reviewing scripts in `packages/rs-sdk/scripts/`, avoid suggesting additional error handling and timeout management that complicate the script without adding significant value.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-10-18T15:37:21.329Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2249
File: packages/dashmate/test/unit/status/scopes/platform.spec.js:466-467
Timestamp: 2024-10-18T15:37:21.329Z
Learning: In test files for the dashmate project, such as `packages/dashmate/test/unit/status/scopes/platform.spec.js`, prefer to keep mocks explicitly in tests rather than consolidating them, to increase readability.
Applied to files:
packages/dashmate/configs/getConfigFileMigrationsFactory.js
📚 Learning: 2024-10-19T06:10:00.808Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2255
File: packages/dashmate/src/status/scopes/platform.js:112-116
Timestamp: 2024-10-19T06:10:00.808Z
Learning: In the Dashmate project, when fetching from Tenderdash RPC endpoints like `/status`, `/net_info`, and `/abci_info`, if one request fails, it's likely that the others will fail as well, so individual error handling for each fetch may not be necessary.
Applied to files:
packages/dashmate/docs/services/gateway.md
📚 Learning: 2024-10-06T16:18:07.994Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
Applied to files:
packages/rs-drive-abci/src/query/service.rspackages/dapi-grpc/protos/platform/v0/platform.proto
📚 Learning: 2024-10-08T13:28:03.529Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2227
File: packages/rs-drive-abci/src/platform_types/platform_state/mod.rs:141-141
Timestamp: 2024-10-08T13:28:03.529Z
Learning: When converting `PlatformStateV0` to `PlatformStateForSavingV1` in `packages/rs-drive-abci/src/platform_types/platform_state/mod.rs`, only version `0` needs to be handled in the match on `platform_state_for_saving_structure_default` because the changes are retroactive.
Applied to files:
packages/rs-drive-abci/src/query/service.rspackages/dapi-grpc/protos/platform/v0/platform.proto
📚 Learning: 2025-09-05T06:00:56.491Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2761
File: packages/rs-drive/src/query/filter.rs:268-272
Timestamp: 2025-09-05T06:00:56.491Z
Learning: For document subscription filters in rs-drive, validation should not require index availability checks like queries do. Subscription filtering has different validation requirements than query optimization.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-10T10:30:24.653Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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-drive-abci/src/query/service.rs
📚 Learning: 2025-10-09T15:59:33.162Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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-drive-abci/src/query/service.rs
📚 Learning: 2024-10-29T14:40:54.727Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/core/transaction.rs:0-0
Timestamp: 2024-10-29T14:40:54.727Z
Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Use PascalCase for Rust types
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-11-03T10:39:11.242Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2305
File: packages/rs-drive-abci/src/abci/handler/finalize_block.rs:81-0
Timestamp: 2024-11-03T10:39:11.242Z
Learning: In `packages/rs-drive-abci/src/abci/handler/finalize_block.rs`, the use of `.expect("commit transaction")` after `app.commit_transaction(platform_version)` is intentional due to the provided comment explaining its necessity. Do not flag this usage in future reviews.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Applies to packages/rs-*/**/*.rs : Use SCREAMING_SNAKE_CASE for Rust constants
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-11-20T20:43:41.185Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/tests/strategy_tests/masternodes.rs:212-220
Timestamp: 2024-11-20T20:43:41.185Z
Learning: In `packages/rs-drive-abci/tests/strategy_tests/masternodes.rs`, the pattern of generating a `PrivateKey`, converting it to bytes, and reconstructing a `BlsPrivateKey` from those bytes is intentional. Avoid suggesting to simplify this code in future reviews.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-22T10:53:12.111Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2259
File: packages/rs-dapi-client/src/dapi_client.rs:137-139
Timestamp: 2024-10-22T10:53:12.111Z
Learning: In `packages/rs-dapi-client/src/dapi_client.rs`, when passing data into asynchronous code, ensure that data structures are `Send + Sync`. Using `Arc<AtomicUsize>` is necessary for the retry counter.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-12-03T15:00:50.385Z
Learnt from: lklimek
Repo: dashpay/platform PR: 1924
File: packages/rs-sdk/src/sdk.rs:855-870
Timestamp: 2024-12-03T15:00:50.385Z
Learning: In `packages/rs-sdk/src/sdk.rs`, remember that `rustls_pemfile::certs` returns an iterator over `Result`, not a `Result` directly. Handle it accordingly in future code reviews.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-12-05T09:29:38.918Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2375
File: packages/rs-drive-abci/Cargo.toml:61-63
Timestamp: 2024-12-05T09:29:38.918Z
Learning: In the `drive-abci` package, avoid adding unused dependencies like `hashbrown` to `Cargo.toml`. The team relies on CI to detect dependency version issues.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:105-105
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In `run_block_proposal` in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, when retrieving `last_block_time_ms`, it's acceptable to use `platform_state` instead of `block_platform_state`, even after updating the protocol version.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
📚 Learning: 2024-10-30T11:04:33.634Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/platform/fetch_unproved.rs:0-0
Timestamp: 2024-10-30T11:04:33.634Z
Learning: In `packages/rs-sdk/src/platform/fetch_unproved.rs`, the `execute()` method consumes the request object, so cloning the request is necessary before passing it to `execute()` and `maybe_from_unproved_with_metadata`.
Applied to files:
packages/rs-drive-abci/src/query/service.rs
🧬 Code graph analysis (1)
packages/rs-drive-abci/src/query/service.rs (5)
packages/rs-drive-abci/src/abci/app/full.rs (3)
platform(48-50)event_bus(60-62)new(37-44)packages/rs-drive-abci/src/abci/app/consensus.rs (3)
platform(51-53)event_bus(63-65)new(37-47)packages/rs-drive-abci/src/abci/app/mod.rs (2)
platform(32-32)event_bus(24-26)packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs (3)
mpsc(46-48)drop(24-26)new(17-20)packages/rs-sdk/src/platform/events.rs (1)
subscribe_platform_events(17-61)
🪛 markdownlint-cli2 (0.18.1)
packages/dashmate/docs/services/gateway.md
109-109: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
109-109: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
110-110: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
110-110: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
111-111: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
111-111: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
112-112: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
112-112: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
⏰ 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). (16)
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Tests
- 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 JS packages / Build JS
- GitHub Check: Rust crates security audit
- 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)
🔇 Additional comments (19)
packages/dapi-grpc/protos/platform/v0/platform.proto (3)
43-50: Critical issue resolved: bool-in-oneof footgun eliminated.The previous footgun of using
boolinsideoneofhas been correctly eliminated by using empty message types (AllEvents,BlockCommitted). This ensures proper presence semantics across language implementations.
177-183: Well-documented server-streaming RPC with clear handshake semantics.The RPC comment correctly describes the server-streaming nature (not bi-directional), aligns with gRPC-Web limitations noted in PR discussions, and the handshake mechanism is well-explained. The signature properly matches a server-streaming subscription pattern suitable for web clients.
52-76: Event hierarchy is well-structured with appropriate JS compatibility markers.
PlatformEventV0and nested types (BlockMetadata,BlockCommitted,StateTransitionFinalized) are cleanly organized. The use ofjstype = JS_STRINGon 64-bit fields (lines 54–55) correctly addresses JavaScript number precision limits. EmptyKeepalivemessage is appropriate for connection liveness.packages/dashmate/docs/services/gateway.md (1)
106-106: LGTM: Clear explanation of timeout behavior.The description accurately explains the shared timeout model between rs-dapi and Envoy, and clearly documents the five-second grace period for structured error responses.
packages/dashmate/configs/getConfigFileMigrationsFactory.js (8)
1167-1175: LGTM: Proper initialization of deprecated marker.This block correctly initializes the
deprecatedmarker from defaults before it's removed later in the migration. The nested conditional logic properly handles both missing objects and undefined properties.
1336-1339: LGTM: Clean removal of deprecated DAPI marker.This correctly removes the legacy
deprecatedmarker after it's been used for migration purposes.
1341-1367: LGTM: Thoughtful consolidation of legacy upstreams.The migration correctly consolidates the old
dapiApianddapiCoreStreamsupstreams into a singlersDapiupstream by taking the maximum of theirmaxRequestsvalues. The fallback to defaults when no valid values exist is appropriate.
1369-1371: LGTM: Appropriate guard clause.This guard correctly protects the subsequent DAPI-specific migration logic when the DAPI configuration doesn't exist.
1373-1376: LGTM: Proper initialization with early return.This correctly initializes
rsDapifrom defaults for configs that don't have it yet, with an early return to skip unnecessary transformation logic.
1378-1389: LGTM: Proper extraction of legacy timeout values.This block correctly extracts timeout values from the legacy
apisection before deleting it, preserving both the oldwaitForStResultTimeoutfield and thetimeoutsobject for migration to the new structure.
1391-1400: LGTM: Standard docker image updates.The docker image updates follow the established pattern used throughout this migration file, with proper existence checks before accessing nested properties.
1402-1425: LGTM: Comprehensive timeout normalization and cleanup.This block correctly normalizes the timeout structure by:
- Computing each timeout value from multiple potential sources using proper fallback chains with the
??operator- Creating a clean, unified
rsDapi.timeoutsobject with exactly three properties- Removing all legacy timeout fields from both
rsDapiand the gateway listenerThe fallback logic properly handles undefined values at each level, ensuring defaults are used when legacy values are missing.
packages/rs-drive-abci/src/query/service.rs (7)
13-13: LGTM: Imports and constant.The new imports support event bus integration and streaming. The buffer size of 128 is reasonable for platform event streams.
Also applies to: 51-51, 55-55, 59-59, 62-66
71-83: LGTM: Worker cleanup via Drop.The Drop implementation properly aborts all background workers, preventing resource leaks when QueryService is dropped. Using
Vec<JoinHandle>with explicit cleanup addresses the previous concern about task accumulation.
94-103: LGTM: Constructor updated.The constructor properly accepts and stores the event_bus parameter, and initializes the workers vector.
281-320: LGTM: Filter adapter implementation.The
PlatformFilterAdaptercorrectly implementsEventBusFiltersemantics:
- Keepalive events always pass through (essential for the keepalive mechanism)
- Filter variants (All, BlockCommitted, StateTransitionResult) are properly matched
- Optional tx_hash filtering for StateTransitionResult is correctly handled
894-913: LGTM: Request validation.The validation logic properly handles:
- Filter kind requirement (addresses previous review comment)
- Keepalive interval range (25-300 seconds) with appropriate error messages
- Zero keepalive value to disable keepalive
930-947: LGTM: Handshake pattern.The handshake message (empty event with subscription_id) confirms subscription creation to the client before streaming events. Error handling for client disconnection during handshake is appropriate.
949-1016: LGTM: Event forwarding worker.The worker implementation correctly:
- Uses
tokio::time::timeoutto implement keepalive intervals- Forwards events when received
- Emits keepalive events when timeout elapses
- Handles stream closure gracefully
- Logs disconnection and completion
| - Core streaming endpoints: derived from `platform.dapi.rsDapi.timeouts.coreStreams` (default 600 000 ms) | ||
| - waitForStateTransitionResult endpoint: derived from `platform.dapi.rsDapi.timeouts.waitForStateTransitionResult` (default 120 000 ms) | ||
| - subscribePlatformEvents endpoint: derived from `platform.dapi.rsDapi.timeouts.subscribePlatformEvents` (default 600 000 ms) | ||
| - All streaming routes share a fixed Envoy `idle_timeout` of 300s so that connections can stay open while rs-dapi manages its own deadlines. |
There was a problem hiding this comment.
Fix markdown indentation for list items.
These bullet points have 1 space of indentation when they should have 0. They are top-level list items, not nested items.
Apply this diff to fix the indentation:
-- Core streaming endpoints: derived from `platform.dapi.rsDapi.timeouts.coreStreams` (default 600 000 ms)
-- waitForStateTransitionResult endpoint: derived from `platform.dapi.rsDapi.timeouts.waitForStateTransitionResult` (default 120 000 ms)
-- subscribePlatformEvents endpoint: derived from `platform.dapi.rsDapi.timeouts.subscribePlatformEvents` (default 600 000 ms)
-- All streaming routes share a fixed Envoy `idle_timeout` of 300s so that connections can stay open while rs-dapi manages its own deadlines.
+- Core streaming endpoints: derived from `platform.dapi.rsDapi.timeouts.coreStreams` (default 600 000 ms)
+- waitForStateTransitionResult endpoint: derived from `platform.dapi.rsDapi.timeouts.waitForStateTransitionResult` (default 120 000 ms)
+- subscribePlatformEvents endpoint: derived from `platform.dapi.rsDapi.timeouts.subscribePlatformEvents` (default 600 000 ms)
+- All streaming routes share a fixed Envoy `idle_timeout` of 300s so that connections can stay open while rs-dapi manages its own deadlines.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Core streaming endpoints: derived from `platform.dapi.rsDapi.timeouts.coreStreams` (default 600 000 ms) | |
| - waitForStateTransitionResult endpoint: derived from `platform.dapi.rsDapi.timeouts.waitForStateTransitionResult` (default 120 000 ms) | |
| - subscribePlatformEvents endpoint: derived from `platform.dapi.rsDapi.timeouts.subscribePlatformEvents` (default 600 000 ms) | |
| - All streaming routes share a fixed Envoy `idle_timeout` of 300s so that connections can stay open while rs-dapi manages its own deadlines. | |
| - Core streaming endpoints: derived from `platform.dapi.rsDapi.timeouts.coreStreams` (default 600 000 ms) | |
| - waitForStateTransitionResult endpoint: derived from `platform.dapi.rsDapi.timeouts.waitForStateTransitionResult` (default 120 000 ms) | |
| - subscribePlatformEvents endpoint: derived from `platform.dapi.rsDapi.timeouts.subscribePlatformEvents` (default 600 000 ms) | |
| - All streaming routes share a fixed Envoy `idle_timeout` of 300s so that connections can stay open while rs-dapi manages its own deadlines. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
109-109: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
109-109: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
110-110: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
110-110: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
111-111: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
111-111: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
112-112: Inconsistent indentation for list items at the same level
Expected: 0; Actual: 1
(MD005, list-indent)
112-112: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
🤖 Prompt for AI Agents
In packages/dashmate/docs/services/gateway.md around lines 109 to 112, the four
bullet list items are incorrectly indented with a leading space; remove that
single leading space for each line so they are top-level list items (no
indentation) and ensure Markdown renders them as a primary list.
Issue being fixed or feature implemented
platform/v0/platform.proto:8, packages/rs-drive-abci/src/abci/handler/finalize_block.rs:1, packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs:1).
dashmate/templates/platform/gateway/envoy.yaml.dot:1, packages/rs-sdk/src/platform/events.rs:1).
What was done?
the authoritative producer (packages/dapi-grpc/protos/platform/v0/platform.proto:8, packages/rs-drive-abci/src/abci/handler/finalize_block.rs:1, packages/rs-dash-event-bus/src/event_bus.rs:1).
(packages/rs-dapi/src/services/platform_service/subscribe_platform_events.rs:1, packages/rs-dapi/src/metrics.rs:89, packages/rs-dapi/src/server/grpc.rs:21).
event subscriptions (packages/dashmate/configs/getConfigFileMigrationsFactory.js:1164, packages/dashmate/templates/platform/gateway/envoy.yaml.dot:1, packages/dashmate/docs/config/dapi.md:39).
semantics (packages/rs-sdk/src/platform/events.rs:1, packages/rs-sdk/tests/fetch/platform_events.rs:1, packages/wasm-sdk/src/error.rs:169).
Note
This is only preliminary implementation, adding necessary scaffolding to emit and deliver events to user.
Implemented events are more a proof-of-concept. Further work is needed to emit meaningful events, like document update, etc.
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only