Skip to content

fix: count literal Miniscript witness selectors for fee sizing - #7098

Draft
antuz123 wants to merge 1 commit into
sprintfrom
codex/r5-witness-fee-fix-20261022
Draft

antuz123 wants to merge 1 commit into
sprintfrom
codex/r5-witness-fee-fix-20261022

Conversation

@antuz123

@antuz123 antuz123 commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Publication status — 24 October 2026

Completed code, regression fixtures and documentation are already published in this existing draft at 514d2d656da17336312fef98989c01c2113ef9f1; local and remote heads match, with no unpublished implementation. No duplicate PR or commit was created on resume. The current R5 integration #7068 checkpoint 8b56c6b51831e69f4b67a77754f431f5dcf4711c already includes this commit by verified ancestry, and its fee helper is byte-identical. No patch was replayed. #7082 remains the original frozen 3eb92fc8 checkpoint.

The completed hosted results below were run on 22 October for this unchanged PR head. They do not transfer to newer R5 source. Native validation is being coordinated separately: no native build, rerun, ready-for-review transition or native-trigger label was started by this resume. The PR remains draft / NEEDS_LOCAL; no merge or release approval is implied.

Count only the pinned Miniscript satisfier's complete 0 / 1 witness tokens when estimating input size. This replaces the destructive placeholder-stripping chain flagged by CodeQL; signature/key counts, script sizing, rounding and coin selection retain their existing logic.

The fee helper is identical in sprint and R5 #7082 at 3eb92fc8a622a3e09926d49f47243c163e32d009. This focused PR targets sprint; the same patch and tests also passed on a disposable copy of that R5 checkpoint. It does not modify or integrate #7082. Production diff: four inserted / five removed lines; remaining files are regression evidence and documentation.

Evidence and validation

  • No accessible successor to the unavailable candidate was recovered from fetched refs/history, recent PR searches or CodeQL artifacts. Historical 140 + 104 comparisons are not used to validate this patch.
  • Fresh immutable R5 baseline: 78 policies / 927 generated witnesses, plus 120 complete public fee API scenarios using real compiler and coin selection. Read-only baseline verifier PASS.
  • Prepatch: 213 equivalence/anchor checks PASS; ten new malformed-text regressions FAIL as expected.
  • Patched PR source: 223/223 PASS (30.395s). Patched R5 checkpoint: 223/223 PASS (31.100s). Isolated Node/Jest configuration executes actual production AST bodies without native/backend imports; this is not a full app or native test result.
  • Repository-config focused Jest: 223/223 PASS (13.665s); JavaScript syntax and git diff --check PASS. Scoped operations-file ESLint FAILS with the same 10 errors / 12 warnings as sprint (zero normalized diagnostic additions). Frozen install on unchanged sprint package/lock inputs FAILS: lockfile needs updating. Full repository/native acceptance is not claimed.
  • Completed hosted CI: run 39832893591, attempt 1, actual merge checkout e072f055560877b76be9526d3140ed870b27b4e5 (same Git tree as this head). Tests 120013424994: 18 suites / 406 tests PASS, zero snapshots, 17.673s. The dependency install was not frozen and rewrote lockfiles in the disposable runner; this does not repair the recorded frozen-install failure.
  • Hosted lint command FAIL: job 120013425406 reports 4,540 errors / 913 warnings, exit 1. Its green job conclusion comes from continue-on-error, not clean lint.
  • Native gate remains FAIL: keeper/local-validation, latest completed job 120015986997, awaits trusted exact-source native validation. Its legacy Mac wording is not a request for the owner's personal Mac. Native execution remains with the separate coordinator.
  • Current-source local R5 CodeQL complete: CLI 2.27.2 / JavaScript queries 2.4.7, all 89 default code-scanning queries. Baseline: 2 results including the target sanitization result. Patched R5 composite: target result absent; one inherited High result remains unresolved. Both analyses exit 0 / executionSuccessful, zero warning/error notifications; coverage 888/888 → 891/891 JS/TS and 13/13 Actions files. This is local analysis of the exact R5 checkpoint plus this patch/test/doc delta, not an integrated or hosted R5 acceptance result.
  • Current PR GitHub CodeQL PASS: analysis run 39832893584, attempt 1 / job 120013425456, completed SUCCESS on head 514d2d656da17336312fef98989c01c2113ef9f1. The separate security check 120015222886 completed SUCCESS: No new alerts in code changed by this pull request, zero annotations. This scoped check does not clear the inherited High result from the full R5 scan.
  • Focused patch SHA256 613479473a14d2872f8b28ee7c712d9f736a8a9ea1c05d1773f04a06b6f37a99; patched R5 operations blob 85ca30c4527f31ffcc166ade5391e81459013fa2. Baseline SARIF SHA256 5405ac92137d6f8f76a6dbed54890058b53cd1733415546dc9df358f863cb66f; patched SARIF b3bb4aa1a4c9e6ab4a2be947e7937ee43a967702644ef71511eaec5d3910c930. Complete SARIF/source manifests and the additional inherited finding are retained in local security evidence for maintainer review under SECURITY.md.

Security evidence and limits

Recovered actual failing security check 119985857101: High incomplete multi-character sanitization, operations/index.ts:110–111, possible remaining <script. It is a pattern-based problem query; numeric downstream use alone does not clear it. Alert/dataflow and analysis APIs returned 403 Resource not accessible by integration; no alert number or end-to-end path is invented. Actions analysis execution success is separate from security-check success.

Equivalence is bounded to the pinned generated witness grammar. Malformed/unsupported tokens intentionally stop contributing through stripped remnants. This is not a general witness parser or an HTML sanitizer. The original #7082 security check and other inherited R5/native/dependency findings remain unresolved on their original sources. No suppression, dismissal, paid reviewer activation, merge or release action. Access to both code-scanning endpoints was rechecked on 24 October and still returns HTTP403; the evidence does not establish full official alert access.

See investigation and reproduction instructions.

Codex local handoff

State: NEEDS_LOCAL
PR: #7098
Base: sprint
Head branch: codex/r5-witness-fee-fix-20261022
Head commit: 514d2d6
Goal and acceptance criteria: Remove destructive witness selector stripping while preserving generated-witness input allowances and public fee behavior; obtain current-source CodeQL results.
Changes made: Literal 0/1 token count; fresh immutable R5 vectors, production-body harness, regression tests and bounded investigation notes.
Cloud checks: 223/223 focused checks PASS on PR source and separately patched R5 checkpoint; immutable baseline verifier PASS (78 policies, 927 witnesses, 120 API scenarios); syntax and diff checks PASS. Full local R5 CodeQL suite complete: target absent, one inherited High unresolved. GitHub CodeQL analysis and separate security check PASS (run39832893584 / check120015222886). Scoped lint and frozen install FAIL as recorded above. Hosted CI Tests PASS (18 suites/406 tests, run39832893591); hosted lint command FAILS (4540 errors/913 warnings) despite tolerated green job. keeper/local-validation remains FAIL awaiting separately coordinated native validation.
Required local checks: IOS, ANDROID
Local checks requested: Hosted iOS/Android simulator/emulator checks of normal, custom and send-max fees for ordinary and Miniscript wallets, including nested-key/timelock/inheritance paths, on the eventual exact combined candidate. No personal Mac prerequisite. Not run in this bounded security task.
Risks / known gaps: Full repository/native acceptance not established; installed legacy keeper/local-validation gate FAILS awaiting exact-source validation (not a personal Mac request); code-scanning APIs return403; original R5 findings/native/physical/integration requirements remain separate. No general witness-validation or full wallet-security claim.
Release readiness: NOT READY.

This branch has not been deployed

No deployments
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.

2 participants