Skip to content

feat(audit): Stellar chain module cryptographic audit report and fixes - #93

Merged
truthixify merged 4 commits into
wraith-protocol:developfrom
Timrossid:audit/stellar-module-crypto
Jun 25, 2026
Merged

feat(audit): Stellar chain module cryptographic audit report and fixes#93
truthixify merged 4 commits into
wraith-protocol:developfrom
Timrossid:audit/stellar-module-crypto

Conversation

@Timrossid

@Timrossid Timrossid commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR delivers the independent cryptographic audit of the Stellar chain module (issue #55).

Deliverables

Audit report

\�udits/2026-06-author-stellar-module.md\ — covers every primitive in \src/chains/stellar/:

  • 0 Critical, 0 High, 2 Medium, 2 Low, 9 Informational findings
  • Cross-references RFC 8032 and noble-curves implementation
  • Coordinated disclosure: no Critical/High findings

Findings & Fixes

Finding Severity Fix
signWithScalar nonce derivation Medium Documented deviation from RFC 8032 (necessary — no seed available)
Missing zero-scalar guard in signWithScalar Medium Added range check \scalar ∈ (0, L)\
Missing zero-scalar guard in deriveStealthPrivateScalar Low Added guard + skip in scanAnnouncements
Small-order point handling Low Graceful via try/catch (DoS only, no fund loss)
All other findings Info Verified correct (domain separation, LE encoding, bias, clamping)

Tests

  • signWithScalar test vectors (17 tests): determinism, edge cases, scalar=0/L-1, empty/1MB messages, LE encoding, cross-validation with @noble/curves
  • Clamping reproducer test in keys.test.ts
  • signStellarTransaction cross-validation in spend.test.ts
  • E2e signing step added to e2e.test.ts

Files changed

  • \src/chains/stellar/scalar.ts\ — scalar range guard in signWithScalar
  • \src/chains/stellar/scan.ts\ — skip zero-scalar candidates
  • \src/chains/stellar/spend.ts\ — zero-scalar guard in deriveStealthPrivateScalar
  • \ est/chains/stellar/signwithscalar-vectors.test.ts\ — new test file (17 test vectors)
  • \ est/chains/stellar/keys.test.ts\ — clamping + domain separation reproducer tests
  • \ est/chains/stellar/spend.test.ts\ — signStellarTransaction cross-validation
  • \ est/chains/stellar/e2e.test.ts\ — signing step included in full flow
  • \�udits/2026-06-author-stellar-module.md\ — full audit report

Closes #55

Jefferson Youashi and others added 2 commits June 2, 2026 00:20
…raith-protocol#45)

* perf(stellar): prefilter scans with public view tags

* test(stellar): cover legacy view-tag scanner
…fixes

Completes independent cryptographic audit of the Stellar chain module.
(issue wraith-protocol#55)

Findings:
- 0 Critical, 0 High, 2 Medium, 2 Low, 9 Informational
- All cryptographic primitives verified correct
- Custom signWithScalar implementation justified and cross-validated

Fixes applied:
- Add zero-scalar guard in signWithScalar (scalar must be in (0, L))
- Add stealth private scalar validation in deriveStealthPrivateScalar
- Skip zero-scalar candidates in scanAnnouncements
- Verify signatures with @noble/curves ed25519.verify()

New test coverage:
- signWithScalar test vectors (17 tests): determinism, edge cases,
  boundary values (scalar=1, scalar=L-1), empty/1MB messages, LE encoding
- Clamping reproducer test (keys.test.ts)
- signStellarTransaction cross-validation (spend.test.ts)
- E2e signing step added

Closes wraith-protocol#55
@drips-wave

drips-wave Bot commented Jun 24, 2026

Copy link
Copy Markdown

@Timrossid Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@truthixify
truthixify changed the base branch from main to develop June 24, 2026 14:49
@truthixify

Copy link
Copy Markdown
Contributor

The audit doc is solid. 13 findings with right severity tiers, the signWithScalar justification reads correctly, cross-validation tests are concrete. Good XL deliverable.

Same blocker as #88/#89/#91/#92: branch predates view-tag batching landing on develop, so the diff includes those files as conflicts.

git fetch origin
git rebase origin/develop
# during rebase, drop changes to src/chains/stellar/scan.ts, stealth.ts (the view-tag bits), test/chains/stellar/scan.test.ts, test/chains/stellar/bench/scan.bench.ts, docs/chains/stellar-view-tag-batching.md
# keep: audits/2026-06-author-stellar-module.md, test/chains/stellar/signwithscalar-vectors.test.ts, your spend.test.ts/keys.test.ts/e2e.test.ts additions, the scalar.ts 3-line fix, src/chains/stellar/spend.ts changes, the index.ts re-exports
git push --force-with-lease

Once rebased the audit + test fixtures land cleanly.

@truthixify

Copy link
Copy Markdown
Contributor

Clean merge. The audit report (236 lines), view-tag-batching doc, scalar/scan/spend/stealth fixes, and signWithScalar vectors are exactly what we needed. Bench harness is a nice bonus. Thanks @Timrossid.

@truthixify
truthixify merged commit f7def68 into wraith-protocol:develop Jun 25, 2026
maztah1 added a commit to maztah1/sdk that referenced this pull request Jul 2, 2026
Bring the WebGPU spike branch up to date with develop post-wraith-protocol#93 and wraith-protocol#116:

src/chains/stellar/constants.ts
  - Add SCHEME_ID_V1, SCHEME_ID_V2, ANNOUNCE_EVENT_SYMBOL, VIEW_TAG_BUCKET_COUNT
    (from wraith-protocol#93 audit / develop)

src/chains/stellar/scan.ts
  - Add scanAnnouncementsStream (streaming / low-memory variant) from develop
  - Accept SCHEME_ID_V2 announcements in scanAnnouncements + stream
  - Add zero-scalar guard: skip stealthPrivateScalar <= 0n (wraith-protocol#93 security fix)

src/chains/stellar/stealth.ts
  - Merge JSDoc from develop (detailed DKSAP steps + @deprecated note on
    computeViewTag)

src/chains/stellar/index.ts
  - Re-export SCHEME_ID_V1/V2, ANNOUNCE_EVENT_SYMBOL, VIEW_TAG_BUCKET_COUNT
  - Re-export scanAnnouncementsStream, scanAnnouncementsLegacySharedSecretTag
  - Re-export computeAnnouncementViewTag

src/chains/stellar/webgpu/scan-webgpu.ts
  - Accept SCHEME_ID_V2 in the GPU prefilter path (mirrors scan.ts change)

pnpm-workspace.yaml
  - Add allowBuilds config + root + examples/* entries from develop

packages/test-vectors/vectors/{ckb,evm,solana}.json
  - Generate deterministic vectors for CKB, EVM, Solana chains
    (fixes three pre-existing test failures from commit 764b751)

packages/test-vectors/scripts/generate-multichain.ts
  - New generator script for the three missing vector sets

All 31 test files / 2036 tests pass.
truthixify pushed a commit that referenced this pull request Jul 29, 2026
* test(test-vectors): add differential harness across sdk versions

No safety net today catches a version bump that silently changes
cryptographic output for existing inputs — semver numbers alone don't
verify actual behavior. Add differential.ts: it installs the pinned
reference version (vN-1, named in differential.config.json) into a
throwaway directory, runs every vectors/stellar.json fixture through both
that build and the workspace tip via the shared differential-runner.mjs,
and diffs the outputs field by field. Unwaived diffs fail the run; waivers
in differences.json require a non-empty reason and the script refuses to
run without one.

Running the harness for real against the pinned reference (1.3.0) surfaced
two genuine issues:

- generateStealthAddress's default view-tag scheme changed (legacy prefix
  to the domain-separated v2 prefix) in commit f7def68 (PR #93), before the
  1.4.0 minor, without a major bump. This is a real, already-shipped
  protocol change — documented and waived in differences.json with the
  commit reference, exactly the kind of entry this file exists to hold.
- signWithScalar had a leftover debug `console.log` that wrote to stdout on
  every signing call, corrupting the runner's JSON-over-stdout protocol
  (and explaining the stray "SCALAR:" noise in the existing test output).
  Removed it.

Wire a `differential` job into ci.yml that runs on every PR touching
src/chains/** (via dorny/paths-filter) and unconditionally on push to
main/develop, so the acceptance criterion of a green run on develop is
checked directly rather than assumed.

Closes #132

* fix(ci): repair lockfile broken by the develop merge

The 'Merge branch develop into feat/132-test-vectors-differential-harness'
merge commit left pnpm-lock.yaml missing an entry for
@stellar/stellar-sdk@13.3.0, so every CI job died within seconds at the
first 'pnpm install --frozen-lockfile' step, before continue-on-error
steps further down even ran. Regenerate the lockfile with
'pnpm install --no-frozen-lockfile'.

Verified locally after the fix: pnpm install --frozen-lockfile succeeds,
format/build/test all pass (1073 tests), the differential harness still
runs clean against the merged-in chains code, and every Examples CI
matrix example (including the new stellar-chrome-extension from upstream)
installs, type-checks, and builds.

* fix(ci): remove the bun job reintroduced by a bad merge resolution

CI/bun was cancelled after the full 6-hour job ceiling:

  test/leaks/scan-leak.test.ts:
    Error: The operation was canceled.

Reproduced locally: `bun test test/leaks/scan-leak.test.ts` hangs
indefinitely with zero output (killed after 30s), most likely inside its
v8.getHeapSnapshot() stream handling, which bun's own --timeout doesn't
preempt. That file is already excluded from the pnpm `test` script for
being slow/resource-intensive; bun just turns "slow" into "hangs forever."

Excluding it isn't enough on its own, though: upstream/develop already
removed this job entirely in 885ec6e ("ci: drop the aspirational bun job"),
because bun test also fails ~45 more tests unrelated to the hang (vi.hoisted
and vi.importActual aren't implemented in bun's vitest-compat layer, so
every sdk-react/sdk-vue mocking-based test fails, plus a stale dist/
build artifact and a genuine assertion-style mismatch in one pipeline
test). None of that has anything to do with this PR's changes.

The job is only present on this branch because an earlier merge from
develop resolved the conflict in ci.yml by keeping it instead of taking
upstream's deletion. Remove it again so this branch matches the decision
upstream already made and merged, rather than re-fighting it here.

Verified: this branch's ci.yml now diffs from upstream/develop's only by
the addition of this PR's own `differential` job. format/build/test all
still pass locally (1073 tests).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Independent cryptographic audit of Stellar chain module

2 participants