Skip to content

Conversation

@DrEverr
Copy link
Member

@DrEverr DrEverr commented Oct 26, 2025

Resolves #715

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 26, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for Gray Paper version 0.7.2 (stable release).
  • Bug Fixes

    • Removed support for Gray Paper 0.6.7 and 0.7.2-preview to streamline version compatibility.
  • Chores

    • Updated default supported version to 0.7.2.
    • Simplified internal version-compatibility logic by removing conditional branching.
    • Updated CI workflows and test vectors to align with current version support.
    • Updated compliance checklist documentation.

Walkthrough

This PR upgrades Gray Paper version support from 0.6.7/0.7.2-preview to stable 0.7.2. It removes the V0_6_7 enum variant, updates DEFAULT_VERSION to GpVersion.V0_7_2, and eliminates version-conditional codec logic across block encoding, work packages/reports, and state serialization. Legacy test data files (test-block-0-6-5.ts, safrole-seal-067.test.ts) are deleted, CI workflows for 0.6.7 are removed, and version-gated branches in tests are replaced with fixed expectations. The changes consolidate previously branching encoding paths into single, unified codec definitions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Codec field ordering verification: Ensure field reordering in work-item.ts, work-package.ts, and work-report.ts is consistent and correctly unified
  • Version enum and DEFAULT_VERSION impacts: Verify all references to removed V0_6_7 are cleaned up and DEFAULT_VERSION propagates correctly
  • Test expectation updates: Validate updated hash values, state roots, and expected results align with 0.7.2 specification
  • Workflow completeness: Confirm all 0.6.7-specific and preview workflows are fully removed without orphaned references
  • Header codec unification: Verify header.ts and header.test.ts codec simplification maintains correct field semantics

Possibly related PRs

Suggested reviewers

  • tomusdrw
  • skoszuta
  • mateuszsikora

Poem

🐰✨ Hops through old code with a gleeful bound,
Sweeping away what's 0.6.7-bound!
Codecs unified, simpler and bright,
0.7.2 shines—the future looks right! 🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive The pull request contains no description provided by the author. While the check is intentionally lenient and only requires the description to not be off-topic, the absence of any description means there is no information to evaluate against the criterion that the description should be "related in some way to the changeset." An empty description neither clearly demonstrates relevance to the changes nor is unambiguously misleading, creating an indeterminate state. Consider adding a brief description explaining the rationale for deprecating version 0.6.7, the transition to 0.7.2 as the default version, and the scope of changes (including workflow removals, codec simplifications, and cleanup of legacy compatibility logic). This will provide reviewers with valuable context for understanding the full scope and implications of the changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Delete GP v0.6.7 from compatibility support" accurately and clearly summarizes the primary change across the changeset. The deletion of version 0.6.7 support is pervasive throughout the codebase, affecting many files including the compatibility enum, test files, workflow configurations, and various codec and schema definitions. While the PR also involves updating the default version to 0.7.2 (changing 0.7.2-preview to stable 0.7.2), the predominant change by scope is the removal of the legacy 0.6.7 version branch and its associated compatibility logic. The title is specific, concise, and directly communicates the main modification without unnecessary details.

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
packages/jam/jam-host-calls/accumulate/transfer.ts (2)

34-36: Remove version-conditional logic for basicGasCost.

This version check is inconsistent with the PR's objective to consolidate to 0.7.2 and eliminate version-gated logic. Since the codebase now targets 0.7.2 exclusively, this should use the simplified 0.7.2 behavior.

Apply this diff to remove the version check:

-  basicGasCost = Compatibility.isGreaterOrEqual(GpVersion.V0_7_2)
-    ? tryAsSmallGas(10)
-    : (regs: IHostCallRegisters) => tryAsGas(10n + regs.get(TRANSFER_GAS_FEE_REG));
+  basicGasCost = tryAsSmallGas(10);

69-75: Remove version-conditional logic in execute method.

This version check is inconsistent with the PR's objective to consolidate to 0.7.2 and eliminate version-gated branches. Since the codebase now targets 0.7.2 exclusively, the gas subtraction logic should always execute.

Apply this diff to remove the version check:

     // All good!
     if (transferResult.isOk) {
-      if (Compatibility.isGreaterOrEqual(GpVersion.V0_7_2)) {
-        // substracting value `t`
-        const underflow = gas.sub(tryAsGas(transferGasFee));
-        if (underflow) {
-          return PvmExecution.OOG;
-        }
+      // subtracting value `t`
+      const underflow = gas.sub(tryAsGas(transferGasFee));
+      if (underflow) {
+        return PvmExecution.OOG;
       }
       regs.set(IN_OUT_REG, HostCallResult.OK);
       return;
packages/jam/block/header.ts (2)

24-24: Update documentation link to current Gray Paper version.

The link references v=0.6.4, but this PR upgrades to version 0.7.2. Please update the version parameter in the URL to reflect the current version.

As per coding guidelines.

-* https://graypaper.fluffylabs.dev/#/68eaa1f/0e34030e3603?v=0.6.4
+* https://graypaper.fluffylabs.dev/#/68eaa1f/0e34030e3603?v=0.7.2

97-97: Update documentation link to current Gray Paper version.

The link references v=0.6.4, but this PR upgrades to version 0.7.2. Please update the version parameter in the URL to reflect the current version.

As per coding guidelines.

-* https://graypaper.fluffylabs.dev/#/68eaa1f/370202370302?v=0.6.4
+* https://graypaper.fluffylabs.dev/#/68eaa1f/370202370302?v=0.7.2
packages/jam/block/work-package.ts (1)

13-13: Update documentation link to current Gray Paper version.

The link references v=0.6.4, but this PR upgrades to version 0.7.2. Please update the version parameter in the URL to reflect the current version.

As per coding guidelines.

-/** Constrained by I=16 https://graypaper.fluffylabs.dev/#/68eaa1f/417a00417a00?v=0.6.4 */
+/** Constrained by I=16 https://graypaper.fluffylabs.dev/#/68eaa1f/417a00417a00?v=0.7.2 */
🧹 Nitpick comments (9)
packages/jam/state/test.utils.ts (3)

392-393: Consolidation looks good; add provenance/check to avoid drift.

Great to pin TEST_AVAILABILITY_ASSIGNMENT to a single blob. Please add a short comment with the source (snapshot, block/hash) and optionally a quick decode/round‑trip sanity check in a nearby test to guard against accidental edits.


400-404: Mapping keyed at V0_7_1 while targeting 0.7.2.

selectIfGreaterOrEqual will use the V0_7_1 entry for V0_7_2, which is fine. For clarity and future maintenance, consider adding an explicit [GpVersion.V0_7_2] entry (even if identical) or confirm that the 0.7.1 root is indeed unchanged in 0.7.2.


482-485: Ensure c10 payload stays in sync with TEST_AVAILABILITY_ASSIGNMENT.

The updated c10 hex starts with 0x01…; verify it equals AvailabilityAssignment.Codec.encode(decoded(TEST_AVAILABILITY_ASSIGNMENT)) for 0.7.2. To prevent future divergence, consider generating this value programmatically in a test and asserting equality to this fixture.

packages/jam/transition/disputes/disputes.test.ts (1)

130-135: Address the commented test logic.

The test logic for verifying cleared availability assignments is commented out with a TODO note. This reduces test coverage for the disputes transition.

Should this test logic be:

  1. Re-enabled with updated test data for 0.7.2?
  2. Moved to a separate issue/PR?
  3. Removed entirely if no longer applicable?

Would you like me to help regenerate the test data or open an issue to track this?

packages/jam/block/work-item.ts (1)

120-124: Avoid overshooting size hints for importSegments

Setting typicalLength to MAX_NUMBER_OF_SEGMENTS likely overestimates encoded size and can degrade pre-allocation heuristics. Prefer a realistic typical (e.g., 0–16) or omit if not needed.

-    importSegments: codecKnownSizeArray(ImportSpec.Codec, {
-      minLength: 0,
-      maxLength: MAX_NUMBER_OF_SEGMENTS,
-      typicalLength: MAX_NUMBER_OF_SEGMENTS,
-    }),
+    importSegments: codecKnownSizeArray(ImportSpec.Codec, {
+      minLength: 0,
+      maxLength: MAX_NUMBER_OF_SEGMENTS,
+      typicalLength: 16, // or remove if not representative
+    }),

Also, ensure any graypaper.fluffylabs.dev links in this file target 0.7.2 or omit version pins if “latest” is intended. As per coding guidelines.

packages/jam/state/statistics.test.ts (2)

9-11: Skip rationale OK; consider pruning dead path

Since 0.6.7 is dropped, either remove this case or keep a clear TODO with target (issue/PR) to re-enable with 0.7.2 vectors. The current skip is fine.


16-16: Redundant suite override in a skipped test

Compatibility.overrideSuite(TestSuite.JAMDUNA) is inert under it.skip. Safe to remove or leave with a comment if you plan to revive this case.

packages/jam/block/work-report.ts (1)

21-21: Update graypaper links to 0.7.2

Links pinned to 0.6.x: please retarget to 0.7.2 (or drop the v= to follow “latest”) to match this upgrade.

- * https://graypaper.fluffylabs.dev/#/5f542d7/130e01130e01?v=0.6.2
+ * https://graypaper.fluffylabs.dev/#/5f542d7/130e01130e01?v=0.7.2
...
- * https://graypaper.fluffylabs.dev/#/cc517d7/131c01132401?v=0.6.5
+ * https://graypaper.fluffylabs.dev/#/cc517d7/131c01132401?v=0.7.2

As per coding guidelines.

Also applies to: 55-56

packages/jam/state/statistics.ts (1)

84-86: Refresh graypaper links to 0.7.2

Links pinned to 0.6.4 and 0.7.1 should target 0.7.2 (or remove version pin if “latest” is preferred) to match this PR’s scope. As per coding guidelines.

- * https://graypaper.fluffylabs.dev/#/68eaa1f/18f10318f103?v=0.6.4
+ * https://graypaper.fluffylabs.dev/#/68eaa1f/18f10318f103?v=0.7.2
...
- * https://graypaper.fluffylabs.dev/#/1c979cb/199802199802?v=0.7.1
+ * https://graypaper.fluffylabs.dev/#/1c979cb/199802199802?v=0.7.2

Also applies to: 143-144

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5746fdc and 05598fc.

📒 Files selected for processing (30)
  • .github/workflows/build-qa.yml (1 hunks)
  • .github/workflows/vectors-jamduna-067.yml (0 hunks)
  • .github/workflows/vectors-w3f-davxy-067.yml (0 hunks)
  • .github/workflows/vectors-w3f.yml (0 hunks)
  • README.md (1 hunks)
  • packages/core/utils/compatibility.test.ts (5 hunks)
  • packages/core/utils/compatibility.ts (2 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-package.ts (2 hunks)
  • packages/jam/block/work-report.ts (2 hunks)
  • packages/jam/jam-host-calls/accumulate/transfer.ts (1 hunks)
  • packages/jam/jam-host-calls/fetch.ts (1 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-merkleization/keys.ts (2 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 (1 hunks)
  • packages/jam/state/statistics.ts (3 hunks)
  • packages/jam/state/test.utils.ts (2 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 (6)
  • .github/workflows/vectors-jamduna-067.yml
  • .github/workflows/vectors-w3f-davxy-067.yml
  • packages/jam/safrole/safrole-seal-067.test.ts
  • packages/jam/block/test-work-report-0-6-5.ts
  • .github/workflows/vectors-w3f.yml
  • packages/jam/block/test-block-0-6-5.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts

⚙️ CodeRabbit configuration file

**/*.ts: rules from ./CODESTYLE.md should be adhered to.

**/*.ts: Any function whose documentation mention it must not be used in production code,
can be safely used in *.test.ts files. Other usage should be carefuly reviewed
and the comment must explain why it's safe to use there.

**/*.ts: as conversions must not be used. Suggest using tryAs conversion methods.

**/*.ts: Classes with static Codec field must have private constructor and static create method.

**/*.ts: Casting a bigint (or U64) using Number(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/jam-host-calls/fetch.ts
  • packages/jam/block/header.test.ts
  • packages/jam/state/statistics.test.ts
  • packages/jam/jam-host-calls/accumulate/transfer.ts
  • packages/jam/safrole/safrole-seal.test.ts
  • packages/jam/block/test-helpers.ts
  • packages/jam/state-merkleization/serialized-state.ts
  • packages/jam/block/header.ts
  • packages/jam/transition/disputes/disputes.test.ts
  • packages/jam/transition/reports/verify-credentials.test.ts
  • packages/jam/block/work-item.ts
  • packages/jam/jamnp-s/protocol/ce-133-work-package-submission.test.ts
  • packages/core/utils/compatibility.test.ts
  • packages/jam/state-merkleization/keys.ts
  • packages/jam/transition/hasher.test.ts
  • packages/jam/state-merkleization/state-entries.test.ts
  • packages/jam/block/work-package.ts
  • packages/jam/state/test.utils.ts
  • packages/jam/state/statistics.ts
  • packages/jam/block/work-report.ts
  • packages/core/utils/compatibility.ts
  • packages/jam/transition/block-verifier.test.ts
packages/core/**/*.ts

⚙️ CodeRabbit configuration file

packages/core/**/*.ts: Core packages must not import any JAM-related packages
(i.e. packages defined under packages/jam/**)

Files:

  • packages/core/utils/compatibility.test.ts
  • packages/core/utils/compatibility.ts
🧬 Code graph analysis (12)
packages/jam/state/statistics.test.ts (3)
packages/jam/config/chain-spec.ts (1)
  • tinyChainSpec (106-122)
packages/core/codec/decoder.ts (1)
  • Decoder (12-379)
packages/core/bytes/bytes.ts (1)
  • BytesBlob (18-171)
packages/jam/jam-host-calls/accumulate/transfer.ts (1)
packages/core/pvm-host-calls/host-call-handler.ts (1)
  • tryAsHostCallIndex (11-11)
packages/jam/safrole/safrole-seal.test.ts (10)
packages/jam/block/header.ts (1)
  • Header (112-173)
packages/core/bytes/bytes.ts (1)
  • Bytes (176-242)
packages/core/hash/hash.ts (2)
  • HASH_SIZE (10-10)
  • HASH_SIZE (11-11)
packages/jam/block/common.ts (4)
  • tryAsTimeSlot (18-18)
  • tryAsValidatorIndex (23-23)
  • tryAsPerValidator (56-62)
  • tryAsPerEpochBlock (72-78)
packages/core/crypto/bandersnatch.ts (6)
  • BANDERSNATCH_VRF_SIGNATURE_BYTES (10-10)
  • BANDERSNATCH_VRF_SIGNATURE_BYTES (11-11)
  • BANDERSNATCH_KEY_BYTES (6-6)
  • BANDERSNATCH_KEY_BYTES (7-7)
  • BLS_KEY_BYTES (22-22)
  • BLS_KEY_BYTES (23-23)
packages/jam/config/chain-spec.ts (1)
  • tinyChainSpec (106-122)
packages/jam/block/utils.ts (1)
  • reencodeAsView (10-13)
packages/jam/safrole/safrole-seal.ts (1)
  • SafroleSeal (32-165)
packages/jam/state/validator-data.ts (3)
  • ValidatorData (27-51)
  • VALIDATOR_META_BYTES (19-19)
  • VALIDATOR_META_BYTES (20-20)
packages/jam/state/safrole-data.ts (1)
  • SafroleSealingKeysData (30-91)
packages/jam/block/header.ts (3)
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/ed25519.ts (3)
  • ED25519_KEY_BYTES (11-11)
  • ED25519_KEY_BYTES (12-12)
  • Ed25519Key (23-23)
packages/jam/transition/disputes/disputes.test.ts (3)
packages/core/codec/encoder.ts (1)
  • Encoder (73-480)
packages/jam/block/work-report.ts (1)
  • WorkReport (105-127)
packages/jam/config/chain-spec.ts (1)
  • tinyChainSpec (106-122)
packages/jam/block/work-item.ts (3)
packages/jam/block/common.ts (2)
  • ServiceId (26-26)
  • ServiceGas (31-31)
packages/jam/block/codec.ts (1)
  • codecKnownSizeArray (47-57)
packages/jam/block/work-item-segment.ts (1)
  • MAX_NUMBER_OF_SEGMENTS (12-12)
packages/core/utils/compatibility.test.ts (1)
packages/core/utils/compatibility.ts (1)
  • Compatibility (47-106)
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-175)
packages/core/collections/sized-array.ts (1)
  • FixedSizeArray (18-54)
packages/jam/state/test.utils.ts (1)
packages/core/utils/compatibility.ts (1)
  • Compatibility (47-106)
packages/jam/state/statistics.ts (4)
packages/core/codec/descriptor.ts (1)
  • Descriptor (67-144)
packages/jam/block/common.ts (2)
  • ServiceId (26-26)
  • tryAsServiceId (28-28)
packages/core/utils/compatibility.ts (1)
  • Compatibility (47-106)
packages/core/numbers/index.ts (1)
  • tryAsU32 (48-51)
packages/jam/block/work-report.ts (6)
packages/jam/block/refine-context.ts (2)
  • RefineContext (53-90)
  • AuthorizerHash (15-15)
packages/core/numbers/index.ts (2)
  • tryAsU32 (48-51)
  • isU16 (45-45)
packages/jam/block/common.ts (2)
  • tryAsCoreIndex (37-37)
  • ServiceGas (31-31)
packages/jam/block/work-result.ts (1)
  • WorkResult (118-163)
packages/core/collections/sized-array.ts (1)
  • FixedSizeArray (18-54)
packages/jam/block/work-package.ts (1)
  • tryAsWorkItemsCount (17-23)
packages/jam/transition/block-verifier.test.ts (1)
packages/core/utils/result.ts (2)
  • Result (4-4)
  • Result (81-111)
🪛 GitHub Actions: Build - Lint & Test
packages/core/utils/compatibility.test.ts

[error] 52-52: File content differs from formatting output. ESLint/Prettier formatting mismatch detected during 'npm run qa'.

🪛 GitHub Actions: VECTORS - javajam
packages/core/utils/compatibility.ts

[error] 28-28: ReferenceError: Cannot access 'ALL_VERSIONS_IN_ORDER' before initialization in parseCurrentVersion.

🔇 Additional comments (34)
packages/jam/jam-host-calls/accumulate/transfer.ts (1)

23-23: LGTM! Static host-call index aligns with PR objectives.

The change from dynamic version-dependent index resolution to a constant value correctly simplifies the code by removing version-conditional logic.

packages/jam/transition/hasher.test.ts (2)

25-25: LGTM: Clean removal of unused imports.

The removal of Compatibility and GpVersion imports aligns with the elimination of version-gated test logic, leaving only the imports that are actually used in the test.


154-157: Verify the expected hash value is correct for version 0.7.2.

The test now uses a single hard-coded expected hash instead of version-conditional logic. Please confirm that the test suite passes with this new expected value to ensure the hash is correct for the consolidated 0.7.2 codec implementation.

packages/jam/jamnp-s/protocol/ce-133-work-package-submission.test.ts (2)

10-10: LGTM - Clean import cleanup.

Removing the unused Compatibility and GpVersion imports after eliminating version-guarded logic is appropriate.


50-51: LGTM - Version consolidation achieved.

Replacing the version-guarded logic with a fixed hex constant for 0.7.2 correctly implements the PR objective to consolidate version support. The test is now simpler and eliminates runtime branching.

packages/jam/safrole/safrole-seal.test.ts (2)

1-24: LGTM!

The imports and test setup are appropriate for the Safrole seal verification tests.


125-225: LGTM! Well-structured test data constants.

The test data constants are properly structured:

  • TEST_VALIDATOR_KEYS correctly uses tryAsPerValidator with 6 validators matching tinyChainSpec
  • TEST_VALIDATOR_DATA appropriately extends keys with zero BLS keys and metadata (acceptable for seal verification testing)
  • SEALING_KEYS correctly uses tryAsPerEpochBlock with 12 keys matching the epoch length

The data is properly typed and parsed with appropriate opaque type conversions.

packages/jam/block/test-helpers.ts (2)

8-10: Changes are structurally correct but require manual verification of test data version.

The simplification successfully removes version-gating logic as intended by the PR. However, the test data files (test-block.ts and test-work-report.ts) contain binary hex strings without version information, making it impossible to verify they represent 0.7.2 codec data through static analysis alone.

Key findings:

  • All version-gating conditionals removed from testBlockHex() and testWorkReportHex()
  • Legacy version-related imports removed
  • No version-gating logic detected in consumer files
  • Test data files unchanged (still valid exports)

Action required: Verify that test-block.ts and test-work-report.ts represent 0.7.2 codec data by checking the PR description, commit history, or the w3f spec reference mentioned in the file comment.


20-22: Verify that ./test-work-report.ts contains 0.7.2 test data.

The file exists and the simplification correctly mirrors testBlockHex(). However, the exported hex string lacks explicit version documentation. Confirm that this test data corresponds to version 0.7.2 by cross-referencing with the w3f codec test files or project documentation.

packages/jam/state-merkleization/serialized-state.ts (3)

3-3: LGTM! Type-only import is appropriate.

BytesBlob is only used in type annotations throughout the file, making the type-only import correct and helping reduce bundle size.


8-8: LGTM! Type-only import is appropriate.

U32 is only used in the type annotation for the len parameter (line 232), making the type-only import correct.


19-19: Import cleanup verified — no issues found.

The removed imports (GpVersion, Compatibility, safeAllocUint8Array, u32AsLeBytes) are not referenced anywhere in the file. TEST_COMPARE_USING is correctly retained (line 19) and used (line 63). The simplified getStorage implementation directly uses serialize.serviceStorage without version branching, and all callers throughout the codebase work correctly with this change.

packages/jam/state-merkleization/state-entries.test.ts (1)

59-64: Confirm test passes with updated hash before merging.

The fallback hash change is logically sound: the new fallback (0xcf33ddfb0987283f7614652d7eb4d3509e5efd93466a4b28ab4865cc912a66e1) is the same value that was previously mapped to V0_7_0, now promoted to cover all versions prior to V0_7_1. Removing V0_6_7 and V0_7_0 entries aligns with the PR objective to consolidate to 0.7.2 support.

However, the npm test command failed in the sandbox (tsx unavailable). Please run the test locally to verify the updated expected hash produces the correct state root:

npm test -- --grep "should update the state"
packages/jam/transition/reports/verify-credentials.test.ts (2)

9-9: LGTM: Import simplification aligns with version consolidation.

Removing the Compatibility and GpVersion imports is appropriate as the tests no longer require version-conditional logic after consolidating to version 0.7.2.


225-225: Message format and construction logic are correct; hash value requires test verification.

The hardcoded message on line 225 follows the correct format:

  • Prefix verified: 0x6a616d5f67756172616e746565 correctly encodes "jam_guarantee"
  • Structure verified: Constructed via BytesBlob.blobFromParts(JAM_GUARANTEE, hash.raw) per the signingPayload() function (verify-credentials.ts:121-123)
  • Gray Paper 0.7.2 alignment: Matches specification reference at https://graypaper.fluffylabs.dev/#/5f542d7/155200155200

The hash value (suffix) depends on the test work report generated from newWorkReport({ core: 0 }) via an external package. Verify the test passes during CI to confirm this hash is correctly computed from the test data.

README.md (1)

16-17: LGTM!

The checklist updates accurately reflect the completion of 0.7.1 and 0.7.2 support.

.github/workflows/build-qa.yml (1)

29-29: LGTM!

The CI workflow correctly adds 0.7.2 testing, replacing the removed 0.6.7 and 0.7.2-preview test runs.

packages/jam/jam-host-calls/fetch.ts (1)

91-95: LGTM! Documentation updated to current version.

The annotation and Gray Paper link have been correctly updated to reflect version 0.7.2 specification.

As per coding guidelines

packages/jam/state-merkleization/keys.ts (2)

7-7: LGTM! Import cleanup aligned with version consolidation.

The removal of Compatibility and GpVersion imports is consistent with the removal of version-gated logic throughout the codebase.


40-47: LGTM! Enum expanded correctly.

The addition of Theta and adjustment of Delta align with the 0.7.2 specification.

packages/core/utils/compatibility.test.ts (1)

47-82: LGTM! Test expectations updated correctly.

The test assertions now use the updated version set (V0_7_0, V0_7_1, V0_7_2), which aligns with the enum changes in compatibility.ts.

packages/jam/transition/block-verifier.test.ts (3)

17-17: LGTM! Unnecessary imports removed.

The removal of Compatibility and GpVersion imports is consistent with eliminating version-gated test logic.


128-132: LGTM! Test expectations now deterministic.

Replacing version-conditional error messages with fixed expectations improves test clarity and maintainability.


196-196: LGTM! Fixed expected hash for 0.7.2.

The test now expects a deterministic header hash value appropriate for the 0.7.2 version.

packages/jam/transition/disputes/disputes.test.ts (2)

13-13: LGTM! Unnecessary imports removed.

Consistent with other test files, version-gating imports are no longer needed.


88-90: LGTM! Hash assertions now deterministic.

The fixed hash values replace version-conditional assertions, making tests more predictable.

packages/core/utils/compatibility.ts (3)

1-5: LGTM! Version enum updated correctly.

The removal of V0_6_7 and promotion of V0_7_2 from preview to stable aligns with the PR objectives.


13-13: LGTM! Default version updated.

Setting DEFAULT_VERSION to V0_7_2 is correct for this upgrade.


34-45: LGTM! Suite parsing updated correctly.

The parseCurrentSuite function correctly validates against the TestSuite enum values.

packages/jam/state/statistics.ts (2)

15-20: Suite-gated ServiceId codec: OK

W3F_DAVXY → u32; otherwise varU32 with conversions. Looks consistent with current suites.


154-176: Rewritten Review Comment:

Field order in fallback and V0_7_1 codecs is identical—there is no flip between versions. Verify that ServiceStatistics codec consumers handle the consistent layout correctly and add tests explicitly decoding both codec paths.

Likely an incorrect or invalid review comment.

packages/jam/block/header.test.ts (1)

34-35: Static expectations post‑unification verified

Verification confirms no version gating remains. Test expectations at lines 34-35 and 38-39 are properly set as static hex strings for sealed and unsealed header encodings without any conditional logic or runtime version branching.

packages/jam/block/header.ts (1)

113-124: LGTM: Codec simplification is clean.

The removal of version-conditional logic in favor of a single, explicit descriptor is a positive change that simplifies the codebase and improves maintainability.

packages/jam/block/work-package.ts (1)

38-48: LGTM: Version-agnostic codec is an improvement.

The consolidation from version-conditional codec variants to a single, unified descriptor simplifies the codebase and eliminates potential version-related encoding inconsistencies.

Comment on lines +113 to +126
static Codec = codec.Class(WorkItem, {
service: codec.u32.asOpaque<ServiceId>(),
codeHash: codec.bytes(HASH_SIZE).asOpaque<CodeHash>(),
refineGasLimit: codec.u64.asOpaque<ServiceGas>(),
accumulateGasLimit: codec.u64.asOpaque<ServiceGas>(),
exportCount: codec.u16,
payload: codec.blob,
importSegments: codecKnownSizeArray(ImportSpec.Codec, {
minLength: 0,
maxLength: MAX_NUMBER_OF_SEGMENTS,
typicalLength: MAX_NUMBER_OF_SEGMENTS,
}),
extrinsic: codec.sequenceVarLen(WorkItemExtrinsicSpec.Codec),
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Unified WorkItem.Codec LGTM; please confirm spec order

Field set matches the intended 0.7.2 shape (notably exportCount before payload). Please confirm against GP 0.7.2 to avoid a silent wire-compat drift.


🏁 Script executed:

#!/bin/bash
# Inspect WorkItem field ordering and ensure no leftover version gating in block codecs
rg -nP 'WorkItem.*Codec|V0_6_7|V0_7_0|Compatibility\.(is|isGreaterOrEqual)\(' packages/jam/block -C2

Length of output: 1196



---

🏁 Script executed:

```shell
#!/bin/bash
# Check for documentation links and comments in work-item.ts that may need version updates
rg -nP 'greypa|graypaper|http|https|spec|0\.7' packages/jam/block/work-item.ts -C3

Length of output: 1227



---

🏁 Script executed:

```shell
#!/bin/bash
# Search for all graypaper links in codebase to identify version pattern and correct hash for 0.7.2
rg -nP 'graypaper\.fluffylabs\.dev.*#' packages/jam -A1 -B1 | head -100

Length of output: 7278



---

🏁 Script executed:

```shell
#!/bin/bash
# Search for 0.7.2 graypaper links to identify correct version hash
rg -nP 'graypaper\.fluffylabs\.dev.*0\.7\.2' packages/jam

Length of output: 3506



---

🏁 Script executed:

```shell
#!/bin/bash
# Get all graypaper links in work-item.ts to identify all that need updating
cat packages/jam/block/work-item.ts | grep -nP 'graypaper|http'

Length of output: 128


Update graypaper links to GP 0.7.2 in work-item.ts

The WorkItem.Codec structure is unified and correct. However, two graypaper documentation links in this file must be updated to point to GP 0.7.2:

  • Line 31 (WorkItemExportedSegment.treeRoot): Change hash from 579bd12 to ab2cdbd
  • Line 110 (WorkItem class): Change hash from 579bd12 to ab2cdbd

Current links reference v=0.5.4; they must reflect the target version (0.7.2) per coding guidelines.

🤖 Prompt for AI Agents
In packages/jam/block/work-item.ts around lines 31 and 110 (and the codec block
shown at 113–126 for context), update the graypaper links that currently point
to v=0.5.4 with hash 579bd12: change the version to v=0.7.2 and replace the hash
579bd12 with ab2cdbd in both places (WorkItemExportedSegment.treeRoot at ~line
31 and the WorkItem class reference at ~line 110) so both links consistently
reference GP 0.7.2 with hash ab2cdbd.

Comment on lines 105 to +126
export class WorkReport extends WorkReportNoCodec {
static Codec: typeof WorkReportCodec = Compatibility.isGreaterOrEqual(GpVersion.V0_7_0)
? WorkReportCodec
: WorkReportCodecPre070;
static Codec = codec.Class(WorkReportNoCodec, {
workPackageSpec: WorkPackageSpec.Codec,
context: RefineContext.Codec,
coreIndex: codec.varU32.convert(
(o) => tryAsU32(o),
(i) => {
if (!isU16(i)) {
throw new Error(`Core index exceeds U16: ${i}`);
}
return tryAsCoreIndex(i);
},
),
authorizerHash: codec.bytes(HASH_SIZE).asOpaque<AuthorizerHash>(),
authorizationGasUsed: codec.varU64.asOpaque<ServiceGas>(),
authorizationOutput: codec.blob,
segmentRootLookup: readonlyArray(codec.sequenceVarLen(WorkPackageInfo.Codec)),
results: codec.sequenceVarLen(WorkResult.Codec).convert(
(x) => x,
(items) => FixedSizeArray.new(items, tryAsWorkItemsCount(items.length)),
),
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Bind Codec to WorkReport and align with ctor/create pattern

WorkReport.Codec = codec.Class(WorkReportNoCodec, …) instantiates the base type, not WorkReport, and violates our pattern (private constructor + static create on the class that holds Codec).

Apply:

-  static Codec = codec.Class(WorkReportNoCodec, {
+  static Codec = codec.Class(WorkReport, {
     workPackageSpec: WorkPackageSpec.Codec,
     context: RefineContext.Codec,
     coreIndex: codec.varU32.convert(
       (o) => tryAsU32(o),
       (i) => {
         if (!isU16(i)) {
           throw new Error(`Core index exceeds U16: ${i}`);
         }
         return tryAsCoreIndex(i);
       },
     ),
     authorizerHash: codec.bytes(HASH_SIZE).asOpaque<AuthorizerHash>(),
     authorizationGasUsed: codec.varU64.asOpaque<ServiceGas>(),
     authorizationOutput: codec.blob,
     segmentRootLookup: readonlyArray(codec.sequenceVarLen(WorkPackageInfo.Codec)),
     results: codec.sequenceVarLen(WorkResult.Codec).convert(
       (x) => x,
       (items) => FixedSizeArray.new(items, tryAsWorkItemsCount(items.length)),
     ),
   });

Then add inside class WorkReport:

// Ensure consumers get WorkReport instances
static create(v: CodecRecord<WorkReport>) {
  return new WorkReport(
    v.workPackageSpec,
    v.context,
    v.coreIndex,
    v.authorizerHash,
    v.authorizationOutput,
    v.segmentRootLookup,
    v.results,
    v.authorizationGasUsed,
  );
}

private constructor(
  workPackageSpec: WorkPackageSpec,
  context: RefineContext,
  coreIndex: CoreIndex,
  authorizerHash: AuthorizerHash,
  authorizationOutput: BytesBlob,
  segmentRootLookup: readonly WorkPackageInfo[],
  results: FixedSizeArray<WorkResult, WorkItemsCount>,
  authorizationGasUsed: ServiceGas,
) {
  super(
    workPackageSpec,
    context,
    coreIndex,
    authorizerHash,
    authorizationOutput,
    segmentRootLookup,
    results,
    authorizationGasUsed,
  );
}

As per coding guidelines.


🏁 Script executed:

#!/bin/bash
# Find usages that rely on WorkReport identity
rg -nP 'instanceof\s+WorkReport|WorkReport\.create|codec\.Class\(WorkReportNoCodec' -C2

Length of output: 5390



---

🏁 Script executed:

```shell
#!/bin/bash
# Inspect the actual WorkReport class definition
cat packages/jam/block/work-report.ts | head -150

Length of output: 4444


🏁 Script executed:

#!/bin/bash
# Also check if WorkReportNoCodec has a create method or constructor
rg -n 'class WorkReportNoCodec|static create|private constructor' packages/jam/block/work-report.ts -A5

Length of output: 690



---

🏁 Script executed:

```shell
#!/bin/bash
# Check if any code explicitly types/expects WorkReportNoCodec
rg -n 'WorkReportNoCodec' packages/jam -g '!*.ts' --type=ts -C2 | head -80

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check if Codec is used elsewhere and how
rg -n 'WorkReport\.Codec' packages/jam --type=ts -C2

Length of output: 7085



---

🏁 Script executed:

```shell
#!/bin/bash
# Check graypaper links in work-report.ts
rg -n 'graypaper\.fluffylabs\.dev' packages/jam/block/work-report.ts

Length of output: 205



---

🏁 Script executed:

```shell
#!/bin/bash
# Check what graypaper versions are used elsewhere in the codebase
rg 'graypaper\.fluffylabs\.dev.*\?v=' packages/jam --type=ts | cut -d'=' -f2 | sort | uniq -c | sort -rn

Length of output: 471


Bind Codec to WorkReport, add factory pattern, and update graypaper links

The current WorkReport.Codec = codec.Class(WorkReportNoCodec, …) violates the guideline: classes with static Codec must have private constructor and static create method. The Codec produces WorkReportNoCodec instances, not WorkReport, causing all 12+ inherited create() calls to return the base type instead of the derived type.

Essential refactor: Bind Codec to WorkReport, add private constructor and static create override:

-  static Codec = codec.Class(WorkReportNoCodec, {
+  static Codec = codec.Class(WorkReport, {
     workPackageSpec: WorkPackageSpec.Codec,
     context: RefineContext.Codec,
     coreIndex: codec.varU32.convert(
       (o) => tryAsU32(o),
       (i) => {
         if (!isU16(i)) {
           throw new Error(`Core index exceeds U16: ${i}`);
         }
         return tryAsCoreIndex(i);
       },
     ),
     authorizerHash: codec.bytes(HASH_SIZE).asOpaque<AuthorizerHash>(),
     authorizationGasUsed: codec.varU64.asOpaque<ServiceGas>(),
     authorizationOutput: codec.blob,
     segmentRootLookup: readonlyArray(codec.sequenceVarLen(WorkPackageInfo.Codec)),
     results: codec.sequenceVarLen(WorkResult.Codec).convert(
       (x) => x,
       (items) => FixedSizeArray.new(items, tryAsWorkItemsCount(items.length)),
     ),
   });
+
+  static create(v: CodecRecord<WorkReport>) {
+    return new WorkReport(
+      v.workPackageSpec,
+      v.context,
+      v.coreIndex,
+      v.authorizerHash,
+      v.authorizationOutput,
+      v.segmentRootLookup,
+      v.results,
+      v.authorizationGasUsed,
+    );
+  }
+
+  private constructor(
+    workPackageSpec: WorkPackageSpec,
+    context: RefineContext,
+    coreIndex: CoreIndex,
+    authorizerHash: AuthorizerHash,
+    authorizationOutput: BytesBlob,
+    segmentRootLookup: readonly WorkPackageInfo[],
+    results: FixedSizeArray<WorkResult, WorkItemsCount>,
+    authorizationGasUsed: ServiceGas,
+  ) {
+    super(
+      workPackageSpec,
+      context,
+      coreIndex,
+      authorizerHash,
+      authorizationOutput,
+      segmentRootLookup,
+      results,
+      authorizationGasUsed,
+    );
+  }

Update graypaper links to current version (0.7.2):

  • Line 21: change ?v=0.6.2 to ?v=0.7.2
  • Line 55: change ?v=0.6.5 to ?v=0.7.2
  • Line 93: add ?v=0.7.2 to the segment-root lookup link

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +26 to 122
it("should verify a valid fallback mode seal and entropySource", async () => {
// based on test-vectors/w3f-davxy_070/traces/fallback/00000002.json
const header = Header.create({
parentHeaderHash: Bytes.parseBytes(
"0x74ad675f8d6480a17b6ec0178962ea0166053c384689044c6f4cd38c97c2776d",
HASH_SIZE,
).asOpaque(),
priorStateRoot: Bytes.parseBytes(
"0x4542b8bd55b25f52767e37c1c72004fefdd068878084e9c87c3ab0dc38543173",
HASH_SIZE,
).asOpaque(),
extrinsicHash: Bytes.parseBytes(
"0x189d15af832dfe4f67744008b62c334b569fcbb4c261e0f065655697306ca252",
HASH_SIZE,
).asOpaque(),
timeSlotIndex: tryAsTimeSlot(2),
epochMarker: null,
ticketsMarker: null,
offendersMarker: [],
bandersnatchBlockAuthorIndex: tryAsValidatorIndex(3),
entropySource: Bytes.parseBytes(
"0x21237c35f11cd849a27ffa62e4aeb1c9a06bca2e42b89e16f93932d773b4ed5e7df1d7c48986eeb1313462aec31668dbfa6d3e499b457c678320ce0bb0fb611be3b6b240e1cd757e624d50cb1a163ca5c6348f97b782f5db74f8877eae593a0d",
BANDERSNATCH_VRF_SIGNATURE_BYTES,
).asOpaque(),
seal: Bytes.parseBytes(
"0x732cef37ec4d9f100aca7445a486afc3fa1015056e3377905168e7b88d40286e68d943e77c0e5f5539c40416cd494b50aeb227ba55701d64e5586c790aebc60c1eba819c07c1b6f8fbca0d7765caaa61e494271c925df7ee42e6a19b0d3d2313",
BANDERSNATCH_VRF_SIGNATURE_BYTES,
).asOpaque(),
});
const spec = tinyChainSpec;
const headerView = reencodeAsView(Header.Codec, header, spec);

const safroleSeal = new SafroleSeal(bandersnatch);
const result = await safroleSeal.verifyHeaderSeal(headerView, {
currentValidatorData: TEST_VALIDATOR_DATA,
sealingKeySeries: SEALING_KEYS,
currentEntropy: Bytes.parseBytes(
"0x405c80c1f6a2d5a0f8dbc56996f04230221100d9500244648f02a795d7850eac",
HASH_SIZE,
).asOpaque(),
});
const safroleSeal = new SafroleSeal(bandersnatch);
const result = await safroleSeal.verifyHeaderSeal(headerView, {
currentValidatorData: TEST_VALIDATOR_DATA,
sealingKeySeries: SEALING_KEYS,
currentEntropy: Bytes.parseBytes(
"0x405c80c1f6a2d5a0f8dbc56996f04230221100d9500244648f02a795d7850eac",
HASH_SIZE,
).asOpaque(),
});

assert.deepStrictEqual(result, {
isError: false,
isOk: true,
ok: Bytes.parseBytes(
"0xc13af3d0cbdb7174590f34518e3beb05708935ceaee242e7ba11a94ca87bd007",
HASH_SIZE,
).asOpaque(),
});
assert.deepStrictEqual(result, {
isError: false,
isOk: true,
ok: Bytes.parseBytes("0xc13af3d0cbdb7174590f34518e3beb05708935ceaee242e7ba11a94ca87bd007", HASH_SIZE).asOpaque(),
});
});

it("should verify a valid ticket seal and entropySource", async () => {
// based on test-vectors/w3f-davxy_070/traces/safrole/00000002.json
const header = Header.create({
parentHeaderHash: Bytes.parseBytes(
"0x74ad675f8d6480a17b6ec0178962ea0166053c384689044c6f4cd38c97c2776d",
HASH_SIZE,
).asOpaque(),
priorStateRoot: Bytes.parseBytes(
"0x4542b8bd55b25f52767e37c1c72004fefdd068878084e9c87c3ab0dc38543173",
HASH_SIZE,
).asOpaque(),
extrinsicHash: Bytes.parseBytes(
"0x189d15af832dfe4f67744008b62c334b569fcbb4c261e0f065655697306ca252",
HASH_SIZE,
).asOpaque(),
timeSlotIndex: tryAsTimeSlot(2),
epochMarker: null,
ticketsMarker: null,
offendersMarker: [],
bandersnatchBlockAuthorIndex: tryAsValidatorIndex(3),
entropySource: Bytes.parseBytes(
"0x21237c35f11cd849a27ffa62e4aeb1c9a06bca2e42b89e16f93932d773b4ed5e7df1d7c48986eeb1313462aec31668dbfa6d3e499b457c678320ce0bb0fb611be3b6b240e1cd757e624d50cb1a163ca5c6348f97b782f5db74f8877eae593a0d",
BANDERSNATCH_VRF_SIGNATURE_BYTES,
).asOpaque(),
seal: Bytes.parseBytes(
"0x732cef37ec4d9f100aca7445a486afc3fa1015056e3377905168e7b88d40286e68d943e77c0e5f5539c40416cd494b50aeb227ba55701d64e5586c790aebc60c1eba819c07c1b6f8fbca0d7765caaa61e494271c925df7ee42e6a19b0d3d2313",
BANDERSNATCH_VRF_SIGNATURE_BYTES,
).asOpaque(),
});
const spec = tinyChainSpec;
const headerView = reencodeAsView(Header.Codec, header, spec);
it("should verify a valid ticket seal and entropySource", async () => {
// based on test-vectors/w3f-davxy_070/traces/safrole/00000002.json
const header = Header.create({
parentHeaderHash: Bytes.parseBytes(
"0x74ad675f8d6480a17b6ec0178962ea0166053c384689044c6f4cd38c97c2776d",
HASH_SIZE,
).asOpaque(),
priorStateRoot: Bytes.parseBytes(
"0x4542b8bd55b25f52767e37c1c72004fefdd068878084e9c87c3ab0dc38543173",
HASH_SIZE,
).asOpaque(),
extrinsicHash: Bytes.parseBytes(
"0x189d15af832dfe4f67744008b62c334b569fcbb4c261e0f065655697306ca252",
HASH_SIZE,
).asOpaque(),
timeSlotIndex: tryAsTimeSlot(2),
epochMarker: null,
ticketsMarker: null,
offendersMarker: [],
bandersnatchBlockAuthorIndex: tryAsValidatorIndex(3),
entropySource: Bytes.parseBytes(
"0x21237c35f11cd849a27ffa62e4aeb1c9a06bca2e42b89e16f93932d773b4ed5e7df1d7c48986eeb1313462aec31668dbfa6d3e499b457c678320ce0bb0fb611be3b6b240e1cd757e624d50cb1a163ca5c6348f97b782f5db74f8877eae593a0d",
BANDERSNATCH_VRF_SIGNATURE_BYTES,
).asOpaque(),
seal: Bytes.parseBytes(
"0x732cef37ec4d9f100aca7445a486afc3fa1015056e3377905168e7b88d40286e68d943e77c0e5f5539c40416cd494b50aeb227ba55701d64e5586c790aebc60c1eba819c07c1b6f8fbca0d7765caaa61e494271c925df7ee42e6a19b0d3d2313",
BANDERSNATCH_VRF_SIGNATURE_BYTES,
).asOpaque(),
});
const spec = tinyChainSpec;
const headerView = reencodeAsView(Header.Codec, header, spec);

const safroleSeal = new SafroleSeal(bandersnatch);
const result = await safroleSeal.verifyHeaderSeal(headerView, {
currentValidatorData: TEST_VALIDATOR_DATA,
sealingKeySeries: SEALING_KEYS,
currentEntropy: Bytes.parseBytes(
"0x405c80c1f6a2d5a0f8dbc56996f04230221100d9500244648f02a795d7850eac",
HASH_SIZE,
).asOpaque(),
});
const safroleSeal = new SafroleSeal(bandersnatch);
const result = await safroleSeal.verifyHeaderSeal(headerView, {
currentValidatorData: TEST_VALIDATOR_DATA,
sealingKeySeries: SEALING_KEYS,
currentEntropy: Bytes.parseBytes(
"0x405c80c1f6a2d5a0f8dbc56996f04230221100d9500244648f02a795d7850eac",
HASH_SIZE,
).asOpaque(),
});

assert.deepStrictEqual(result, {
isError: false,
isOk: true,
ok: Bytes.parseBytes(
"0xc13af3d0cbdb7174590f34518e3beb05708935ceaee242e7ba11a94ca87bd007",
HASH_SIZE,
).asOpaque(),
});
assert.deepStrictEqual(result, {
isError: false,
isOk: true,
ok: Bytes.parseBytes("0xc13af3d0cbdb7174590f34518e3beb05708935ceaee242e7ba11a94ca87bd007", HASH_SIZE).asOpaque(),
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Duplicate test with misleading description - missing ticket mode coverage.

Both test cases are completely identical in implementation and use the same SEALING_KEYS constant, which is created via SafroleSealingKeysData.keys() (line 171). According to the SafroleSealingKeysData implementation, .keys() creates fallback mode (kind = SafroleSealingKeysKind.Keys), not ticket mode.

This means:

  1. Both tests are verifying fallback mode seal verification, despite the second test claiming to test "ticket seal"
  2. There is no actual test coverage for ticket-based seal verification (regular Safrole mode)
  3. The second test provides no additional value over the first

To properly test ticket mode, the second test should:

  • Use SafroleSealingKeysData.tickets() instead of .keys()
  • Use appropriate ticket data matching the test vector from "safrole/00000002.json"
  • Likely have different expected results

Consider creating a separate TICKET_SEALING_KEYS constant using SafroleSealingKeysData.tickets() with proper ticket data, and update the second test to use it. Alternatively, if both test vectors genuinely use fallback mode, update the second test's description to reflect this, though this would still leave ticket mode verification untested.

Copy link
Contributor

@tomusdrw tomusdrw Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't delete that. There are many tests that are not present in any other place. We should rather start using 0.7.0 for these and ignore the tests that are updated in other locations.

authorizationGasUsed: codec.varU64.asOpaque<ServiceGas>(),
});

export class WorkReport extends WorkReportNoCodec {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base class is no longer needed.

Comment on lines +9 to +10
// TODO [MaSo] Tests only for 0.6.7: which is not supported.
it.skip("should decode statistics data 1 with no error", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we supposed to do with that TODO? Either make them work or remove if they are no longer valid.

(i) => tryAsServiceId(i),
);
const codecServiceId: Descriptor<ServiceId> = Compatibility.isSuite(TestSuite.W3F_DAVXY)
? codec.u32.asOpaque<ServiceId>()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you double check if this is still needed? I think at some point GP was specifying var-len encoding, but hopefuly that should be fixed now and we don't need this special-casing?

Comment on lines +154 to +155
extrinsicSize: codec.varU32,
exports: codecVarU16,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid errors this should rather match the codec.

Comment on lines +131 to +135
// if (stateUpdate !== undefined) {
// const clearedAvailabilityAssignment = stateUpdate.availabilityAssignment;
// assert.strictEqual(clearedAvailabilityAssignment[0], null);
// assert.strictEqual(clearedAvailabilityAssignment[1], availabilityAssignment2);
// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please skip the entire test without commenting out the assertions.

@DrEverr DrEverr marked this pull request as draft October 27, 2025 09:22
@DrEverr DrEverr changed the title Update to 0.7.2 + deleted 0.6.7 Deleted 0.6.7 Oct 27, 2025
@DrEverr DrEverr changed the title Deleted 0.6.7 Delete 0.6.7 Oct 27, 2025
@DrEverr DrEverr changed the title Delete 0.6.7 Delete GP v0.6.7 from compatibility support Oct 27, 2025
@github-actions
Copy link

View all
File Benchmark Ops
bytes/hex-from.ts[0] parse hex using Number with NaN checking 136286.1 ±0.54% 84.81% slower
bytes/hex-from.ts[1] parse hex from char codes 897418.35 ±0.33% fastest ✅
bytes/hex-from.ts[2] parse hex from string nibbles 549001.01 ±0.48% 38.82% slower
math/count-bits-u64.ts[0] standard method 1673570.48 ±0.31% 86.49% slower
math/count-bits-u64.ts[1] magic 12385818.28 ±0.45% fastest ✅
math/count-bits-u32.ts[0] standard method 94990495.71 ±1.88% 55.93% slower
math/count-bits-u32.ts[1] magic 215537438.37 ±8.5% fastest ✅
math/add_one_overflow.ts[0] add and take modulus 227109334.72 ±9.59% 11.67% slower
math/add_one_overflow.ts[1] condition before calculation 257113381.36 ±5.99% fastest ✅
hash/index.ts[0] hash with numeric representation 193.83 ±0.06% 20.24% slower
hash/index.ts[1] hash with string representation 119.74 ±0.15% 50.73% slower
hash/index.ts[2] hash with symbol representation 182.69 ±0.44% 24.83% slower
hash/index.ts[3] hash with uint8 representation 162.3 ±0.27% 33.22% slower
hash/index.ts[4] hash with packed representation 243.02 ±4.44% fastest ✅
hash/index.ts[5] hash with bigint representation 186.98 ±0.89% 23.06% slower
hash/index.ts[6] hash with uint32 representation 181.15 ±5.16% 25.46% slower
logger/index.ts[0] console.log with string concat 7928181.96 ±53.54% fastest ✅
logger/index.ts[1] console.log with args 1206139.52 ±72.82% 84.79% slower
collections/map-set.ts[0] 2 gets + conditional set 97857.58 ±22.64% fastest ✅
collections/map-set.ts[1] 1 get 1 set 59589.11 ±0.66% 39.11% slower
codec/bigint.compare.ts[0] compare custom 262481142.59 ±5.54% 3.69% slower
codec/bigint.compare.ts[1] compare bigint 272549604.35 ±5.29% fastest ✅
codec/bigint.decode.ts[0] decode custom 167022551.35 ±2.46% fastest ✅
codec/bigint.decode.ts[1] decode bigint 68504666.34 ±2.13% 58.98% slower
bytes/hex-to.ts[0] number toString + padding 203212.65 ±0.79% fastest ✅
bytes/hex-to.ts[1] manual 11846.59 ±0.88% 94.17% slower
math/mul_overflow.ts[0] multiply and bring back to u32 257153171.75 ±6.31% 0.36% slower
math/mul_overflow.ts[1] multiply and take modulus 258082527.29 ±5.23% fastest ✅
math/switch.ts[0] switch 254727473.9 ±5.62% fastest ✅
math/switch.ts[1] if 236873221.21 ±6.22% 7.01% slower
codec/encoding.ts[0] manual encode 1806632.52 ±0.69% 18.55% slower
codec/encoding.ts[1] int32array encode 2218133.35 ±0.57% fastest ✅
codec/encoding.ts[2] dataview encode 2163634.88 ±1.27% 2.46% slower
codec/decoding.ts[0] manual decode 9733057.41 ±1.49% 94.54% slower
codec/decoding.ts[1] int32array decode 178312724.51 ±4.58% fastest ✅
codec/decoding.ts[2] dataview decode 176102314.04 ±4.31% 1.24% slower
codec/view_vs_object.ts[0] Get the first field from Decoded 324683.54 ±1.58% 4.19% slower
codec/view_vs_object.ts[1] Get the first field from View 62069.89 ±1.56% 81.68% slower
codec/view_vs_object.ts[2] Get the first field as view from View 61678.19 ±2.6% 81.8% slower
codec/view_vs_object.ts[3] Get two fields from Decoded 300188.21 ±1.79% 11.42% slower
codec/view_vs_object.ts[4] Get two fields from View 55679.21 ±1.34% 83.57% slower
codec/view_vs_object.ts[5] Get two fields from materialized from View 114721.95 ±1.2% 66.15% slower
codec/view_vs_object.ts[6] Get two fields as views from View 55265.85 ±1.62% 83.69% slower
codec/view_vs_object.ts[7] Get only third field from Decoded 338875.61 ±1.1% fastest ✅
codec/view_vs_object.ts[8] Get only third field from View 65472.82 ±1.94% 80.68% slower
codec/view_vs_object.ts[9] Get only third field as view from View 69971.8 ±0.9% 79.35% slower
codec/view_vs_collection.ts[0] Get first element from Decoded 19851.41 ±1.17% 56.14% slower
codec/view_vs_collection.ts[1] Get first element from View 45265.99 ±1.18% fastest ✅
codec/view_vs_collection.ts[2] Get 50th element from Decoded 20174.5 ±1.25% 55.43% slower
codec/view_vs_collection.ts[3] Get 50th element from View 24225.72 ±1.58% 46.48% slower
codec/view_vs_collection.ts[4] Get last element from Decoded 21270.08 ±0.89% 53.01% slower
codec/view_vs_collection.ts[5] Get last element from View 15583.37 ±1% 65.57% slower
bytes/compare.ts[0] Comparing Uint32 bytes 18749.32 ±1.03% 11.6% slower
bytes/compare.ts[1] Comparing raw bytes 21210.14 ±0.78% fastest ✅
collections/map_vs_sorted.ts[0] Map 317290.15 ±0.98% fastest ✅
collections/map_vs_sorted.ts[1] Map-array 84702.72 ±2.57% 73.3% slower
collections/map_vs_sorted.ts[2] Array 48396.73 ±11.76% 84.75% slower
collections/map_vs_sorted.ts[3] SortedArray 184438.6 ±1.86% 41.87% slower
hash/blake2b.ts[0] our hasher 2.15 ±2.64% fastest ✅
hash/blake2b.ts[1] blake2b js 0.05 ±1.83% 97.67% slower
crypto/ed25519.ts[0] native crypto 5.26 ±19.11% 82.64% slower
crypto/ed25519.ts[1] wasm lib 10.81 ±0.17% 64.32% slower
crypto/ed25519.ts[2] wasm lib batch 30.3 ±0.65% fastest ✅

Benchmarks summary: 63/63 OK ✅

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.

GP 0.7.2 support

3 participants