sdk%feat(doc): introduce Zensical-based documentation site, WASM-based demos, GitHub Pages workflow#10
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds two interactive WASM browser samples ( ChangesWASM Samples, Documentation Site, and Tooling
Sequence DiagramssequenceDiagram
participant Browser as Browser (index.js)
participant Worker as worker.js (×N Web Workers)
participant SolverWASM as demo_solver.wasm
participant ParserWASM as demo_parser.wasm
rect rgba(100, 149, 237, 0.5)
note over Browser,ParserWASM: Object Parser sample
Browser->>ParserWASM: init() — load module
ParserWASM-->>Browser: ready
Browser->>ParserWASM: parse_block_hex(hex) or parse_tx_hex(hex)
ParserWASM-->>Browser: envelope JSON {data, warnings}
Browser->>Browser: buildTree() — render expandable DOM tree
end
rect rgba(144, 238, 144, 0.5)
note over Browser,SolverWASM: Genesis Solver sample
Browser->>SolverWASM: init() — load module
SolverWASM-->>Browser: ready, load preset
Browser->>SolverWASM: merkle_root(scriptSig, scriptPubkey, amount)
SolverWASM-->>Browser: merkle root string
Browser->>Worker: postMessage({header fields, nonceStart, nonceCount})
Worker->>SolverWASM: scanhash(batch)
loop per batch
SolverWASM-->>Worker: ScanResult
Worker-->>Browser: {progress, totalHashes}
end
alt solution found
Worker-->>Browser: {ok: true, result}
Browser->>Browser: grace timeout → display nonce/hash
else exhausted
Worker-->>Browser: {done}
Browser->>Browser: "Nonce space exhausted"
end
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Warning This pull request may have conflicts, please coordinate with the authors of these pull requests. Potential conflicts |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
80-92:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
defaultis being described as a feature toggle when it is really baseline capability.The docs should describe
no_std + allocas the always-available baseline and avoid implying that Cargo'sdefaultfeature enables it.
README.md#L80-L92: reword the table row to describe the baseline rather than a feature flag.docs/zen/index.md#L82-L92: make the same wording change in the docs site page.🤖 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 `@README.md` around lines 80 - 92, The table describing features incorrectly frames `no_std + alloc` as a Cargo feature flag called `default` when it is actually the always-available baseline capability. In README.md (lines 80-92) and docs/zen/index.md (lines 82-92), reword the `default` table row to clearly describe `no_std + alloc` as the baseline that is always available, rather than implying it is enabled by Cargo's default feature. Ensure the updated wording makes it clear this is not a feature toggle but rather the foundation that all other features build upon.
🤖 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.
Inline comments:
In @.github/workflows/pages.yml:
- Around line 32-34: The "Install wasm-pack" step uses curl to execute a remote
shell script (curl | sh), which presents a supply-chain security risk without
cryptographic verification. Replace the run command to use cargo install
wasm-pack with an explicit version pinned and the --locked flag to compile from
source or use verified crates.io distribution, eliminating the shell script
injection risk and ensuring reproducible builds.
- Around line 21-24: Replace all mutable GitHub Actions version references with
immutable full commit SHAs to eliminate supply-chain vulnerabilities. Locate
each actions/ reference that uses `@v`* or `@master` tags (such as actions/checkout,
and any other third-party actions), and pin each to its corresponding full
commit SHA instead. Additionally, add persist-credentials: false to the checkout
action step since this workflow does not perform any git push operations or
require credential persistence.
In `@contrib/lint/lint_codeql.py`:
- Around line 55-57: The path keys emitted for source files must match what the
QL consumer expects in the sourceLineContent join predicates. The file key
emission (around lines 69-74) was changed to repo-root-relative format with the
pkgs/ prefix, but the corresponding path capture pattern in
contrib/codeql/lib/policy.qll still expects paths without this prefix, breaking
the sourceLineContent join. Align the path format consistently by either
removing the pkgs/ prefix from the emitted file keys in the generator (lines
69-74, 196-201) or updating the path capture pattern in policy.qll to match the
new format with the pkgs/ prefix included.
In `@contrib/samples/parser/parser.rs`:
- Around line 66-69: Add a `# Errors` documentation section to both parser
functions that return Result types. The first function with rustdoc starting
around line 66 needs documentation explaining what errors can be returned, and
the second function around lines 90-92 has the same requirement. Each `# Errors`
section should describe the failure conditions that cause the Result to return
an error variant, following the rustdoc coding guidelines for Result-returning
functions.
In `@contrib/samples/parser/style.css`:
- Around line 3-4: The font-family-name-quotes stylelint rule is being violated
due to inconsistent quoting of font family names. In
contrib/samples/parser/style.css at lines 3-4 (anchor location) and lines 32-33
(sibling location), remove the quotes around 'SFMono-Regular' to make it
consistent with the stylelint configuration and ensure the font family names
follow the proper quoting convention for the configured rule.
In `@contrib/samples/solver/index.js`:
- Around line 157-166: The current isU32 and isI32 functions use parseInt which
accepts partial input with trailing garbage (e.g., parseInt("123abc", 10)
returns 123), allowing malformed values to pass validation. Replace the
parseInt-based validation with a stricter approach that validates the entire
input string matches the numeric format exactly, rejecting any trailing
non-numeric characters. This same issue appears in the functions at lines
234-240 and must also be fixed there to ensure consistent validation across all
numeric type-checking functions.
In `@contrib/samples/solver/style.css`:
- Around line 51-52: In the font-family property declaration, remove the single
quotes around the font name SFMono-Regular (changing 'SFMono-Regular' to
SFMono-Regular) to comply with the stylelint font-family-name-quotes rule, while
keeping the quotes around other font names like 'SF Mono' and 'Liberation Mono'
as they are multi-word names that require quoting.
In `@contrib/samples/solver/worker.js`:
- Around line 13-18: The `await ready` statement in the worker onmessage handler
is executing before the try block, which means WASM initialization failures
won't be caught and the main thread will hang without an error. Move the `await
ready` statement inside the try block (right after the opening brace of try) so
that any WASM initialization failures are properly caught by the catch block and
appropriate error handling can occur.
In `@contrib/zen/__init__.py`:
- Around line 82-86: The blank line handling in the body_line loop (lines 82–86)
breaks admonition block formatting by not maintaining indentation. When
body_line is falsy (line 85), the code appends an empty string "", but it should
append four spaces " " instead to keep blank lines indented at the same level
as non-empty body lines. This prevents MkDocs admonition blocks from terminating
prematurely when encountering paragraph separators.
In `@docs/README.md`:
- Around line 16-18: The extras notation in the three dependency bullets for
Zensical, PyMarkdown, and rjsmin uses an incorrect format of `[.dev]` which is
inconsistent with the install command reference at line 14. Change the notation
from `[.dev]` to `[dev]` in all three bullet points to match the correct extras
syntax and maintain consistency throughout the documentation.
In `@pkgs/primitives/src/prelude.rs`:
- Around line 15-21: The round function shim defined in the cfg_if block in
prelude.rs (lines 15-21) is only used in serialize.rs which is gated by the
serde feature, but the shim itself is defined unconditionally whenever std is
available, causing dead-code warnings in builds without serde. Wrap the entire
cfg_if block that contains the round function definition with an additional
#[cfg(feature = "serde")] attribute so the shim is only compiled when the serde
feature is actually enabled, matching the feature gate of its only caller in
serialize.rs.
---
Outside diff comments:
In `@README.md`:
- Around line 80-92: The table describing features incorrectly frames `no_std +
alloc` as a Cargo feature flag called `default` when it is actually the
always-available baseline capability. In README.md (lines 80-92) and
docs/zen/index.md (lines 82-92), reword the `default` table row to clearly
describe `no_std + alloc` as the baseline that is always available, rather than
implying it is enabled by Cargo's default feature. Ensure the updated wording
makes it clear this is not a feature toggle but rather the foundation that all
other features build upon.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 185583a3-0d70-40b8-8dd6-5fce2f99f612
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockcontrib/samples/Cargo.lockis excluded by!**/*.lockdocs/zen/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (46)
.github/workflows/build_msrv.yml.github/workflows/build_nightly.yml.github/workflows/build_stable.yml.github/workflows/pages.yml.gitignoreCLAUDE.mdREADME.mdcontrib/__init__.pycontrib/build_docs.pycontrib/codeql/codeql-config.ymlcontrib/codeql/import.qlcontrib/codeql/lib/policy.qllcontrib/common.pycontrib/js/eslint.config.mjscontrib/lint/common.pycontrib/lint/common.pycontrib/lint/lint_codeql.pycontrib/lint/lint_javascript.pycontrib/lint/lint_markdown.pycontrib/lint/lint_rust.pycontrib/lint/lint_semgrep.pycontrib/lint_all.pycontrib/samples/Cargo.tomlcontrib/samples/common.csscontrib/samples/parser/Cargo.tomlcontrib/samples/parser/index.jscontrib/samples/parser/parser.rscontrib/samples/parser/style.csscontrib/samples/solver/Cargo.tomlcontrib/samples/solver/index.jscontrib/samples/solver/solver.rscontrib/samples/solver/style.csscontrib/samples/solver/worker.jscontrib/zen/__init__.pydocs/README.mddocs/guide_rust.mddocs/zen/index.mddocs/zen/samples/index.mddocs/zen/samples/parser/index.mddocs/zen/samples/solver/index.mddocs/zen/style.csspkgs/primitives/Cargo.tomlpkgs/primitives/src/prelude.rspkgs/primitives/src/serialize.rspyproject.tomlzensical.toml
Comment on lines
+21
to
+24
| - name: Checkout | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "pages.yml" -path "*/.github/workflows/*" | head -5Repository: dashpay/base-sdk
Length of output: 90
🏁 Script executed:
cat -n ./.github/workflows/pages.ymlRepository: dashpay/base-sdk
Length of output: 2265
🌐 Web query:
GitHub Actions security best practices pin commit SHA persist-credentials
💡 Result:
Securing GitHub Actions workflows involves two critical practices: pinning actions to commit SHAs to ensure immutability [1][2] and explicitly managing credential persistence to prevent unauthorized access [3][4]. Pinning Actions to Commit SHAs Pinning an action to a full-length commit SHA is the only way to ensure the action is an immutable release [1]. Unlike tags or branches, which are mutable and can be updated or redirected by a malicious actor to compromise your workflow [2][5], a commit SHA is immutable once a commit exists [2]. - Implementation: Reference the full-length SHA in your workflow file. It is recommended to include the version tag as a comment for readability [6][5]. - uses: actions/checkout@3df4ab1 # v4.0.0 - Governance: GitHub supports organization-level policies that enforce SHA pinning for all actions [7][2]. If enabled, any workflow using a tag or branch will fail [2][6]. - Automation: Use tools like Dependabot or Renovate (specifically the helpers:pinGitHubActionDigests preset) to automate updates to your pinned SHAs, ensuring you benefit from security patches without sacrificing immutability [2][6]. Managing Credential Persistence The actions/checkout action defaults to persist-credentials: true, which stores the GITHUB_TOKEN (or other credentials) in the runner's local Git configuration [8][3]. This exposes the token to every subsequent step in the job, increasing the risk of credential leakage if a malicious action or script is executed [3][4][9]. - Implementation: Explicitly set persist-credentials: false in your checkout steps unless your workflow specifically requires pushing commits back to the repository [3][4]. - uses: actions/checkout@v4 with: persist-credentials: false - Security Rationale: Setting this to false prevents the token from being persisted in the Git configuration file, thereby limiting the risk of it being accessed by third-party actions, logged, or accidentally included in build artifacts [3][10]. If your workflow must push changes, use a scoped GITHUB_TOKEN with the minimum required permissions [3][11]. Additional Best Practices - Principle of Least Privilege: Always set the default permissions for the GITHUB_TOKEN to read-only at the workflow or job level and grant specific write access only when necessary [12][11]. - Automated Auditing: Use tools like Zizmor or similar linters to audit your workflows for missing SHA pins and unsafe credential configurations [6][3][10].
Citations:
- 1: https://docs.github.com/en/actions/reference/security/secure-use
- 2: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
- 3: https://actsense.dev/vulnerabilities/unsafe_checkout/
- 4: https://adaptive-enforcement-lab.com/secure/github-actions-security/third-party-actions/common-actions/
- 5: https://devopsil.com/articles/2026-03-22-github-actions-security-hardening
- 6: https://architecture.lullabot.com/adr/20260505-pin-github-actions-to-hash/
- 7: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 8: https://github.com/actions/checkout
- 9: https://github.com/suzuki-shunsuke/ghalint/blob/main/docs/policies/013.md
- 10: https://sisaku-security.github.io/lint/docs/rules/artipacked/
- 11: https://github.com/github/docs/blob/962a1c8dccb8c0f66548b324e5b921b5e4fbc3d6/content/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions.md
- 12: https://docs.github.com/actions/security-for-github-actions
Pin third-party GitHub Actions to immutable commit SHAs and disable credential persistence.
Lines 22, 27, 36, 44, 56, and 76 use mutable version tags (@v*/@master), which creates supply-chain vulnerabilities. Pin each action to its full commit SHA instead. Additionally, add persist-credentials: false to the checkout step (lines 21-24) since this workflow performs no git push or authentication operations.
Suggested hardening diff
- name: Checkout
uses: actions/checkout@v6
+ uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0
with:
fetch-depth: 1
+ persist-credentials: false
- name: Install Rust toolchain
- uses: dtolnay/rust-toolchain@master
+ uses: dtolnay/rust-toolchain@<pinned-commit-sha>
- name: Set up Python
- uses: actions/setup-python@v6
+ uses: actions/setup-python@<pinned-commit-sha>
- name: Manage cargo registry
- uses: actions/cache@v5
+ uses: actions/cache@<pinned-commit-sha>
- name: Upload Pages artifact
- uses: actions/upload-pages-artifact@v5
+ uses: actions/upload-pages-artifact@<pinned-commit-sha>
- name: Deploy to GitHub Pages
- uses: actions/deploy-pages@v5
+ uses: actions/deploy-pages@<pinned-commit-sha>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Checkout | |
| uses: actions/checkout@v6 | |
| with: | |
| fetch-depth: 1 | |
| - name: Checkout | |
| uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0 | |
| with: | |
| fetch-depth: 1 | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 21-24: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 22-22: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/pages.yml around lines 21 - 24, Replace all mutable GitHub
Actions version references with immutable full commit SHAs to eliminate
supply-chain vulnerabilities. Locate each actions/ reference that uses `@v`* or
`@master` tags (such as actions/checkout, and any other third-party actions), and
pin each to its corresponding full commit SHA instead. Additionally, add
persist-credentials: false to the checkout action step since this workflow does
not perform any git push operations or require credential persistence.
Source: Linters/SAST tools
Comment on lines
+66
to
+69
| /// Parses a hex-encoded raw block or block header and returns a JSON string. | ||
| /// Inputs of exactly 80 bytes are decoded as a block header; longer inputs are | ||
| /// decoded as a full block. | ||
| #[wasm_bindgen] |
There was a problem hiding this comment.
Add # Errors sections to the exported Result APIs.
Both public parser functions return Result<_, _> but their rustdoc currently omits the required # Errors section.
Proposed fix
/// Parses a hex-encoded raw block or block header and returns a JSON string.
/// Inputs of exactly 80 bytes are decoded as a block header; longer inputs are
/// decoded as a full block.
+///
+/// # Errors
+/// Returns an error if the input is empty, not valid hex, decoding fails,
+/// or JSON serialization fails.
#[wasm_bindgen]
pub fn parse_block_hex(hex_str: &str) -> Result<String, String> {
@@
/// Parses a hex-encoded raw transaction and returns a JSON string.
+///
+/// # Errors
+/// Returns an error if the input is empty, not valid hex, transaction decoding
+/// fails, or JSON serialization fails.
#[wasm_bindgen]
pub fn parse_tx_hex(hex_str: &str) -> Result<String, String> {As per coding guidelines, "Document # Errors for Result-returning functions."
Also applies to: 90-92
🤖 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 `@contrib/samples/parser/parser.rs` around lines 66 - 69, Add a `# Errors`
documentation section to both parser functions that return Result types. The
first function with rustdoc starting around line 66 needs documentation
explaining what errors can be returned, and the second function around lines
90-92 has the same requirement. Each `# Errors` section should describe the
failure conditions that cause the Result to return an error variant, following
the rustdoc coding guidelines for Result-returning functions.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/pages.yml (1)
22-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin workflow actions to immutable commit SHAs.
These
uses:references are still tag/branch-based, so execution can drift without a PR diff. Pin each action to a full 40-char SHA (optionally keep the version tag as an inline comment).#!/bin/bash # Verify any non-SHA-pinned actions in this workflow. rg -nP '^\s*uses:\s*[^@]+@(?![0-9a-f]{40}\b).+' .github/workflows/pages.ymlAlso applies to: 28-28, 37-37, 45-45, 57-57, 77-77
🤖 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 @.github/workflows/pages.yml at line 22, Replace all tag and branch-based action references in the workflow file with their immutable full 40-character commit SHAs. For the actions/checkout action at line 22 and all other uses statements at lines 28, 37, 45, 57, and 77, change each reference from formats like `@v6`, `@main`, or other tag/branch formats to the corresponding full commit SHA (you can optionally retain the version tag as an inline comment for documentation). Verify the changes by running the provided regex command to ensure no non-SHA-pinned actions remain in the workflow.Source: Linters/SAST tools
🤖 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.
Inline comments:
In `@contrib/samples/common.css`:
- Line 1: The `@import` statement in common.css is using the url() function
notation, which violates the configured stylelint import-notation rule. Remove
the url() function wrapper and instead use direct string notation for the import
statement, placing the URL directly as a string argument to `@import`.
---
Duplicate comments:
In @.github/workflows/pages.yml:
- Line 22: Replace all tag and branch-based action references in the workflow
file with their immutable full 40-character commit SHAs. For the
actions/checkout action at line 22 and all other uses statements at lines 28,
37, 45, 57, and 77, change each reference from formats like `@v6`, `@main`, or other
tag/branch formats to the corresponding full commit SHA (you can optionally
retain the version tag as an inline comment for documentation). Verify the
changes by running the provided regex command to ensure no non-SHA-pinned
actions remain in the workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b0146ef6-3aa8-482f-8e1a-12cb85f7ad15
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockcontrib/samples/Cargo.lockis excluded by!**/*.lockdocs/zen/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (44)
.github/workflows/build_msrv.yml.github/workflows/build_nightly.yml.github/workflows/build_stable.yml.github/workflows/pages.yml.gitignoreREADME.mdcontrib/__init__.pycontrib/build_docs.pycontrib/codeql/codeql-config.ymlcontrib/codeql/import.qlcontrib/codeql/lib/files.qllcontrib/codeql/lib/policy.qllcontrib/common.pycontrib/js/eslint.config.mjscontrib/lint/common.pycontrib/lint/common.pycontrib/lint/lint_codeql.pycontrib/lint/lint_javascript.pycontrib/lint/lint_rust.pycontrib/lint/lint_semgrep.pycontrib/lint_all.pycontrib/samples/Cargo.tomlcontrib/samples/common.csscontrib/samples/parser/Cargo.tomlcontrib/samples/parser/index.jscontrib/samples/parser/parser.rscontrib/samples/parser/style.csscontrib/samples/solver/Cargo.tomlcontrib/samples/solver/index.jscontrib/samples/solver/solver.rscontrib/samples/solver/style.csscontrib/samples/solver/worker.jscontrib/zen/__init__.pydocs/README.mddocs/zen/index.mddocs/zen/samples/index.mddocs/zen/samples/parser/index.mddocs/zen/samples/solver/index.mddocs/zen/style.csspkgs/primitives/Cargo.tomlpkgs/primitives/src/prelude.rspkgs/primitives/src/serialize.rspyproject.tomlzensical.toml
✅ Files skipped from review due to trivial changes (7)
- contrib/codeql/lib/files.qll
- .github/workflows/build_stable.yml
- docs/zen/samples/index.md
- docs/zen/samples/parser/index.md
- contrib/samples/parser/style.css
- contrib/codeql/codeql-config.yml
- README.md
🚧 Files skipped from review as they are similar to previous changes (27)
- contrib/init.py
- docs/README.md
- .github/workflows/build_nightly.yml
- pkgs/primitives/src/serialize.rs
- contrib/samples/Cargo.toml
- .github/workflows/build_msrv.yml
- docs/zen/samples/solver/index.md
- docs/zen/style.css
- pkgs/primitives/Cargo.toml
- contrib/samples/solver/Cargo.toml
- contrib/lint/lint_javascript.py
- pkgs/primitives/src/prelude.rs
- contrib/lint/lint_semgrep.py
- contrib/js/eslint.config.mjs
- zensical.toml
- contrib/lint/common.py
- contrib/samples/solver/style.css
- contrib/lint_all.py
- contrib/samples/parser/index.js
- pyproject.toml
- contrib/samples/solver/worker.js
- contrib/samples/parser/Cargo.toml
- contrib/samples/parser/parser.rs
- contrib/samples/solver/solver.rs
- contrib/codeql/import.ql
- contrib/samples/solver/index.js
- contrib/zen/init.py
5 tasks
thephez
requested changes
Jun 16, 2026
thephez
left a comment
thephez
left a comment
Collaborator
There was a problem hiding this comment.
A few minor things to look at
thephez
reviewed
Jun 16, 2026
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
.github/workflows/pages.yml (1)
22-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin all GitHub Actions to immutable commit SHAs.
Mutable refs (
@v*,@master) are still used, which leaves this workflow exposed to upstream ref retargeting. Please pin eachuses:entry to a full 40-char SHA.#!/bin/bash # Verify all unpinned uses in this workflow rg -nP '^\s*uses:\s*[^@]+@(?![0-9a-f]{40}\b).+$' .github/workflows/pages.ymlAlso applies to: 28-28, 37-37, 45-45, 57-57, 77-77
🤖 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 @.github/workflows/pages.yml at line 22, All GitHub Actions uses entries in the workflow file are using mutable version references like `@v6` instead of immutable full 40-character commit SHAs. Replace each mutable uses entry (including actions/checkout@v6 and others referenced at lines 28, 37, 45, 57, and 77) with its corresponding action pinned to a complete 40-character commit SHA to prevent upstream ref retargeting and ensure workflow security.Source: Linters/SAST tools
contrib/samples/solver/index.js (1)
199-205:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNumeric validation is still bypassable via permissive parsing.
parseBits()and DASH amount conversion still use permissive parsing, so malformed values (e.g. trailing garbage) can be accepted and used in solve payload construction.Suggested hardening
function parseBits() { const v = bitsInput.value.trim(); if (bitsMode === "hex") { - return parseInt(v, 16); + if (!/^(?:0x)?[0-9a-fA-F]{1,8}$/.test(v)) return NaN; + return Number.parseInt(v, 16); } - return parseInt(v, 10); + if (!/^(0|[1-9]\d*)$/.test(v)) return NaN; + return Number.parseInt(v, 10); } @@ function getAmountDuffs() { const v = amountInput.value.trim(); if (amountMode === "duffs") { - return v; + return /^(0|[1-9]\d*)$/.test(v) ? v : ""; } - return String(Math.round(parseFloat(v) * DUFFS_PER_DASH)); + if (!/^(?:0|[1-9]\d*)(?:\.\d{1,8})?$/.test(v)) return ""; + return String(Math.round(Number(v) * DUFFS_PER_DASH)); }Does JavaScript `parseInt("123abc", 10)` return `123` by ignoring trailing characters?Also applies to: 234-239, 243-249
🤖 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 `@contrib/samples/solver/index.js` around lines 199 - 205, The numeric validation for difficulty and amount values is still vulnerable to permissive parsing that accepts trailing garbage characters. The amount validation correctly uses a strict regex pattern /^\d+$/ to ensure the entire string contains only digits, but the parseBits() function and DASH amount conversion still use permissive parsing (e.g., JavaScript's parseInt which ignores trailing characters). Apply the same strict validation pattern used for amount validation to the difficulty and amount conversion functions, ensuring they reject any input with trailing non-numeric characters. This fix should be applied consistently across all locations where numeric values are parsed (the isU32(String(parseBits())) block and the amount conversion sections mentioned at lines 234-239 and 243-249).
🧹 Nitpick comments (2)
contrib/samples/solver/style.css (1)
110-144: 💤 Low valueConsider using CSS variables for admonition colors.
The admonition colors are hardcoded hex values. While this may be intentional to match GitHub alert styling, using CSS variables would improve maintainability and make theme customization easier.
♻️ Example refactor using CSS variables
Define variables in the root scope:
`#wasm-genesis` { --genesis-warn-bg: `#fef9e7`; --genesis-warn-fg: `#6e5b00`; --genesis-warn-border: `#d4a017`; /* ... similar for note and danger */ } [data-md-color-scheme="slate"] `#wasm-genesis` { --genesis-warn-bg: `#3b3000`; --genesis-warn-fg: `#fcd34d`; --genesis-warn-border: `#ca8a04`; /* ... similar for note and danger */ }Then use them:
`#wasm-genesis` .genesis-warn { border-left: 3px solid var(--genesis-warn-border); background: var(--genesis-warn-bg); color: var(--genesis-warn-fg); }🤖 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 `@contrib/samples/solver/style.css` around lines 110 - 144, The .genesis-warn, .genesis-note, and .genesis-danger classes contain hardcoded hex color values that should be replaced with CSS variables for improved maintainability and theme customization. Define CSS custom properties (variables) at the `#wasm-genesis` scope for each admonition type covering background, text color, and border colors, then define corresponding variables again within the [data-md-color-scheme="slate"] `#wasm-genesis` block for dark mode styling. Finally, update each of the three admonition classes (.genesis-warn, .genesis-note, .genesis-danger) to use var() references instead of hardcoded hex values.contrib/zen/__init__.py (1)
2-2: 💤 Low valueConsider removing or changing the encoding declaration.
The
latin-1encoding declaration is unusual for Python 3, which defaults to UTF-8. Since the file content appears to be pure ASCII, the encoding declaration is unnecessary and could cause issues if non-ASCII characters are added later.♻️ Simplify by removing the encoding line
#!/usr/bin/env python3 -# coding: latin-1🤖 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 `@contrib/zen/__init__.py` at line 2, Remove the `# coding: latin-1` encoding declaration from the beginning of the file in contrib/zen/__init__.py. Python 3 defaults to UTF-8 encoding, so this explicit declaration is unnecessary for ASCII-only content and can cause issues if non-ASCII characters are introduced later. Simply delete this line to align with Python 3 best practices.
🤖 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.
Inline comments:
In `@contrib/build_docs.py`:
- Around line 85-89: The shutil.copy2 calls in the _copy_artifacts function
assume that the destination directory path already exists, but they do not
create these directories if they are missing, which will cause
FileNotFoundError. Before each shutil.copy2 call (including the one for
common.css and the additional ones mentioned at lines 104-108), add a line to
create the parent directories by calling mkdir with parents=True and
exist_ok=True on the destination path's parent directory to ensure all required
directories exist before attempting to copy files.
In `@contrib/lint/lint_markdown.py`:
- Around line 41-45: The prefix path construction using str(repo_root) + "/"
only handles forward slashes, which fails when the linter outputs backslash
paths on Windows or other platforms. Fix the prefix variable to use a
platform-agnostic path separator (use pathlib.Path or os.path.normpath to
normalize the repo_root path), and ensure that when replacing the prefix in both
the stdout and stderr lines within the respective for loops, the replacement
accounts for the normalized path separator used by the operating system. This
will make the path stripping consistent across different path separator
conventions.
In `@contrib/zen/__init__.py`:
- Line 53: The direct dictionary access to _ALERT_KIND using match.group(1) at
line 53 will raise a KeyError if the source Markdown contains an unknown alert
type (e.g., [!ERROR] or [!INFO]). Add validation to check if the alert type from
match.group(1) exists as a key in the _ALERT_KIND dictionary before accessing
it. Choose a defensive approach either to gracefully skip unknown alerts by
using a default value or .get() method, or fail fast by raising a clear error
message that describes which invalid alert type was encountered.
In `@docs/zen/index.md`:
- Around line 3-5: The documentation intro in the SDK overview section currently
uses "interact with the Dash blockchain" which misleadingly implies networking
and runtime chain interaction capabilities. Revise the opening description to
clarify that the Base SDK's scope is limited to encoding, decoding,
construction, and validation of blockchain data structures, removing any
language that suggests network or runtime interaction capabilities. Focus the
description on the data-level operations and the rust-bitcoin foundation it
builds upon rather than implying full blockchain interaction.
---
Duplicate comments:
In @.github/workflows/pages.yml:
- Line 22: All GitHub Actions uses entries in the workflow file are using
mutable version references like `@v6` instead of immutable full 40-character
commit SHAs. Replace each mutable uses entry (including actions/checkout@v6 and
others referenced at lines 28, 37, 45, 57, and 77) with its corresponding action
pinned to a complete 40-character commit SHA to prevent upstream ref retargeting
and ensure workflow security.
In `@contrib/samples/solver/index.js`:
- Around line 199-205: The numeric validation for difficulty and amount values
is still vulnerable to permissive parsing that accepts trailing garbage
characters. The amount validation correctly uses a strict regex pattern /^\d+$/
to ensure the entire string contains only digits, but the parseBits() function
and DASH amount conversion still use permissive parsing (e.g., JavaScript's
parseInt which ignores trailing characters). Apply the same strict validation
pattern used for amount validation to the difficulty and amount conversion
functions, ensuring they reject any input with trailing non-numeric characters.
This fix should be applied consistently across all locations where numeric
values are parsed (the isU32(String(parseBits())) block and the amount
conversion sections mentioned at lines 234-239 and 243-249).
---
Nitpick comments:
In `@contrib/samples/solver/style.css`:
- Around line 110-144: The .genesis-warn, .genesis-note, and .genesis-danger
classes contain hardcoded hex color values that should be replaced with CSS
variables for improved maintainability and theme customization. Define CSS
custom properties (variables) at the `#wasm-genesis` scope for each admonition
type covering background, text color, and border colors, then define
corresponding variables again within the [data-md-color-scheme="slate"]
`#wasm-genesis` block for dark mode styling. Finally, update each of the three
admonition classes (.genesis-warn, .genesis-note, .genesis-danger) to use var()
references instead of hardcoded hex values.
In `@contrib/zen/__init__.py`:
- Line 2: Remove the `# coding: latin-1` encoding declaration from the beginning
of the file in contrib/zen/__init__.py. Python 3 defaults to UTF-8 encoding, so
this explicit declaration is unnecessary for ASCII-only content and can cause
issues if non-ASCII characters are introduced later. Simply delete this line to
align with Python 3 best practices.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 242fdc01-610c-4230-8702-d39ed95edf31
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockcontrib/samples/Cargo.lockis excluded by!**/*.lockdocs/zen/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (47)
.github/workflows/build_msrv.yml.github/workflows/build_nightly.yml.github/workflows/build_stable.yml.github/workflows/pages.yml.gitignoreCLAUDE.mdREADME.mdcontrib/__init__.pycontrib/build_docs.pycontrib/codeql/codeql-config.ymlcontrib/codeql/import.qlcontrib/codeql/lib/files.qllcontrib/codeql/lib/policy.qllcontrib/common.pycontrib/js/eslint.config.mjscontrib/lint/common.pycontrib/lint/common.pycontrib/lint/lint_codeql.pycontrib/lint/lint_javascript.pycontrib/lint/lint_markdown.pycontrib/lint/lint_rust.pycontrib/lint/lint_semgrep.pycontrib/lint_all.pycontrib/samples/Cargo.tomlcontrib/samples/common.csscontrib/samples/parser/Cargo.tomlcontrib/samples/parser/index.jscontrib/samples/parser/parser.rscontrib/samples/parser/style.csscontrib/samples/solver/Cargo.tomlcontrib/samples/solver/index.jscontrib/samples/solver/solver.rscontrib/samples/solver/style.csscontrib/samples/solver/worker.jscontrib/zen/__init__.pydocs/README.mddocs/guide_rust.mddocs/zen/index.mddocs/zen/samples/index.mddocs/zen/samples/parser/index.mddocs/zen/samples/solver/index.mddocs/zen/style.csspkgs/primitives/Cargo.tomlpkgs/primitives/src/prelude.rspkgs/primitives/src/serialize.rspyproject.tomlzensical.toml
✅ Files skipped from review due to trivial changes (8)
- contrib/lint/lint_semgrep.py
- contrib/init.py
- docs/zen/samples/index.md
- contrib/samples/parser/style.css
- README.md
- CLAUDE.md
- docs/guide_rust.md
- docs/zen/style.css
🚧 Files skipped from review as they are similar to previous changes (27)
- contrib/codeql/codeql-config.yml
- docs/README.md
- contrib/samples/Cargo.toml
- contrib/codeql/lib/files.qll
- docs/zen/samples/parser/index.md
- pkgs/primitives/src/prelude.rs
- .gitignore
- contrib/lint_all.py
- pkgs/primitives/Cargo.toml
- contrib/samples/solver/worker.js
- pyproject.toml
- pkgs/primitives/src/serialize.rs
- contrib/lint/lint_javascript.py
- contrib/lint/common.py
- contrib/samples/parser/Cargo.toml
- .github/workflows/build_stable.yml
- contrib/samples/parser/parser.rs
- .github/workflows/build_msrv.yml
- zensical.toml
- contrib/samples/common.css
- contrib/samples/solver/solver.rs
- .github/workflows/build_nightly.yml
- docs/zen/samples/solver/index.md
- contrib/samples/solver/Cargo.toml
- contrib/lint/common.py
- contrib/js/eslint.config.mjs
- contrib/samples/parser/index.js
Comment on lines
+41
to
+45
| prefix = str(repo_root) + "/" | ||
| for line in result.stdout.splitlines(): | ||
| print(line.replace(prefix, "")) | ||
| for line in result.stderr.splitlines(): | ||
| print(line.replace(prefix, ""), file=sys.stderr) |
There was a problem hiding this comment.
Normalize repo-root path stripping across path separators.
This replacement only handles / paths. If the linter emits \ paths, repo-root trimming won’t apply and output becomes inconsistent.
Proposed fix
- prefix = str(repo_root) + "/"
- for line in result.stdout.splitlines():
- print(line.replace(prefix, ""))
- for line in result.stderr.splitlines():
- print(line.replace(prefix, ""), file=sys.stderr)
+ prefixes = (f"{repo_root}/", f"{repo_root}\\")
+
+ def strip_repo_root(line: str) -> str:
+ for prefix in prefixes:
+ if line.startswith(prefix):
+ return line[len(prefix):]
+ return line
+
+ for line in result.stdout.splitlines():
+ print(strip_repo_root(line))
+ for line in result.stderr.splitlines():
+ print(strip_repo_root(line), file=sys.stderr)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| prefix = str(repo_root) + "/" | |
| for line in result.stdout.splitlines(): | |
| print(line.replace(prefix, "")) | |
| for line in result.stderr.splitlines(): | |
| print(line.replace(prefix, ""), file=sys.stderr) | |
| prefixes = (f"{repo_root}/", f"{repo_root}\\") | |
| def strip_repo_root(line: str) -> str: | |
| for prefix in prefixes: | |
| if line.startswith(prefix): | |
| return line[len(prefix):] | |
| return line | |
| for line in result.stdout.splitlines(): | |
| print(strip_repo_root(line)) | |
| for line in result.stderr.splitlines(): | |
| print(strip_repo_root(line), file=sys.stderr) |
🤖 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 `@contrib/lint/lint_markdown.py` around lines 41 - 45, The prefix path
construction using str(repo_root) + "/" only handles forward slashes, which
fails when the linter outputs backslash paths on Windows or other platforms. Fix
the prefix variable to use a platform-agnostic path separator (use pathlib.Path
or os.path.normpath to normalize the repo_root path), and ensure that when
replacing the prefix in both the stdout and stderr lines within the respective
for loops, the replacement accounts for the normalized path separator used by
the operating system. This will make the path stripping consistent across
different path separator conventions.
| index += 1 | ||
| continue | ||
|
|
||
| level = _ALERT_KIND[match.group(1)] |
There was a problem hiding this comment.
Add validation to prevent KeyError on unknown alert types.
If the source Markdown contains an alert type not defined in _ALERT_KIND (e.g., [!ERROR] or [!INFO]), line 53 will raise a KeyError and crash the documentation build. This creates a fragile preprocessing step.
🛡️ Defensive handling options
Option 1: Skip unknown alerts (graceful degradation)
- level = _ALERT_KIND[match.group(1)]
+ alert_type = match.group(1)
+ if alert_type not in _ALERT_KIND:
+ output.append(line)
+ index += 1
+ continue
+ level = _ALERT_KIND[alert_type]
output.append(f"!!! {level}")Option 2: Fail fast with clear error message
- level = _ALERT_KIND[match.group(1)]
+ alert_type = match.group(1)
+ if alert_type not in _ALERT_KIND:
+ msg = f"Unsupported GFM alert type: {alert_type}"
+ raise ValueError(msg)
+ level = _ALERT_KIND[alert_type]
output.append(f"!!! {level}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| level = _ALERT_KIND[match.group(1)] | |
| alert_type = match.group(1) | |
| if alert_type not in _ALERT_KIND: | |
| msg = f"Unsupported GFM alert type: {alert_type}" | |
| raise ValueError(msg) | |
| level = _ALERT_KIND[alert_type] |
🤖 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 `@contrib/zen/__init__.py` at line 53, The direct dictionary access to
_ALERT_KIND using match.group(1) at line 53 will raise a KeyError if the source
Markdown contains an unknown alert type (e.g., [!ERROR] or [!INFO]). Add
validation to check if the alert type from match.group(1) exists as a key in the
_ALERT_KIND dictionary before accessing it. Choose a defensive approach either
to gracefully skip unknown alerts by using a default value or .get() method, or
fail fast by raising a clear error message that describes which invalid alert
type was encountered.
Comment on lines
+3
to
+5
| The Base SDK for Dash is a collection of packages that enable applications to parse, construct and interact with the | ||
| Dash blockchain. This is achieved by leveraging the [`rust-bitcoin`](https://github.com/rust-bitcoin/rust-bitcoin) | ||
| ecosystem to provide a framework that builds upon existing, familiar APIs and contract expectations. |
There was a problem hiding this comment.
Clarify SDK scope to avoid implying built-in networking.
The current intro wording can read as runtime chain/network interaction. Consider tightening this to encoding/decoding, construction, and validation of blockchain data structures.
Based on learnings, no crate currently implements networking and the SDK scope is encoding/decoding plus data types.
🤖 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 `@docs/zen/index.md` around lines 3 - 5, The documentation intro in the SDK
overview section currently uses "interact with the Dash blockchain" which
misleadingly implies networking and runtime chain interaction capabilities.
Revise the opening description to clarify that the Base SDK's scope is limited
to encoding, decoding, construction, and validation of blockchain data
structures, removing any language that suggests network or runtime interaction
capabilities. Focus the description on the data-level operations and the
rust-bitcoin foundation it builds upon rather than implying full blockchain
interaction.
Source: Learnings
5 tasks
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
2 participants
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.
Motivation
Base SDK is built to run in resource-constrained environments with fewer platform assumptions, till now, the benefits have been largely theoretical. This pull request is part of a series to start building web-ready documentation with examples. As we target
no_std+alloc, WASM support is trivial, allowing us to write interactive demos building on the same foundations used for native applications.Future pull requests will document the protocol and the SDK's intended usage patterns as part of a more traditional series but this pull request for now sets up the infrastructure to allow these future contributions (mostly likely after the API shape of the SDK is more clear).
Additional Information
f64::round()is not available incore(see rust-lang/rust#137578), causing compile failures forwasm32-unknown-unknown.To remedy this,
libmis pulled in as the community-standardno_stdreplacement (source) and the shim inprelude.rsdispatches tof64::round()whenstdis available.CodeQL's
fileCfgLinesregex captured paths relative topkgs/butsourceLineContentreturns paths relative to root after multi-directory expansion, causing silent mismatches. The regex was updated to align the two.Shared utilities introduced in base-sdk#5 are located in
contrib/lint/common.pyand are inaccessible to code outside the lint directory. As the docs builder has overlapping needs, it has been moved tocontrib.py(without the executable bit, just like__init__.py) with a symlink left incontrib/lint/to allowing existing lint scripts to run modified.The published docs are built using Zensical (a fork of MkDocs) with additional preprocessing, minification and build steps for WASM-enabled demos. It was chosen as Zensical provides relatively modern design sensibilities OOTB, extensibility that suits our needs and allowed us to maintain our documentation in Markdown, so it can be rendered on GitHub.
The WASM sample crates live in a separate workspace (
contrib/samples) rather than as members of the root workspace to keep WASM-specific dependencies (e.g.wasm-bindgen) out of the dependency tree.pkg/directories is excluded from JS minification becauserjsmincan corrupt thewasm-bindgenoutput, which relies on specific string patterns for module instantiation.Breaking Changes
None expected.
How Has This Been Tested?
Checklist