docs: add CLAUDE.md with development guidance#6741
docs: add CLAUDE.md with development guidance#6741PastaPastaPasta merged 1 commit intodashpay:developfrom
Conversation
|
Warning Rate limit exceeded@PastaPastaPasta has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughA new documentation file named 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CLAUDE.md (3)
1-3: Document header & placement could better align with existing repo conventionsConsider adding:
- The standard Dash documentation header (copyright / license) so the doc is unambiguously covered under the project’s license.
- Moving the file under
doc/(ordocs/) to keep documentation co-located and discoverable alongside the rest of the developer guides.-# CLAUDE.md +# CLAUDE.md +# +# Copyright (c) 2024 Dash Core Group +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php.
7-8: Minor grammar: add missing determiner
The codebase is primarily C++ using C++20 standard.
Add “the” before C++20 standard for readability.-Dash Core … primarily C++ using C++20 standard. +Dash Core … primarily C++ using the C++20 standard.
151-162: Prerequisite forbearshould be mentioned
bear(to emitcompile_commands.json) is not installed by default on most systems. Adding a one-liner helps:-# Generate compile_commands.json for IDEs -bear -- make -j$(nproc) +# Generate compile_commands.json for IDEs +# sudo apt install bear +bear -- make -j$(nproc)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CLAUDE.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CLAUDE.md
[uncategorized] ~7-~7: You might be missing the article “the” here.
Context: ...ce. The codebase is primarily C++ using C++20 standard. ## Build Commands ### Setup...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~85-~85: Possible missing article found.
Context: ...### Core Extension Pattern Dash extends Bitcoin Core through composition rather than co...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~179-~179: Loose punctuation mark.
Context: ...shd ``` ## Branch Structure - master: Stable releases - `develop`: Active dev...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ 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). (10)
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_multiprocess-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: arm-linux-build / Build source
🔇 Additional comments (1)
CLAUDE.md (1)
22-27: Wallet-breaking--with-incompatible-bdbflag deserves an explicit warningBuilding with
--with-incompatible-bdblinks against the system Berkeley DB ≥ 5.x and renders wallet files incompatible with releases built against BDB 4.8.
Please call this out so new contributors don’t accidentally corrupt wallets.-./configure --with-incompatible-bdb # Use system Berkeley DB +./configure --with-incompatible-bdb # ⚠️ Links against system BDB ≥5; wallets become incompatible with official releases
kwvg
left a comment
kwvg
left a comment
There was a problem hiding this comment.
Additionally, it'll require
- Instructions on not looking into
- Under any circumstances:
.github,guix-build*,releases- Vendored dependencies like
src/{dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}andsrc/crypto/{ctaes,x11}
- Vendored dependencies like
- Unless prompted:
depends,ci,contrib,doc
- Under any circumstances:
- Clarification that the codebase in is C++20 (located in
src), a unit test suite in (located insrc/test,src/wallet/test,src/qt/test) but the functional test suite is intest/functionaland that's written in Python (with the minimum version set in.python-version) - Instructions on how to build using
dependsand use that by default - Known-good developer defaults like
--enable-reduce-exports --enable-suppress-external-warnings --enable-werror --enable-debug --enable-crash-hooks --enable-stacktraces --disable-hardening
CLAUDE.md
Outdated
| make -j$(nproc) | ||
|
|
||
| # Memory-constrained systems | ||
| ./configure CXXFLAGS="--param ggc-min-expand=1 --param ggc-min-heapsize=32768" |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CLAUDE.md (2)
117-130: Specify language for fenced ASCII-art block to satisfy markdown-lint (MD040).Markdown-lint flags the block because it lacks a language identifier.
Addingtext(orplaintext) avoids the warning without altering rendering.-``` +```text Dash Core Components ├── Bitcoin Core Foundation (Blockchain, consensus, networking) ├── Masternodes (Infrastructure) @@ └── Spork System (Feature control)
7-7: Minor grammar nit—insert “the” before “C++20 standard”.Current phrasing reads a bit abrupt.
-Dash Core is a cryptocurrency project … using C++20 standard +Dash Core is a cryptocurrency project … using the C++20 standard
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CLAUDE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
CLAUDE.md (6)
Learnt from: PastaPastaPasta
PR: dashpay/dash#6564
File: contrib/containers/deploy/Dockerfile:1-1
Timestamp: 2025-02-11T19:25:05.487Z
Learning: The deploy container for Dash Core is designed to be minimal, requiring only basic container functionality without complex init systems or service management.
Learnt from: PastaPastaPasta
PR: dashpay/dash#6564
File: contrib/containers/deploy/Dockerfile:1-1
Timestamp: 2025-02-11T19:25:05.487Z
Learning: The Dash Core deploy container runs a single process and doesn't require complex init systems, service supervision, or system utilities that Phusion baseimage provides, making vanilla Ubuntu a more suitable base image choice.
Learnt from: kwvg
PR: dashpay/dash#6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.
Learnt from: kwvg
PR: dashpay/dash#6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Learnt from: kwvg
PR: dashpay/dash#6629
File: src/evo/netinfo.h:248-256
Timestamp: 2025-05-10T00:54:30.887Z
Learning: The Dash codebase includes a specialization for serializing std::shared_ptr objects in src/serialize.h, which allows direct serialization of shared_ptr instances with stream operators.
🪛 LanguageTool
CLAUDE.md
[uncategorized] ~7-~7: You might be missing the article “the” here.
Context: ...ce. The codebase is primarily C++ using C++20 standard (minimum compiler requirements...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~17-~17: Consider shortening or rephrasing this to strengthen your wording.
Context: ...de Under any circumstances, do not make changes to: - guix-build* - Build system files -...
(MAKE_CHANGES)
[uncategorized] ~115-~115: Possible missing article found.
Context: ...### Core Extension Pattern Dash extends Bitcoin Core through composition rather than co...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~230-~230: Loose punctuation mark.
Context: ...shd ``` ## Branch Structure - master: Stable releases - `develop`: Active dev...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
117-117: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (5)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_multiprocess-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
🔇 Additional comments (1)
CLAUDE.md (1)
46-55: Double-check the advice to--disable-hardeningin developer builds.Disabling hardening options (stack canaries, FORTIFY, RELRO, etc.) speeds up local builds but can mask UB-related crashes that only appear in hardened CI artifacts.
Consider adding a cautionary note or recommending re-building without this flag before pushing.
CLAUDE.md
Outdated
| ./configure # Basic configuration | ||
| # Build dependencies first (recommended) | ||
| cd depends | ||
| make -j"$(( $(nproc) - 1 ))" |
There was a problem hiding this comment.
Use HOST=x86_64-pc-linux-gnu make -C depends -j "$(( $(nproc) - 1 ))" instead, Claude Code can behave erratically when changing directories and then expends additional effort to make sense of relative paths.
CLAUDE.md
Outdated
| cd .. | ||
|
|
||
| # Configure with depends | ||
| ./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu # Adjust architecture as needed |
There was a problem hiding this comment.
Should we define HOST somewhere and place the # Adjust architecture as needed comment there instead, then replace the triplet with HOST in all occurrences.
Also, list of all valid hosts are defined in depends/README.md
CLAUDE.md
Outdated
| - `SporkStore`: Spork state persistence | ||
| - `NetFulfilledRequestStore`: Network request tracking | ||
| - **CEvoDb**: Specialized database for Evolution/deterministic masternode data | ||
| - **CRecoveredSigsDb**: LLMQ recovered signature storage |
There was a problem hiding this comment.
There are a two more Dash-specific CDBWrapper usages, should probably list them under CDBWrapper in the same vein as CFlatDB.
CDKGSessionManager(llmq/dkgdb)CQuorumManager(llmq/quorumdb)
| 3. **Masternode System**: Deterministic masternode lists and management | ||
| 4. **LLMQ System**: Distributed consensus for advanced features | ||
| 5. **Dash Services**: CoinJoin, governance, InstantSend, ChainLocks | ||
| ``` |
There was a problem hiding this comment.
Should still mention the special transactions, earlier suggestion was complementary. Modification of block and transaction primitives should be documented, probably list of all special transactions would help.
There was a problem hiding this comment.
Earlier revision of this PR mentioned special transactions and it got lost on revision, suggestion was to bring it back.
Line 88 in 1dc13e7
CLAUDE.md
Outdated
| - **ValidationInterface**: Event distribution for block/transaction processing | ||
| - **ChainstateManager**: Enhanced with Dash-specific validation | ||
| - **Chainstate Initialization**: Separated into `src/node/chainstate.*` | ||
| - **MessagesSerializer**: Special transaction serialization |
There was a problem hiding this comment.
Potential typo? Couldn't find an entity named MessagesSerializer, should probably direct towards src/evo/specialtx.h for payload (de)ser routines.
CLAUDE.md
Outdated
| - `guix-build*` - Build system files | ||
| - `releases` - Release artifacts | ||
| - Vendored dependencies: | ||
| - `src/{dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}` |
There was a problem hiding this comment.
Forgot about src/crc32c, also a vendored dependency. Should we point towards EXCLUDE_DIRS in contrib/devtools/copyright_header.py as the definitive list of vendored/external code?
There was a problem hiding this comment.
I think that would be non-ideal. You don't really want to say "for x look at y" because then each time you run cc, it has to read that file too. Ideally, you want it to just be able to read CLAUDE.md and then have all the context it really needs to just go.
There was a problem hiding this comment.
Is there a way to write a comment that Claude ignores? Something like "this list should be in sync with this authoritative source" for maintainers?
CLAUDE.md
Outdated
|
|
||
| # Developer configuration (recommended) | ||
| ./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu \ | ||
| --enable-reduce-exports \ |
There was a problem hiding this comment.
nit: alphabetical (same for arguments and expressions)
CLAUDE.md
Outdated
| ## Project Overview | ||
|
|
||
| Dash Core is a cryptocurrency project extending Bitcoin Core with advanced features including masternodes, instant payments, privacy mixing, and decentralized governance. The codebase is primarily C++ using C++20 standard. | ||
| Dash Core is a cryptocurrency project extending Bitcoin Core with advanced features including masternodes, instant payments, privacy mixing, and decentralized governance. The codebase is primarily C++ using C++20 standard (minimum compiler requirements: Clang 16 or GCC 11.1). |
There was a problem hiding this comment.
Should point out minimum compiler requirements are defined in doc/dependencies.md as authoritative source
There was a problem hiding this comment.
why? how would it use that information to perform better?
There was a problem hiding this comment.
It's not for Claude, it's for the maintainer (comment)
| - **Core Code**: `src/` - C++20 implementation | ||
| - **Unit Tests**: `src/test/`, `src/wallet/test/`, `src/qt/test/` - C++ tests | ||
| - **Functional Tests**: `test/functional/` - Python tests (minimum version in `.python-version`) | ||
|
|
There was a problem hiding this comment.
Should we add a "hints" header and consolidate references to dependencies.md, .python_version here? It can be expanded to include test/util/data/non-backported.txt for list of Dash-specific files.
There was a problem hiding this comment.
I don't fully follow what you mean
There was a problem hiding this comment.
The suggestion was to move all references to other documents under a separate header so if for example, Claude needed to identify if code is Dash-specific or from upstream, it could refer to test/util/data/non-backported.txt as a hint. This suggestion can be ignored if tokens could be wasted from extraneous file reading,
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
CLAUDE.md (1)
40-40: Re-introduce theHOSTtriplet when buildingdepends
Using theHOSTenv keeps build artefacts predictable and avoids architecture-guessing issues Claude has hit before.-make -C depends -j"$(( $(nproc) - 1 ))" +HOST=x86_64-pc-linux-gnu make -C depends -j"$(( $(nproc) - 1 ))" # Adjust HOST as needed
🧹 Nitpick comments (3)
CLAUDE.md (3)
17-29: Clarify the “avoiddoc/” guidanceThe bullet list instructs Claude not to touch
doc/, yet this very file is documentation. This could confuse future maintainers (and Claude) about whether updating documentation is permissible.Proposed tweak:
- - `doc` - Documentation + - `doc` - Documentation (except for CLAUDE.md and explicitly-requested docs)
34-65: Reduce duplication in the build-configuration blockMultiple
./configureinvocations inside a single fenced block make the recommended flow hard to follow. Consider grouping flags or providing a single canonical command followed by deltas.Example consolidation:
-# Configure with depends (use the path shown in depends build output) -# Example paths: depends/x86_64-pc-linux-gnu, depends/aarch64-apple-darwin24.3.0 -./configure --prefix=$(pwd)/depends/[platform-triplet] - -# Developer configuration (recommended) -./configure --prefix=$(pwd)/depends/[platform-triplet] \ - --disable-hardening \ - --enable-crash-hooks \ - --enable-debug \ - --enable-reduce-exports \ - --enable-stacktraces \ - --enable-suppress-external-warnings \ - --enable-werror +# Configure (replace `[triplet]` with path printed by `depends`) +CONFIGURE_FLAGS="--prefix=$(pwd)/depends/[triplet] \ + --disable-hardening \ + --enable-crash-hooks \ + --enable-debug \ + --enable-reduce-exports \ + --enable-stacktraces \ + --enable-suppress-external-warnings \ + --enable-werror" +./configure $CONFIGURE_FLAGSThis keeps a single source of truth while still showing the full flag set.
117-117: Add a language hint to satisfy markdown-lint (MD040)The architecture ASCII diagram is in an unlabeled fenced block.
-``` +```text
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CLAUDE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
CLAUDE.md (9)
Learnt from: PastaPastaPasta
PR: dashpay/dash#6564
File: contrib/containers/deploy/Dockerfile:1-1
Timestamp: 2025-02-11T19:25:05.487Z
Learning: The deploy container for Dash Core is designed to be minimal, requiring only basic container functionality without complex init systems or service management.
Learnt from: PastaPastaPasta
PR: dashpay/dash#6564
File: contrib/containers/deploy/Dockerfile:1-1
Timestamp: 2025-02-11T19:25:05.487Z
Learning: The Dash Core deploy container runs a single process and doesn't require complex init systems, service supervision, or system utilities that Phusion baseimage provides, making vanilla Ubuntu a more suitable base image choice.
Learnt from: kwvg
PR: dashpay/dash#6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.
Learnt from: kwvg
PR: dashpay/dash#6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
PR: dashpay/dash#6532
File: src/net.cpp:4329-4329
Timestamp: 2025-01-14T09:07:12.446Z
Learning: Keep suggestions focused on the scope of the current commit/PR. Avoid suggesting unrelated improvements that should be handled in separate PRs, even if technically valid.
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Learnt from: kwvg
PR: dashpay/dash#6629
File: src/evo/netinfo.h:248-256
Timestamp: 2025-05-10T00:54:30.887Z
Learning: The Dash codebase includes a specialization for serializing std::shared_ptr objects in src/serialize.h, which allows direct serialization of shared_ptr instances with stream operators.
🪛 LanguageTool
CLAUDE.md
[uncategorized] ~7-~7: You might be missing the article “the” here.
Context: ...ce. The codebase is primarily C++ using C++20 standard (minimum compiler requirements...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~17-~17: Consider shortening or rephrasing this to strengthen your wording.
Context: ...de Under any circumstances, do not make changes to: - guix-build* - Build system files -...
(MAKE_CHANGES)
[uncategorized] ~115-~115: Possible missing article found.
Context: ...### Core Extension Pattern Dash extends Bitcoin Core through composition rather than co...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~233-~233: Loose punctuation mark.
Context: ...shd ``` ## Branch Structure - master: Stable releases - `develop`: Active dev...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
117-117: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: win64-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
Add comprehensive documentation for Claude Code including: - Build and testing commands - High-level architecture overview - Key Dash-specific components (masternodes, LLMQs, CoinJoin, governance) - Development workflow and debugging instructions - Critical interfaces and design patterns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
PastaPastaPasta
force-pushed
the
claude-md-init
branch
from
44cf556 to
2a1e4d4
Compare
3 participants
Summary
Test plan
🤖 Generated with Claude Code