feat(platform-wallet): surface ProRegTx masternode details in iOS via rust-dashcore bump#4112
Conversation
Picks up dashpay/rust-dashcore#876 (FFISpecialTransactionPayload on FFITransactionRecord, exposing ProRegTx service IP), #867 (AssetLock routing covers CoinJoin/DashPay accounts) and #868 (CoinJoin gap limit 30 -> 100 for dashj parity). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nsaction records Extracts the DIP-3 provider payload (service address, collateral outpoint, owner/voting key hashes, and proTxHash for ProUpServTx) from dashcore's typed special_transaction_payload where the persister builds TransactionRecordFFI, persists the optional fields on PersistentTransaction, and renders a Masternode section in the transaction detail view for provider registration/update-service transactions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe update pins Dashcore dependencies to a new revision, extracts DIP-3 provider payloads through Rust FFI, persists them in Swift transaction records, and displays provider registration or service-update details in the example app. ChangesProvider payload pipeline
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Dashcore
participant RustFFI
participant SwiftPersistence
participant TransactionDetailView
Dashcore->>RustFFI: decode DIP-3 provider payload
RustFFI->>SwiftPersistence: expose provider fields and presence flags
SwiftPersistence->>TransactionDetailView: provide persisted provider values
TransactionDetailView->>TransactionDetailView: render provider details conditionally
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
🔍 Review in progress — actively reviewing now (commit 627cb6c) |
…dashcore-876 # Conflicts: # packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swift
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift (1)
91-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
copyableHashRowusage into the existing Transaction ID / Block Hash blocks too.The new
copyableHashRow(title:value:)helper duplicates the exact tap-to-copy markup already inlined for "Transaction ID" (Line 204) and "Block Hash" (Line 232). Since the helper now exists, replacing both call sites removes ~50 lines of triplicated layout code.♻️ Proposed refactor to reuse the new helper
- // Transaction ID - VStack(alignment: .leading, spacing: 8) { - Text("Transaction ID") - .font(.caption) - .foregroundColor(.secondary) - - Button { - copyToClipboard(transaction.txidHex) - } label: { - HStack { - Text(transaction.txidHex) - .font(.system(.footnote, design: .monospaced)) - .foregroundColor(.primary) - .lineLimit(nil) - .fixedSize(horizontal: false, vertical: true) - - Spacer() - - Image(systemName: "doc.on.doc") - .font(.caption) - .foregroundColor(.blue) - } - .padding() - .background(Color(UIColor.secondarySystemBackground)) - .cornerRadius(8) - } - } - - // Block Hash (if available) - if let blockHash = blockHashHex { - VStack(alignment: .leading, spacing: 8) { - Text("Block Hash") - .font(.caption) - .foregroundColor(.secondary) - - Button { - copyToClipboard(blockHash) - } label: { - HStack { - Text(blockHash) - .font(.system(.footnote, design: .monospaced)) - .foregroundColor(.primary) - .lineLimit(nil) - .fixedSize(horizontal: false, vertical: true) - - Spacer() - - Image(systemName: "doc.on.doc") - .font(.caption) - .foregroundColor(.blue) - } - .padding() - .background(Color(UIColor.secondarySystemBackground)) - .cornerRadius(8) - } - } - } + copyableHashRow(title: "Transaction ID", value: transaction.txidHex) + + if let blockHash = blockHashHex { + copyableHashRow(title: "Block Hash", value: blockHash) + }Also applies to: 204-260
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift` around lines 91 - 155, Update the existing Transaction ID and Block Hash sections in TransactionDetailView to call copyableHashRow(title:value:) instead of duplicating their inline tap-to-copy layout. Preserve their current titles, values, and conditional rendering while removing only the repeated markup.packages/rs-platform-wallet-ffi/src/core_wallet_types.rs (1)
956-975: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd ProUpServTx fixture coverage for
provider_payload_fields. TheProviderUpdateServicePayloadTypebranch has no test, so the service-address reconstruction andpro_tx_hashextraction can regress silently; add a known ProUpServTx fixture alongsideprovider_registration_payload_fields_extracted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-platform-wallet-ffi/src/core_wallet_types.rs` around lines 956 - 975, Add a ProUpServTx fixture-based test alongside provider_registration_payload_fields_extracted, exercising provider_payload_fields with ProviderUpdateServicePayloadType. Assert the reconstructed service_address and extracted pro_tx_hash match the known fixture values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/rs-platform-wallet-ffi/src/core_wallet_types.rs`:
- Around line 956-975: Add a ProUpServTx fixture-based test alongside
provider_registration_payload_fields_extracted, exercising
provider_payload_fields with ProviderUpdateServicePayloadType. Assert the
reconstructed service_address and extracted pro_tx_hash match the known fixture
values.
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift`:
- Around line 91-155: Update the existing Transaction ID and Block Hash sections
in TransactionDetailView to call copyableHashRow(title:value:) instead of
duplicating their inline tap-to-copy layout. Preserve their current titles,
values, and conditional rendering while removing only the repeated markup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5dbd9ff-b7c7-4587-b844-b1d65b8b1c52
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.tomlpackages/rs-platform-wallet-ffi/src/core_wallet_types.rspackages/swift-sdk/Sources/SwiftDashSDK/Persistence/Models/PersistentTransaction.swiftpackages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/TransactionDetailView.swift
…ture test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Both CodeRabbit nitpicks addressed in 2391dd2 (and the v4.1-dev merge conflict with #4109's PersistentTransaction helpers is resolved in cd0e46d — both sides kept):
Verified: 🤖 Addressed by Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.1-dev #4112 +/- ##
==========================================
Coverage 87.42% 87.42%
==========================================
Files 2640 2643 +3
Lines 333446 333632 +186
==========================================
+ Hits 291502 291684 +182
- Misses 41944 41948 +4
🚀 New features to boost your workflow:
|
…nd-example-app Conflict: Cargo.lock (base bumped grovedb 5.0.1 + rust-dashcore for the masternode-list work) — took base's resolution and re-resolved for the branch-only workspace members. Merge adaptation: WalletRestoreEntryFFI gained provider_special_txs(+count) (#4112/#4116); the JNI load path initializes them null/0 — the documented "wallet has no provider special txs" value — since Android has no provider-special-tx persistence yet (masternode-list parity tracked as a follow-up). Also lands the Sdk teardown behavioral test the review asked for: SdkLifecycleTest (4 tests over a native-free Sdk.forLifecycleTest seam — handle 0 skips the Cleaner's native destroy) pins close-awaits-lease, post-close fail-fast, cancellation-cannot-strand-cleanup, and the AutoCloseable.close() fence. The nested-C-export panic-containment finding is intentionally NOT addressed here: shumkov confirmed the mechanism and scoped the fix into follow-up #4121 (cfg-gated panic-catch inert under the iOS panic=abort profiles + PlatformWalletError surfacing); an ABI-wide change from this PR would cut across that. Verified: cargo fmt, platform-wallet-ffi lib tests (155), JNI check (shielded), workspace clippy --all-features -D warnings, gradle :sdk+:app tests (SdkLifecycleTest 4/4). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Issue being fixed or feature implemented
dashpay/rust-dashcore#876 landed typed DIP-2/DIP-3 special-transaction payloads (ProRegTx service IP, collateral, key hashes) plus provider-record retention, but nothing in the platform stack consumed them: a masternode registration in the wallet showed up as just "Provider Registration" with no details. This PR bumps the pin to pick that work up and surfaces the payload end-to-end in the iOS app.
What was done?
Dependency bump (
Cargo.toml/Cargo.lock): rust-dashcore234e3c43→337f6ccc, picking up #876 (typed special-tx payloads + provider retention), #867 (AssetLock routing covers CoinJoin/DashPay accounts), #868 (CoinJoin gap limit 30 → 100).Rust —
rs-platform-wallet-ffi/src/core_wallet_types.rs(all DIP-3 decoding stays in Rust):TransactionRecordFFIgains optional provider fields:provider_service_address(C string "ip:port"),provider_pro_tx_hash, collateral txid+vout, owner/voting key hashes — inline arrays withhas_*flags following the existingblock_hash/has_feepattern, freed infree_wallet_changeset_ffi.provider_payload_fields()seam decodesdashcore::TransactionPayload::{ProviderRegistration, ProviderUpdateService}wheretx_record_to_ffibuilds the record (ProRegTx: service/collateral/key hashes; ProUpServTx: service + proTxHash).Swift SDK (pure persist/bridge per architecture rules):
PersistentTransactiongains matching optional columns + hex display helpers;PlatformWalletPersistenceHandler.upsertTransactionmarshals the FFI fields gated on thehas_*flags.SwiftExampleApp:
TransactionDetailViewrenders a "Masternode" section (service address, proTxHash, collateral, key hashes with tap-to-copy rows) for provider registration / update-service transactions.Follow-ups deliberately out of scope: payout address (needs network-dependent script→address encoding), operator BLS key, and proTxHash follow in the compact-filter watch set (rust-dashcore#861 remainder — without it, ProUpServTx updates for owned masternodes don't match during sync).
How Has This Been Tested?
platform-wallet-ffi:provider_registration_payload_fields_extracted(rust-dashcore's testnet collateral-ProRegTx fixture; asserts service1.2.5.6:19999, owner/voting key hashes, collateral) andnon_provider_tx_has_no_provider_fields.cargo test -p platform-wallet-ffigreen; clippy clean.cargo check --workspace --all-targetsgreen against the new pin.build_ios.sh --target sim(warnings-as-errors) green; regenerated headers carry the new fields.ZPERSISTENTTRANSACTION), 1,775 pre-existing transaction rows intact.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes