-
Notifications
You must be signed in to change notification settings - Fork 45
build(sdk): remove unmaintained rustls-pemfile, update lru to 0.16.3 #2983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3.0-dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughLRU dependency bumped to v0.16.3 in three workspace packages. In rs-sdk, Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ gRPC Query Coverage Report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/rs-sdk/src/sdk.rs:
- Around line 1375-1395: The test test_load_ca_certificate hardcodes a
Linux-specific path (/etc/ssl/certs) causing failures on macOS/Windows; change
it to use a bundled test fixture or skip on non-Linux: add a PEM file under
tests/fixtures (e.g., tests/fixtures/test-ca.pem) and update
test_load_ca_certificate to call
SdkBuilder::new_mock().with_ca_certificate_file(<fixture path>) or wrap the test
with cfg(target_os = "linux")/cfg_attr to skip on non-Linux platforms so
SdkBuilder::new_mock and with_ca_certificate_file load a known portable file
instead of relying on /etc/ssl/certs.
- Around line 953-959: The code calls
rustls_pki_types::CertificateDer::from_pem_file which returns DER bytes but then
passes that DER to Certificate::from_pem (used in the with_ca_certificate flow);
rename the misleading variable and either read the raw PEM file bytes instead
(use std::fs::read to produce pem_bytes and pass those to Certificate::from_pem)
or convert the DER back to PEM before calling Certificate::from_pem; update the
variable name (avoid using `pem` for DER) and ensure the call site uses either
Certificate::from_der (if available) or Certificate::from_pem with actual PEM
bytes so TLS setup via with_ca_certificate receives the correct encoding.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
packages/rs-dapi-client/Cargo.tomlpackages/rs-sdk-trusted-context-provider/Cargo.tomlpackages/rs-sdk/Cargo.tomlpackages/rs-sdk/src/sdk.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Rust code must passcargo clippy --workspacelinter checks
Rust code must be formatted usingcargo fmt --all
**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-sdk/src/sdk.rs
🧠 Learnings (16)
📓 Common learnings
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.
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.
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.
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.
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use Rust for core platform components (Drive, DAPI server, DPP implementation)
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-drive-abci/tests/strategy_tests/main.rs:1162-1162
Timestamp: 2024-10-04T09:08:47.901Z
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`.
📚 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-dapi-client/Cargo.tomlpackages/rs-sdk/Cargo.tomlpackages/rs-sdk-trusted-context-provider/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-dapi-client/Cargo.tomlpackages/rs-sdk/Cargo.toml
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/rs-dapi-client/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-dapi-client/Cargo.tomlpackages/rs-sdk/Cargo.tomlpackages/rs-sdk-trusted-context-provider/Cargo.toml
📚 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-client/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-dapi-client/Cargo.toml
📚 Learning: 2025-11-25T13:10:47.943Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T13:10:47.943Z
Learning: Applies to **/*.rs : Use PascalCase for Rust type names
Applied to files:
packages/rs-sdk/Cargo.toml
📚 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/Cargo.tomlpackages/rs-sdk/src/sdk.rs
📚 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-sdk/Cargo.toml
📚 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-sdk/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-sdk/Cargo.toml
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use Rust for core platform components (Drive, DAPI server, DPP implementation)
Applied to files:
packages/rs-sdk/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-sdk/src/sdk.rs
📚 Learning: 2024-10-29T14:16:40.972Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/src/address_list.rs:63-73
Timestamp: 2024-10-29T14:16:40.972Z
Learning: In the Rust SDK, it's acceptable to use `expect()` that panics on errors in the `Mockable` trait implementations during testing, and the `Mockable` trait will be refactored in a future PR.
Applied to files:
packages/rs-sdk/src/sdk.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-sdk/src/sdk.rs
⏰ 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). (9)
- GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image - GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: JS NPM security audit
- GitHub Check: Rust crates security audit
- GitHub Check: Determine changed packages
- 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 (4)
packages/rs-dapi-client/Cargo.toml (1)
59-59: LGTM! Consistent lru version bump across the workspace.packages/rs-sdk-trusted-context-provider/Cargo.toml (1)
21-21: LGTM! Consistent lru version bump.packages/rs-sdk/Cargo.toml (2)
40-40: LGTM! Version bump to 0.16.3 addresses RUSTSEC-2026-0002 (IterMut memory-corruption vulnerability fixed January 7, 2026).
24-24: LGTM! Replacing unmaintainedrustls-pemfilewithrustls-pki-typesaddresses RUSTSEC-2025-0134. Version 1.13.2 is current (latest as of December 17, 2025).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/rs-sdk/src/sdk.rs:
- Around line 953-976: The code parses the certificate file with
rustls_pki_types::pem::from_buf which returns decoded DER bytes (pem_cert) but
then incorrectly calls Certificate::from_pem with those DER bytes; instead,
construct the tonic certificate from the original PEM file bytes (`pem`) so the
PEM markers are present. Keep the validation via from_buf (or use its result to
verify SectionKind::Certificate) but replace the Certificate::from_pem(pem_cert)
call with Certificate::from_pem(pem) (or otherwise feed the original PEM-encoded
bytes), then return self.with_ca_certificate(...) using that PEM-based
Certificate.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-sdk/src/sdk.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Rust code must passcargo clippy --workspacelinter checks
Rust code must be formatted usingcargo fmt --all
**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-sdk/src/sdk.rs
🧠 Learnings (6)
📓 Common learnings
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.
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.
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.
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.
📚 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/src/sdk.rs
📚 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/src/sdk.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/src/sdk.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-sdk/src/sdk.rs
📚 Learning: 2024-10-29T14:16:40.972Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/src/address_list.rs:63-73
Timestamp: 2024-10-29T14:16:40.972Z
Learning: In the Rust SDK, it's acceptable to use `expect()` that panics on errors in the `Mockable` trait implementations during testing, and the `Mockable` trait will be refactored in a future PR.
Applied to files:
packages/rs-sdk/src/sdk.rs
⏰ 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). (18)
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / 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: 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 (1)
packages/rs-sdk/src/sdk.rs (1)
1392-1412: Test coverage looks appropriate for Linux environments.The test correctly targets Linux-only systems and locates a system CA certificate to exercise
with_ca_certificate_file. However, the test's effectiveness depends on fixing the DER/PEM issue identified above.One minor observation: some
.pemfiles in/etc/ssl/certsmay be certificate bundles (containing multiple certificates). The current implementation only validates the first section, which is acceptable for single-CA use cases but worth noting.
|
Closes #2880 |
Issue being fixed or feature implemented
rustls-pemfile is unmaintained as per https://rustsec.org/advisories/RUSTSEC-2025-0134
LRU has minor issue https://rustsec.org/advisories/RUSTSEC-2026-0002
What was done?
How Has This Been Tested?
Github Actions green
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.