RPC: JIP-2 data types #765
Conversation
- switched to zod 4 for built-in support of base64 - updated param schemas - added result schemas - 2-way encoding of data base64 <-> uint8array - improved rpc errors to match jip-2 more closely (still some work left to be done)
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdates the RPC subsystem: bumps zod in package.json, introduces withValidation to wrap RPC handlers (explicit params/result schemas), replaces many RpcMethod exports with validated wrappers and changes return shapes from positional arrays to structured objects (e.g., BlockDescriptor). Adds RpcErrorCode enum and extends RpcError with optional data. Switches hash handling from Bytes.fromNumbers to Bytes.fromBlob, replaces array-based method registry initialization with a Map populated via methods.set(...), adds a notImplemented handler, changes client error propagation to forward raw error objects, and updates e2e tests accordingly. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas to focus on:
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)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bin/rpc/src/methods/service-data.ts (1)
9-19: Update documentation to reflect error-throwing behavior.The documentation comment states that "null is returned if the block's posterior state is not known" (line 11), but the implementation now throws an
RpcErroron line 26 when the state is not found. Please update the documentation to accurately describe the error-throwing behavior.Apply this diff:
/** * https://hackmd.io/@polkadot/jip2#serviceData - * Returns the service data for the given service ID. The data are encoded as per the GP. `null` is - * returned if the block's posterior state is not known. `Some(None)` is returned if there is no value - * associated with the given service ID. + * Returns the service data for the given service ID. The data are encoded as per the GP. + * Throws RpcError if the block's posterior state is not known. Returns `null` if there is no value + * associated with the given service ID. * @param [ * Hash - The header hash indicating the block whose posterior state should be used for the query. * ServiceId - The ID of the service. * ] - * @returns Either null or Blob + * @returns Blob or null + * @throws RpcError when the block's posterior state is not found */bin/rpc/src/methods/service-preimage.ts (1)
7-18: Update documentation to reflect error-throwing behavior.The documentation should mention that
RpcErroris thrown when the block's posterior state is not found (line 25) or when the service is not found (line 31). Currently, it only describes the null return case for missing preimage.Apply this diff:
/** * https://hackmd.io/@polkadot/jip2#servicePreimage * Returns the preimage associated with the given service ID and hash in the posterior state of the * block with the given header hash. `null` is returned if there is no preimage associated with the * given service ID and hash. * @param [ * Hash - The header hash indicating the block whose posterior state should be used for the query. * ServiceId - The ID of the service. * Hash - The hash. * ] - * @returns Either null or Blob + * @returns Blob or null + * @throws RpcError when the block's posterior state or service is not found */
🧹 Nitpick comments (5)
bin/rpc/src/types.ts (1)
99-111: Simplify Uint8Array conversion in codec decode functions.The
Uint8Array.from(Buffer.from(v, "base64"))pattern can be simplified sinceBuffer.from()already returns a Uint8Array-compatible buffer. The extraUint8Array.from()call creates an unnecessary copy.Apply this diff to simplify the decode functions:
export const Hash = z.codec( z.base64(), zUint8Array.refine((v) => v.length === HASH_SIZE, "Invalid hash length."), { - decode: (v) => Uint8Array.from(Buffer.from(v, "base64")), + decode: (v) => Buffer.from(v, "base64"), encode: (v) => Buffer.from(v).toString("base64"), }, ); export const Slot = zU32; export const BlobArray = z.codec(z.base64(), zUint8Array, { - decode: (v) => Uint8Array.from(Buffer.from(v, "base64")), + decode: (v) => Buffer.from(v, "base64"), encode: (v) => Buffer.from(v).toString("base64"), });bin/rpc/src/methods/parent.ts (1)
39-39: Use RpcError for consistency with error handling pattern.This error path uses a generic
Errorwhile other error paths in this method useRpcError. For consistency and to align with the standardized error handling approach, consider usingRpcErrorhere as well.Apply this diff:
- throw new Error(`Parent (${parentHash}) not found.`); + throw new RpcError(RpcErrorCode.Other, `Parent (${parentHash}) not found.`);bin/rpc/src/methods/not-implemented.ts (1)
4-10: Consider z.unknown() instead of z.any() for semantic correctness.While
z.any()works for this placeholder implementation,z.unknown()would be more semantically appropriate as it better expresses that the parameters exist but are not validated, rather than bypassing the type system entirely.Apply this diff:
export const notImplemented = withValidation( () => { throw new RpcError(RpcErrorCode.Other, "Method not implemented"); }, - z.any(), - z.any(), + z.unknown(), + z.unknown(), );bin/rpc/src/methods/service-data.ts (1)
20-39: LGTM! Consider using more specific error code.The implementation correctly:
- Uses
Bytes.fromBlobfor Uint8Array hash parameters (decoded by the Hash schema)- Throws
RpcErrorfor missing state with a descriptive message- Returns raw bytes that will be base64-encoded by the BlobArray schema
- Defines appropriate input/output validation schemas
If
RpcErrorCodeincludes a more specific code for unavailable blocks (e.g.,BlockUnavailable), consider using it instead ofOtheron line 26 for clearer error semantics.bin/rpc/src/methods/service-preimage.ts (1)
19-44: LGTM! Consider using more specific error codes.The implementation correctly:
- Uses
Bytes.fromBlobfor both hash parameters (decoded by the Hash schema)- Implements three-level validation: state existence, service existence, preimage existence
- Returns
preimage.rawdirectly, consistent with the learning thatgetPreimage()returnsBytesBlob | null- Provides descriptive error messages for missing state and service
- Returns null for missing preimage, matching the documented behavior
- Defines appropriate validation schemas
Based on learnings
If
RpcErrorCodeincludes more specific codes for these scenarios (e.g.,BlockUnavailable,ServiceNotFound), consider using them instead ofOtheron lines 25 and 31 for more precise error categorization.
📜 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 (16)
bin/rpc/package.json(1 hunks)bin/rpc/src/client.ts(1 hunks)bin/rpc/src/method-loader.ts(1 hunks)bin/rpc/src/methods/best-block.ts(2 hunks)bin/rpc/src/methods/list-services.ts(2 hunks)bin/rpc/src/methods/not-implemented.ts(1 hunks)bin/rpc/src/methods/parent.ts(2 hunks)bin/rpc/src/methods/service-data.ts(2 hunks)bin/rpc/src/methods/service-preimage.ts(2 hunks)bin/rpc/src/methods/service-request.ts(2 hunks)bin/rpc/src/methods/service-value.ts(2 hunks)bin/rpc/src/methods/state-root.ts(2 hunks)bin/rpc/src/methods/statistics.ts(2 hunks)bin/rpc/src/server.ts(5 hunks)bin/rpc/src/types.ts(1 hunks)bin/rpc/test/e2e.ts(3 hunks)
🧰 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:
bin/rpc/src/methods/not-implemented.tsbin/rpc/test/e2e.tsbin/rpc/src/methods/parent.tsbin/rpc/src/types.tsbin/rpc/src/methods/best-block.tsbin/rpc/src/methods/service-request.tsbin/rpc/src/methods/service-value.tsbin/rpc/src/methods/service-data.tsbin/rpc/src/client.tsbin/rpc/src/methods/service-preimage.tsbin/rpc/src/methods/list-services.tsbin/rpc/src/method-loader.tsbin/rpc/src/server.tsbin/rpc/src/methods/state-root.tsbin/rpc/src/methods/statistics.ts
🧠 Learnings (3)
📚 Learning: 2025-05-22T18:08:38.725Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 381
File: bin/rpc/src/server.ts:99-112
Timestamp: 2025-05-22T18:08:38.725Z
Learning: RPC methods should implement strict parameter validation, including checking the exact number of parameters and their types. This helps catch errors early when users confuse similar methods (e.g., serviceData vs serviceValue) and prevents cryptic errors from undefined parameters being passed to internal code.
Applied to files:
bin/rpc/src/types.tsbin/rpc/src/methods/service-request.tsbin/rpc/src/methods/service-value.tsbin/rpc/src/methods/service-data.tsbin/rpc/src/methods/service-preimage.tsbin/rpc/src/methods/list-services.tsbin/rpc/src/method-loader.tsbin/rpc/src/server.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:
bin/rpc/src/methods/service-value.tsbin/rpc/src/methods/service-data.tsbin/rpc/src/methods/service-preimage.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:
bin/rpc/src/methods/service-data.tsbin/rpc/src/methods/list-services.ts
🧬 Code graph analysis (14)
bin/rpc/src/methods/not-implemented.ts (1)
bin/rpc/src/types.ts (2)
withValidation(84-92)RpcError(62-70)
bin/rpc/test/e2e.ts (2)
bin/rpc/src/types.ts (1)
BlockDescriptor(115-118)bin/rpc/src/methods/best-block.ts (1)
bestBlock(11-31)
bin/rpc/src/methods/parent.ts (2)
bin/rpc/src/types.ts (4)
withValidation(84-92)RpcError(62-70)Hash(99-106)BlockDescriptor(115-118)packages/core/bytes/bytes.ts (1)
Bytes(177-243)
bin/rpc/src/types.ts (4)
packages/jam/database/blocks.ts (1)
BlocksDb(9-30)packages/jam/database/states.ts (1)
StatesDb(31-51)packages/jam/state/state.ts (2)
State(52-210)EnumerableState(37-45)packages/jam/config/chain-spec.ts (1)
ChainSpec(41-106)
bin/rpc/src/methods/best-block.ts (1)
bin/rpc/src/types.ts (5)
withValidation(84-92)RpcError(62-70)Hash(99-106)NoArgs(114-114)BlockDescriptor(115-118)
bin/rpc/src/methods/service-request.ts (4)
bin/rpc/src/types.ts (5)
withValidation(84-92)Hash(99-106)ServiceId(112-112)PreimageLength(113-113)Slot(107-107)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)packages/core/numbers/index.ts (1)
tryAsU32(45-48)
bin/rpc/src/methods/service-value.ts (3)
bin/rpc/src/types.ts (5)
withValidation(84-92)RpcError(62-70)Hash(99-106)ServiceId(112-112)BlobArray(108-111)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)
bin/rpc/src/methods/service-data.ts (6)
packages/jam/state-merkleization/serialize.ts (1)
serviceData(185-190)bin/rpc/src/types.ts (5)
withValidation(84-92)RpcError(62-70)Hash(99-106)ServiceId(112-112)BlobArray(108-111)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)packages/core/codec/encoder.ts (1)
Encoder(73-480)packages/jam/state/service.ts (1)
ServiceAccountInfo(82-154)
bin/rpc/src/methods/service-preimage.ts (3)
bin/rpc/src/types.ts (5)
withValidation(84-92)RpcError(62-70)Hash(99-106)ServiceId(112-112)BlobArray(108-111)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)
bin/rpc/src/methods/list-services.ts (2)
bin/rpc/src/types.ts (3)
withValidation(84-92)Hash(99-106)ServiceId(112-112)packages/core/bytes/bytes.ts (1)
Bytes(177-243)
bin/rpc/src/method-loader.ts (2)
bin/rpc/src/types.ts (1)
RpcMethodRepo(94-94)bin/rpc/src/methods/not-implemented.ts (1)
notImplemented(4-10)
bin/rpc/src/server.ts (1)
bin/rpc/src/types.ts (1)
DatabaseContext(79-82)
bin/rpc/src/methods/state-root.ts (2)
bin/rpc/src/types.ts (3)
withValidation(84-92)RpcError(62-70)Hash(99-106)packages/core/bytes/bytes.ts (1)
Bytes(177-243)
bin/rpc/src/methods/statistics.ts (5)
packages/jam/state-merkleization/serialize.ts (1)
statistics(148-152)packages/jam/state-merkleization/serialized-state.ts (1)
statistics(163-165)bin/rpc/src/types.ts (4)
withValidation(84-92)RpcError(62-70)Hash(99-106)BlobArray(108-111)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/state/statistics.ts (1)
StatisticsData(271-291)
🔇 Additional comments (16)
bin/rpc/src/client.ts (1)
110-110: LGTM: Error propagation now preserves structured error data.The change from wrapping the error in a new Error object to passing the raw
response.errorobject preserves the structured error information (code, message, and data fields), which aligns with the newRpcErrortype and enables clients to access error codes and additional data.bin/rpc/package.json (1)
36-36: LGTM: Zod 4 upgrade aligns with new validation patterns.The upgrade to zod ^4.1.12 enables the new
z.codecandz.base64()features used throughout the codebase for type-safe encoding/decoding. The caret range is appropriate for managing minor and patch updates.bin/rpc/src/types.ts (3)
66-66: LGTM: Error data field enables structured error context.Adding the optional
dataparameter toRpcErroraligns with JSON-RPC 2.0 specification and enables methods to include additional context (e.g., encoded hash values) in error responses.
72-77: LGTM: Application-specific error codes improve error handling.The
RpcErrorCodeenum provides typed, domain-specific error codes that enable clients to handle different error scenarios programmatically. The values (1, 2, 3) are appropriately distinct from JSON-RPC standard error codes (negative values).
84-92: LGTM: withValidation standardizes RPC method definitions.The
withValidationwrapper provides a type-safe pattern for defining RPC methods with explicit parameter and result schemas, enabling consistent validation and encoding across all methods.bin/rpc/src/methods/parent.ts (2)
21-21: LGTM: Bytes.fromBlob is the correct method for Uint8Array input.The change from
Bytes.fromNumberstoBytes.fromBlobcorrectly handles the Uint8Array input from the validated hash parameter.
42-45: LGTM: Structured return type improves API clarity.Returning a
BlockDescriptorobject instead of a tuple provides better clarity and type safety for API consumers.bin/rpc/src/server.ts (3)
32-32: LGTM: Error response now includes data field.Adding
error.datato the error response structure enables methods to provide additional context (such as encoded hash values) in error responses, consistent with JSON-RPC 2.0 specification.
168-168: LGTM: Default empty array prevents undefined params.Using
params ?? []ensures that the validation function always receives a defined value, which is correct for methods that acceptNoArgs(empty tuple) and prevents potential validation errors from undefined values.Also applies to: 180-180
204-211: LGTM: Result encoding ensures proper data serialization.The addition of
resultSchema.encode()is essential for converting internal types (e.g., Uint8Array) to wire formats (e.g., base64 strings) before sending responses. This aligns with the new codec-based schema definitions.bin/rpc/src/methods/best-block.ts (1)
11-31: LGTM: Consistent implementation of withValidation pattern.The
bestBlockmethod correctly implements the new validation pattern with proper error handling, structured return type, and encoded error data. The implementation is consistent with other migrated methods.bin/rpc/test/e2e.ts (3)
9-13: LGTM: Temporary type definition is appropriate.The local
BlockDescriptortype definition is a reasonable temporary workaround until client-side validation and typing are implemented. The TODO comment clearly indicates this is technical debt to be addressed.
93-105: LGTM: Error assertions validate structured error format.The test correctly validates the new error structure including the
code,message, anddatafields, ensuring that encoded hash values are properly included in error responses.
64-68: LGTM: Tests properly validate base64-encoded hash values.The test assertions correctly use base64-encoded hash values, which is consistent with the new
Hashcodec implementation. This ensures the encoding/decoding logic is properly exercised in the test suite.Also applies to: 75-79, 86-90
bin/rpc/src/methods/service-data.ts (1)
7-7: LGTM!The imports correctly bring in the new validation utilities and type definitions needed for the withValidation wrapper pattern.
bin/rpc/src/methods/service-preimage.ts (1)
5-5: LGTM!The imports are consistent with the new validation pattern and include all necessary utilities.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bin/rpc/src/methods/parent.ts (1)
7-18: Update documentation to reflect the new implementation.The documentation comment describes behavior that no longer matches the implementation:
- States the method can return
null, but the new implementation throwsRpcErrorinstead- Describes the return value as a tuple
[Hash, Slot], but it now returns an object withheader_hashandslotfieldsPlease update the documentation to accurately describe the new error-throwing behavior and the
BlockDescriptorobject return type.
🧹 Nitpick comments (1)
bin/rpc/src/methods/parent.ts (1)
38-43: Consider usingBlockUnavailablefor consistency.When the parent header is missing from the database (lines 38-43), the code uses
RpcErrorCode.Other. For consistency with the earlier error handling (lines 24-28), consider usingRpcErrorCode.BlockUnavailablehere as well, since this is also a case of an unavailable block.Apply this diff if you agree:
if (parentHeader === null) { throw new RpcError( - RpcErrorCode.Other, + RpcErrorCode.BlockUnavailable, `The hash of parent was found (${parentHash}) but its header doesn't exist in the database.`, ); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bin/rpc/src/methods/list-services.ts(2 hunks)bin/rpc/src/methods/parent.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- bin/rpc/src/methods/list-services.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:
bin/rpc/src/methods/parent.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 381
File: bin/rpc/src/server.ts:99-112
Timestamp: 2025-05-22T18:08:38.725Z
Learning: RPC methods should implement strict parameter validation, including checking the exact number of parameters and their types. This helps catch errors early when users confuse similar methods (e.g., serviceData vs serviceValue) and prevents cryptic errors from undefined parameters being passed to internal code.
🧬 Code graph analysis (1)
bin/rpc/src/methods/parent.ts (2)
bin/rpc/src/types.ts (4)
withValidation(84-92)RpcError(62-70)Hash(99-106)BlockDescriptor(115-118)packages/core/bytes/bytes.ts (1)
Bytes(177-243)
⏰ 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). (7)
- GitHub Check: run (22.x)
- GitHub Check: run (22.x)
- GitHub Check: run (22.x)
- GitHub Check: benchmarks (22.x)
- GitHub Check: jam-e2e (22.x)
- GitHub Check: test (22.x)
- GitHub Check: e2e (22.x)
🔇 Additional comments (2)
bin/rpc/src/methods/parent.ts (2)
19-52: LGTM! Excellent use of withValidation for type safety.The refactoring to use
withValidationprovides strong input/output validation:
- Input schema
z.tuple([Hash])ensures exactly one Hash parameter is provided (addressing strict parameter validation requirements)- Output schema
BlockDescriptorguarantees consistent return shape- Improved error handling with structured
RpcErrorinstancesThis aligns well with the JIP-2 migration objectives.
Based on learnings
21-21: LGTM! Correct usage ofBytes.fromBlob.The change from
Bytes.fromNumberstoBytes.fromBlobis appropriate sinceheaderHashis now aUint8Arrayafter being decoded from base64 by theHashcodec.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
bin/rpc/src/methods/service-value.ts (1)
26-28: UseBlockUnavailable(with encoded hash) for missing posterior state.We still throw
RpcErrorCode.Otherwhen the posterior state is absent, which hides the actionable signal clients expect for unavailable blocks and omits the encoded hash payload. Please switch toRpcErrorCode.BlockUnavailableand include the base64 header hash, consistent with the other RPCs and earlier feedback.- if (state === null) { - throw new RpcError(RpcErrorCode.Other, `State not found for block: ${hashOpaque.toString()}`); - } + if (state === null) { + throw new RpcError( + RpcErrorCode.BlockUnavailable, + `State not found for block: ${hashOpaque.toString()}`, + Hash.encode(hashOpaque.raw), + ); + }bin/rpc/src/methods/statistics.ts (1)
24-24: UseRpcErrorCode.BlockUnavailablefor missing state.When the posterior state is not found, the error code should be
RpcErrorCode.BlockUnavailableto allow clients to distinguish block-not-found from generic errors.Apply this diff:
- throw new RpcError(RpcErrorCode.Other, `State not found for block: ${hashOpaque.toString()}`); + throw new RpcError( + RpcErrorCode.BlockUnavailable, + `State not found for block: ${hashOpaque.toString()}`, + hashOpaque.raw, + );bin/rpc/src/methods/list-services.ts (1)
22-22: UseRpcErrorCode.BlockUnavailablefor missing state.The error code should be
BlockUnavailablerather thanOtherto provide clients with actionable error information.Apply this diff:
- throw new RpcError(RpcErrorCode.Other, `Posterior state not found for block: ${hashOpaque.toString()}`); + throw new RpcError( + RpcErrorCode.BlockUnavailable, + `Posterior state not found for block: ${hashOpaque.toString()}`, + hashOpaque.raw, + );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
bin/rpc/src/methods/best-block.ts(2 hunks)bin/rpc/src/methods/list-services.ts(2 hunks)bin/rpc/src/methods/not-implemented.ts(1 hunks)bin/rpc/src/methods/parent.ts(2 hunks)bin/rpc/src/methods/service-data.ts(2 hunks)bin/rpc/src/methods/service-preimage.ts(2 hunks)bin/rpc/src/methods/service-request.ts(2 hunks)bin/rpc/src/methods/service-value.ts(2 hunks)bin/rpc/src/methods/state-root.ts(2 hunks)bin/rpc/src/methods/statistics.ts(2 hunks)bin/rpc/src/types.ts(1 hunks)bin/rpc/test/e2e.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- bin/rpc/src/methods/not-implemented.ts
- bin/rpc/src/methods/best-block.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:
bin/rpc/test/e2e.tsbin/rpc/src/methods/service-data.tsbin/rpc/src/methods/service-preimage.tsbin/rpc/src/methods/statistics.tsbin/rpc/src/methods/state-root.tsbin/rpc/src/methods/service-request.tsbin/rpc/src/methods/parent.tsbin/rpc/src/methods/list-services.tsbin/rpc/src/types.tsbin/rpc/src/methods/service-value.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 381
File: bin/rpc/src/server.ts:99-112
Timestamp: 2025-05-22T18:08:38.725Z
Learning: RPC methods should implement strict parameter validation, including checking the exact number of parameters and their types. This helps catch errors early when users confuse similar methods (e.g., serviceData vs serviceValue) and prevents cryptic errors from undefined parameters being passed to internal code.
📚 Learning: 2025-05-22T18:08:38.725Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 381
File: bin/rpc/src/server.ts:99-112
Timestamp: 2025-05-22T18:08:38.725Z
Learning: RPC methods should implement strict parameter validation, including checking the exact number of parameters and their types. This helps catch errors early when users confuse similar methods (e.g., serviceData vs serviceValue) and prevents cryptic errors from undefined parameters being passed to internal code.
Applied to files:
bin/rpc/src/methods/service-data.tsbin/rpc/src/methods/service-preimage.tsbin/rpc/src/methods/service-request.tsbin/rpc/src/methods/list-services.tsbin/rpc/src/types.tsbin/rpc/src/methods/service-value.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:
bin/rpc/src/methods/service-data.tsbin/rpc/src/methods/list-services.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:
bin/rpc/src/methods/service-data.tsbin/rpc/src/methods/service-preimage.tsbin/rpc/src/methods/service-value.ts
🧬 Code graph analysis (10)
bin/rpc/test/e2e.ts (2)
bin/rpc/src/types.ts (1)
BlockDescriptor(115-118)bin/rpc/src/methods/best-block.ts (1)
bestBlock(11-27)
bin/rpc/src/methods/service-data.ts (4)
bin/rpc/src/types.ts (5)
withValidation(84-92)Hash(99-106)ServiceId(112-112)BlobArray(108-111)RpcError(62-70)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)packages/core/codec/encoder.ts (1)
Encoder(73-480)
bin/rpc/src/methods/service-preimage.ts (3)
bin/rpc/src/types.ts (5)
withValidation(84-92)Hash(99-106)ServiceId(112-112)BlobArray(108-111)RpcError(62-70)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)
bin/rpc/src/methods/statistics.ts (4)
bin/rpc/src/types.ts (4)
withValidation(84-92)Hash(99-106)BlobArray(108-111)RpcError(62-70)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/core/codec/encoder.ts (1)
Encoder(73-480)packages/jam/state/statistics.ts (1)
StatisticsData(271-291)
bin/rpc/src/methods/state-root.ts (2)
bin/rpc/src/types.ts (3)
withValidation(84-92)Hash(99-106)RpcError(62-70)packages/core/bytes/bytes.ts (1)
Bytes(177-243)
bin/rpc/src/methods/service-request.ts (4)
bin/rpc/src/types.ts (5)
withValidation(84-92)Hash(99-106)ServiceId(112-112)PreimageLength(113-113)Slot(107-107)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)packages/core/numbers/index.ts (1)
tryAsU32(45-48)
bin/rpc/src/methods/parent.ts (2)
bin/rpc/src/types.ts (4)
withValidation(84-92)Hash(99-106)BlockDescriptor(115-118)RpcError(62-70)packages/core/bytes/bytes.ts (1)
Bytes(177-243)
bin/rpc/src/methods/list-services.ts (2)
bin/rpc/src/types.ts (4)
withValidation(84-92)Hash(99-106)ServiceId(112-112)RpcError(62-70)packages/core/bytes/bytes.ts (1)
Bytes(177-243)
bin/rpc/src/types.ts (4)
packages/jam/database/blocks.ts (1)
BlocksDb(9-30)packages/jam/database/states.ts (1)
StatesDb(31-51)packages/jam/state/state.ts (2)
State(52-210)EnumerableState(37-45)packages/jam/config/chain-spec.ts (1)
ChainSpec(41-106)
bin/rpc/src/methods/service-value.ts (3)
bin/rpc/src/types.ts (5)
withValidation(84-92)Hash(99-106)ServiceId(112-112)BlobArray(108-111)RpcError(62-70)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)
⏰ 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: benchmarks (22.x)
- GitHub Check: e2e (22.x)
- GitHub Check: run (22.x)
- GitHub Check: run (22.x)
- GitHub Check: run (22.x)
- GitHub Check: test (22.x)
🔇 Additional comments (7)
bin/rpc/src/methods/service-request.ts (1)
21-46: LGTM! Well-structured validation and error handling.The implementation correctly uses
withValidationwith explicit parameter and result schemas, aligning with the learning that RPC methods should implement strict parameter validation. The consistent null-return pattern for missing state/service/slots provides clear semantics.bin/rpc/src/types.ts (5)
62-70: LGTM! Extended error handling.The optional
dataparameter allows attaching context (like the hash that wasn't found) to error responses, improving debugging for clients.
72-77: LGTM! Well-defined error codes.The
RpcErrorCodeenum provides semantic error categories aligned with JIP-2 requirements, enabling clients to distinguish missing blocks from missing work reports or DA segments.
84-92: LGTM! Clean validation wrapper.The
withValidationfunction properly couples schemas with methods and follows the parameter ordering suggested in past review comments. This provides the strict parameter validation recommended in learnings.Based on learnings
115-118: LGTM! Structured block descriptor.The
BlockDescriptorobject withheader_hashandslotprovides a cleaner API surface than positional arrays, aligning with JIP-2 structured response format.
99-106: No changes required—base64 validation is working as intended.The
z.base64()validator checks input with an internal base64 checker and rejects invalid input, producing a Zod issue with code "invalid_format". This validation occurs before the decode function executes, so the code already ensures invalid base64 inputs are caught by Zod's validation layer.bin/rpc/src/methods/service-preimage.ts (1)
19-44: Confirm inconsistent error-handling strategy requires JIP-2 spec alignment.The codebase shows a mix of error-handling approaches:
service-request.tsconsistently returnsnullfor all missing entities (state, service, slots)service-preimage.tsthrowsRpcErrorfor missing state and service but returnsnullfor missing preimageservice-data.tsthrows for missing state but returnsnullfor missing serviceservice-value.tsthrows for missing state but returnsnullfor service and storage valuesThe mixed approach in
service-preimage.ts(lines 27, 33 throw; line 39 returnsnull) is confirmed. Verify this aligns with the JIP-2 spec requirements referenced in the docstring, as the inconsistency across similar methods suggests either a bug or intentional differentiation that needs documenting.
View all
Benchmarks summary: 63/63 OK ✅ |
This is the first step in a series of changes to align with the new JIP-2 spec (#756).