Skip to content

fix(currency): wrong ID retrieval for native USDC - #1774

Open
alexandre-abrioux-rf wants to merge 2 commits into
masterfrom
fix-currency-manager-id
Open

alexandre-abrioux-rf wants to merge 2 commits into
masterfrom
fix-currency-manager-id

Conversation

@alexandre-abrioux-rf

@alexandre-abrioux-rf alexandre-abrioux-rf commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Issue

When retrieving the native USDC currency on matic:

  • id: USDCn-matic
  • symbol: USDC
  • chain: matic

; with currencyManager.from(<symbol>, <chain>), it retrieves the wrong currency:

  • id: USDC-matic
  • symbol: USDC.e
  • chain: matic

Changes

  • Added a test to surface the issue
  • Added a fix

@alexandre-abrioux-rf alexandre-abrioux-rf changed the title Fix currency manager fix(currency): wrong detection for native USDC Sep 24, 2026
@alexandre-abrioux-rf alexandre-abrioux-rf changed the title fix(currency): wrong detection for native USDC fix(currency): wrong ID retrieval for native USDC Sep 24, 2026
Comment thread .gitignore
Comment on lines +29 to +30
/coverage/
/reports/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(unrelated) These folders should be ignored / they can appear when running Jest from the root folder

@greptile-apps

greptile-apps Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR should not merge as a fix for default-manager Polygon USDC lookup until the default currency list supports the intended result.

Findings

  1. P1 Default lookup remains wrong ▶

Summary

The PR changes symbol-and-network lookup to use an existing currency’s ID, adds a native Polygon USDC test using a custom list, and ignores root-level test output directories.

  • The test demonstrates the new lookup behavior for a custom manager, but not for the default manager.

Reviews (1) · Last reviewed commit: "fix the test"

Comment thread packages/currency/test/currencyManager.test.ts
Comment on lines +238 to +241
static currencyId(
currency: { symbol: string; network?: string },
knownCurrencies?: CurrencyTypes.CurrencyDefinition[],
): string {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the existing static contract, and introduced an optional argument so an instance of CurrencyManager can pass down its existing known currencies whenever possible. I could not change it to a non-static method easily because it is also used in static fromInput.

@alexandre-abrioux-rf
alexandre-abrioux-rf marked this pull request as ready for review September 24, 2026 13:36
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