Repository navigation
chore(deps)!: bump dashpay/platform to v4.3-dev at protocol 13 - #1016
Merged
Merged
Conversation
Pin dash-sdk, rs-sdk-trusted-context-provider, platform-wallet and platform-wallet-storage to dashpay/platform#4764 head 4d1db8a5, which contains the full v4.3-dev branch plus the available-signer key selection fix DET relies on for HASH160 DashPay profile keys. This commit alone does not compile: protocol version 14 API changes are adapted in follow-up commits on this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Make DET compile against dashpay/platform v4.3-dev without hiding the new variants behind catch-alls: - same_key compares identity public keys of either version through one identity tuple. disabled_at, total_budget and expires_at are state that Platform moves (disable, IdentityKeyLimitsUpdate); whether a key has a budget or an expiry is fixed for its life and still identifies it. A V1 key without limits equals its V0 wire form. - Key info shows contract-group bounds. - PlatformWalletError::TokenOperationFailed is classified through the normal SDK error mapping; an otherwise unclassified failure becomes the new TaskError::TokenOperationFailed, which names the operation. - StateTransitionCreationOptions carries no action fee agreement from the signing-override helper; document transitions add their own. - Tests and fixtures cover the new enum variants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
21d70b3c restores the IdentitySettersV0 import that the v4.3-dev merge in dashpay/platform#4764 dropped; 4d1db8a5 did not compile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c5c4d316 is the head of dashpay/platform#4764 (fix/dashpay-available-signer): 21d70b3c plus signer-aware identity key selection in platform-wallet (277b5356, 088499f4). The pin still sits on an unmerged PR branch; re-pin to v4.3-dev once #4764 merges. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…m#4764 merges Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The platform pin sits on fix/dashpay-available-signer (dashpay/platform#4764), which carries all of v4.3-dev plus the available-signer fix DET depends on, so it is not yet an ancestor of any release line. The wallet-storage migration fingerprint is unchanged (18 files). Temporary: once #4764 merges, re-pin to a commit on v4.3-dev, set the target branch to v4.3-dev and rerun --update. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The platform pin knows protocol 14, so `PlatformVersion::latest()` is now PLATFORM_V14, while mainnet and testnet run protocol 13. DET must not build or validate protocol-14 structures yet. - Add `DET_PLATFORM_VERSION` (PLATFORM_V13), the single source for `default_platform_version`: `AppContext::platform_version()` (validation, building, signing), the SDK's initial version seed and therefore system-contract loading. - Platform info loaded the Withdrawals contract and computed the daily withdrawal limit at `latest()`; both now use `DET_PLATFORM_VERSION`, and a test proves the limit is protocol 13's flat 2000 Dash, not protocol 14's relative one. - Update the platform#4231 TODO (the fix is in the pin now) and the devnet seeding TODO; rename the v12 seeding test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DET targets protocol 13, so the new upstream variants only need the simplest correct handling to compile, not protocol-14 semantics. - `PlatformWalletError::TokenOperationFailed` is upstream's typed successor of `TokenError` with the same message; map it through the same generic `WalletBackend` arm. Drop `TokenOperationKind`, `TaskError::TokenOperationFailed` and `from_token_operation_failure`. - `same_key` keeps the original exhaustive V0 comparison. V1 keys (protocol 14 only) compare strictly apart from `disabled_at`, and a V1 key never equals a V0 one, since upstream writes a limit-less key as V0. The limit-raise semantics of `IdentityKeyLimitsUpdate` are left for protocol 14 support. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: dashpay/dash-evo-tool/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request updates platform dependency pins, moves DET to protocol 13, aligns withdrawal operations with that version, adds identity key and contract-group handling, and updates wallet error mapping and token distribution UI behavior. ChangesPlatform version and migration pinning
Identity key and contract-bound handling
Wallet and token behavior updates
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~30 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to DET now targets protocol 13 consistently, with updated identity, withdrawal, wallet, and token handling. The supplied checks report no dependency-bump regression, so the change is ready to merge with normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 12 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
Collaborator
|
|
github-actions
Bot
added
the
waiting-bots
lklimek
approved these changes
Sep 22, 2026
Collaborator
Author
Backend E2E on testnet (protocol 13), identity keysRun at
Test-harness issues found (pre-existing, not in this PR):
|
lklimek
added
the
claudius-review
dashpay/platform#4764 merged as 9f7ed169, whose tree is identical to the previous pin c5c4d316, so no code changes follow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lklimek
enabled auto-merge (squash)
September 22, 2026 12:56
Contributor
|
/self-reviewed |
Contributor
|
@coderabbitai review all |
Contributor
|
✅ Action performedFull review finished. |
5 tasks done
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
⚠️ DEGRADED — Final validation — Phase 1 + Phase 2
⚠️ DEGRADED review. The primary review models were unavailable (gpt-6-astraunavailable: All credentials for model gpt-6-astra are cooling down (last error: usage_limit_reached: The usage limit has been reache), so this review ran on stand-in models:gpt-5.6-luna→muse-spark-1.3-contributor,gpt-5.6-sol→muse-spark-1.3-contributor,gpt-5.6-terra→muse-spark-1.3-contributor,gpt-6-astra→muse-spark-1.3-contributor. Both review phases and the independent verifiers still ran, but on weaker models, with Phase 1 capped athigheffort. Treat the verdict as provisional; a full-strength re-review will run on the next push once the primary models are back.
Protocol-13 pin is correctly scoped and verified against the pinned upstream sources. No blocking defects found; the four remaining items are new-arm test gaps and low-risk consistency nits introduced by the pin switch.
🟡 2 suggestion(s) | 💬 2 nitpick(s)
2 finding(s) not shown inline (the lines are not part of this PR's diff)
💬 Nitpick: FAILED (5) query value is now filtered against the protocol-13 withdrawals contract, whose status enum stops at 4
src/backend_task/platform_info.rs:783-789
Both completed-withdrawals queries now load the withdrawals contract at DET_PLATFORM_VERSION (v1, status enum 0-4, verified in the pinned withdrawals-contract schemas) but still filter on WithdrawalStatus::FAILED (5), which only exists in the v2 schema. If Drive validates where-values against the property enum the query errors on protocol-13 networks; otherwise the value is dead but harmless. Dropping the v14-only value keeps the query consistent with the schema this PR pins.
value: Value::Array(vec![
Value::U8(WithdrawalStatus::COMPLETE as u8),
Value::U8(WithdrawalStatus::EXPIRED as u8),
]),
source: glm-5.3-flash (phase1-reviewer: general)
💬 Nitpick: Stale platform#4231 restore-condition comment contradicts the updated cross-reference
src/backend_task/platform_info.rs:619-627
context/mod.rs now states the platform#4231 fix is merged and contained in the current pin, but this arm still conditions restoration on the fix merging and the pin advancing past rev a18bd158, which is no longer the pinned rev (Cargo.lock is at 9f7ed169). Deferring the live-fetch restoration is the right minimal-scope call; the comment should stop asserting conditions that no longer hold so future readers do not re-litigate the workaround.
// TODO(platform#4231): restore the commented-out live fetch below and drop the
// hardcoded multiplier. The fix is merged upstream and contained in the current
// pin (9f7ed169), whose proof verifier accepts the descending-epoch-without-start
// query shape `fetch_current` sends again; the previously broken shape failed on
// every DAPI node, cycling the SDK's whole address pool and surfacing as
// `DapiAllAddressesExhausted` in unrelated flows such as identity top-up. This
// task also runs automatically on every SPV Syncing->Synced transition, so keep
// that pool-cycling risk in mind when restoring.
source: glm-5.3-flash (phase1-reviewer: general, rust-quality)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 3: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: general); reviewer 4: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: rust-quality); final verifier: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: astra-verifier, role: final-verifier)
- Degraded mode:
gpt-6-astraunavailable: All credentials for model gpt-6-astra are cooling down (last error: usage_limit_reached: The usage limit has been reache (detected by probe, since 2026-09-20T20:45:34Z); stand-insgpt-5.6-luna→muse-spark-1.3-contributor,gpt-5.6-sol→muse-spark-1.3-contributor,gpt-5.6-terra→muse-spark-1.3-contributor,gpt-6-astra→muse-spark-1.3-contributor; Phase 1 effort capped athigh - Triage:
normalbymuse-spark-1.3-contributor(standing in forgpt-6-astra) (effort low) — Dependency bump to v4.3-dev with minimal compile adaptations and explicit protocol 13 pinning preserves existing withdrawal, signing, and migration behavior without changing a critical surface. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— rust-quality (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 82% left, weekly 59% left; passed overgemini-3.8-flash-high(lane failed) - Fresh verifier:
muse-spark-1.3-contributor(standing in forgpt-6-astra) — final-verifier; agentastra-verifier - Phase 2 reviewers:
muse-spark-1.3-contributor(standing in forgpt-6-astra) — general (completed, effort high); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6-astra) — rust-quality (completed, effort high); agentphase2-reviewer
🤖 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 `src/model/qualified_identity/encrypted_key_storage.rs`:
- [SUGGESTION] src/model/qualified_identity/encrypted_key_storage.rs:1186-1194: V1 expires_at comparison has no test coverage
same_key compares both total_budget and expires_at for V1 keys, but the test only varies total_budget (1_000 vs 5_000) and never mutates expires_at. A regression that dropped the expires_at conjunct would still pass, leaving one conjunct of the new V1 branch unpinned.
In `src/wallet_backend/mod.rs`:
- [SUGGESTION] src/wallet_backend/mod.rs:3456-3460: TokenOperationFailed bucket in identity classifier is untested
identity_op_error_kind buckets TokenOperationFailed as Rejected, but no test constructs that variant. The existing register/top-up tests cover Sdk, broadcast, finality, and Other cases, so the only arm that interprets a struct variant with payload could be moved to a different bucket with no test failure, silently changing identity-flow messaging.
In `src/backend_task/platform_info.rs`:
- [NITPICK] src/backend_task/platform_info.rs:783-789: FAILED (5) query value is now filtered against the protocol-13 withdrawals contract, whose status enum stops at 4
Both completed-withdrawals queries now load the withdrawals contract at DET_PLATFORM_VERSION (v1, status enum 0-4, verified in the pinned withdrawals-contract schemas) but still filter on WithdrawalStatus::FAILED (5), which only exists in the v2 schema. If Drive validates where-values against the property enum the query errors on protocol-13 networks; otherwise the value is dead but harmless. Dropping the v14-only value keeps the query consistent with the schema this PR pins.
- [NITPICK] src/backend_task/platform_info.rs:619-627: Stale platform#4231 restore-condition comment contradicts the updated cross-reference
context/mod.rs now states the platform#4231 fix is merged and contained in the current pin, but this arm still conditions restoration on the fix merging and the pin advancing past rev a18bd158, which is no longer the pinned rev (Cargo.lock is at 9f7ed169). Deferring the live-fetch restoration is the right minimal-scope call; the comment should stop asserting conditions that no longer hold so future readers do not re-litigate the workaround.
Out-of-scope follow-up suggestions (1)
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.
- Backend-e2e harness silently reports skipped masternode tests as ok — The author's testnet comment documents 8 of 28 masternode e2e tests reporting ok without executing because opt_env returns early on a missing variable, plus a stale TC-MN-020 expectation. This is pre-existing harness behavior untouched by this PR, but it weakens the backend-e2e evidence for any PR that relies on it.
- Follow-up: Separate maintainer-tracked issue: make the e2e harness fail or emit an explicit skipped marker when required E2E_MN_* variables are absent, and update TC-MN-020's expected TaskError variant.
Comment on lines
+1186
to
+1194
| fn same_key_rejects_a_v1_key_against_v0_or_other_limits() { | ||
| let base = IdentityPublicKey::random_key(0, Some(1), PlatformVersion::latest()); | ||
| let limited = base.clone().with_limits(Some(1_000), None); | ||
| let raised = base.clone().with_limits(Some(5_000), None); | ||
|
|
||
| assert!(!same_key(&base, &limited)); | ||
| assert!(!same_key(&limited, &base)); | ||
| assert!(!same_key(&limited, &raised)); | ||
| } |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: V1 expires_at comparison has no test coverage
same_key compares both total_budget and expires_at for V1 keys, but the test only varies total_budget (1_000 vs 5_000) and never mutates expires_at. A regression that dropped the expires_at conjunct would still pass, leaving one conjunct of the new V1 branch unpinned.
Suggested change
| fn same_key_rejects_a_v1_key_against_v0_or_other_limits() { | |
| let base = IdentityPublicKey::random_key(0, Some(1), PlatformVersion::latest()); | |
| let limited = base.clone().with_limits(Some(1_000), None); | |
| let raised = base.clone().with_limits(Some(5_000), None); | |
| assert!(!same_key(&base, &limited)); | |
| assert!(!same_key(&limited, &base)); | |
| assert!(!same_key(&limited, &raised)); | |
| } | |
| fn same_key_rejects_a_v1_key_against_v0_or_other_limits() { | |
| let base = IdentityPublicKey::random_key(0, Some(1), PlatformVersion::latest()); | |
| let limited = base.clone().with_limits(Some(1_000), None); | |
| let raised = base.clone().with_limits(Some(5_000), None); | |
| let extended = base.clone().with_limits(Some(1_000), Some(1_800_000)); | |
| assert!(!same_key(&base, &limited)); | |
| assert!(!same_key(&limited, &base)); | |
| assert!(!same_key(&limited, &raised)); | |
| assert!(!same_key(&limited, &extended)); | |
| assert!(!same_key(&extended, &limited)); | |
| } |
source: muse-spark-1.3-contributor (phase2-reviewer: rust-quality)
Comment on lines
+3456
to
+3460
| // Network / broadcast rejections. A failed token operation carries the | ||
| // SDK rejection that caused it. | ||
| P::Sdk(_) | P::TransactionBroadcast(_) | P::TokenOperationFailed { .. } => { | ||
| IdentityOpErrorKind::Rejected | ||
| } |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: TokenOperationFailed bucket in identity classifier is untested
identity_op_error_kind buckets TokenOperationFailed as Rejected, but no test constructs that variant. The existing register/top-up tests cover Sdk, broadcast, finality, and Other cases, so the only arm that interprets a struct variant with payload could be moved to a different bucket with no test failure, silently changing identity-flow messaging.
source: muse-spark-1.3-contributor (phase2-reviewer: rust-quality)
lklimek
disabled auto-merge
September 22, 2026 17:58
lklimek
added a commit
that referenced
this pull request
Sep 24, 2026
…slow SPV sync (#1017) * test(e2e): expect IdentityKeyVerificationFailed for a key not on the masternode #860 split the string-carrying KeyInputValidationFailed into a format-only KeyInputValidationFailed(KeyInputError) and IdentityKeyVerificationFailed (KeyVerificationError). A well-formed WIF that matches no OWNER key is a verification failure, so TC-MN-020 now asserts NoMatchingKey { owner }. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(e2e): clamp masternode withdrawals to the protocol per-transition limits A tenth of a long-running testnet evonode's balance (~1321 tDASH) exceeds max_withdrawal_amount (500 DASH), so consensus rejected TC-MN-050. Clamp the amount to system_limits of the platform version DET builds with, and fail with a fund-the-identity message when the balance is below the minimum. Applies to TC-MN-050, TC-MN-051 and TC-MN-053. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(e2e): fail masternode cases loudly on missing E2E_MN_* inputs The masternode cases returned early when an input was unset, reporting ok without running, and read names (E2E_MN_PRO_TX_HASH, *_WIF) the operator .env never used. Adopt the operator names as canonical (E2E_MN_PROTX_HASH, E2E_MN_OWNER_KEY, E2E_MN_PAYOUT_KEY, E2E_MN_VOTING_KEY): the loaders take WIF or hex, so _KEY is accurate. A missing input now panics with setup instructions and names the rename when a legacy spelling is set; legacy names are not aliases, to avoid ambiguous precedence. TC-MN-007 no longer needs a key: the ProTxHash parse fails before any key check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(e2e): expect MalformedProTxHash for a malformed masternode ProTxHash #860 gave masternode/evonode loads the ProTxHash-specific MalformedProTxHash; TC-MN-007 still expected IdentifierParsingError and never noticed because a missing payout key skipped it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wallet): report an insecure wallet data folder instead of disk-space advice A wallet database whose folder (or an ancestor) other accounts can modify is refused upstream with WalletStorageError::InsecureParentDir, which DET mapped to the generic 'check disk space and restart' copy. Map it to a dedicated WalletDataFolderInsecure variant that tells the user to make the folders writable only by their account; the folder and reason stay in the source chain. Classify it as a terminal storage-open error, since retrying cannot fix permissions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(e2e): stop the workdir scan on a store refusal the next slot cannot avoid open_available_workdir treated every store-open error as 'slot still open in this process', so an insecure_parent_dir refusal (group-writable TMPDIR) cycled all 10 slots and ended blaming lock files. Classify on the typed upstream variants: only WalletStorageError::AlreadyOpen and SecretStoreError::AlreadyLocked mean the slot is held. Anything else returns WorkdirOpenError::StoreRefused at once, and the panic prints the full source chain, which names the offending directory and the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(e2e): resume a slow SPV sync on its own slot instead of retrying init A fixed 600s SPV wait panicked init; the retry could not reopen the slot (its SPV lock and k/v claim stay held in-process), so it moved to a fresh slot and restarted sync from genesis, which cannot finish in 600s either. Wait on the same runtime while the monotonic spv_progress_token advances, failing after SPV_STALL_WINDOW (180s) without progress or at SPV_SYNC_CAP (1800s, the former 3 x 600s worst case). The pure spv_wait_verdict and spv_progress_advanced decisions are unit-tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(platform-info): read the current epoch live again Review comment on #1016: the TODO(platform#4231) in the CurrentEpochInfo arm was stale. The fix is in this repo's pin (9f7ed169): ExtendedEpochInfo::fetch_current now resolves the epoch with two proved explicit-start queries the verifier accepts, so the query shape that burned the SDK address pool is gone. Restore the live fetch, which publishes the network's own fee multiplier and protocol version, and keep the previous behaviour (default multiplier plus the DPNS-driven ratchet) as the logged degraded path when the fetch fails. It belongs in this PR because the review comment landed on #1016 after it merged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(platform-info): pin the completed-withdrawal status filter Review comment on #1016 asked whether filtering on WithdrawalStatus::FAILED (5) is valid while the contract is loaded at protocol 13, whose status enum stops at EXPIRED (4). A schema enum constrains documents, not query values: nothing in the query path checks membership, and the value encodes through the status property's type at both schema versions. So the filter stays as is, with the reasoning recorded where it is sent, the two call sites sharing one helper, a unit test on encoding, and live testnet coverage of both withdrawal queries plus the restored epoch fetch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(identity-keys): pin the expires_at conjunct in same_key Review comment on #1016: same_key_rejects_a_v1_key_against_v0_or_other_limits varied only total_budget, so removing the expires_at comparison would not have failed it. Compare two keys with an identical budget and different expiries, in both directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(wallet-backend): cover TokenOperationFailed in identity_op_error_kind Review comment on #1016: the arm bucketing network rejections had no test for TokenOperationFailed. Assert the Rejected bucket for it alongside Sdk and TransactionBroadcast, which share the arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(platform-info): stop dash_to_credits! silently zeroing the test total dash_to_credits! stringifies its argument and parses the text, so a Rust numeric separator is not a digit to it: dash_to_credits!(1_000_000) is 0 credits. daily_withdrawal_limit_uses_det_platform_version_not_latest ran on a zero total and still passed, because protocol 14 returns its 500 Dash floor there, which differs from protocol 13's flat 2000 Dash anyway. Drop the separator, assert the total, and pin the protocol-14 expectation to the 4000 Dash cap, which only a genuinely large total produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(e2e): report why no workdir slot could be opened The scan swallowed every I/O and locking failure: create_dir_all errors were dropped with .ok(), a lock-file open error and any flock failure both meant 'try the next slot', so an unwritable TMPDIR, a full disk or a filesystem without flock all ended as 'all slots busy, kill other test processes'. try_lock_exclusive now returns Ok(false) only for EWOULDBLOCK and an error otherwise (LockUnsupported); a non-permission I/O failure stops the scan with DirectoryUnusable. PermissionDenied stays ambiguous between a foreign slot and an unwritable parent, so it keeps scanning and is reported at the end when no slot was ever genuinely taken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(e2e): name the SPV error when the sync wait gives up The progress wait ignored SpvStatus::Error, so a sync that died mid-flight was reported as a bare stall without the recorded SPV error. Log the error when it first appears (the status clears itself, so the log is the only record), and report ErroredAndStalled, carrying spv_last_error, when the wait ends while SPV is errored; Stalled and Exhausted now carry the status they observed. Not a fail-fast check: SpvStatus::Error is not terminal here. EventBridge recomputes the status from every on_progress, so a manager error is cleared by the next progress event, and aborting on it would abandon a sync that recovers on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(platform-info): stop the degraded epoch copy claiming network knowledge The fallback rendering said the multiplier 'is the one every network charges today', which this build cannot know once the epoch fetch failed, and pointed at an upstream issue number. Say it is the app's default and ask the user to try again, and pin both in the test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 5 (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
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.
TL;DR: Moves the Dash Platform dependency to
v4.3-dev(9f7ed169). DET is adapted only as far as it needs to build against it: no protocol version 14 features are added. DET's local platform version is fixed at protocol 13, which is what mainnet and testnet run.User story
As a DET maintainer, I want DET on the current Dash Platform dependency, and fully compatible with the protocol 13 networks users are on today, without shipping protocol 14 features that cannot be tested yet.
Scenario
Base flow
DET depends on four pinned
dashpay/platformgit crates:dash-sdk,rs-sdk-trusted-context-provider,platform-walletandplatform-wallet-storage.Actual behavior
v4.3-dev. Protocol 14 adds enum variants, a required field and a changed signature that no longer fit DET's exhaustive matches and struct literals.PLATFORM_V12table, while mainnet and testnet run protocol 13.platform_info.rswould pick up protocol 14 values throughPlatformVersion::latest().Expected behavior
v4.3-dev.Detailed discussion
Dependency pin
9f7ed169, the head ofv4.3-dev: the merge of dashpay/platform#4764, which carries the available-signer fix that fix(dashpay): accept HASH160 profile authentication keys #991 depends on.scripts/migration-fixtures/) targetsv4.3-dev. The migration fingerprint is unchanged (18 migrations).Protocol 13
DET_PLATFORM_VERSION = PLATFORM_V13insrc/context/mod.rs. It feeds:AppContext::platform_version()(validation, building, signing);platform_info.rsloads the withdrawals contract and computes the daily withdrawal limit with it instead oflatest(), which is now protocol 14. Protocol 14 changes how that limit is computed; a test asserts the protocol 13 value (2000 Dash) and that it differs fromlatest().latest()use is test-only.Minimal type adaptation
action_fee_agreement: Noneon document transitions. DET never sets it.ContractBounds::ContractGroupandTokenDistributionType::OncePerIdentity.PlatformWalletError::TokenOperationFailedis the typed successor ofTokenError(String)with the same message. It maps through the same generic wallet-backend arm, so users see what they saw before.same_keycompares V1 keys exhaustively, ignoring onlydisabled_at. A V0 key never equals a V1 key.Backward compatibility
IdentityPublicKey::V1,TokenDistributionRules::V1orContractGroupbounds.Relation to #1015
#1015 carries the full protocol 14 support on top of this bump. It is on hold until protocol 14 reaches testnet.
Testing
cargo clippy --all-features --all-targets -- -D warningsis clean, andcargo fmt --all -- --checkpasses.--lib: 2587 passed.kittest: 329 passed.masternode_tab::remove_flow_deletes_associated_voter_identityfailed once under host load and passed on reruns.migration-matrix: 61 passed.backend-e2eon testnet (identity keys): in progress, results will follow in a comment.Checklist
cargo fmt --allcargo clippy --all-features --all-targets -- -D warningsv4.3-dev(fix(platform-wallet): select available signing keys across identity operations platform#4764 merged)🤖 Generated with Claude Code
Summary by CodeRabbit
PR Hygiene ·
64a35f6/self-reviewedonce the bots are donedash-evo-tool(Cargo.lock,Cargo.toml,scripts/migration-fixtures/check-pin-ancestry.shand 12 more) — lklimekWhen every box is checked the
PR Hygienecheck passes and this can merge.