human-legible CAIP-350 text identifiers for chains and addresses#35
human-legible CAIP-350 text identifiers for chains and addresses#35
Conversation
📝 WalkthroughWalkthroughThis PR replaces the single Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRsSuggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0xrcinus
left a comment
0xrcinus
left a comment
There was a problem hiding this comment.
Nice! Left some comments on the schemas, which may need to be reflected across the other files. Mostly labelling, but also wondering if we can simplify some of the data structures (e.g. inputs and outputs could have chain at the top level)
Co-authored-by: Orca <227982352+0xrcinus@users.noreply.github.com>
Co-authored-by: Orca <227982352+0xrcinus@users.noreply.github.com>
Co-authored-by: Orca <227982352+0xrcinus@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/references.md (1)
1-1:⚠️ Potential issue | 🟡 MinorHeading hierarchy is inverted — top-level heading is
###while sub-sections use##.The document title on line 1 is an
h3(###), but the child sections on lines 5 and 23 areh2(##). This breaks semantic heading hierarchy and will render oddly in most Markdown renderers.Proposed fix
-### External references: related APIs and intent protocols +# External references: related APIs and intent protocolsOr alternatively, demote the subsections to
###.Also applies to: 5-5, 23-23
specs/openapi.yaml (1)
2083-2107: 🛠️ Refactor suggestion | 🟠 MajorPath schemas inline the entire request/response bodies instead of using
$ref.The path definitions duplicate the component schemas entirely. For example, the
/v1/quotesPOST request body (lines 2094-2302) is a full copy of theGetQuoteRequestcomponent schema (lines 234-443). The same applies to all response schemas.This compounds the
$refissue flagged above — with references, the paths section could be reduced to just a few lines per endpoint:requestBody: required: true content: application/json: schema: $ref: '#/components/schemas/GetQuoteRequest'
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 38-50: The README contains JSON code blocks that include
JavaScript-style comments which make the blocks invalid JSON; update the blocks
that currently start with ```json (the two example blocks showing "chain" and
"address") to either use ```jsonc so comments are allowed or remove the comment
lines and move the explanatory text above each block—ensure the rendered fence
language matches the content (jsonc for comments, json for strict JSON) and keep
the two example objects intact.
In `@schemas/typescript/types.ts`:
- Around line 8-18: The doc comment for the Chain type incorrectly references
CAIP-350; update the descriptive text for export type Chain to reference CAIP-2
(chain identifier text representation `<namespace>:<reference>`) and correct the
same wording for the ChainAddress.chain field so both describe CAIP-2 rather
than CAIP-350; also regenerate or update any generated schemas that include the
same incorrect description so they reflect CAIP-2 consistently.
In `@specs/openapi.yaml`:
- Around line 237-248: Top-level GetQuoteRequest defines user as a ChainAddress
object with properties chain and address, but inputs[] currently uses user as a
plain string with a separate sibling chain field; decide on a single canonical
shape and make them consistent: either change inputs[].user to the ChainAddress
object (include both chain and address) or change top-level user to a string and
keep chain at input level, and add or update descriptions to clarify if they
intentionally differ; update the OpenAPI schema entries for GetQuoteRequest,
inputs[], and the user property (and mention ChainAddress type if you introduce
or reuse it) so both representations match and the intent is documented.
- Around line 17-38: The components schemas Chain, NativeAddress, and
ChainAddress are defined but never referenced, causing duplicated inline
definitions; update all properties that currently inline chain, address, or
chain+address objects to reference the shared components instead (use $ref:
'#/components/schemas/Chain' for any chain property, $ref:
'#/components/schemas/NativeAddress' for address strings, and $ref:
'#/components/schemas/ChainAddress' for composite chain+address objects), and
either remove the unused NativeAddress if truly unnecessary or replace all
inline address fields to reference it to avoid dead code and future duplication.
🧹 Nitpick comments (3)
docs/references.md (1)
41-42: Pending reference link for LI.FI Catalyst.This is a known placeholder. Consider opening a tracking issue so it doesn't get forgotten.
Would you like me to open an issue to track adding the LI.FI Catalyst canonical URL?
schemas/typescript/types.ts (1)
160-190: Intentional asymmetry betweenGetQuoteRequest.user(ChainAddress) andInput.user/Output.receiver(NativeAddress) — consider adding a clarifying note.
GetQuoteRequest.useris aChainAddressobject (line 343), whileInput.user(line 170) andOutput.receiver(line 224) areNativeAddressstrings withchainat the parent level. This is a sound design (avoids redundancy since all fields in an Input/Output share the same chain), but the different shapes for "user" at different nesting levels could trip up integrators.A brief JSDoc note on
GetQuoteRequest.userexplaining why it's aChainAddress(e.g., "identifies the user and their home chain for refund routing, independent of individual input/output chains") would help.Also applies to: 214-246, 341-343
schemas/typescript/schemas.generated.ts (1)
278-291: Same description string repeated three times ingetAssetsResponseSchema.The "Map where keys are CAIP-2 chain identifiers…" description is applied to the key schema (line 282), the value schema (line 285), and the outer
.describe()(line 289). Since this file is auto-generated byts-to-zod, this is likely a generator artifact — but if you control the generation config, it would be cleaner to only keep the outermost.describe().
| ```json | ||
| // USDC on Ethereum mainnet | ||
| { | ||
| "chain": "eip155:1", | ||
| "address": "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48" | ||
| } | ||
|
|
||
| // vitalik.eth on Base | ||
| { | ||
| "chain": "eip155:8453", | ||
| "address": "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045" | ||
| } | ||
| ``` |
There was a problem hiding this comment.
JSON code blocks contain // comments, which are invalid JSON syntax.
Lines 39 and 45 use JavaScript-style comments inside ```json blocks. While fine for human readers, tooling that validates or syntax-highlights JSON will flag or break on these. Consider switching to ```jsonc (JSON with Comments) or removing the comments and placing them above each block.
Proposed fix (option A: use jsonc)
-```json
+```jsonc🤖 Prompt for AI Agents
In `@README.md` around lines 38 - 50, The README contains JSON code blocks that
include JavaScript-style comments which make the blocks invalid JSON; update the
blocks that currently start with ```json (the two example blocks showing "chain"
and "address") to either use ```jsonc so comments are allowed or remove the
comment lines and move the explanatory text above each block—ensure the rendered
fence language matches the content (jsonc for comments, json for strict JSON)
and keep the two example objects intact.
| Chain: | ||
| type: string | ||
| pattern: ^0x0001[a-fA-F0-9]+$ | ||
| description: |- | ||
| Cross-chain compatible address format per EIP-7930 version 1 encoded format (0x0001 + chain ID + address) for | ||
| unambiguous cross-chain identification. | ||
| Chain identifier following the CAIP-350 chain identifier text representation (`<namespace>:<reference>`). | ||
| For EVM chains, uses eip155 namespace with chain ID. | ||
| NativeAddress: | ||
| type: string | ||
| description: |- | ||
| The address in its standard text representation for the specified chain. | ||
| Format varies by chain namespace. | ||
| ChainAddress: | ||
| type: object | ||
| properties: | ||
| chain: | ||
| type: string | ||
| description: Chain identifier following the CAIP-350 chain identifier text representation (`<namespace>:<reference>`) | ||
| address: | ||
| type: string | ||
| description: The address in its standard text representation for the specified chain | ||
| required: | ||
| - chain | ||
| - address |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the three new component schemas are not referenced via $ref
echo "=== Searching for \$ref usage of Chain, NativeAddress, ChainAddress ==="
echo "Chain references:"
rg '\$ref.*#/components/schemas/Chain' specs/openapi.yaml | head -20
echo ""
echo "NativeAddress references:"
rg '\$ref.*#/components/schemas/NativeAddress' specs/openapi.yaml | head -20
echo ""
echo "ChainAddress references:"
rg '\$ref.*#/components/schemas/ChainAddress' specs/openapi.yaml | head -20
echo ""
echo "=== Verify schemas are defined in components ==="
sed -n '17,38p' specs/openapi.yaml
echo ""
echo "=== Count inline Chain definitions (chain field with CAIP-350 description) ==="
rg -c 'Chain identifier following the CAIP-350' specs/openapi.yaml
echo ""
echo "=== Sample of inline ChainAddress-like structures (chain: + address:) ==="
rg -n 'type: object' specs/openapi.yaml | grep -A 10 'properties:' | head -50
echo ""
echo "=== Count distinct 'chain:' property definitions ==="
rg 'chain:$' specs/openapi.yaml | wc -l
echo ""
echo "=== Count distinct 'address:' property definitions ==="
rg 'address:$' specs/openapi.yaml | wc -lRepository: openintentsframework/oif-specs
Length of output: 1355
🏁 Script executed:
#!/bin/bash
# Check if NativeAddress is used inline at all (or completely unused)
echo "=== Searching for 'standard text representation' description (NativeAddress concept) ==="
rg -n 'The address in its standard text representation for the specified chain' specs/openapi.yaml | head -20
echo ""
echo "=== Check lines around NativeAddress definition to see if it's referenced ==="
sed -n '10,50p' specs/openapi.yaml
echo ""
echo "=== Verify CAIP-350 is the correct standard (search comments/descriptions) ==="
rg -i 'caip-2|caip-10|caip-350' specs/openapi.yaml | head -10
echo ""
echo "=== Show all property definitions with 'chain' that use $ref vs inline ==="
rg -B2 -A2 'type: string' specs/openapi.yaml | grep -A 2 'chain:' | head -40Repository: openintentsframework/oif-specs
Length of output: 6235
Component schemas Chain, NativeAddress, and ChainAddress are defined in components but never referenced via $ref, resulting in significant schema duplication.
The three new schemas are defined at lines 17-38 but not used anywhere via $ref. Instead:
- The
Chainidentifier description ("Chain identifier following the CAIP-350...") is duplicated 15+ times inline NativeAddressschema is completely unused — none of the 14+ inlineaddress:properties reference itChainAddressis inlined repeatedly instead of using$ref
This creates a maintainability burden: any future change to these schemas (e.g., adding a pattern constraint for validation) would require updates across 30+ locations.
Refactor to use $ref throughout:
Example refactoring approach
# In components, keep the definitions (already correct)
components:
schemas:
Chain: { ... }
NativeAddress: { ... }
ChainAddress: { ... }
# In all property definitions, use $ref:
properties:
chain:
$ref: '#/components/schemas/Chain'
address:
$ref: '#/components/schemas/NativeAddress'
to:
$ref: '#/components/schemas/ChainAddress'
user:
$ref: '#/components/schemas/ChainAddress'This is especially important for NativeAddress, which is currently dead code and should either be removed or actively used throughout the spec.
🤖 Prompt for AI Agents
In `@specs/openapi.yaml` around lines 17 - 38, The components schemas Chain,
NativeAddress, and ChainAddress are defined but never referenced, causing
duplicated inline definitions; update all properties that currently inline
chain, address, or chain+address objects to reference the shared components
instead (use $ref: '#/components/schemas/Chain' for any chain property, $ref:
'#/components/schemas/NativeAddress' for address strings, and $ref:
'#/components/schemas/ChainAddress' for composite chain+address objects), and
either remove the unused NativeAddress if truly unnecessary or replace all
inline address fields to reference it to avoid dead code and future duplication.
0xrcinus
left a comment
0xrcinus
left a comment
There was a problem hiding this comment.
Small comment about nested & duplicate chain identifiers
| /** Destination contract in EIP-7930 address format */ | ||
| to: Address; | ||
| /** Destination contract in CAIP-350 address format */ | ||
| to: ChainAddress; |
There was a problem hiding this comment.
Does this make sense as a nested chainAddress, or put chainId at the top level?
| * assets: [ | ||
| * { | ||
| * address: "0x000100000101A0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", | ||
| * address: { chain: "eip155:1", address: "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48" }, |
There was a problem hiding this comment.
Is the chain necessary here? This is already constrained to a single network
| * @example { chain: "eip155:1", address: "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48" } - USDC on Ethereum | ||
| */ | ||
| address: Address; | ||
| address: ChainAddress; |
There was a problem hiding this comment.
Does this make sense as a nested object or chain & address at top level?
| * assets: [ | ||
| * { | ||
| * address: "0x000100000101A0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", | ||
| * address: { chain: "eip155:1", address: "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48" }, |
There was a problem hiding this comment.
Same note as above, this feels like we're duplicating the chain a few times unnecessarily
There was a problem hiding this comment.
♻️ Duplicate comments (1)
schemas/typescript/types.ts (1)
14-14:⚠️ Potential issue | 🟡 MinorSolana example uses truncated CAIP-2 form instead of full CAIP-350 identifier.
The Solana mainnet example should use the full 44-character base58btc genesis blockhash per CAIP-350, not the 32-character CAIP-2 truncation.
Proposed fix
- * `@example` "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp" - Solana mainnet + * `@example` "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdpKuc147dw2N9d" - Solana mainnetBased on learnings: "Always use the full 44-char identifier when documenting Solana chain identifiers per CAIP-350."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schemas/typescript/types.ts` at line 14, Update the `@example` Solana chain identifier in schemas/typescript/types.ts to use a full CAIP-350 44-character base58btc genesis blockhash rather than the truncated CAIP-2 form; locate the `@example` string "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp" and replace it with a full 44-character base58btc genesis blockhash per CAIP-350 so the example demonstrates the correct identifier format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@schemas/typescript/types.ts`:
- Line 14: Update the `@example` Solana chain identifier in
schemas/typescript/types.ts to use a full CAIP-350 44-character base58btc
genesis blockhash rather than the truncated CAIP-2 form; locate the `@example`
string "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp" and replace it with a full
44-character base58btc genesis blockhash per CAIP-350 so the example
demonstrates the correct identifier format.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68e8860d-d8f6-4972-91da-9d3dffc9261d
📒 Files selected for processing (4)
schemas/typescript/schemas.generated.tsschemas/typescript/types.tsscripts/generate-openapi.tsspecs/openapi.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- schemas/typescript/schemas.generated.ts
3 participants
Related discussion: #33
Summary by CodeRabbit
New Features
Documentation
Refactor