Skip to content

fix: revert Arbitrum unchained OpenAPI spec pinning - #11092

Merged
kaladinlight merged 3 commits into
developfrom
feat_revert_arbitrum_unchained_openapi_spec
Nov 14, 2025
Merged

fix: revert Arbitrum unchained OpenAPI spec pinning#11092
kaladinlight merged 3 commits into
developfrom
feat_revert_arbitrum_unchained_openapi_spec

Conversation

@gomesalexandre

@gomesalexandre gomesalexandre commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Description

Reverts c7ced5c - to be merged when upstream spec is happy again

Issue (if applicable)

closes N/A

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

None

Testing

  • CI is happy

Engineering

  • ^

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)
  • ^

Screenshots (if applicable)

Summary by CodeRabbit

  • Chores

    • Updated Arbitrum coin stack configuration to fetch API specifications from the development branch instead of a fixed commit, enabling continuous updates
  • Refactor

    • Standardized internal type definitions across Solana parser test utilities to maintain consistency and improve code maintainability

@coderabbitai

coderabbitai Bot commented Nov 14, 2025

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Updated type imports in Solana parser test files from Token to TokenDetails and changed the Arbitrum coin stack configuration to reference the develop branch URL instead of a fixed commit for code generation.

Changes

Cohort / File(s) Change Summary
Arbitrum Coin Stack Configuration
packages/unchained-client/openapitools.json
Updated inputSpec for arbitrum coin stack from fixed commit URL to develop branch URL for dynamic code generation source.
Solana Parser Test Type Updates
packages/unchained-client/src/solana/parser/__tests__/mockData/tokens.ts, packages/unchained-client/src/solana/parser/__tests__/solana.test.ts
Type import rebinding: changed from Token to TokenDetails while maintaining local alias names. Updated mock factory type annotation to reflect new type.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Straightforward type alias rebindings across test files with consistent patterns
  • Configuration update with minimal scope (single coin stack reference)
  • No logic modifications, only type import adjustments

Poem

🐰 With whiskers twitching, I spy the change,
TokenDetails hops into range,
From Token old to details new,
The types align, the tests ring true,
Arbitrum's branch will pave the way,
Hopping forward, come what may! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly describes the main change: reverting the Arbitrum OpenAPI spec pinning. It accurately summarizes the primary purpose of the changeset across all modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat_revert_arbitrum_unchained_openapi_spec

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d5bf283 and 396f518.

📒 Files selected for processing (3)
  • packages/unchained-client/openapitools.json (1 hunks)
  • packages/unchained-client/src/solana/parser/__tests__/mockData/tokens.ts (1 hunks)
  • packages/unchained-client/src/solana/parser/__tests__/solana.test.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10232
File: packages/unchained-client/openapitools.json:61-61
Timestamp: 2025-08-08T10:23:16.843Z
Learning: In shapeshift/web, for temporary “monkey patch” PRs (e.g., packages/unchained-client/openapitools.json using jsDelivr CDN refs like cosmos/mayachain), gomesalexandre is fine with branch-based URLs and does not want SHA pinning. Treat this as a scoped exception to their general preference for pinned dependencies/refs.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10232
File: packages/unchained-client/openapitools.json:230-230
Timestamp: 2025-08-08T10:23:06.773Z
Learning: In shapeshift/web, for temporary monkey patches (e.g., OpenAPI inputSpec URLs in packages/unchained-client/openapitools.json), gomesalexandre is not concerned about commit SHA pinning; tag-based CDN URLs (e.g., jsDelivr branch) are acceptable during the temporary period.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:27:02.203Z
Learning: In shapeshift/web, while gomesalexandre generally prefers pinned dependencies for security, semver with a caret range (^7.7.2) is acceptable as an exception since it's already a transient dependency of many packages and is being made explicit for hdwallet support.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
📚 Learning: 2025-08-08T10:23:06.773Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10232
File: packages/unchained-client/openapitools.json:230-230
Timestamp: 2025-08-08T10:23:06.773Z
Learning: In shapeshift/web, for temporary monkey patches (e.g., OpenAPI inputSpec URLs in packages/unchained-client/openapitools.json), gomesalexandre is not concerned about commit SHA pinning; tag-based CDN URLs (e.g., jsDelivr branch) are acceptable during the temporary period.

Applied to files:

  • packages/unchained-client/openapitools.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install and Cache
🔇 Additional comments (4)
packages/unchained-client/openapitools.json (1)

152-152: Approved: Revert arbitrum spec pinning to develop branch.

This change aligns the arbitrum inputSpec with the pattern used across all other coinstacks and successfully reverts the previous commit pinning. The develop branch URL is consistent with your established precedent for temporary spec stabilization phases in this configuration file (per learnings from PR #10232). CI confirms the generated code surfaces remain unaffected.

packages/unchained-client/src/solana/parser/__tests__/solana.test.ts (2)

15-15: LGTM! Mock type updated consistently.

The mock function return type correctly updated to match the new TokenDetails import.


5-5: Import path is incorrect and will fail compilation.

The import import type { ParsedTx, TokenDetails } from '../../parser' resolves to src/parser/index.ts, which only exports export * as thormaya from './thormaya'. These types are not available from that module.

ParsedTx, TokenDetails, and V1Api are defined in src/solana/parser/types.ts and re-exported from src/solana/parser/index.ts. The import should use:

  • '../types' to import from src/solana/parser/types.ts, or
  • '..' to import from src/solana/parser/index.ts
⛔ Skipped due to learnings
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/pages/Assets/Asset.tsx:1-1
Timestamp: 2025-08-05T22:41:35.473Z
Learning: In the shapeshift/web codebase, component imports use direct file paths like '@/components/ComponentName/ComponentName' rather than barrel exports. The AssetAccountDetails component should be imported as '@/components/AssetAccountDetails/AssetAccountDetails', not from a directory index.
packages/unchained-client/src/solana/parser/__tests__/mockData/tokens.ts (1)

2-2: Fix the broken import path - TokenDetails cannot be imported from non-existent module.

The import import type { TokenDetails as ApiToken } from '../../types' is broken:

  • The module path '../../types' resolves to a non-existent directory
  • TokenDetails is not defined anywhere in the codebase
  • The test file successfully imports TokenDetails from '../../parser' instead

Either use the correct import path from the parser module or define/export TokenDetails from an appropriate location. The current import will cause build/compilation errors.

⛔ Skipped due to learnings
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/pages/Assets/Asset.tsx:1-1
Timestamp: 2025-08-05T22:41:35.473Z
Learning: In the shapeshift/web codebase, component imports use direct file paths like '@/components/ComponentName/ComponentName' rather than barrel exports. The AssetAccountDetails component should be imported as '@/components/AssetAccountDetails/AssetAccountDetails', not from a directory index.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/components/MultiHopTrade/components/TradeInput/components/HighlightedTokens.tsx:14-14
Timestamp: 2025-08-08T15:00:22.321Z
Learning: In shapeshift/web reviews for NeOMakinG, avoid nitpicks to change deep-relative imports to '@/…' alias paths within feature/non-refactor PRs; defer such style-only changes to a dedicated follow-up refactor unless they fix an issue.

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.

❤️ Share

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

@kaladinlight
kaladinlight marked this pull request as ready for review November 14, 2025 16:02
@kaladinlight
kaladinlight requested a review from a team as a code owner November 14, 2025 16:02
@kaladinlight
kaladinlight enabled auto-merge (squash) November 14, 2025 16:03
@gomesalexandre gomesalexandre changed the title fix: revert Arbitrum unchaiend OpenAPI spec pinning fix: revert Arbitrum unchained OpenAPI spec pinning Nov 14, 2025
@kaladinlight
kaladinlight merged commit b023a92 into develop Nov 14, 2025
4 checks passed
@kaladinlight
kaladinlight deleted the feat_revert_arbitrum_unchained_openapi_spec branch November 14, 2025 16:11
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.

2 participants