fix: map eth-flow order sell token #154
Conversation
WalkthroughAdds Eth Flow order awareness to trade notifications: a new isEthFlowOrder flag is propagated from getTradeNotifications to fromTradeToNotification, which uses it to resolve sellToken as the chain native currency for Eth Flow orders; otherwise, it continues fetching the ERC-20 sell token. Imports chain metadata for native currency lookup. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GTN as getTradeNotifications
participant Repo as erc20Repository
participant SDK as ALL_SUPPORTED_CHAINS_MAP
participant FTN as fromTradeToNotification
Note over GTN: Determine isEthFlowOrder (owner ∈ ethFlowAddresses)
GTN->>FTN: fromTradeToNotification({..., isEthFlowOrder, sellTokenAddress, chainId, ...})
alt isEthFlowOrder == true
FTN->>SDK: Lookup nativeCurrency by chainId
SDK-->>FTN: nativeCurrency (sellToken)
else
FTN->>Repo: getErc20ByAddress(sellTokenAddress)
Repo-->>FTN: ERC-20 sellToken
end
FTN-->>GTN: Notification payload
Note over FTN,GTN: Control flow unchanged otherwise
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/notification-producer/src/producers/trade/fromTradeToNotification.ts (1)
45-50: Guard against missing chain metadata; provide safe fallback.If a new chain is introduced without nativeCurrency in ALL_SUPPORTED_CHAINS_MAP, this will throw. Add a fallback to ERC-20 lookup and log once.
- const sellToken = isEthFlowOrder - ? ALL_SUPPORTED_CHAINS_MAP[chainId].nativeCurrency - : await erc20Repository.get( - chainId, - getAddress(sellTokenAddress) - ); + const chainMeta = ALL_SUPPORTED_CHAINS_MAP[chainId]; + const sellToken = isEthFlowOrder + ? (chainMeta?.nativeCurrency ?? + (await erc20Repository.get(chainId, getAddress(sellTokenAddress)))) + : await erc20Repository.get(chainId, getAddress(sellTokenAddress)); + if (isEthFlowOrder && !chainMeta?.nativeCurrency) { + logger.warn(`${prefix} Missing nativeCurrency for chainId=${chainId}; fell back to ERC-20 sellTokenAddress.`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/notification-producer/src/producers/trade/fromTradeToNotification.ts(4 hunks)apps/notification-producer/src/producers/trade/getTradeNotifications.ts(1 hunks)
⏰ 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). (4)
- GitHub Check: docker (apps/api)
- GitHub Check: docker (apps/notification-producer)
- GitHub Check: docker (apps/telegram)
- GitHub Check: docker (apps/twap)
🔇 Additional comments (3)
apps/notification-producer/src/producers/trade/getTradeNotifications.ts (1)
116-121: Only one call site for fromTradeToNotification and it already includes isEthFlowOrder
Confirmed via project-wide search that getTradeNotifications.ts is the sole caller and it’s updated correctly.apps/notification-producer/src/producers/trade/fromTradeToNotification.ts (2)
1-1: Import is fine; consider defensive access for chain metadata.Since ALL_SUPPORTED_CHAINS_MAP is used for runtime lookups, ensure callers only pass SupportedChainId values present in this map.
16-16: AllfromTradeToNotificationcalls include the newisEthFlowOrderprop; downstream compatibility verified.
| ? ALL_SUPPORTED_CHAINS_MAP[chainId].nativeCurrency | ||
| : await erc20Repository.get( | ||
| chainId, | ||
| getAddress(sellTokenAddress) |
There was a problem hiding this comment.
erc20 repo already does that behind the scenes.
Just use the 0xeee address instead of the conditional.
Use native token symbol in notification template message instead of wrapped
Summary by CodeRabbit