chore: bump rust-dashcore dependency to latest commit#3397
Open
chore: bump rust-dashcore dependency to latest commit#3397
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR updates workspace dependency revisions, removes Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
ZocoLini
changed the title
Member
|
already been done |
Collaborator
Author
|
honestly, I was working on it bcs I know what has changed in rust-dashcode but I can see the commit was just changed to one thats not even merged yet, without updating any other code, and I also saw 2 open PRs and 1 closed trying to update it |
ZocoLini
force-pushed
the
chore/bump-dashcore
branch
from
3931d0b to
4b1d139
Compare
ZocoLini
force-pushed
the
chore/bump-dashcore
branch
from
4b1d139 to
9aeef00
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (3)
packages/rs-sdk-ffi/build_ios.sh (1)
81-81:⚠️ Potential issue | 🟡 MinorRemove
key-wallet-manager/srcfrom hash computation.Line 81 still includes
key-wallet-manager/srcin thefindcommand for detecting local rust-dashcore changes, butkey-wallet-managerhas been removed from the workspace dependencies and the cargo clean loop (line 98). This inconsistency could trigger unnecessary clean builds whenkey-wallet-managersource changes, even though that crate is no longer used.Proposed fix
- CURRENT_HASH=$(find "$RUST_DASHCORE_DIR/dash/src" "$RUST_DASHCORE_DIR/key-wallet/src" "$RUST_DASHCORE_DIR/dash-spv/src" "$RUST_DASHCORE_DIR/dash-spv-ffi/src" "$RUST_DASHCORE_DIR/key-wallet-manager/src" -name '*.rs' 2>/dev/null | sort | xargs cat 2>/dev/null | shasum -a 256 | cut -d' ' -f1) + CURRENT_HASH=$(find "$RUST_DASHCORE_DIR/dash/src" "$RUST_DASHCORE_DIR/key-wallet/src" "$RUST_DASHCORE_DIR/dash-spv/src" "$RUST_DASHCORE_DIR/dash-spv-ffi/src" -name '*.rs' 2>/dev/null | sort | xargs cat 2>/dev/null | shasum -a 256 | cut -d' ' -f1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk-ffi/build_ios.sh` at line 81, Remove the removed crate from the hash computation: update the CURRENT_HASH find invocation to exclude "key-wallet-manager/src" so it matches the workspace changes and the cargo clean loop; specifically, edit the find arguments referenced by the CURRENT_HASH variable (the find command that lists "$RUST_DASHCORE_DIR/...") to drop the "key-wallet-manager/src" entry so local rust-dashcore changes no longer consider that directory when computing the hash.packages/rs-platform-wallet/README.md (1)
23-25:⚠️ Potential issue | 🟡 MinorUpdate code example to use the new import path.
The code example still references
key_wallet_manager::wallet_manager::WalletManager, but this import path no longer exists after removingkey-wallet-manager. The example should use the updated import fromkey_wallet::manager::WalletManageras shown inexamples/basic_usage.rs.Proposed fix
use platform_wallet::PlatformWalletInfo; -use key_wallet_manager::wallet_manager::WalletManager; +use key_wallet::manager::WalletManager; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-platform-wallet/README.md` around lines 23 - 25, The README code example uses the removed import path key_wallet_manager::wallet_manager::WalletManager; update that import to the new path key_wallet::manager::WalletManager to match examples/basic_usage.rs and the current crate layout, ensuring the use statement alongside PlatformWalletInfo and WalletInfoInterface is adjusted to key_wallet::manager::WalletManager.packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift (1)
280-292:⚠️ Potential issue | 🟡 MinorConsider strengthening concurrency safety for Sendable event handlers storing
@MainActorreferences.Event handlers (
SPVProgressUpdateEventHandlerImpl,SPVSyncEventsHandlerImpl,SPVNetworkEventsHandlerImpl,SPVWalletEventsHandlerImpl,SPVClientErrorEventsHandlerImpl) are markedSendablebut store a reference toWalletService, which is@MainActor. In strict concurrency mode, this creates a type safety violation (non-Sendablereference inSendabletype), even though current mitigations viaTask {@mainactorin ... }function correctly for FFI callback dispatch.While strict concurrency checking is not currently enabled and existing code properly dispatches to the main thread, consider using a weak reference or actor-isolated pattern to future-proof the implementation against strict concurrency enforcement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift` around lines 280 - 292, The event handler types (e.g., SPVProgressUpdateEventHandlerImpl) are Sendable but hold a strong reference to a `@MainActor` WalletService, which will break strict concurrency checks; change the stored walletService to a weak optional (e.g., weak var walletService: WalletService?) in SPVProgressUpdateEventHandlerImpl (and the other handlers named in the review), update the initializer to store the weak reference, and in onProgressUpdate unwrap the optional (guard let ws = walletService else { return }) before calling Task { `@MainActor` in ws.syncProgress = progress } so the handler no longer retains a non-Sendable reference while preserving MainActor dispatch.
🧹 Nitpick comments (6)
packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs (1)
37-42: Consider using structured logging instead ofeprintln!.The
eprintln!macro writes directly to stderr, which is less flexible than structured logging. If the crate usestracingorlog, consider using those for consistency with the rest of the codebase.This is a minor observation and may be outside the scope of this dependency bump PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs` around lines 37 - 42, Replace the eprintln! call in wallet_transaction_checker.rs inside the fetch_identity_and_contacts_for_asset_lock error branch with the crate's structured logging macro (e.g., tracing::error! or log::error!) so the failure is recorded consistently; call tracing::error!("Failed to fetch identity for asset lock for wallet {:?}, tx {:?}: {:?}", wallet.id, tx.id, e) (or similar contextual fields available) and add the necessary use/import for tracing::error (or log::error) at the top of the file to ensure the logger is available.packages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVEventHandler.swift (3)
565-567: Remove trailing semicolon.Static analysis flagged the trailing semicolon which is not idiomatic Swift.
🔧 Proposed fix
protocol SPVClientErrorEventsHandler: AnyObject { - func onClientError(_ error: String); + func onClientError(_ error: String) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVEventHandler.swift` around lines 565 - 567, The protocol declaration for SPVClientErrorEventsHandler contains a trailing semicolon after the method signature in func onClientError(_ error: String); — remove the semicolon so the signature reads func onClientError(_ error: String) to match Swift style and satisfy static analysis; update the protocol SPVClientErrorEventsHandler accordingly.
260-271: UnusedconfirmedTxidsandconfirmedTxidCountparameters.The new FFI callback signature includes
confirmedTxidsandconfirmedTxidCountparameters that are not being forwarded to the Swift handler. If this data is relevant, consider extendingSPVSyncEventsHandler.onBlocksProcessedto accept the confirmed transaction IDs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVEventHandler.swift` around lines 260 - 271, The callback onSpvBlockProcessedCallbackC currently ignores the confirmedTxids and confirmedTxidCount parameters; update the FFI bridge to convert confirmedTxids (UnsafePointer<Byte32>?) into a Swift [Data] (using confirmedTxidCount to determine length, similar to bytePtrIntoData) and forward that array to the SPV sync handler by extending rawPtrIntoSpvSyncEventsHandler().onBlocksProcessed to accept a confirmedTxids parameter (e.g., onBlocksProcessed(height:hash:newAddressCount:confirmedTxids:)), then update the call site in onSpvBlockProcessedCallbackC to pass the converted confirmedTxids array. Ensure null/zero-count handling is safe.
473-500: Unusedstatus: FFITransactionContextparameter.The
statusparameter is received from FFI but not passed tohandler.onTransactionReceived(...). If transaction status context is meaningful, consider extendingSPVWalletEventsHandler.onTransactionReceivedto include it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVEventHandler.swift` around lines 473 - 500, The FFI `status: FFITransactionContext` parameter in onSpvTransactionReceivedCallbackC is currently unused; update the SPV event path to pass it through by adding a corresponding parameter to SPVWalletEventsHandler.onTransactionReceived and forwarding `status` from onSpvTransactionReceivedCallbackC into that call (adjusting all implementations of SPVWalletEventsHandler to accept and handle the new FFITransactionContext argument), or if you intend to ignore it, explicitly consume it (e.g., assign to _ or call a mapping function) to avoid unused-parameter warnings; locate the change at onSpvTransactionReceivedCallbackC and the SPVWalletEventsHandler.onTransactionReceived declarations/implementations and update them consistently.packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift (2)
370-378: Empty error handler implementation - consider logging errors.
SPVClientErrorEventsHandlerImpl.onClientErrordoes nothing with received errors. Consider at minimum logging the error or updatinglastSyncError.♻️ Proposed implementation
internal final class SPVClientErrorEventsHandlerImpl: SPVClientErrorEventsHandler, Sendable { private let walletService: WalletService init(walletService: WalletService) { self.walletService = walletService } - func onClientError(_ error: String) {} + func onClientError(_ error: String) { + SDKLogger.error("SPV client error: \(error)") + + Task { `@MainActor` in + walletService.lastSyncError = SPVError.syncFailed(error) + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift` around lines 370 - 378, SPVClientErrorEventsHandlerImpl.onClientError currently ignores errors; update it to record and surface failures by calling into the owning WalletService: set or update WalletService.lastSyncError (or an appropriate error property) with the received error string and emit a log entry via the walletService logger (or use os_log) including the error text and context (e.g., "SPV client error"). Ensure you reference SPVClientErrorEventsHandlerImpl.onClientError and WalletService when implementing the update so the handler stores the error and logs it for debugging and state inspection.
107-153: Consider documenting the two-phase initialization pattern.The init creates an SPVClient with dummy handlers, extracts the wallet manager, destroys it, then recreates with real handlers that reference
self. While the comments explain this is necessary for self-referencing handlers, a brief doc comment onWalletServiceexplaining this lifecycle would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift` around lines 107 - 153, Add a short doc comment to WalletService describing the two-phase initialization lifecycle: explain that init temporarily constructs an SPVClient with dummy SPVEventHandlers to obtain the shared CoreWalletManager via CoreWalletManager(spvClient:spvClient, modelContainer:), then destroys that client (spvClient.destroy()) and re-initializes a second SPVClient with real SPVEventHandlers that capture self for event callbacks; mention reasons (handlers need self) and any implications for error handling or ordering around self.walletManager and self.spvClient to help future maintainers locate the pattern around init, SPVClient, SPVEventHandlers, spvClient, and walletManager.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/rs-platform-wallet/src/platform_wallet_info/wallet_info_interface.rs`:
- Around line 115-117: The method mark_instant_send_utxos currently panics via
todo!(); replace it with a delegation to the wrapped wallet info instance used
elsewhere in this impl—call the inner/wrapped object's mark_instant_send_utxos
with the provided &dashcore::Txid and return its bool result (ensure you use the
same field name used by other methods in this impl, e.g., self.inner or
self.wrapped, and respect &mut self for the mutable call).
---
Outside diff comments:
In `@packages/rs-platform-wallet/README.md`:
- Around line 23-25: The README code example uses the removed import path
key_wallet_manager::wallet_manager::WalletManager; update that import to the new
path key_wallet::manager::WalletManager to match examples/basic_usage.rs and the
current crate layout, ensuring the use statement alongside PlatformWalletInfo
and WalletInfoInterface is adjusted to key_wallet::manager::WalletManager.
In `@packages/rs-sdk-ffi/build_ios.sh`:
- Line 81: Remove the removed crate from the hash computation: update the
CURRENT_HASH find invocation to exclude "key-wallet-manager/src" so it matches
the workspace changes and the cargo clean loop; specifically, edit the find
arguments referenced by the CURRENT_HASH variable (the find command that lists
"$RUST_DASHCORE_DIR/...") to drop the "key-wallet-manager/src" entry so local
rust-dashcore changes no longer consider that directory when computing the hash.
In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift`:
- Around line 280-292: The event handler types (e.g.,
SPVProgressUpdateEventHandlerImpl) are Sendable but hold a strong reference to a
`@MainActor` WalletService, which will break strict concurrency checks; change the
stored walletService to a weak optional (e.g., weak var walletService:
WalletService?) in SPVProgressUpdateEventHandlerImpl (and the other handlers
named in the review), update the initializer to store the weak reference, and in
onProgressUpdate unwrap the optional (guard let ws = walletService else { return
}) before calling Task { `@MainActor` in ws.syncProgress = progress } so the
handler no longer retains a non-Sendable reference while preserving MainActor
dispatch.
---
Nitpick comments:
In
`@packages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rs`:
- Around line 37-42: Replace the eprintln! call in wallet_transaction_checker.rs
inside the fetch_identity_and_contacts_for_asset_lock error branch with the
crate's structured logging macro (e.g., tracing::error! or log::error!) so the
failure is recorded consistently; call tracing::error!("Failed to fetch identity
for asset lock for wallet {:?}, tx {:?}: {:?}", wallet.id, tx.id, e) (or similar
contextual fields available) and add the necessary use/import for tracing::error
(or log::error) at the top of the file to ensure the logger is available.
In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift`:
- Around line 370-378: SPVClientErrorEventsHandlerImpl.onClientError currently
ignores errors; update it to record and surface failures by calling into the
owning WalletService: set or update WalletService.lastSyncError (or an
appropriate error property) with the received error string and emit a log entry
via the walletService logger (or use os_log) including the error text and
context (e.g., "SPV client error"). Ensure you reference
SPVClientErrorEventsHandlerImpl.onClientError and WalletService when
implementing the update so the handler stores the error and logs it for
debugging and state inspection.
- Around line 107-153: Add a short doc comment to WalletService describing the
two-phase initialization lifecycle: explain that init temporarily constructs an
SPVClient with dummy SPVEventHandlers to obtain the shared CoreWalletManager via
CoreWalletManager(spvClient:spvClient, modelContainer:), then destroys that
client (spvClient.destroy()) and re-initializes a second SPVClient with real
SPVEventHandlers that capture self for event callbacks; mention reasons
(handlers need self) and any implications for error handling or ordering around
self.walletManager and self.spvClient to help future maintainers locate the
pattern around init, SPVClient, SPVEventHandlers, spvClient, and walletManager.
In `@packages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVEventHandler.swift`:
- Around line 565-567: The protocol declaration for SPVClientErrorEventsHandler
contains a trailing semicolon after the method signature in func onClientError(_
error: String); — remove the semicolon so the signature reads func
onClientError(_ error: String) to match Swift style and satisfy static analysis;
update the protocol SPVClientErrorEventsHandler accordingly.
- Around line 260-271: The callback onSpvBlockProcessedCallbackC currently
ignores the confirmedTxids and confirmedTxidCount parameters; update the FFI
bridge to convert confirmedTxids (UnsafePointer<Byte32>?) into a Swift [Data]
(using confirmedTxidCount to determine length, similar to bytePtrIntoData) and
forward that array to the SPV sync handler by extending
rawPtrIntoSpvSyncEventsHandler().onBlocksProcessed to accept a confirmedTxids
parameter (e.g.,
onBlocksProcessed(height:hash:newAddressCount:confirmedTxids:)), then update the
call site in onSpvBlockProcessedCallbackC to pass the converted confirmedTxids
array. Ensure null/zero-count handling is safe.
- Around line 473-500: The FFI `status: FFITransactionContext` parameter in
onSpvTransactionReceivedCallbackC is currently unused; update the SPV event path
to pass it through by adding a corresponding parameter to
SPVWalletEventsHandler.onTransactionReceived and forwarding `status` from
onSpvTransactionReceivedCallbackC into that call (adjusting all implementations
of SPVWalletEventsHandler to accept and handle the new FFITransactionContext
argument), or if you intend to ignore it, explicitly consume it (e.g., assign to
_ or call a mapping function) to avoid unused-parameter warnings; locate the
change at onSpvTransactionReceivedCallbackC and the
SPVWalletEventsHandler.onTransactionReceived declarations/implementations and
update them consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4daba1b2-98ed-48a5-b722-a55d3d1374a7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
Cargo.tomlpackages/rs-dpp/Cargo.tomlpackages/rs-dpp/src/lib.rspackages/rs-drive-abci/src/platform_types/masternode/v0/mod.rspackages/rs-platform-wallet/Cargo.tomlpackages/rs-platform-wallet/README.mdpackages/rs-platform-wallet/examples/basic_usage.rspackages/rs-platform-wallet/src/lib.rspackages/rs-platform-wallet/src/platform_wallet_info/wallet_info_interface.rspackages/rs-platform-wallet/src/platform_wallet_info/wallet_transaction_checker.rspackages/rs-sdk-ffi/build_ios.shpackages/rs-sdk-ffi/src/lib.rspackages/rs-sdk-ffi/src/unified.rspackages/rs-sdk/Cargo.tomlpackages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVClient.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Core/SPV/SPVEventHandler.swiftpackages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift
💤 Files with no reviewable changes (5)
- packages/rs-dpp/src/lib.rs
- packages/rs-platform-wallet/src/lib.rs
- packages/rs-sdk-ffi/src/lib.rs
- packages/rs-dpp/Cargo.toml
- packages/rs-sdk-ffi/src/unified.rs
packages/rs-platform-wallet/src/platform_wallet_info/wallet_info_interface.rs
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3397 +/- ##
============================================
- Coverage 79.93% 79.93% -0.01%
============================================
Files 2861 2861
Lines 280230 280236 +6
============================================
Hits 223993 223993
- Misses 56237 56243 +6
🚀 New features to boost your workflow:
|
Contributor
|
✅ 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: "e6a0d70651a92448119f1ea1c541379f90a06ae6d611bfce036996dc7a97cbea"
)Xcode manual integration:
|
ZocoLini
force-pushed
the
chore/bump-dashcore
branch
from
a25b8ca to
399bc7f
Compare
thepastaclaw
suggested changes
Mar 26, 2026
Collaborator
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Verified 2 findings in the full PR diff (origin/v3.1-dev...HEAD). Both convergent findings are real and directly introduced on this branch. The on_transaction_status_changed report is not included because the source shows the callback slot was added and left nil, but the available repo code does not prove that this now causes a user-visible regression.
Reviewed commit: 399bc7f
🔴 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 `packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift`:
- [BLOCKING] lines 123-152: WalletService binds walletManager to an SPV client instance that it immediately destroys
In the PR diff, `WalletService.init` now creates `self.spvClient` with dummy handlers, constructs `walletManager` from that client, then calls `self.spvClient.destroy()` and replaces it with a second `SPVClient` using real handlers. That leaves `walletManager` backed by the first, destroyed Rust client instead of the live one stored in `self.spvClient`. The later `initializeNewSPVClient()` path recreates `walletManager` after creating the final client, which reinforces that the initializer is inconsistent and likely wrong.
In `packages/rs-sdk-ffi/src/lib.rs`:
- [BLOCKING] lines 32-35: PR removes unified FFI exports but keeps shipped headers declaring `dash_unified_sdk_*`
The branch deletes `packages/rs-sdk-ffi/src/unified.rs` and removes `mod unified;` / `pub use unified::*;` from `packages/rs-sdk-ffi/src/lib.rs`, so the `dash_unified_sdk_*` symbols are no longer implemented. However, the committed headers still declare those APIs in `packages/rs-sdk-ffi/include/dash_sdk_ffi.h:2148-2219` and `packages/rs-sdk-ffi/test_header.h:1608-1677`, and cbindgen configs still include the `dash_unified_sdk_*` prefix. Consumers compiling against the shipped headers will still expect those symbols and can fail at link/load time.
packages/swift-sdk/Sources/SwiftDashSDK/Core/Services/WalletService.swift
Show resolved
Hide resolved
Comment on lines
32
to
35
| mod system; | ||
| mod token; | ||
| mod types; | ||
| mod unified; | ||
| mod utils; |
Collaborator
There was a problem hiding this comment.
🔴 Blocking: PR removes unified FFI exports but keeps shipped headers declaring dash_unified_sdk_*
The branch deletes packages/rs-sdk-ffi/src/unified.rs and removes mod unified; / pub use unified::*; from packages/rs-sdk-ffi/src/lib.rs, so the dash_unified_sdk_* symbols are no longer implemented. However, the committed headers still declare those APIs in packages/rs-sdk-ffi/include/dash_sdk_ffi.h:2148-2219 and packages/rs-sdk-ffi/test_header.h:1608-1677, and cbindgen configs still include the dash_unified_sdk_* prefix. Consumers compiling against the shipped headers will still expect those symbols and can fail at link/load time.
source: codex
🤖 Fix this 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 `packages/rs-sdk-ffi/src/lib.rs`:
- [BLOCKING] lines 32-35: PR removes unified FFI exports but keeps shipped headers declaring `dash_unified_sdk_*`
The branch deletes `packages/rs-sdk-ffi/src/unified.rs` and removes `mod unified;` / `pub use unified::*;` from `packages/rs-sdk-ffi/src/lib.rs`, so the `dash_unified_sdk_*` symbols are no longer implemented. However, the committed headers still declare those APIs in `packages/rs-sdk-ffi/include/dash_sdk_ffi.h:2148-2219` and `packages/rs-sdk-ffi/test_header.h:1608-1677`, and cbindgen configs still include the `dash_unified_sdk_*` prefix. Consumers compiling against the shipped headers will still expect those symbols and can fail at link/load time.
Collaborator
Author
There was a problem hiding this comment.
@QuantumExplorer how are you updating the headers?? any script to generate them or I should use cbindgen cli directly??
ZocoLini
force-pushed
the
chore/bump-dashcore
branch
from
399bc7f to
45b23c9
Compare
shumkov
requested changes
Mar 27, 2026
| @@ -12,9 +12,6 @@ pub use dashcore; | |||
| #[cfg(feature = "core_key_wallet")] | |||
| pub use key_wallet; | |||
|
|
|||
Collaborator
There was a problem hiding this comment.
We stll have manager and there is a feature flag to enable or disable it. For some crates we need it enabled and for some of them - disabled.
Collaborator
Author
There was a problem hiding this comment.
Tell me one crate that need the manager to be enable and other one that needs it to be disble
shumkov
added a commit
that referenced
this pull request
Mar 27, 2026
Update rust-dashcore from 8cbae416 to 88eacdf1 (v0.42-dev HEAD). Changes: - Remove key-wallet-manager workspace dep (merged into key-wallet::manager) - Set key-wallet default-features = false at workspace level for WASM safety - Add core_key_wallet_manager feature in dpp (enables key-wallet/manager) - Add core_key_wallet_manager to rs-sdk spv-client feature - Platform-wallet explicitly enables key-wallet/manager - Remove "std" feature from dashcore dep in dpp (removed upstream) - Update WalletInfoInterface: add mark_instant_send_utxos, monitor_revision - Update WalletTransactionChecker: &mut Wallet, update_balance param - Swift SDK: migrate to unified FFIEventCallbacks pattern - build_ios.sh: add header generation fallback, forward declarations - Remove unified.rs (replaced by direct FFI usage) Based on colleague's PR #3397 with proper feature gating preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shumkov
added a commit
that referenced
this pull request
Mar 27, 2026
Update rust-dashcore from 8cbae416 to 88eacdf1 (v0.42-dev HEAD). Changes: - Remove key-wallet-manager workspace dep (merged into key-wallet::manager) - Set key-wallet default-features = false at workspace level for WASM safety - Add core_key_wallet_manager feature in dpp (enables key-wallet/manager) - Add core_key_wallet_manager to rs-sdk spv-client feature - Platform-wallet explicitly enables key-wallet/manager - Remove "std" feature from dashcore dep in dpp (removed upstream) - Update WalletInfoInterface: add mark_instant_send_utxos, monitor_revision - Update WalletTransactionChecker: &mut Wallet, update_balance param - Swift SDK: migrate to unified FFIEventCallbacks pattern - build_ios.sh: add header generation fallback, forward declarations - Remove unified.rs (replaced by direct FFI usage) Based on colleague's PR #3397 with proper feature gating preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shumkov
added a commit
that referenced
this pull request
Mar 27, 2026
Update rust-dashcore from 8cbae416 to 88eacdf1 (v0.42-dev HEAD). Changes: - Remove key-wallet-manager workspace dep (merged into key-wallet::manager) - Set key-wallet default-features = false at workspace level for WASM safety - Add core_key_wallet_manager feature in dpp (enables key-wallet/manager) - Add core_key_wallet_manager to rs-sdk spv-client feature - Platform-wallet explicitly enables key-wallet/manager - Remove "std" feature from dashcore dep in dpp (removed upstream) - Update WalletInfoInterface: add mark_instant_send_utxos, monitor_revision - Update WalletTransactionChecker: &mut Wallet, update_balance param - Swift SDK: migrate to unified FFIEventCallbacks pattern - build_ios.sh: add header generation fallback, forward declarations - Remove unified.rs (replaced by direct FFI usage) Based on colleague's PR #3397 with proper feature gating preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I am glad to announce, this PR goes through CI
Changes made:
How did I test this??
I am open to discuss the changes made in this PR with real humans and adjust the code if needed
Summary by CodeRabbit
Release Notes
Breaking Changes
Improvements
Removals