Conversation
WalkthroughDocumentation-only changes update multichain asset identity, tokenized-assets and freeze-status contracts, issuer CLI requirements, planned architecture, and Alpaca redemption endpoint documentation. ChangesMultichain documentation updates
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
0xgleb
changed the title
5443d22 to
738ad9b
Compare
0xgleb
changed the title
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/multichain-implementation-plan.md`:
- Around line 68-74: Slice 2 currently covers the SQL migration/view rebuild for
the new TokenizedAsset aggregate id, but it also needs the aggregate-store
rekeying/backfill plan because the live write path still uses the old
underlying-only key. Update the plan to explicitly include rekeying the
store.send() path in TokenizedAsset handling and the POST response flow in
src/tokenized_asset/api.rs so it keys and returns {underlying}:{network} instead
of just underlying, and call out a backfill/migration step to prevent
cross-chain collisions.
In `@SPEC.md`:
- Around line 2269-2302: Add a cutover note for the `?network=` breaking change
in the internal tokenized-asset endpoints. Clarify in the multi-chain section
whether Slice 2 requires a lockstep deploy with the client update or a
versioned/dual-read transition, since `GET /tokenized-assets/{underlying}` and
`.../status` now depend on `network` and will return 422 without it. Reference
the existing `TokenizedAsset` aggregate id and the internal endpoint contract so
readers understand how existing callers should migrate.
- Around line 944-945: Clarify the Slice 2 spec around row merging in SPEC.md:
the behavior in the underlying/token merge section changes the contract because
coalescing duplicate (underlying, token) entries alters response cardinality and
ordering. Update the wording to explicitly mark this as a breaking semantic
change, and define deterministic ordering for the tokens and networks fields so
clients can rely on stable results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7fdce01b-38f1-47a5-8fbc-78141580d3d2
📒 Files selected for processing (2)
SPEC.mddocs/multichain-implementation-plan.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
⚙️ CodeRabbit configuration file
Focus on the contents of the docs and not on cosmetic things like markdown formatting. We use markdown files for various docs including but not limited to the north star system specification, SPEC.md, the plan for how to get there, ROADMAP.md, guidelines for AI contributors, AGENTS.md, project overview and instructions for human contributors, README.md. Think about the target audience of a document when deciding what comment to leave. For specifications and designs, suggest potential product, architectural, and UI/UX improvements. For plans, suggest changes that would make things more parallelizable and deliverable-focused. For instructions, suggest better rules and guidelines and point out missing instructions. In all cases, flag needless bloat, prefer clear concise writing, and consider the structure of the document and order of the sections
Files:
docs/multichain-implementation-plan.mdSPEC.md
*
⚙️ CodeRabbit configuration file
Focus on providing constructive criticism. Whenever you see a suboptimal approach, suggest more idiomatic or robust alternative(s). Flag potential footguns. Suggest FP alternatives to mutable/imperative code. Point out architectural flaws like leaky abstractions, tight coupling, wrong level of abstraction, poor type modeling, over-abstraction, unclear domain boundaries. Code should generally be organized based on business concerns rather than technical aspects - suggest improvements if you find violations. Point out gaps in test coverage but suggest tests that are not too coupled to the implementation and actually test domain invariants and business logic
Files:
SPEC.md
🧠 Learnings (1)
📚 Learning: 2026-02-14T15:59:50.889Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 118
File: docs/apalis.md:81-94
Timestamp: 2026-02-14T15:59:50.889Z
Learning: In the ST0x-Technology/st0x.issuance repository, markdown files should not be linted for formatting issues. Do not suggest or enforce Markdown linting rules (e.g., MD040) or cosmetic markdown fixes in docs/*.md files; treat Markdown formatting as outside the review scope for this repo.
Applied to files:
docs/multichain-implementation-plan.md
738ad9b to
233c8c8
Compare
233c8c8 to
f79828b
Compare
2f48519 to
7151ff9
Compare
hardyjosh
left a comment
hardyjosh
left a comment
There was a problem hiding this comment.
Reviewed with Josh. The design is sound — the slice boundaries with per-slice deploy gates are right, and "registry miss → typed failure, never fall back to Base" is exactly the invariant that matters (a mint routed to the wrong chain is the catastrophic failure mode). Also verified the Alpaca list-response change against src/tokenized_asset/api.rs on main — it is a genuine doc fix, the code already serves { tokens, networks[] }.
One stack-level question: this docs-only gate PR sits on feat/dynamic-asset-monitoring → refactor/apalis, so design sign-off can only merge after two feature stacks land. If ChainRegistry does not genuinely build on dynamic asset monitoring, consider rebasing the gate (and Slice 1) onto main so the design record isn't hostage to unrelated review cycles.
Inline comments for the rest. The cross-repo Slice 2 item now has a Linear issue: RAI-1212.
| `…/status` require `?network=` (422 if missing). Alpaca ITN list | ||
| (`GET /tokenized-assets`) keeps `{ tokens, networks[] }`. | ||
|
|
||
| **Cutover:** Lockstep deploy — issuance, `st0x-issuance-client`, and liquidity |
There was a problem hiding this comment.
What is the rollback story for the lockstep window? Three deployables ship together with no transition — if issuance rolls back after liquidity deploys (or vice versa), the freeze guard is getting 422s. Does it fail open or fail closed?
A one-release window where ?network= is optional-defaulting-to-base would decouple the three deploys, and it doesn't violate invariant 2 (that is about persisted-aggregate runtime lookups, not API param defaults). If lockstep is still preferred for simplicity, fine — but the freeze-guard 422 behaviour should be stated here as part of the cutover plan.
There was a problem hiding this comment.
Documented in SPEC.md Rollback (lines 2328–2332): roll back issuance, client, and liquidity together; a mixed-version window fail-closes liquidity rebalancing on 422 until restored.
|
|
||
| - SPEC multichain section matches this plan (AssetKey, API break, deploy | ||
| constraints). | ||
| - Alpaca Phase 0 comms sent |
There was a problem hiding this comment.
Resolved from Alpaca's public docs — no comms needed for the mechanics (mint carries network, redemption is our callback with network + tx_hash, list shape matches what we already serve). Findings recorded on RAI-1099; the issue is closed.
The one live constraint: Alpaca's network enum is solana, base, arbitrum, ethereum, binance, ton, tron, mantle (+ cronos) — no Tempo, no HyperEVM. That is now a comment on RAI-1094; we ask Alpaca to add a value once the target chain is decided. Suggest rewording this gate item to "Alpaca network enum covers the chosen chain (RAI-1094)".
There was a problem hiding this comment.
Acknowledged — Alpaca mint carries network; redemption is our callback with network + tx_hash. No HyperEVM in the Alpaca enum; informational only.
| constraints). | ||
| - Alpaca Phase 0 comms sent | ||
| ([RAI-1099](https://linear.app/makeitrain/issue/RAI-1099)). | ||
| - [RAI-1205](https://linear.app/makeitrain/issue/RAI-1205) migration runbook |
There was a problem hiding this comment.
The RAI-1205 rekey (events.aggregate_id + snapshots, in place, on the prod event store) is the single scariest operation in this plan, and the runbook it gates on doesn't exist yet. Suggest making the runbook a deliverable of the RAI-1205 PR itself, and requiring it to cover: backup/restore, a dry run against a copy of the prod DB, and idempotency (safe to re-run if it dies mid-migration).
There was a problem hiding this comment.
Runbook added on the asset-key branch: docs/runbooks/tokenized-asset-aggregate-rekey.md. The plan cites this path in the RAI-1205 section.
| and `subgraph_url` from per-chain config — same fields as legacy single-chain | ||
| startup. | ||
|
|
||
| **Cross-repo ([RAI-1205](https://linear.app/makeitrain/issue/RAI-1205) + |
There was a problem hiding this comment.
This coordinated liquidity change is now tracked as RAI-1212 (blocks the RAI-1205 deploy) — worth citing here so it doesn't get lost.
There was a problem hiding this comment.
Cited in the plan cross-repo section and SPEC cutover note — RAI-1212 blocks the RAI-1205 deploy window.
7151ff9 to
01dc95e
Compare
af9a550 to
54d9c89
Compare
7c23bb7 to
6152132
Compare
54d9c89 to
24bea0f
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| 2. `POST /v1/accounts/{account_id}/tokenization/callback/redeem` - Initiate | ||
| redemption |
There was a problem hiding this comment.
The endpoint path changed to a callback endpoint (/tokenization/callback/redeem) but the description still says "Initiate redemption". Callback endpoints are for confirmations/completions, not initiations (as shown by the mint callback on line 309 which says "Confirm mint completed"). This creates a contradiction.
2. `POST /v1/accounts/{account_id}/tokenization/callback/redeem` - Confirm redemption
completedThe description should be updated to match the callback pattern, or if the intent is to keep "Initiate redemption", the endpoint path should be /tokenization/redeem (not /tokenization/callback/redeem).
| 2. `POST /v1/accounts/{account_id}/tokenization/callback/redeem` - Initiate | |
| redemption | |
| 2. `POST /v1/accounts/{account_id}/tokenization/callback/redeem` - Confirm redemption | |
| completed | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
6152132 to
80ba089
Compare
24bea0f to
70dcef6
Compare
82b0808 to
dbb137c
Compare
70dcef6 to
694a78c
Compare
Merge activity
|
github-actions
Bot
added
the
externally-merged
4 participants
Motivation
Before multichain implementation PRs land, the approved design needs to be
recorded in SPEC.md so the stack follows
docs/workflow.md. Delivery tracking(stack map, external gates, acceptance criteria) lives on the Linear umbrella
RAI-1098, not in repo docs.
Implements RAI-1203. First PR in the multichain MVP stack.
Solution
semantics, asset-identity break, cutover/rollback constraints, invariants,
rejected alternatives, and the legacy flat-var config sunset (target
CHAIN_<NETWORK>_*shape)GET /tokenized-assetsresponse to{ tokens, networks[] }Checks
By submitting this for review, I'm confirming I've done the following:
cargo testandcargo clippySummary by CodeRabbit
New Features
Bug Fixes
Documentation
{underlying}:{network}asset key and query requirements.