Update dependencies #778
Update dependencies #778
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR moves the top-level "type": "module" to package.json root, relaxes devDependency versions, and bumps rollup. It adds an explicit Uint8Array return type in safeAllocUint8Array and removes an unnecessary Uint8Array cast in message concatenation. Multiple numeric-parsing sites were changed to use explicit radix or Number(...) instead of implicit parseInt. The TransitionHasher constructor was simplified to accept only keccak and blake2b (removing ChainSpec), and all call sites/tests were updated. A hex benchmark fix returns parsed values, and biome.jsonc schema version was updated. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
Comment |
![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
🧹 Nitpick comments (1)
packages/core/json-parser/index.test.ts (1)
56-56: Note the behavior change from parseInt to Number.Similar to the change in
bin/tci/args.ts, this switches fromNumber.parseInt(v)toNumber(v). While both can parse the hex string"0x123"correctly,Number()has stricter parsing rules and can handle floating-point values.For consistency across the codebase, please ensure this change aligns with the intended numeric parsing strategy.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
benchmarks/bytes/hex-from.ts(1 hunks)bin/jam/test/e2e.ts(1 hunks)bin/tci/args.ts(1 hunks)bin/test-runner/state-transition/state-transition.ts(1 hunks)biome.jsonc(1 hunks)package.json(1 hunks)packages/core/json-parser/index.test.ts(1 hunks)packages/core/networking/bin/test.ts(1 hunks)packages/jam/config-node/jip-chain-spec.ts(1 hunks)packages/jam/transition/accumulate/accumulate-data.ts(1 hunks)packages/jam/transition/block-verifier.test.ts(1 hunks)packages/jam/transition/hasher.test.ts(1 hunks)packages/jam/transition/hasher.ts(0 hunks)packages/workers/block-generator/generator.ts(1 hunks)packages/workers/importer/main.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/jam/transition/hasher.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 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/networking/bin/test.tspackages/core/json-parser/index.test.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/networking/bin/test.tspackages/jam/transition/accumulate/accumulate-data.tspackages/jam/transition/block-verifier.test.tspackages/jam/transition/hasher.test.tspackages/workers/importer/main.tsbenchmarks/bytes/hex-from.tsbin/test-runner/state-transition/state-transition.tsbin/jam/test/e2e.tspackages/jam/config-node/jip-chain-spec.tsbin/tci/args.tspackages/workers/block-generator/generator.tspackages/core/json-parser/index.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 625
File: benchmarks/package.json:8-15
Timestamp: 2025-09-15T14:37:04.214Z
Learning: In the FluffyLabs/typeberry monorepo, the workspace:* protocol does not work for internal typeberry dependencies. The working approach is to use "*" as the version specifier for internal monorepo packages instead of "workspace:*".
🧬 Code graph analysis (6)
packages/jam/transition/accumulate/accumulate-data.ts (1)
packages/jam/block/common.ts (1)
ServiceGas(31-31)
packages/jam/transition/block-verifier.test.ts (2)
packages/jam/transition/hasher.ts (1)
TransitionHasher(9-62)packages/core/hash/blake2b.ts (1)
Blake2b(8-44)
packages/jam/transition/hasher.test.ts (1)
packages/jam/transition/hasher.ts (1)
TransitionHasher(9-62)
packages/workers/importer/main.ts (1)
packages/jam/transition/hasher.ts (1)
TransitionHasher(9-62)
bin/test-runner/state-transition/state-transition.ts (1)
packages/jam/transition/hasher.ts (1)
TransitionHasher(9-62)
packages/workers/block-generator/generator.ts (1)
packages/jam/transition/hasher.ts (1)
TransitionHasher(9-62)
⏰ 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)
- GitHub Check: e2e (22.x)
- GitHub Check: test (22.x)
- GitHub Check: run (22.x)
- GitHub Check: state_transition (22.x)
- GitHub Check: run (22.x)
- GitHub Check: run (22.x)
🔇 Additional comments (11)
packages/core/networking/bin/test.ts (1)
90-90: LGTM! Explicit radix prevents octal interpretation.Adding the explicit radix 10 is a good practice that prevents strings with leading zeros from being incorrectly parsed as octal numbers.
packages/jam/config-node/jip-chain-spec.ts (1)
17-17: LGTM! Explicit radix improves parsing safety.Adding the explicit radix 10 is consistent with the broader pattern in this PR and prevents potential issues with octal interpretation.
bin/jam/test/e2e.ts (1)
67-67: LGTM! Explicit radix ensures correct block number parsing.Adding the explicit radix 10 is appropriate for parsing block numbers and aligns with the broader improvements in this PR.
benchmarks/bytes/hex-from.ts (1)
97-97: LGTM! Returning the value prevents dead code elimination.Returning the computed value is good practice in benchmarks as it prevents the optimizer from potentially eliminating the computation, ensuring accurate performance measurements.
bin/tci/args.ts (1)
64-64: Change fromNumber.parseInt()toNumber()improves CLI argument validation—concern is not applicable.The original review raised concerns about stricter parsing behavior, but actually the opposite is true. Using
Number(v)with theNumber.isNaN()check is more strict:
parseInt("42abc")silently returns42and discards the invalid suffixNumber("42abc")returnsNaN, which triggers the error:"Cannot parse '42abc' as a number"For CLI arguments expecting integers (port, timestamp, validatorindex), rejecting malformed input like
"42abc"is the correct behavior. The implementation is sound:
- No fractional values are expected (all tests use integers)
- Hex notation like
"0xbeef"works correctly withNumber()- Downstream conversions (
tryAsU16,tryAsTimeSlot,tryAsValidatorIndex) provide additional range validationbiome.jsonc (1)
2-2: Schema version is valid and accessible; review configuration for compatibility with documented breaking changes.The schema URL at
https://biomejs.dev/schemas/2.3.4/schema.jsonis accessible (HTTP 200). Between Biome 2.0.6 and 2.3.4, there are several documented breaking changes:
- Configuration format:
include/ignorereplaced withincludes(different glob semantics);deniedGlobalschanged from array to record structure- CLI/API: renamed and removed options (
--skip-errors→--skip-parse-errors)- Linter: v2.1.x changed
noImportCyclesrule to ignore type-only imports by defaultThese changes may require updating the biome.jsonc configuration if your project uses any of these deprecated patterns. Run
biome migrateto automatically update your configuration, or manually verify compatibility.bin/test-runner/state-transition/state-transition.ts (1)
94-94: LGTM! Constructor call updated correctly.The
TransitionHasherconstructor call has been correctly updated to match the new signature that no longer requires theChainSpecparameter.packages/jam/transition/hasher.test.ts (1)
33-33: LGTM! Test updated correctly.The test correctly instantiates
TransitionHasherwith the updated constructor signature, removing thespecparameter.packages/workers/block-generator/generator.ts (1)
75-75: LGTM! Constructor call updated correctly.The
TransitionHasherinstantiation has been correctly updated to use only the keccak and blake2b hashers, removing thechainSpecparameter as per the updated API.packages/workers/importer/main.ts (1)
25-25: LGTM! Constructor call updated correctly.The
TransitionHasherinstantiation has been properly updated to remove thechainSpecparameter, correctly awaiting both the keccak and blake2b hashers.packages/jam/transition/block-verifier.test.ts (1)
28-28: LGTM! Test updated correctly.The test correctly instantiates
TransitionHasherwith the updated constructor signature, removing thespecparameter and properly awaiting both hashers.
View all
Benchmarks summary: 63/63 OK ✅ |
No description provided.