update book - #11556
Open
ripatel-fd wants to merge 2 commits into
Open
update book#11556ripatel-fd wants to merge 2 commits into
ripatel-fd wants to merge 2 commits into
Conversation
ripatel-fd
commented
Sep 16, 2026
ripatel-fd
commented
Contributor
- book: update cryptography page
- book: add feature gates page
┌─ ⚡ PERF · 30a2ebe vs main@fd7003c ─────────────────────────────────
│ SUITE BASELINE NEW Δ
│ replay tps, mainnet 29,462 tps 29,508 tps · +0.16%
│ bench tps, localnet 832,513 tps 831,297 tps · -0.15%
│ snapshot load, testnet 9.22 s 9.04 s · -1.97%
│ mem total, mainnet 162.38 GiB 162.38 GiB · 0.00%
│ mem total, testnet 93.38 GiB 93.38 GiB · 0.00%
│ mem total, ag mainnet 198.86 GiB 198.86 GiB · 0.00%
│ mem total, ag testnet 129.86 GiB 129.86 GiB · 0.00%
│ clean compile, firedancer 8.32 s 8.38 s · +0.79%
│ binary size, firedancer 68.63 MB 68.63 MB · 0.00%
├─────────────────────────────────────────────────────────────────────
@@ 0 REGRESSIONS · 0 WARNINGS · 0 IMPROVED · 9 NOISE @@
└─────────────────────────────────────────────────────────────────────history · 2 pushes ┌─ HISTORY · Δ vs main, per push, newest first ───────────────────────────────────────────
│ HEAD TPS BENCH SNAP MEM·M MEM·T AG·M AG·T COMPILE BINARY
│ 30a2ebe +0.16% -0.15% -1.97% 0.00% 0.00% 0.00% 0.00% +0.79% 0.00%
│ 60f748e -0.26% +0.15% +0.24% 0.00% 0.00% 0.00% 0.00% +1.18% 0.00%
└───────────────────────────────────────────────────────────────────────────────────────── |
cmoyes-jump
previously approved these changes
Sep 16, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The documentation contains incorrect feature IDs, an unsupported syscall claim, and a broken anchor link.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates protocol documentation and exposes the new feature-gates reference in the book navigation.
Changes:
- Adds the feature-gate eligibility table.
- Expands cryptography implementation documentation.
- Adds navigation for the feature-gates page.
File summaries
| File | Description |
|---|---|
book/guide/protocol/feature_gates.md |
Documents supported feature gates. |
book/guide/protocol/cryptography.md |
Expands cryptography coverage. |
book/.vitepress/config.mts |
Adds feature-gates navigation. |
Review details
Suppressed comments (3)
book/guide/protocol/feature_gates.md:146
- The documented key is inconsistent with
src/flamenco/features/feature_map.json:140and the generated table, which useexpH2pp...forenable_big_mod_exp_syscall. This makes the bounty eligibility table identify the wrong feature account.
| `EBq48m8irRKuE7ZnMTLvLg2UuGSqhe8s8oMqnmja1fJw` | `enable_big_mod_exp_syscall` |
book/guide/protocol/cryptography.md:123
- Remove the duplicated verb in “utilizing using.”
- A portable reference backend utilizing using [fiat-crypto](https://github.com/firedancer-io/firedancer/blob/main/src/third_party/fiat-crypto/secp256k1_montgomery_64.c).
book/guide/protocol/cryptography.md:138
- The singular subject
Secp384r1requires “is,” not “are.”
Secp384r1 (NIST P-384) are similarly provided.
- Files reviewed: 3/3 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
topointon-jump
previously approved these changes
Sep 16, 2026
ripatel-fd
dismissed stale reviews from topointon-jump and cmoyes-jump
via
September 16, 2026 21:45
9dc2b04
ripatel-fd
force-pushed
the
ripatel/feature-gates-book-page
branch
from
September 16, 2026 21:46
9dc2b04 to
30a2ebe
Compare
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Several cryptographic descriptions are inaccurate or omit relevant implementations.
Review details
Suppressed comments (5)
Previously missed (5) — in code that hasn't changed since the last review.
book/guide/protocol/cryptography.md:56
- Poseidon is defined over the BN254 scalar field, not over the elliptic curve itself (
fd_poseidon.h:4). Describing the field explicitly avoids conflating the hash domain with curve operations.
book/guide/protocol/cryptography.md:146 - This wording implies both hashes serve both purposes, but Firedancer uses SHA-384 for P-384 ECDSA while Ed25519 uses SHA-512; SHA-512 is also used with RSA signatures. Separate the uses to describe the implemented algorithms accurately.
book/guide/protocol/cryptography.md:149 - After combining SHA-256, SHA-384, and SHA-512 into one section, this link still covers only
src/ballet/sha256. SHA-384/SHA-512 have a separate implementation insrc/ballet/sha512, so readers currently cannot reach the implementation for two of the documented algorithms.
book/guide/protocol/cryptography.md:172 fd_merlin.ccallsfd_keccak256_core, which selects either the s2n-bignum backend or the portable C backend. Attributing Strobe-128 unconditionally to s2n-bignum contradicts the backend description immediately above and is false for builds withoutFD_HAS_S2NBIGNUM.
book/guide/protocol/feature_gates.md:3- The opening is a sentence fragment and makes eligibility sound unconditional before introducing the Mainnet-activation requirement. State both requirements together so the bounty scope is unambiguous.
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Several claims conflict with the current TLS, hashing, and SVM implementations.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 5
- Review effort level: Balanced
|
|
||
| Used in Solana for: | ||
| - The `Secp256r1SigVerify1111111111111111111111111` native precompile. | ||
| - TLS 1.3 client and server authentication (`ECDSA_SECP256R1_SHA256`). |
| SHA-256 is used for computing PoH, computing PDAs, and generally any | ||
| Solana protocol hashing requirements, and also the SVM `sol_sha256` syscall. | ||
|
|
||
| SHA-384 and SHA-512 are used for ECDSA in X.509/TLS and Ed25519. |
Comment on lines
+148
to
+149
| [Implementation](https://github.com/firedancer-io/firedancer/tree/89ed44b4c521e314487b8f0145532dc1aa43953e/src/ballet/sha256) | ||
| written by hand, providing both scalar and SIMD/SHA-NI batched optimized variants. |
Comment on lines
+171
to
172
| written by hand, using the [core Keccak-256](#keccak-256) provided by | ||
| `s2n-bignum`. |
| | `A8xyMHZovGXFkorFqEmVH2PKGLiBip5JD7jt4zsUWo4H` | `remove_congestion_multiplier_from_fee_calculation` | | ||
| | `DdLwVYuvDz26JohmgSbA7mjpJFgX5zP2dkp8qsF2C33V` | `cap_transaction_accounts_data_size` | | ||
| | `4UDcAfQ6EcA6bdcadkeHpkarkhZGJ7Bpq7wTAiRMjkoi` | `disable_builtin_loader_ownership_chains` | | ||
| | `expH2ppKPW2ANEdEmAjfhSEcnBQJfmoX4FjuNpe9ttg` | `enable_big_mod_exp_syscall` | |
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.