Conversation
fix errors delete 0.6.7
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe changes remove support for Gray Paper v0.6.7 and the JAMDUNA test suite, consolidating many previously version-gated codec and data paths into single, version-agnostic implementations. Numerous test-runner scripts and GitHub workflow files for 0.6.7 were deleted or updated to target 0.7.x and test-vector refs were updated. Multiple modules dropped Compatibility/GpVersion branches (affecting codecs, key derivation, serialized-state, and tests), several test fixtures and expected hashes were updated, and package/test-runner script entries were adjusted. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
Comment |
DrEverr
requested review from
mateuszsikora and
tomusdrw
and removed request for
mateuszsikora
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/jam/state-merkleization/keys.ts (1)
108-121: RemovelegacyServiceNestedas dead code.The function is not used anywhere in the codebase and can be safely deleted. After removal of 0.6.7 compatibility, this function became orphaned with no callers.
🧹 Nitpick comments (5)
bin/test-runner/w3f-070.ts (1)
1-27: w3f-070 runner wiring looks correct; consider merging CLIignoredpatterns if needed.The runner cleanly delegates to
mainwith the newtest-vectors/w3f-070root, enforces JSON patterns, and documents why specific files/dirs are ignored. The logging and error‑handling flow matches the other test-runner patterns and respects the current TypeScript guidelines (no unsafe casts, noCodecclasses, etc.).One optional tweak: if
parseArgscan also supplyignoredpatterns, your hard-codedignored: [...]will override them. If you want to allow callers to extend the ignore set instead of replacing it, you could merge them, e.g.ignored: [...(args.ignored ?? []), "..."].packages/jam/state-merkleization/serialized-state.ts (1)
3-4: Serialized state cleanup matches dropped legacy compatibilitySwitching
BytesBlob/U32to type-only imports and narrowing utils usage toTEST_COMPARE_USINGis a good dependency cleanup. SimplifyingSerializedService.getStorageto always useserialize.serviceStorage(this.blake2b, this.serviceId, rawKey)removes the obsolete legacy key-format path and is consistent with only supporting 0.7.x versions now.If any tests or docs still mention the old storage-key variant, consider pruning those in a follow-up to avoid confusion.
Also applies to: 8-9, 19-20, 211-214
packages/jam/jam-host-calls/accumulate/transfer.ts (1)
22-24: Host-call index constant is consistent with supported versionsGiven the prior compatibility logic already resolved to 20 for all versions ≥0.6.7, hard-coding
index = tryAsHostCallIndex(20);preserves behavior for the remaining supported 0.7.x versions and cleanly drops obsolete pre-0.6.7 handling.Consider adding a brief comment noting that earlier indices are intentionally unsupported now, to make this migration explicit to future readers.
packages/jam/state-merkleization/keys.ts (1)
51-58: Update graypaper links to current version.The links at lines 51 and 58 still reference
v=0.6.3. As per coding guidelines, when making changes to code with graypaper links, ensure they point to the current version. Consider updating these tov=0.7.1for consistency with the other updated links in this file.packages/jam/transition/accumulate/operand.ts (1)
12-13: Update graypaper link to current version.The link references
v=0.6.7but should be updated to the current version (e.g.,v=0.7.1) to match the removal of 0.6.7 compatibility. Same applies to the link at line 34.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.github/scripts/load-test-ref.sh(1 hunks).github/workflows/build-qa.yml(2 hunks).github/workflows/vectors-jamduna-067.yml(0 hunks).github/workflows/vectors-javajam.yml(0 hunks).github/workflows/vectors-w3f-davxy-067.yml(0 hunks).github/workflows/vectors-w3f.yml(3 hunks)README.md(0 hunks)bin/test-runner/jamduna-067.ts(0 hunks)bin/test-runner/package.json(1 hunks)bin/test-runner/w3f-070.ts(1 hunks)bin/test-runner/w3f-davxy-067.ts(0 hunks)bin/test-runner/w3f.ts(0 hunks)packages/core/utils/compatibility.test.ts(5 hunks)packages/core/utils/compatibility.ts(1 hunks)packages/jam/block/work-package.ts(2 hunks)packages/jam/jam-host-calls/accumulate/transfer.ts(1 hunks)packages/jam/state-merkleization/keys.ts(1 hunks)packages/jam/state-merkleization/serialized-state.ts(2 hunks)packages/jam/state-merkleization/state-entries.test.ts(1 hunks)packages/jam/state/statistics.test.ts(0 hunks)packages/jam/state/statistics.ts(1 hunks)packages/jam/state/test.utils.ts(1 hunks)packages/jam/transition/accumulate/operand.ts(2 hunks)packages/jam/transition/disputes/disputes.test.ts(2 hunks)
💤 Files with no reviewable changes (8)
- packages/jam/state/statistics.test.ts
- bin/test-runner/w3f-davxy-067.ts
- bin/test-runner/w3f.ts
- README.md
- .github/workflows/vectors-jamduna-067.yml
- .github/workflows/vectors-w3f-davxy-067.yml
- bin/test-runner/jamduna-067.ts
- .github/workflows/vectors-javajam.yml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
⚙️ CodeRabbit configuration file
**/*.ts: rules from./CODESTYLE.mdshould be adhered to.
**/*.ts: Any function whose documentation mention it must not be used in production code,
can be safely used in*.test.tsfiles. Other usage should be carefuly reviewed
and the comment must explain why it's safe to use there.
**/*.ts:asconversions must not be used. Suggest usingtryAsconversion methods.
**/*.ts: Classes withstatic Codecfield must have private constructor and staticcreatemethod.
**/*.ts: Casting abigint(orU64) usingNumber(x)must have an explanation comment why
it is safe.
**/*.ts: When making changes to code with comments containing links (in classes, constants, methods, etc.)
to graypaper.fluffylabs.dev, ensure those links point to the current version for this update.
Files:
packages/jam/transition/disputes/disputes.test.tspackages/jam/jam-host-calls/accumulate/transfer.tspackages/jam/state-merkleization/state-entries.test.tspackages/jam/state/test.utils.tspackages/jam/state/statistics.tspackages/jam/state-merkleization/keys.tspackages/core/utils/compatibility.tspackages/jam/transition/accumulate/operand.tspackages/jam/block/work-package.tsbin/test-runner/w3f-070.tspackages/jam/state-merkleization/serialized-state.tspackages/core/utils/compatibility.test.ts
packages/core/**/*.ts
⚙️ CodeRabbit configuration file
packages/core/**/*.ts: Core packages must not import any JAM-related packages
(i.e. packages defined underpackages/jam/**)
Files:
packages/core/utils/compatibility.tspackages/core/utils/compatibility.test.ts
🧠 Learnings (9)
📚 Learning: 2025-06-18T20:35:13.536Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `typeberry/utils` package has browser compatibility issues due to Node.js-specific code like `measure` function using `process.hrtime()` and `testUtils` importing `node:assert`, causing white screens in browser environments.
Applied to files:
packages/jam/transition/disputes/disputes.test.tspackages/jam/block/work-package.ts
📚 Learning: 2025-06-10T12:06:32.535Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/jam/database-lmdb/states.test.ts:158-160
Timestamp: 2025-06-10T12:06:32.535Z
Learning: In test code, tomusdrw prefers pragmatic solutions over strict type safety when listing all entries would be cumbersome. Using `Object.assign({}, state)` for state updates in tests is acceptable even if it compromises type safety, prioritizing test maintainability and readability.
Applied to files:
packages/jam/transition/disputes/disputes.test.tspackages/jam/state-merkleization/state-entries.test.tspackages/jam/state/test.utils.tspackages/jam/state-merkleization/serialized-state.ts
📚 Learning: 2025-06-10T12:20:17.513Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/jam/state/state-inmemory.ts:141-149
Timestamp: 2025-06-10T12:20:17.513Z
Learning: In the `InMemoryService.copyFrom` function in `packages/jam/state/state-inmemory.ts`, duplicate checking for `(hash, length)` pairs in the lookup history is not necessary because the function operates under the assumption that the input `ServiceEntries` comes from an existing well-formed state, which already maintains the invariant of unique lookup history entries per hash and length combination.
Applied to files:
packages/jam/transition/disputes/disputes.test.tspackages/jam/state-merkleization/state-entries.test.tspackages/jam/state/test.utils.tspackages/jam/state-merkleization/keys.tspackages/jam/block/work-package.tspackages/jam/state-merkleization/serialized-state.ts
📚 Learning: 2025-05-26T21:31:58.688Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 399
File: .github/workflows/vectors-jamduna.yml:78-78
Timestamp: 2025-05-26T21:31:58.688Z
Learning: In the typeberry project, the npm start script in the test-runner workspace is designed to accept test suite names as arguments (e.g., "jamduna", "w3f"). The command `npm start -w typeberry/test-runner jamduna` is the correct way to run the jamduna test suite, not `npm run jamduna`. This is an architectural design choice where the start script acts as a unified entry point for different test suites.
Applied to files:
.github/workflows/vectors-w3f.ymlbin/test-runner/package.jsonbin/test-runner/w3f-070.ts
📚 Learning: 2025-06-10T12:10:10.532Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/jam/state-merkleization/serialize-update.ts:115-126
Timestamp: 2025-06-10T12:10:10.532Z
Learning: In packages/jam/state-merkleization/serialize-update.ts, service removal is handled separately from service updates. The UpdateServiceKind enum does not include a Remove variant. Service removals are handled via the servicesRemoved parameter in serializeUpdate() which is processed by serializeRemovedServices(), while service updates/creations are handled via servicesUpdates parameter processed by serializeServiceUpdates().
Applied to files:
packages/jam/state/statistics.tspackages/jam/state-merkleization/keys.tspackages/jam/state-merkleization/serialized-state.ts
📚 Learning: 2025-06-10T12:04:56.072Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/jam/transition/accumulate/accumulate.ts:193-193
Timestamp: 2025-06-10T12:04:56.072Z
Learning: In the typeberry codebase, the Service.getPreimage() method was updated to return BytesBlob | null directly instead of PreimageItem | null, so the returned value is already a BytesBlob and doesn't need .blob property access.
Applied to files:
packages/jam/state-merkleization/keys.tspackages/jam/state-merkleization/serialized-state.ts
📚 Learning: 2025-11-03T10:12:27.580Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 757
File: packages/jam/state-vectors/index.ts:18-36
Timestamp: 2025-11-03T10:12:27.580Z
Learning: In TypeScript files, the coding guideline requiring classes with `static Codec` field to have a private constructor and static `create` method applies specifically to classes using `codec.Class`. When using `codec.object`, this pattern is not required because the code only cares about the shape, not the instance.
Applied to files:
packages/jam/transition/accumulate/operand.tspackages/jam/block/work-package.ts
📚 Learning: 2025-04-24T19:48:31.051Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 351
File: packages/jam/state-merkleization/index.ts:19-37
Timestamp: 2025-04-24T19:48:31.051Z
Learning: In the typeberry codebase, `Bytes` inherits from `BytesBlob`, so a `Bytes` instance can be directly used where a `BytesBlob` is expected without needing conversion.
Applied to files:
packages/jam/transition/accumulate/operand.ts
📚 Learning: 2025-06-18T20:35:13.536Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `measure` function in `typeberry/utils/debug.ts` attempts environment detection by checking `process === undefined` but still causes bundling issues because bundlers see the `process.hrtime` reference in the Node.js branch.
Applied to files:
packages/jam/block/work-package.ts
🧬 Code graph analysis (6)
packages/jam/transition/disputes/disputes.test.ts (2)
packages/jam/block/work-report.ts (1)
WorkReport(149-153)packages/jam/config/chain-spec.ts (1)
tinyChainSpec(109-126)
packages/jam/jam-host-calls/accumulate/transfer.ts (1)
packages/core/pvm-host-calls/host-call-handler.ts (1)
tryAsHostCallIndex(11-11)
packages/jam/transition/accumulate/operand.ts (3)
packages/jam/block/refine-context.ts (3)
WorkPackageHash(18-18)ExportsRootHash(21-21)AuthorizerHash(15-15)packages/jam/block/common.ts (1)
ServiceGas(31-31)packages/jam/block/work-result.ts (1)
WorkExecResult(28-69)
packages/jam/block/work-package.ts (4)
packages/jam/block/common.ts (1)
ServiceId(26-26)packages/jam/block/refine-context.ts (1)
RefineContext(53-90)packages/jam/block/work-item.ts (1)
WorkItem(112-190)packages/core/collections/sized-array.ts (1)
FixedSizeArray(18-54)
bin/test-runner/w3f-070.ts (1)
bin/test-runner/w3f/runners.ts (1)
runners(54-78)
packages/core/utils/compatibility.test.ts (1)
packages/core/utils/compatibility.ts (1)
Compatibility(54-113)
🔇 Additional comments (12)
.github/workflows/vectors-w3f.yml (1)
1-1: Workflow wiring for w3f-0.7.0 looks consistent; please verify thenpm startarg & dist filename.The version bump, runner invocation, and result/artifact paths all line up with the new
w3f-070.tstest-runner andGP_VERSION=0.7.0. One thing to double‑check: per prior design, thestartscript used to accept logical suite names (e.g.jamduna,w3f) rather than TS filenames. Passingw3f-070.tshere is fine as long asindex.ts(the start entrypoint) now expects that form and emits./dist/w3f-070.txtas configured. Based on learnings, this is the only potential mismatch area to re‑confirm.Also applies to: 14-14, 42-42, 45-45, 51-51
.github/scripts/load-test-ref.sh (1)
2-2: Updated test-vectors REF is isolated and safe.Only the
REFconstant changed; the script behavior is otherwise identical, so this is a straightforward point to the new test-vectors commit.If you haven’t already, it’s worth confirming that this hash in
fluffylabs/test-vectorsindeed contains the neww3f-070vectors you expect.bin/test-runner/package.json (1)
37-44: Neww3f:0.7.0script matches the new runner and versioning scheme.The script correctly wires
GP_VERSION=0.7.0tow3f-070.tsand is consistent with the existingw3f-davxyandjam-conformanceentries, so this looks good..github/workflows/build-qa.yml (1)
5-29: Workflow changes correctly target 0.7.x onlyBranch filters and the explicit
GP_VERSION=0.7.{0,1,2} npm teststeps align with dropping 0.6.7 while still exercising all supported versions. No issues spotted.packages/jam/state/test.utils.ts (1)
399-405: TEST_STATE_ROOT version mapping looks consistent with new version setUsing
selectIfGreaterOrEqualwith a fallback hash for 0.7.0 and a single override forGpVersion.V0_7_1(also used by 0.7.2) matches the updated supported-version set and howCompatibilityworks. Assuming the new fallback was taken from the 0.7.0 vectors, this change looks correct.packages/jam/state-merkleization/state-entries.test.ts (1)
59-66: Updated expected root matches new compatibility semanticsThe new fallback hash and single
GpVersion.V0_7_1entry mirror howTEST_STATE_ROOTis structured (0.7.0 via fallback, 0.7.1+ via override). This keeps the “update state then check root” test aligned with the reduced version set.packages/jam/state/statistics.ts (1)
15-20: ServiceId codec simplification aligns with removing JAMDUNA/0.6.7
codecServiceIdnow only distinguishes the W3F_DAVXY suite (plainu32opaque) from the defaultvarU32encoding, which matches the new supported-version matrix. ThesortKeys: (a, b) => a - bcomparator remains valid for both representations since they are numeric at runtime.Also applies to: 275-277
packages/core/utils/compatibility.test.ts (1)
31-38: Tests accurately cover new 0.7.0–0.7.2 ordering and selection semanticsThe updated assertions for
isGreaterOrEqual,isLessThan, andselectIfGreaterOrEqualcorrectly encode the intended ordering [V0_7_0, V0_7_1, V0_7_2] and how the helper should pick fallback/low/mid/high values. This provides good regression coverage for the trimmed version set.Also applies to: 39-46, 49-60, 63-75, 78-90, 93-104, 107-113
packages/jam/transition/disputes/disputes.test.ts (1)
13-13: Disputes tests now version-agnostic and aligned with new work-report codecImporting only
resultToStringand asserting against fixed work-report hashes removes brittle version-branching in the test while matching the updatedWorkReport.Codecbehavior. Assuming the new hash literals were derived from the 0.7.x vectors, this looks correct.Also applies to: 87-91
packages/core/utils/compatibility.ts (1)
3-14: LGTM!The removal of
GpVersion.V0_6_7andTestSuite.JAMDUNAis consistent with the PR objectives. The updatedALL_VERSIONS_IN_ORDERarray correctly reflects the supported versions.packages/jam/block/work-package.ts (2)
38-48: LGTM!The simplified single
Codecdefinition without version branching is consistent with the PR objectives and follows the class pattern withstatic Codec, private constructor, andstatic createmethod.
6-6: WorkItem.Codec contains dead code due to minimum version requirement.Since V0_7_0 is now the minimum supported version, the
Compatibility.isGreaterOrEqual(GpVersion.V0_7_0)check at line 113 will always evaluate to true. Remove the else branch (the codec definition for pre-0.7.0 format) to simplify the code.
mateuszsikora
left a comment
mateuszsikora
left a comment
There was a problem hiding this comment.
try to look for Compatibility.isLessThan(GpVersion.V0_7_0) in the project - there are a few more places to update
# Conflicts: # .github/workflows/build-qa.yml # .github/workflows/vectors-jamduna-067.yml # .github/workflows/vectors-javajam.yml # .github/workflows/vectors-w3f-davxy-067.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/jam/jamnp-s/protocol/ce-133-work-package-submission.test.ts (1)
50-51: Document the origin/meaning of thetestWorkPackagehex blobThe large inline hex fixture is fine for a test, but it’s opaque. Consider adding a short comment indicating its origin (e.g., which W3F/Gray Paper vector and version it comes from) so it’s easier to regenerate or update when vectors change.
packages/jam/transition/disputes/disputes.test.ts (1)
77-80: Ensure the skipped test is tracked for follow-up.The NOTE explains that test data needs to be obtained from W3F test vectors or regenerated. This is reasonable for now, but consider opening an issue to track re-enabling this test to avoid it being forgotten.
Do you want me to open an issue to track re-enabling this test once the 0.7.0 test data is available?
packages/jam/block/work-report.ts (1)
102-124: Constructor isprotectedbut coding guidelines requireprivate.Per coding guidelines: "Classes with
static Codecfield must have private constructor and staticcreatemethod." The constructor here isprotected. If there are no subclasses requiring this visibility, consider changing toprivatefor consistency.- protected constructor( + private constructor(packages/jam/block/header.test.ts (1)
32-39: Canonical header encoding fixtures look goodThe test now uses a single canonical encoding for both full and unsealed headers, matching the unified
Header.CodecandencodeUnsealedHeaderpath. If these hex fixtures need frequent updates as the spec evolves, consider adding a short comment or helper indicating how they were generated (e.g. script or upstream vector) to simplify future refreshes.packages/jam/block/header.ts (1)
94-105: Update encodeUnsealedHeader Gray Paper link to current version
encodeUnsealedHeaderstill points at av=0.6.4Gray Paper URL in its JSDoc, while the surroundingHeaderdocumentation has been updated tov=0.7.2. To keep docs aligned with the spec version this PR targets (and with the new unified codec), it would be good to update this URL to the appropriate 0.7.x section as well. Likewise, please double‑check theValidatorKeyslink earlier in the file so all header‑related links consistently reference the current Gray Paper version.As per coding guidelines, graypaper links in touched code should track the current spec version.
packages/jam/state/test.utils.ts (3)
21-44: Use of Compatibility/GpVersion here is fine as long as this stays test‑onlyImporting
CompatibilityandGpVersionin this helper is appropriate given it’s consumed by JAM state tests and not by production entrypoints. Just keep in mind the past bundling issues around@typeberry/utilsand ensurepackages/jam/state/test.utils.tsis not pulled into browser‑facing bundles.Based on learnings,
@typeberry/utilsshould stay out of browser paths.
392-405: Availability assignment and state root updates appear self‑consistent
TEST_AVAILABILITY_ASSIGNMENT, the correspondingc10entry inTEST_STATE, andTEST_STATE_ROOT’s fallback were all updated together, which should keep the snapshot, trie contents, and root hash in sync for the new vectors. Since these are hand‑maintained hex blobs, consider adding a brief comment pointing to the exact upstream snapshot / tooling used to regenerate them to reduce the chance of future mismatches.Also applies to: 481-484
496-505: STATISTICS blob duplication between test.utils and statistics.testThe
c13value now usesCompatibility.selectIfGreaterOrEqualwith the same fallback/V0_7_1 STATISTICS blobs as instatistics.test.ts. That keeps the state snapshot compatible with the updatedServiceStatistics.Codec, but it does duplicate a very long literal in two places. If these vectors change again, it might be worth centralizing the STATISTICS hex string in a single exported constant to avoid accidental divergence between the state snapshot and the statistics unit test.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/vectors-w3f-070.yml(3 hunks)packages/jam/block/header.test.ts(1 hunks)packages/jam/block/header.ts(2 hunks)packages/jam/block/test-block-0-6-5.ts(0 hunks)packages/jam/block/test-helpers.ts(2 hunks)packages/jam/block/test-work-report-0-6-5.ts(0 hunks)packages/jam/block/work-item.ts(2 hunks)packages/jam/block/work-report.ts(5 hunks)packages/jam/jamnp-s/protocol/ce-133-work-package-submission.test.ts(2 hunks)packages/jam/safrole/safrole-seal-067.test.ts(0 hunks)packages/jam/safrole/safrole-seal.test.ts(1 hunks)packages/jam/state/statistics.test.ts(1 hunks)packages/jam/state/statistics.ts(5 hunks)packages/jam/state/test.utils.ts(4 hunks)packages/jam/transition/accumulate/operand.ts(1 hunks)packages/jam/transition/block-verifier.test.ts(3 hunks)packages/jam/transition/disputes/disputes.test.ts(3 hunks)packages/jam/transition/hasher.test.ts(2 hunks)packages/jam/transition/reports/verify-credentials.test.ts(2 hunks)
💤 Files with no reviewable changes (3)
- packages/jam/block/test-work-report-0-6-5.ts
- packages/jam/block/test-block-0-6-5.ts
- packages/jam/safrole/safrole-seal-067.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jam/transition/accumulate/operand.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
⚙️ CodeRabbit configuration file
**/*.ts: rules from./CODESTYLE.mdshould be adhered to.
**/*.ts: Any function whose documentation mention it must not be used in production code,
can be safely used in*.test.tsfiles. Other usage should be carefuly reviewed
and the comment must explain why it's safe to use there.
**/*.ts:asconversions must not be used. Suggest usingtryAsconversion methods.
**/*.ts: Classes withstatic Codecfield must have private constructor and staticcreatemethod.
**/*.ts: Casting abigint(orU64) usingNumber(x)must have an explanation comment why
it is safe.
**/*.ts: When making changes to code with comments containing links (in classes, constants, methods, etc.)
to graypaper.fluffylabs.dev, ensure those links point to the current version for this update.
Files:
packages/jam/transition/block-verifier.test.tspackages/jam/transition/hasher.test.tspackages/jam/transition/disputes/disputes.test.tspackages/jam/transition/reports/verify-credentials.test.tspackages/jam/state/statistics.test.tspackages/jam/safrole/safrole-seal.test.tspackages/jam/block/header.test.tspackages/jam/block/test-helpers.tspackages/jam/block/work-report.tspackages/jam/block/header.tspackages/jam/block/work-item.tspackages/jam/state/statistics.tspackages/jam/state/test.utils.tspackages/jam/jamnp-s/protocol/ce-133-work-package-submission.test.ts
🧠 Learnings (8)
📚 Learning: 2025-06-10T12:20:17.513Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/jam/state/state-inmemory.ts:141-149
Timestamp: 2025-06-10T12:20:17.513Z
Learning: In the `InMemoryService.copyFrom` function in `packages/jam/state/state-inmemory.ts`, duplicate checking for `(hash, length)` pairs in the lookup history is not necessary because the function operates under the assumption that the input `ServiceEntries` comes from an existing well-formed state, which already maintains the invariant of unique lookup history entries per hash and length combination.
Applied to files:
packages/jam/transition/hasher.test.ts
📚 Learning: 2025-06-18T20:35:13.536Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `typeberry/utils` package has browser compatibility issues due to Node.js-specific code like `measure` function using `process.hrtime()` and `testUtils` importing `node:assert`, causing white screens in browser environments.
Applied to files:
packages/jam/transition/disputes/disputes.test.tspackages/jam/block/work-item.tspackages/jam/state/test.utils.ts
📚 Learning: 2025-06-10T12:06:32.535Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/jam/database-lmdb/states.test.ts:158-160
Timestamp: 2025-06-10T12:06:32.535Z
Learning: In test code, tomusdrw prefers pragmatic solutions over strict type safety when listing all entries would be cumbersome. Using `Object.assign({}, state)` for state updates in tests is acceptable even if it compromises type safety, prioritizing test maintainability and readability.
Applied to files:
packages/jam/transition/disputes/disputes.test.tspackages/jam/state/test.utils.ts
📚 Learning: 2025-05-26T21:31:58.688Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 399
File: .github/workflows/vectors-jamduna.yml:78-78
Timestamp: 2025-05-26T21:31:58.688Z
Learning: In the typeberry project, the npm start script in the test-runner workspace is designed to accept test suite names as arguments (e.g., "jamduna", "w3f"). The command `npm start -w typeberry/test-runner jamduna` is the correct way to run the jamduna test suite, not `npm run jamduna`. This is an architectural design choice where the start script acts as a unified entry point for different test suites.
Applied to files:
.github/workflows/vectors-w3f-070.yml
📚 Learning: 2025-04-24T19:48:31.051Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 351
File: packages/jam/state-merkleization/index.ts:19-37
Timestamp: 2025-04-24T19:48:31.051Z
Learning: In the typeberry codebase, `Bytes` inherits from `BytesBlob`, so a `Bytes` instance can be directly used where a `BytesBlob` is expected without needing conversion.
Applied to files:
packages/jam/block/test-helpers.ts
📚 Learning: 2025-11-03T10:12:27.580Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 757
File: packages/jam/state-vectors/index.ts:18-36
Timestamp: 2025-11-03T10:12:27.580Z
Learning: In TypeScript files, the coding guideline requiring classes with `static Codec` field to have a private constructor and static `create` method applies specifically to classes using `codec.Class`. When using `codec.object`, this pattern is not required because the code only cares about the shape, not the instance.
Applied to files:
packages/jam/block/header.tspackages/jam/state/statistics.ts
📚 Learning: 2025-06-18T20:35:13.536Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `measure` function in `typeberry/utils/debug.ts` attempts environment detection by checking `process === undefined` but still causes bundling issues because bundlers see the `process.hrtime` reference in the Node.js branch.
Applied to files:
packages/jam/block/work-item.ts
📚 Learning: 2025-06-10T12:10:10.532Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/jam/state-merkleization/serialize-update.ts:115-126
Timestamp: 2025-06-10T12:10:10.532Z
Learning: In packages/jam/state-merkleization/serialize-update.ts, service removal is handled separately from service updates. The UpdateServiceKind enum does not include a Remove variant. Service removals are handled via the servicesRemoved parameter in serializeUpdate() which is processed by serializeRemovedServices(), while service updates/creations are handled via servicesUpdates parameter processed by serializeServiceUpdates().
Applied to files:
packages/jam/state/statistics.ts
🧬 Code graph analysis (4)
packages/jam/transition/block-verifier.test.ts (1)
packages/core/utils/result.ts (2)
Result(4-4)Result(81-111)
packages/jam/block/header.ts (4)
packages/jam/block/common.ts (3)
StateRootHash(43-43)TimeSlot(16-16)ValidatorIndex(21-21)packages/core/crypto/bandersnatch.ts (3)
BANDERSNATCH_VRF_SIGNATURE_BYTES(10-10)BANDERSNATCH_VRF_SIGNATURE_BYTES(11-11)BandersnatchVrfSignature(44-44)packages/core/crypto/index.ts (4)
BANDERSNATCH_VRF_SIGNATURE_BYTES(15-15)BandersnatchVrfSignature(6-6)ED25519_KEY_BYTES(20-20)Ed25519Key(18-18)packages/core/crypto/ed25519.ts (3)
ED25519_KEY_BYTES(11-11)ED25519_KEY_BYTES(12-12)Ed25519Key(23-23)
packages/jam/state/statistics.ts (1)
packages/jam/block/common.ts (1)
ServiceId(26-26)
packages/jam/state/test.utils.ts (1)
packages/core/utils/compatibility.ts (1)
Compatibility(54-113)
🔇 Additional comments (17)
packages/jam/jamnp-s/protocol/ce-133-work-package-submission.test.ts (1)
10-10: OK import source looks correctUsing
OKfrom@typeberry/utilsas the return token forwithNewStreamaligns with the existing call site and keeps the test logic unchanged. No issues here..github/workflows/vectors-w3f-070.yml (1)
1-51: Changes align with PR objectives and are internally consistent.The workflow correctly transitions from 0.6.7 to 0.7.0, with consistent updates across the workflow name (line 1), environment variable (line 14), test runner target (line 42), and artifact paths (lines 45, 51). The filename
vectors-w3f-070.ymlappropriately uses the compact version notation. Thew3f-070.tstest runner target exists and is properly configured to accept the invocationnpm start -w @typeberry/test-runner -- w3f-070.ts, which correctly resolves the suite file and generates the expected output path./dist/w3f-070.txt.packages/jam/safrole/safrole-seal.test.ts (2)
1-24: LGTM!The imports and setup are appropriate for this test file.
125-225: LGTM!The test data constants are properly structured using appropriate type-safe conversion methods (
tryAsPerValidator,tryAsPerEpochBlock) and creation functions.packages/jam/transition/hasher.test.ts (1)
25-25: LGTM!The removal of
CompatibilityandGpVersionimports and the simplification to a fixed expected hash aligns with the PR objective of removing 0.6.7 compatibility. The test now uses a single, version-agnostic expected value.Also applies to: 153-156
packages/jam/transition/reports/verify-credentials.test.ts (1)
9-9: LGTM!The import cleanup and fixed message blob are consistent with the version unification effort across the codebase. The test now asserts against a deterministic expected value.
Also applies to: 224-226
packages/jam/transition/block-verifier.test.ts (1)
17-17: LGTM!The removal of version-conditional logic and use of fixed expected values for error messages and result hashes is consistent with the PR's version unification objective.
Also applies to: 128-132, 196-197
packages/jam/block/test-helpers.ts (1)
8-10: LGTM!Good simplification. The test helpers now return data directly without version-conditional fallbacks, consistent with the codebase-wide removal of 0.6.5/0.6.7 variant handling.
Also applies to: 20-22
packages/jam/transition/disputes/disputes.test.ts (1)
91-93: Hash expectations updated for 0.7.0.The hardcoded expected hashes are appropriate given the test is currently skipped. When the test is re-enabled with proper test data, these assertions will validate the new codec behavior.
packages/jam/block/work-report.ts (2)
57-78: LGTM on the unified Codec!The consolidation from version-conditional codecs to a single
WorkReport.Codecis clean and aligns with the PR's objective. The codec properly handles core index validation with appropriate bounds checking.
55-55: The graypaper link version is already current. The code correctly usesv=0.7.2, which is the current version as of November 2025. No changes needed.packages/jam/block/header.ts (1)
110-124: Unified Header.Codec matches 0.7.x layoutThe new
Header.Codecdefinition is consistent with the documented field layout (parent hash, prior state root, extrinsic hash, timeslot, optional markers, author index, entropy, offenders, seal), and withencodeUnsealedHeaderassuming the seal is the final fixed‑size field. No correctness or compatibility concerns from this change.packages/jam/state/statistics.test.ts (1)
5-20: Version‑aware STATISTICS blob wiring looks consistentUsing
Compatibility.selectIfGreaterOrEqualhere with a fallback blob and a V0_7_1 override aligns with theServiceStatistics.Codecsetup and keeps the test valid across supported Gray Paper versions while remaining simple (decodewith no error). This also effectively replaces the removed older test case.packages/jam/state/statistics.ts (2)
72-90: CoreStatistics single Codec matches 0.7.2 specThe unified
CoreStatistics.Codec(usingvarU32/codecVarU16/codecVarGas) and the updated Gray Paper URL are consistent with the 0.7.2 layout. Using field names increate()and the constructor avoids any dependency on descriptor order, so this change is safe and keeps thecodec.Classpattern intact.
135-170: ServiceStatistics version gating and services map look correctThe
ServiceStatistics.Codecsetup keeps 0.7.0 support via the fallback descriptor and switches to the 0.7.1+ descriptor (withignoreValueWithDefaultfor deprecatedonTransfers*fields) whenCompatibilityreports ≥ V0_7_1. This matches the intended evolution of the statistics schema. UpdatingStatisticsData.Codec.servicesto usecodec.u32.asOpaque<ServiceId>()withsortKeys: (a, b) => a - bis consistent withServiceIdbeing an opaqueU32and preserves deterministic ordering.Also applies to: 236-244
packages/jam/block/work-item.ts (2)
1-7: Import cleanup improves runtime dependency surfaceDropping
Compatibility/GpVersionfrom this runtime module and keeping onlyOpaque/WithDebugfrom@typeberry/utilsreduces coupling to the compatibility helpers and is friendlier for browser bundling, while preserving the existing debug/type utilities.Based on learnings about
@typeberry/utilsin browser builds, this is a nice simplification.
107-126: Unified WorkItem.Codec matches 0.7.2 spec and class shapeThe new
WorkItem.Codecdefinition (service id, code hash, gas limits, export count, payload, boundedimportSegments, and var‑len extrinsics) lines up with the updated Gray Paper link and the constructor field comments. ThecodecKnownSizeArraybounds mirrorMAX_NUMBER_OF_SEGMENTS, and thecodec.Classpattern (private ctor +create) is correctly preserved, so encoding/decoding semantics stay clear and version‑agnostic.Also applies to: 150-172
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/utils/compatibility.ts (1)
13-13: Minor: Fix grammar in comment.The comment should read "the supported versions" rather than "a supported versions".
-// NOTE: Also acts as a supported versions +// NOTE: Also acts as the list of supported versions
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/vectors-w3f-070.yml(3 hunks)bin/test-runner/package.json(1 hunks)packages/core/utils/compatibility.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/core/**/*.ts
⚙️ CodeRabbit configuration file
packages/core/**/*.ts: Core packages must not import any JAM-related packages
(i.e. packages defined underpackages/jam/**)
Files:
packages/core/utils/compatibility.ts
**/*.ts
⚙️ CodeRabbit configuration file
**/*.ts: rules from./CODESTYLE.mdshould be adhered to.
**/*.ts: Any function whose documentation mention it must not be used in production code,
can be safely used in*.test.tsfiles. Other usage should be carefuly reviewed
and the comment must explain why it's safe to use there.
**/*.ts:asconversions must not be used. Suggest usingtryAsconversion methods.
**/*.ts: Classes withstatic Codecfield must have private constructor and staticcreatemethod.
**/*.ts: Casting abigint(orU64) usingNumber(x)must have an explanation comment why
it is safe.
**/*.ts: When making changes to code with comments containing links (in classes, constants, methods, etc.)
to graypaper.fluffylabs.dev, ensure those links point to the current version for this update.
Files:
packages/core/utils/compatibility.ts
🧠 Learnings (1)
📚 Learning: 2025-05-26T21:31:58.688Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 399
File: .github/workflows/vectors-jamduna.yml:78-78
Timestamp: 2025-05-26T21:31:58.688Z
Learning: In the typeberry project, the npm start script in the test-runner workspace is designed to accept test suite names as arguments (e.g., "jamduna", "w3f"). The command `npm start -w typeberry/test-runner jamduna` is the correct way to run the jamduna test suite, not `npm run jamduna`. This is an architectural design choice where the start script acts as a unified entry point for different test suites.
Applied to files:
.github/workflows/vectors-w3f-070.ymlbin/test-runner/package.json
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: e2e (22.x)
- GitHub Check: run (22.x)
- GitHub Check: run (22.x)
- GitHub Check: benchmarks (22.x)
- GitHub Check: run (22.x)
🔇 Additional comments (6)
bin/test-runner/package.json (1)
38-38: LGTM! Script correctly updated to target 0.7.0 vectors.The script properly sets the
GP_VERSIONenvironment variable and targets the neww3f-070.tsrunner, aligning with the PR objective to update W3F vectors to 0.7.0.packages/core/utils/compatibility.ts (2)
3-11: LGTM! Enum updates align with version consolidation.The removal of
V0_6_7fromGpVersionandJAMDUNAfromTestSuitecorrectly implements the PR objective to drop 0.6.7 compatibility and consolidate to 0.7.x versions only.
14-14: LGTM! Version array correctly updated.The
ALL_VERSIONS_IN_ORDERarray now correctly reflects only the 0.7.x versions, and the usage in error messages (lines 32, 78) and version comparison methods remains consistent..github/workflows/vectors-w3f-070.yml (3)
1-1: Workflow name and file naming aligned to 0.7.0.The workflow name has been updated from "w3f-0.6.7" to "w3f-0.7.0", and the filename appears to reflect the version update as well, addressing the past review feedback.
15-15: GP_VERSION updated consistently to 0.7.0.The environment variable has been correctly updated to align with the PR objective to remove 0.6.7 compatibility.
43-43: No issues found with npm start command format and test runner script.The command
npm start -w @typeberry/test-runner -- w3f-070.tscorrectly invokes the test runner:
- The w3f-070.ts script exists at bin/test-runner/w3f-070.ts and properly imports from common.js and w3f/runners.js
- The npm start script (defined as
tsx --test-timeout=900000 ./index.tsin bin/test-runner/package.json) accepts the test suite filename as an argument via process.argv[2]- The output file will be generated at ./dist/w3f-070.txt as expected (index.ts removes the .ts extension and writes to the dist directory)
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
View all
Benchmarks summary: 83/83 OK ✅ |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
3 participants
Uh oh!
There was an error while loading. Please reload this page.