🐛 server: accept bnb, matic, and mon activity assets - #1278
Conversation
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughThe package configuration adds pnpm overrides for selected dependency versions. The workspace audit configuration adds one GitHub Security Advisory to its ignore list. ChangesDependency configuration
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized change allows BNB, MATIC, and MON activity assets while adding coverage for the new behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1278 +/- ##
==========================================
+ Coverage 72.51% 73.04% +0.53%
==========================================
Files 275 276 +1
Lines 13254 13257 +3
Branches 4610 4613 +3
==========================================
+ Hits 9611 9684 +73
+ Misses 3306 3236 -70
Partials 337 337
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0d9b36b4a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| v.object({ | ||
| category: v.picklist(["external", "internal"]), | ||
| asset: v.literal("ETH"), | ||
| asset: v.picklist(["BNB", "ETH", "MATIC", "MON"]), |
There was a problem hiding this comment.
Bypass the token allowlist for native assets
For BNB_MAINNET, MATIC_MAINNET, and MONAD_MAINNET deliveries, the missing contract address causes each newly accepted native asset to be normalized to the single Exa-chain WETH constant before isKnownToken checks the event chain's LI.FI token set. When that set is nonempty and does not contain the unrelated Exa WETH address, known resolves false and the funds-received notification is suppressed. The added test uses Anvil, where isKnownToken returns true unconditionally, so it cannot expose this production behavior; native assets should bypass the ERC-20 allowlist or use a chain-specific native/wrapped address.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0c54d52ca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "abitype>zod": "^4.0.0", | ||
| "axios@1.16.1": "^1.18.0", | ||
| "brace-expansion@5.0.6": "^5.0.7", | ||
| "browserslist@4": "^4.28.7", |
There was a problem hiding this comment.
Restore the native-asset webhook changes
When Alchemy sends an external or internal transfer for BNB, MATIC, or MON, server/hooks/activity.ts:109-112 still accepts only asset: "ETH", so validatorHook rejects the event before account deployment, collateral processing, or notification. Fresh evidence relative to the earlier review is that this target has no server diff at all—its only changes are dependency and audit metadata—so the advertised native-asset fix has disappeared from the proposed commit.
Useful? React with 👍 / 👎.
Summary by CodeRabbit