chore: update rust-dashcore dependency to store TransactionContext in TransactionRecord - #11
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ 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 |
… TransactionRecord Update dashcore/dashcore-rpc git dependencies to 5114330ae564858803d656efba84bb4adacb2a2e for integration testing of dashpay/rust-dashcore#582.
6047c21 to
6b20929
Compare
|
⛔ Final review complete — 2 blocking finding(s) (commit 6b20929) · triage: critical · Phase 2 only (queue backlog) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 2 only (queue backlog)
At head 6b20929, the dependency update cannot meet its stated compilation goal: the manifest is incompatible with the selected upstream revision, and Cargo.lock still references the old tag. Rust 1.91.0 locked build, test, and Clippy checks reproduced the feature-resolution failure offline; isolated manifest corrections independently confirmed the bincode conflict and subsequent lockfile failure. Formatting also fails on unchanged source files; the repository remains unmodified.
Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)
Review provenance
- Triage:
criticalbygpt-6-astra(effort low) — Although only two dependency pointers change, replacing released dashcore and dashcore-rpc versions with a PR commit can alter consensus-sensitive quorum handling, serialization, and RPC behavior, requiring review of the full upstream delta rather than just compilation. - Phase 1 reviewers: not run (skipped for throughput: 24 PRs queued, above the 10 limit)
- Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer
🔴 2 blocking
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `Cargo.toml`:
- [BLOCKING] Cargo.toml:8-13: Adapt the manifest requirements to the selected upstream revision
Revision 5114330 provides dashcore 0.42.0, which no longer defines the requested `std` feature. Consequently, locked build, test, and Clippy commands fail during dependency resolution, before validating the upstream refactor. Removing only `std` exposes a second manifest incompatibility: dashcore requires bincode and bincode_derive `^2.0.1`, and dashcore-rpc-json also requires bincode `^2.0.1`, while this application pins both codec packages to `=2.0.0-rc.3` at lines 19–20. Cargo rejects that graph as well. Remove the obsolete feature and align both codec pins with the selected revision, or remove the unused direct codec dependencies, so this integration can reach compilation.
- [BLOCKING] Cargo.toml:13: Update Cargo.lock alongside the Git dependency pointers
Cargo.lock is unchanged and still records dashcore, dashcore-rpc, and their workspace dependencies from `?tag=v0.40.0#c877c1a74d145e2003d549619698511513db925c`, rather than the selected revision. After correcting the feature and codec requirements in an isolated copy, `cargo build --locked --offline` independently fails because the lockfile needs updating. This also blocks the Dockerfile's `cargo build --release --locked` step. Regenerate and include Cargo.lock with the compatible manifest, then rerun the locked validation commands.
Out-of-scope follow-up suggestions (2)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Existing unauthenticated endpoint allows quorum-cache deletion — src/api.rs exposes POST /quorums/clear without authentication and clears the shared quorum list. Callers can erase cached entries and disrupt hash lookups until the list is repopulated. The route and mutation are unchanged by this dependency-only PR.
- Follow-up: Track removal of the public mutation endpoint or restriction to authenticated administrative access separately.
- Existing request-triggered backend work permits denial-of-service amplification — Public /quorums and /previous handlers execute synchronous Core RPC calls through their loaders, including per-quorum calls. Concurrent /masternodes requests at cache expiry can each initiate a full probe batch because the expiry check does not coalesce refreshes. These concrete amplification paths predate this PR and do not affect its review action.
- Follow-up: Track request-path RPC isolation and coalesced, bounded masternode refreshes as a separate availability issue.
| dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "5114330ae564858803d656efba84bb4adacb2a2e", default-features = false, features = [ | ||
| "std", | ||
| "secp-recovery", | ||
| "bincode", | ||
| ] } | ||
| dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.40.0" } | ||
| dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "5114330ae564858803d656efba84bb4adacb2a2e" } |
There was a problem hiding this comment.
🔴 Blocking: Adapt the manifest requirements to the selected upstream revision
Revision 5114330 provides dashcore 0.42.0, which no longer defines the requested std feature. Consequently, locked build, test, and Clippy commands fail during dependency resolution, before validating the upstream refactor. Removing only std exposes a second manifest incompatibility: dashcore requires bincode and bincode_derive ^2.0.1, and dashcore-rpc-json also requires bincode ^2.0.1, while this application pins both codec packages to =2.0.0-rc.3 at lines 19–20. Cargo rejects that graph as well. Remove the obsolete feature and align both codec pins with the selected revision, or remove the unused direct codec dependencies, so this integration can reach compilation.
source: ['claude']
| "bincode", | ||
| ] } | ||
| dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.40.0" } | ||
| dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "5114330ae564858803d656efba84bb4adacb2a2e" } |
There was a problem hiding this comment.
🔴 Blocking: Update Cargo.lock alongside the Git dependency pointers
Cargo.lock is unchanged and still records dashcore, dashcore-rpc, and their workspace dependencies from ?tag=v0.40.0#c877c1a74d145e2003d549619698511513db925c, rather than the selected revision. After correcting the feature and codec requirements in an isolated copy, cargo build --locked --offline independently fails because the lockfile needs updating. This also blocks the Dockerfile's cargo build --release --locked step. Regenerate and include Cargo.lock with the compatible manifest, then rerun the locked validation commands.
source: ['claude']
2 participants
Automated integration test for dashpay/rust-dashcore#582.
What: Updated all
dashcore/dashcore-rpcgit dependencies to PR HEAD commit (5114330ae564858803d656efba84bb4adacb2a2e).Why: Validates that the changes in rust-dashcore PR #582 (
refactor: store TransactionContext in TransactionRecord) compile and pass CI against this repository.Changes: Dependency pointer update only — no functional code changes.