Skip to content

Remove validation during parsing#932

Merged
tomusdrw merged 2 commits intomainfrom
td-lazy-parser
Mar 19, 2026
Merged

Remove validation during parsing#932
tomusdrw merged 2 commits intomainfrom
td-lazy-parser

Conversation

@tomusdrw
Copy link
Copy Markdown
Member

  • Remove early validation of work items count and ticket attempt

We do check all of these things anyway later in the pipeline, but doing validation during parsing forces us to ignore conformance tests which is becoming a bit cumbersome.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 74674b86-96c3-4b36-a7a2-def85b0e6afd

📥 Commits

Reviewing files that changed from the base of the PR and between 0115148 and 1ee5d9b.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (26)
  • bin/convert/types.ts
  • bin/test-runner/jam-conformance-071.ts
  • bin/test-runner/jam-conformance-072.ts
  • bin/test-runner/w3f/runners.ts
  • bin/test-runner/w3f/safrole.ts
  • packages/jam/block-json/block.ts
  • packages/jam/block-json/common.ts
  • packages/jam/block-json/extrinsic.ts
  • packages/jam/block-json/header.ts
  • packages/jam/block-json/tickets-extrinsic.ts
  • packages/jam/block-json/work-result.ts
  • packages/jam/block/tickets.ts
  • packages/jam/block/work-package.ts
  • packages/jam/jamnp-s/protocol/ce-131-ce-132-safrole-ticket-distribution.test.ts
  • packages/jam/jamnp-s/tasks/ticket-distribution.test.ts
  • packages/jam/safrole/bandersnatch-vrf.test.ts
  • packages/jam/safrole/bandersnatch-vrf.ts
  • packages/jam/safrole/safrole.test.ts
  • packages/jam/state-json/dump.ts
  • packages/jam/state-json/safrole.ts
  • packages/jam/state-vectors/index.ts
  • packages/jam/state/test.utils.ts
  • packages/jam/transition/hasher.test.ts
  • packages/workers/block-authorship/main.ts
  • packages/workers/block-authorship/ticket-generator.test.ts
  • packages/workers/block-authorship/ticket-generator.ts
💤 Files with no reviewable changes (4)
  • packages/workers/block-authorship/main.ts
  • packages/workers/block-authorship/ticket-generator.ts
  • bin/test-runner/jam-conformance-072.ts
  • bin/test-runner/jam-conformance-071.ts

📝 Walkthrough

Summary by CodeRabbit

  • Tests

    • Re-enabled previously skipped test vectors in conformance test suites to increase coverage.
  • Refactor

    • Streamlined JSON parsing and validation to reduce parameter dependencies and improve code maintainability.

Walkthrough

This pull request refactors JSON parsing and type validation across the codebase by removing ChainSpec dependencies. Key changes include converting function-based fromJson handlers into constant-based FromJson values, removing the ChainSpec parameter from tryAsTicketAttempt, updating handler registrations in bin/convert/types.ts to pass function references instead of computed values, and removing previously ignored test vectors from jam-conformance test files. The refactoring spans JSON parsing modules, type definitions, test runners, and worker packages, affecting how JSON parsers are constructed and invoked throughout the codebase.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • FluffyLabs/typeberry\TicketAttempt validation and encoding fix #913: Makes opposite, directly conflicting changes to the same public APIs (tryAsTicketAttempt, headerFromJson, StateTransitionGenesis.fromJson) by adding or requiring ChainSpec parameters where this PR removes them.
  • FluffyLabs/typeberry\Refactor test runner #744: Modifies the same fromJson/runner infrastructure (bin/test-runner/w3f/runners.ts, headerFromJson, StateTransitionGenesis.fromJson) converting per-spec functions into spec-free FromJson values.
  • FluffyLabs/typeberry\Persistent DB and new CLI #351: Introduces the @typeberry/state-json package with identical signature and usage changes across headerFromJson, ticketsExtrinsicFromJson, and StateTransitionGenesis.fromJson.

Suggested reviewers

  • mateuszsikora
  • DrEverr

Poem

🐰 A rabbit hops through TypeScript land,
No specs to pass, the code's more grand!
FromJson constants, steady and true,
Ticket attempts need not ask "who?"—
Simpler paths for parsers to roam, 🎉
In chainless fields, we've found our home!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Remove validation during parsing' accurately reflects the main change across all modified files, which systematically remove ChainSpec validation requirements from parsing logic.
Description check ✅ Passed The description directly relates to the changeset, explaining the rationale for removing early validation of work items count and ticket attempt during parsing.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch td-lazy-parser
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

View all
File Benchmark Ops
bytes/hex-from.ts[0] parse hex using Number with NaN checking 65019.43 ±1.17% 85.89% slower
bytes/hex-from.ts[1] parse hex from char codes 460925.7 ±0.31% fastest ✅
bytes/hex-from.ts[2] parse hex from string nibbles 257922.74 ±0.5% 44.04% slower
bytes/hex-to.ts[0] number toString + padding 89884.04 ±0.68% fastest ✅
bytes/hex-to.ts[1] manual 7318.68 ±0.76% 91.86% slower
codec/bigint.compare.ts[0] compare custom 92314083.24 ±4.95% fastest ✅
codec/bigint.compare.ts[1] compare bigint 86406900.48 ±6.25% 6.4% slower
codec/bigint.decode.ts[0] decode custom 69940958.65 ±3.04% 24.95% slower
codec/bigint.decode.ts[1] decode bigint 93196386.77 ±4.08% fastest ✅
codec/decoding.ts[0] manual decode 9637527.24 ±1.19% 86.14% slower
codec/decoding.ts[1] int32array decode 66213282.37 ±2.47% 4.75% slower
codec/decoding.ts[2] dataview decode 69517541.5 ±2.81% fastest ✅
codec/encoding.ts[0] manual encode 890394.01 ±0.84% 11.44% slower
codec/encoding.ts[1] int32array encode 1005385.92 ±0.39% fastest ✅
codec/encoding.ts[2] dataview encode 991265.32 ±0.32% 1.4% slower
collections/map-set.ts[0] 2 gets + conditional set 71265.54 ±0.11% fastest ✅
collections/map-set.ts[1] 1 get 1 set 47294.4 ±0.1% 33.64% slower
logger/index.ts[0] console.log with string concat 4877048.88 ±39.85% fastest ✅
logger/index.ts[1] console.log with args 1421005.25 ±73.69% 70.86% slower
math/add_one_overflow.ts[0] add and take modulus 96589873.21 ±5.03% 1.52% slower
math/add_one_overflow.ts[1] condition before calculation 98076447.02 ±5.19% fastest ✅
math/count-bits-u32.ts[0] standard method 47163722.47 ±2.23% 47.76% slower
math/count-bits-u32.ts[1] magic 90287629.7 ±3.8% fastest ✅
math/count-bits-u64.ts[0] standard method 5105113.52 ±0.41% 80.66% slower
math/count-bits-u64.ts[1] magic 26400983.48 ±2.25% fastest ✅
math/mul_overflow.ts[0] multiply and bring back to u32 101067189.84 ±4.5% fastest ✅
math/mul_overflow.ts[1] multiply and take modulus 96834687.09 ±5.19% 4.19% slower
math/switch.ts[0] switch 98566668.41 ±4.43% fastest ✅
math/switch.ts[1] if 90840571.18 ±5.44% 7.84% slower
hash/index.ts[0] hash with numeric representation 65.81 ±1.69% 34.06% slower
hash/index.ts[1] hash with string representation 41.84 ±1.95% 58.08% slower
hash/index.ts[2] hash with symbol representation 65.95 ±0.86% 33.92% slower
hash/index.ts[3] hash with uint8 representation 76.23 ±0.31% 23.62% slower
hash/index.ts[4] hash with packed representation 99.8 ±0.33% fastest ✅
hash/index.ts[5] hash with bigint representation 69.81 ±0.92% 30.05% slower
hash/index.ts[6] hash with uint32 representation 92.71 ±0.48% 7.1% slower
bytes/bytes-to-number.ts[0] Conversion with bitops 3094.23 ±5.62% fastest ✅
bytes/bytes-to-number.ts[1] Conversion without bitops 2730.29 ±3.53% 11.76% slower
bytes/compare.ts[0] Comparing Uint32 bytes 11150.6 ±0.32% 2.23% slower
bytes/compare.ts[1] Comparing raw bytes 11405.39 ±0.33% fastest ✅
codec/view_vs_collection.ts[0] Get first element from Decoded 14527.83 ±0.59% 54.53% slower
codec/view_vs_collection.ts[1] Get first element from View 31950.52 ±0.71% fastest ✅
codec/view_vs_collection.ts[2] Get 50th element from Decoded 14546.44 ±0.74% 54.47% slower
codec/view_vs_collection.ts[3] Get 50th element from View 17136.58 ±0.39% 46.37% slower
codec/view_vs_collection.ts[4] Get last element from Decoded 14768.62 ±0.22% 53.78% slower
codec/view_vs_collection.ts[5] Get last element from View 11640.92 ±1.05% 63.57% slower
codec/view_vs_object.ts[0] Get the first field from Decoded 199208.98 ±0.5% 0.42% slower
codec/view_vs_object.ts[1] Get the first field from View 28996.88 ±82.78% 85.51% slower
codec/view_vs_object.ts[2] Get the first field as view from View 50284.17 ±0.61% 74.86% slower
codec/view_vs_object.ts[3] Get two fields from Decoded 200047.89 ±0.46% fastest ✅
codec/view_vs_object.ts[4] Get two fields from View 40228.43 ±0.36% 79.89% slower
codec/view_vs_object.ts[5] Get two fields from materialized from View 80344.83 ±0.4% 59.84% slower
codec/view_vs_object.ts[6] Get two fields as views from View 40249.45 ±0.48% 79.88% slower
codec/view_vs_object.ts[7] Get only third field from Decoded 199353.83 ±0.53% 0.35% slower
codec/view_vs_object.ts[8] Get only third field from View 50245.2 ±0.36% 74.88% slower
codec/view_vs_object.ts[9] Get only third field as view from View 50192.34 ±0.37% 74.91% slower
collections/map_vs_sorted.ts[0] Map 117560.39 ±0.37% fastest ✅
collections/map_vs_sorted.ts[1] Map-array 46492.67 ±0.19% 60.45% slower
collections/map_vs_sorted.ts[2] Array 52964.32 ±0.2% 54.95% slower
collections/map_vs_sorted.ts[3] SortedArray 83187.81 ±0.11% 29.24% slower
collections/hash-dict-vs-blob-dict_delete.ts[0] StringHashDictionary 2647.16 ±0.32% fastest ✅
collections/hash-dict-vs-blob-dict_delete.ts[1] BlobDictionary(1) 2645.41 ±0.36% 0.07% slower
collections/hash-dict-vs-blob-dict_delete.ts[2] BlobDictionary(2) 2642.23 ±0.32% 0.19% slower
collections/hash-dict-vs-blob-dict_delete.ts[3] BlobDictionary(3) 2637.27 ±0.36% 0.37% slower
collections/hash-dict-vs-blob-dict_delete.ts[4] BlobDictionary(4) 2604.42 ±0.33% 1.61% slower
collections/hash-dict-vs-blob-dict_delete.ts[5] BlobDictionary(5) 2587.21 ±1.52% 2.26% slower
collections/hash-dict-vs-blob-dict_get.ts[0] StringHashDictionary 3004.9 ±0.35% 1.7% slower
collections/hash-dict-vs-blob-dict_get.ts[1] BlobDictionary(1) 3046.73 ±0.58% 0.33% slower
collections/hash-dict-vs-blob-dict_get.ts[2] BlobDictionary(2) 3028.41 ±0.38% 0.93% slower
collections/hash-dict-vs-blob-dict_get.ts[3] BlobDictionary(3) 3056.78 ±0.35% fastest ✅
collections/hash-dict-vs-blob-dict_get.ts[4] BlobDictionary(4) 3014.84 ±0.63% 1.37% slower
collections/hash-dict-vs-blob-dict_get.ts[5] BlobDictionary(5) 2923.56 ±2.22% 4.36% slower
collections/hash-dict-vs-blob-dict_set.ts[0] StringHashDictionary 2256.43 ±0.44% 0.18% slower
collections/hash-dict-vs-blob-dict_set.ts[1] BlobDictionary(1) 2260.39 ±0.37% fastest ✅
collections/hash-dict-vs-blob-dict_set.ts[2] BlobDictionary(2) 1472.44 ±68.74% 34.86% slower
collections/hash-dict-vs-blob-dict_set.ts[3] BlobDictionary(3) 1691.01 ±2.35% 25.19% slower
collections/hash-dict-vs-blob-dict_set.ts[4] BlobDictionary(4) 2186.2 ±1.34% 3.28% slower
collections/hash-dict-vs-blob-dict_set.ts[5] BlobDictionary(5) 2229.45 ±0.63% 1.37% slower
hash/blake2b.ts[0] our hasher 1.07 ±0.18% fastest ✅
hash/blake2b.ts[1] blake2b js 0.03 ±0.21% 97.2% slower
crypto/ed25519.ts[0] native crypto 5.703 ±1.09% fastest ✅
crypto/ed25519.ts[1] wasm lib 2.248 ±0.45% 60.58% slower
crypto/ed25519.ts[2] wasm lib batch 2.255 ±0.27% 60.46% slower

Benchmarks summary: 83/83 OK ✅

@tomusdrw tomusdrw added this pull request to the merge queue Mar 19, 2026
Merged via the queue into main with commit 6a657ad Mar 19, 2026
14 checks passed
@tomusdrw tomusdrw deleted the td-lazy-parser branch March 19, 2026 11:01
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