Repository navigation
docs: remove committed working specs and plans - #5060
Merged
Merged
Conversation
CLAUDE.md: a spec, plan or design doc written to drive a change is a working artifact that is never committed, and one that was committed is removed. Twenty such files were committed in July (implementation specs, migration leftovers, a code review log, a PR hardening plan). Remove them; the one fact code comments still pointed at (why a private key in an auto-accept QR is acceptable) moves into the comment, the other pointers are dropped, and the Kotlin workflow no longer watches a deleted path. INTEROP_DESK_CHECK.md stays: it records why it is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedNext included review available in 45 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: dashpay/platform/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (51)
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 |
github-actions
Bot
added
the
waiting-bots
Collaborator
|
🕓 Queued for automated review — 1st in line, estimated start in ~10 min (commit f0a37f8)
|
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final review — Phase 1 only (trivial change)
Verified the supplied preliminary review evidence against head cbf718d. The change deletes 20 working documents, removes their references, and preserves the QR auto-accept security rationale in its code comment; the remaining edits affect documentation, comments, and a workflow path filter, not executable logic. No in-scope findings were identified; this assessment does not claim final combined reviewer coverage.
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: architecture-layering); final verifier: gpt-6-astra (agent: astra-gate-verifier, role: final-verifier)
- Triage:
trivialbygpt-6-astra(effort low) — The diff removes working documentation, updates comments and references, and drops a deleted document from a CI path filter without changing executable logic, tests, cryptography, or key handling. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— architecture-layering (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 84% left, weekly 31% left; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 13% left, 5h 100% left) - Fresh verifier:
gpt-6-astra— final-verifier; agentastra-gate-verifier - Phase 2 reviewers: not run (triage rated this change trivial); this review comments and never approves
Contributor
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
The specs removed in the previous commit held rationale that existed nowhere else. It now sits next to the code it explains: - invitation threat model on the invitation codec - why multi-account contacts stay deferred on newest_received_per_sender (and that function's doc, which sat above SYNC_OVERLAP_MS, moves back onto it) - ignore stays local, and how a cross-device ignore must work, on ignore_contact_sender and the DashPay contract README - count proofs are public, in the book's count-tree chapter - profile sync privacy cost and why it refetches by time window - avatar fingerprints compare by Hamming distance - when the legacy Keychain signer fallback may be removed - what the iOS Keychain protection does and does not guarantee, and why the two signer handles stay separate - Kotlin parity manifest rules, native-binding test limits, and the Compose handle rule in the Kotlin CLAUDE.md - Room's power-loss residual on DashDatabase.create - two manual QA checks in the Android test plan - a dated addendum and the reference-client anchors in the interop desk check Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
packages/rs-platform-wallet/PLAN.md, PERSISTENCE_REDESIGN.md, docs/DASHPAY_MIGRATION_PLAN.md and the SwiftExampleApp shielded-sync-timing spec were working plans. Their designs shipped or were replaced, and the code now documents itself in more detail. The few facts found nowhere else move next to the code: - why WalletManager has no per-wallet locks (a 7x SPV slowdown), on the WalletEvent adapter - why tokens need a watch list (no query lists an identity's tokens), on IdentitySyncManager - DashPay contact pools use key-wallet's gap of 20, not DIP-15's 10, on DEFAULT_CONTACT_GAP_LIMIT - what each IdentityStatus variant means and which the library sets - why the shielded sync timer uses Swift Date() at both edges Comments that pointed at the plans lose the pointer or the plan label, and docs that contradicted the code are corrected: apply_changeset drops cs.core rather than replaying it (so ApplyError::CoreApply is never built), TokenWallet no longer exists, and a cold shielded sync of 1M notes takes about 20 minutes, not 10. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-09-27T23:05:04.047Z |
github-actions
Bot
added
waiting-bots
QuantumExplorer
changed the title
This was referenced Sep 27, 2026
Member
Author
|
Open work that only the removed documents recorded is now filed:
A live doc bug found the same way, the book's contact-request guide encrypting a 78-byte xpub instead of the 69-byte compact form, is fixed in #5087. |
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.
Basic explanation
What this does: deletes 24 planning documents that were committed to the repository: implementation specs, "known leftovers" lists, a code-review log and a PR hardening plan for DashPay and the Kotlin/Swift SDKs (20 files under
docs/), plus three platform-wallet plans and a SwiftExampleApp timing spec. The project's rule (inCLAUDE.md) is that such documents guide the work but are never committed, because they go stale as soon as the code lands; anything worth keeping goes in code comments, tests, commit messages or the PR description.Before deleting them, every document was read against the current code. Most of what they said is already documented where the code lives, and in several places the plan was wrong and the code right. The facts that existed nowhere else now sit next to the code they explain.
Example:
docs/dashpay/MULTI_ACCOUNT_SPEC.md.newest_received_per_sender, the function that enforces it.Value: the repository stops carrying about 16,000 lines of partly stale plans that readers could mistake for current documentation, without losing the reasoning in them.
Risks: Low. Only comments, docs and one test-plan table change; no code. The history keeps every deleted file.
Issue being fixed or feature implemented
CLAUDE.md, "Specs are working artifacts: never commit them": "Delete it once the task ships. If a branch already committed one,git rm --cached <path>before merge." A review of 4.1 and 4.2 found these committed. Their headers identify them as working documents, for exampleSPEC.md("A single working spec for getting the full DashPay flow done"),KOTLIN_MIGRATION_LEFTOVERS.md("Kept so nothing is silently lost when the four stacked PRs collapse into one"),CODE_REVIEW_NOTES.md("Final multi-agent code review"),rs-platform-wallet/PLAN.md(status: active).What was done?
Removed:
docs/dashpay/{BLOCK_SPEC, CONTACTINFO_FORMAT_SPEC, DASHPAY_STATE_ENCAPSULATION_SPEC, DIP15_INVITATIONS_SPEC, DIP_CONFORMANCE_GAPS, IDENTITY_KEY_SCALAR_ELIMINATION_SPEC, KOTLIN_INVITATIONS_SPEC, KOTLIN_MIGRATION_FOLLOWUPS_SPEC, KOTLIN_MIGRATION_LEFTOVERS, KOTLIN_MIGRATION_SPEC, MULTI_ACCOUNT_SPEC, PENDING_CONTACT_CRYPTO_RELOCATION_SPEC, QA_TESTCASES_SPEC, QR_AUTO_ACCEPT_SPEC, SIGNER_SEED_ELIMINATION_SPEC, SPEC, SYNC_CORRECTNESS_SPEC}.mddocs/sdk/{CODE_REVIEW_NOTES, KOTLIN_SWIFT_SHARED_PARITY_SPEC, PR3999_WALLET_LIFECYCLE_HARDENING_SPEC}.mdpackages/rs-platform-wallet/{PLAN, PERSISTENCE_REDESIGN}.md,packages/rs-platform-wallet/docs/DASHPAY_MIGRATION_PLAN.mdpackages/swift-sdk/SwiftExampleApp/docs/shielded-sync-timing-spec.mdKept:
docs/dashpay/INTEROP_DESK_CHECK.md, which says it is kept on purpose as the evidence for the DashPay wire-format decisions. It gains a dated addendum (#4372 now accepts legacy key purposes on receive) and the reference-client source anchors from the deleted conformance-gaps file.Facts moved next to the code:
crypto/invitation.rs# Securitynewest_received_per_sender(whose doc also moves back onto it from aboveSYNC_OVERLAP_MS)contactInfoignore_contact_sender, DashPay contract README[toUserId, $ownerId]index would publish who contacted whomsync_contact_profilescalculate_dhash_fingerprintKeychainSigner.signOnDemandWalletStorage,swift-sdk/CLAUDE.mdkotlin-sdk/CLAUDE.mdDashDatabase.createTEST_PLAN.md(ID-01 note, new ID-17)WalletManagerhas no per-wallet lockscore_bridge.rs)IdentitySyncManagerDEFAULT_CONTACT_GAP_LIMITIdentityStatusvariant, and which the library setsIdentityStatusDate()at both edgesShieldedServiceOther doc fixes in touched files:
apply_changeset's docs said it replayscs.core; it drops it (soApplyError::CoreApplyis never built).TokenWalletmentions removed. Plan labels ("Phase 9a-4", "evo-tool task #130") removed from comments. The shielded cold-sync figure is about 20 minutes for 1M notes, not 10. Pointers to deleted files are removed fromauto_accept.rs(the TTL comment now states its security reasoning),invitation.rs, two Kotlin instrumentation tests, both test plans, the Swift timing comments and thekotlin-sdk-build.ymlpath filter.Open work that only these documents recorded is filed as issues (linked in a comment below).
How Has This Been Tested?
Comment, Markdown and test-plan changes only.
cargo check -p platform-wallet(first commit);rustfmt --checkon every touched Rust file;python3 scripts/check_sdk_parity_manifest.pypasses;git grepfinds no remaining reference to a deleted file outsideCHANGELOG.mdhistory.Breaking Changes
None.
Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
f0a37f8/skip-botsproceeds without the ones not yet reported/self-reviewedonce the bots are donesystem-contracts— you own itkotlin-sdk(packages/kotlin-sdk/CLAUDE.md,packages/kotlin-sdk/KotlinExampleApp/TEST_PLAN.md,packages/kotlin-sdk/sdk/src/androidTest/kotlin/org/dashfoundation/dashsdk/wallet/DashPayUnlockAndSyncTest.ktand 2 more) — HashEngineeringrs-platform-wallet(packages/rs-platform-wallet/PERSISTENCE_REDESIGN.md,packages/rs-platform-wallet/PLAN.md,packages/rs-platform-wallet/docs/DASHPAY_MIGRATION_PLAN.mdand 13 more) — HashEngineering or ZocoLini or llbartekll or romchornyiswift-sdk(packages/swift-sdk/CLAUDE.md,packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift,packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swiftand 3 more) — llbartekll or romchornyiWhen every box is checked the
PR Hygienecheck passes and this can merge.