feat: wire Velora partner fee recipient and safe default fee#436
Open
feat: wire Velora partner fee recipient and safe default fee#436
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdded partnerAddress, partnerFeeBps, and chargeFee across Velora swap API and call paths; introduced normalizePartnerFeeBps validation and defaults in swap constants; propagated chargeFee through swap/leverage/deleverage hooks; and updated the SwapModal UI to show a transaction preview with slippage-aware estimates. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/swap/api/velora.ts (1)
269-306: ValidatepartnerAddressat runtime before building requests.These are tx-critical request boundaries; add a local
isAddressguard forpartnerAddressbefore query/body assembly so malformed runtime input fails fast.Suggested fix
+const normalizePartnerAddress = (partnerAddress: string): Address => { + if (!isAddress(partnerAddress)) { + throw new VeloraApiError('Velora partner address must be a valid EVM address', 400, { partnerAddress }); + } + return partnerAddress as Address; +}; + export const fetchVeloraPriceRoute = async ({ @@ - const effectivePartnerFeeBps = normalizePartnerFeeBps(partnerFeeBps); + const effectivePartnerAddress = normalizePartnerAddress(partnerAddress); + const effectivePartnerFeeBps = normalizePartnerFeeBps(partnerFeeBps); @@ - partnerAddress, + partnerAddress: effectivePartnerAddress, @@ export const buildVeloraTransactionPayload = async ({ @@ - const effectivePartnerFeeBps = normalizePartnerFeeBps(partnerFeeBps); + const effectivePartnerAddress = normalizePartnerAddress(partnerAddress); + const effectivePartnerFeeBps = normalizePartnerFeeBps(partnerFeeBps); @@ - partnerAddress, + partnerAddress: effectivePartnerAddress,As per coding guidelines, "For transaction and position flows: keep endpoint-specific request builders strict..." and "guard null/undefined/stale API and contract fields in all tx-critical paths so malformed data degrades safely".
Also applies to: 340-423
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/swap/api/velora.ts` around lines 269 - 306, In fetchVeloraPriceRoute (and the other Velora request builders in the same file around the 340-423 range), validate partnerAddress with the existing address guard (e.g., isAddress) before assembling URLSearchParams or request bodies; if partnerAddress is missing/invalid, throw a VeloraApiError with a 400 status and include partnerAddress plus context (partner, network) in the error meta so malformed runtime input fails fast. Ensure the same guard is applied to any other tx-critical Velora functions that accept partnerAddress to keep request builders strict.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/swap/api/velora.ts`:
- Around line 138-144: The partner fee validation in normalizePartnerFeeBps is
too weak—replace Number.isInteger check with Number.isSafeInteger and add an
upper bound so partnerFeeBps is between 0 and 10,000 inclusive; on validation
failure continue to throw VeloraApiError with the same message/status but
include the invalid value in the metadata (partnerFeeBps) and use the new
validation logic in the normalizePartnerFeeBps function.
---
Nitpick comments:
In `@src/features/swap/api/velora.ts`:
- Around line 269-306: In fetchVeloraPriceRoute (and the other Velora request
builders in the same file around the 340-423 range), validate partnerAddress
with the existing address guard (e.g., isAddress) before assembling
URLSearchParams or request bodies; if partnerAddress is missing/invalid, throw a
VeloraApiError with a 400 status and include partnerAddress plus context
(partner, network) in the error meta so malformed runtime input fails fast.
Ensure the same guard is applied to any other tx-critical Velora functions that
accept partnerAddress to keep request builders strict.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e0e7493d-1b6d-4e31-8e51-68d2018375f8
📒 Files selected for processing (2)
src/features/swap/api/velora.tssrc/features/swap/constants.ts
Comment on lines
+138
to
+144
| const normalizePartnerFeeBps = (partnerFeeBps: number): number => { | ||
| if (!Number.isInteger(partnerFeeBps) || partnerFeeBps < 0) { | ||
| throw new VeloraApiError('Velora partner fee bps must be a non-negative integer', 400, { partnerFeeBps }); | ||
| } | ||
|
|
||
| return partnerFeeBps; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/features/swap/api/velora.ts | sed -n '130,155p'Repository: antoncoding/monarch
Length of output: 1084
🏁 Script executed:
# Check for any existing safe-integer checks or bps constraints in the codebase
rg -n "isSafeInteger|MAX.*BPS|PARTNER.*FEE" src/features/swap/Repository: antoncoding/monarch
Length of output: 747
🏁 Script executed:
# Look at the full normalizePartnerFeeBps context and its call sites
rg -B5 -A10 "const normalizePartnerFeeBps" src/features/swap/api/velora.tsRepository: antoncoding/monarch
Length of output: 699
🏁 Script executed:
# Check what value range is typical for partner fee bps in configs/constants
rg -n "PARTNER.*FEE|partnerFeeBps" src/features/swap/ -A2 | head -40Repository: antoncoding/monarch
Length of output: 2219
Harden partner fee validation: use isSafeInteger and upper bound.
Number.isInteger alone misses precision constraints. Use Number.isSafeInteger and add an upper bound check (bps cannot exceed 10,000).
Suggested fix
const normalizePartnerFeeBps = (partnerFeeBps: number): number => {
- if (!Number.isInteger(partnerFeeBps) || partnerFeeBps < 0) {
+ const MAX_PARTNER_FEE_BPS = 10_000;
+ if (!Number.isSafeInteger(partnerFeeBps) || partnerFeeBps < 0 || partnerFeeBps > MAX_PARTNER_FEE_BPS) {
throw new VeloraApiError('Velora partner fee bps must be a non-negative integer', 400, { partnerFeeBps });
}
return partnerFeeBps;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const normalizePartnerFeeBps = (partnerFeeBps: number): number => { | |
| if (!Number.isInteger(partnerFeeBps) || partnerFeeBps < 0) { | |
| throw new VeloraApiError('Velora partner fee bps must be a non-negative integer', 400, { partnerFeeBps }); | |
| } | |
| return partnerFeeBps; | |
| }; | |
| const normalizePartnerFeeBps = (partnerFeeBps: number): number => { | |
| const MAX_PARTNER_FEE_BPS = 10_000; | |
| if (!Number.isSafeInteger(partnerFeeBps) || partnerFeeBps < 0 || partnerFeeBps > MAX_PARTNER_FEE_BPS) { | |
| throw new VeloraApiError('Velora partner fee bps must be a non-negative integer', 400, { partnerFeeBps }); | |
| } | |
| return partnerFeeBps; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/swap/api/velora.ts` around lines 138 - 144, The partner fee
validation in normalizePartnerFeeBps is too weak—replace Number.isInteger check
with Number.isSafeInteger and add an upper bound so partnerFeeBps is between 0
and 10,000 inclusive; on validation failure continue to throw VeloraApiError
with the same message/status but include the invalid value in the metadata
(partnerFeeBps) and use the new validation logic in the normalizePartnerFeeBps
function.
coderabbitai
bot
added
the
feature request
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/features/swap/api/velora.ts (1)
138-144:⚠️ Potential issue | 🟡 MinorBound partner fee bps to valid basis-point range and safe integers.
Line 139 currently allows any non-negative integer. This should reject unsafe integers and values above
10_000bps before request building.Suggested fix
const normalizePartnerFeeBps = (partnerFeeBps: number): number => { - if (!Number.isInteger(partnerFeeBps) || partnerFeeBps < 0) { + const MAX_PARTNER_FEE_BPS = 10_000; + if (!Number.isSafeInteger(partnerFeeBps) || partnerFeeBps < 0 || partnerFeeBps > MAX_PARTNER_FEE_BPS) { throw new VeloraApiError('Velora partner fee bps must be a non-negative integer', 400, { partnerFeeBps }); } return partnerFeeBps; };#!/bin/bash set -euo pipefail # Verify current validation logic and whether an upper bound exists. cat -n src/features/swap/api/velora.ts | sed -n '134,150p' # Check related fee constants/usages in swap scope. rg -n -C2 "partnerFeeBps|MAX_PARTNER_FEE_BPS|SWAP_PARTNER_FEE_BPS" src/features/swapAs per coding guidelines: "For transaction and position flows: keep endpoint-specific request builders strict, normalize remote errors into typed app errors, and verify response token/route identities before using previews or tx payloads".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/swap/api/velora.ts` around lines 138 - 144, The normalizePartnerFeeBps function currently only checks for non-negative integers but must also reject unsafe integers and values > 10,000 bps; update normalizePartnerFeeBps to verify Number.isSafeInteger(partnerFeeBps) and partnerFeeBps <= 10000, and if not throw a VeloraApiError (same pattern used currently) including the invalid partnerFeeBps in the error payload so callers can see the offending value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/features/swap/api/velora.ts`:
- Around line 138-144: The normalizePartnerFeeBps function currently only checks
for non-negative integers but must also reject unsafe integers and values >
10,000 bps; update normalizePartnerFeeBps to verify
Number.isSafeInteger(partnerFeeBps) and partnerFeeBps <= 10000, and if not throw
a VeloraApiError (same pattern used currently) including the invalid
partnerFeeBps in the error payload so callers can see the offending value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1fa8c97d-c8af-4058-9bf9-b73e36716046
📒 Files selected for processing (1)
src/features/swap/api/velora.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/features/swap/api/velora.ts (1)
141-147:⚠️ Potential issue | 🟡 MinorUse
isSafeIntegerand add upper bound for partner fee validation.
Number.isIntegeralone passes values outside safe integer range. Bps logically cannot exceed 10,000 (100%).Suggested fix
const normalizePartnerFeeBps = (partnerFeeBps: number): number => { - if (!Number.isInteger(partnerFeeBps) || partnerFeeBps < 0) { + const MAX_PARTNER_FEE_BPS = 10_000; + if (!Number.isSafeInteger(partnerFeeBps) || partnerFeeBps < 0 || partnerFeeBps > MAX_PARTNER_FEE_BPS) { throw new VeloraApiError('Velora partner fee bps must be a non-negative integer', 400, { partnerFeeBps }); } return partnerFeeBps; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/swap/api/velora.ts` around lines 141 - 147, The partner fee validator normalizePartnerFeeBps currently uses Number.isInteger and allows unsafe integers and values >10000; update it to use Number.isSafeInteger(partnerFeeBps) and validate 0 <= partnerFeeBps <= 10000, otherwise throw the same VeloraApiError (include partnerFeeBps in metadata) so out-of-range or unsafe integers are rejected; keep the function signature and return the validated partnerFeeBps.
🧹 Nitpick comments (1)
src/features/swap/components/SwapModal.tsx (1)
389-435: Est/Min receive rows are duplicated; extract a small renderer.Both rows repeat the same tooltip/icon/render branches. A tiny helper component would reduce maintenance noise and keep behavior consistent.
As per coding guidelines: "Run one consolidation pass and remove duplicated logic across files, especially repeated UI blocks, before closing non-trivial changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/swap/components/SwapModal.tsx` around lines 389 - 435, The Est/Min receive blocks in SwapModal are duplicated; create a small reusable component (e.g., ReceiveAmountRow or AmountRow) that takes props like label:string, previewValue (receivePreview | minReceivePreview), zeroPreview (zeroReceivePreview), isQuoting:boolean, and targetToken, and move the shared JSX (tooltip with .full/.compact, TokenIcon usage, and the conditional branches for missing token / quoting) into that component; then replace both duplicated blocks in SwapModal with calls to this new component, ensuring you pass the correct preview prop (receivePreview vs minReceivePreview) and preserve existing classNames and Tooltip/TokenIcon props.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/swap/components/SwapModal.tsx`:
- Around line 294-302: The receivePreview and minReceivePreview memoizers must
be computed only for executable quotes: update both useMemo blocks
(receivePreview, minReceivePreview) to early-return null unless quote exists,
targetToken exists, quote.error is falsy (or null/undefined) and
quote.chainsMatch is true (i.e., only when the quote is in an executable state);
keep using formatTokenAmountPreview and withSlippageFloor for the actual
formatting but gate their invocation behind these checks and keep
swapSlippageBps in the minReceivePreview dependency array.
---
Duplicate comments:
In `@src/features/swap/api/velora.ts`:
- Around line 141-147: The partner fee validator normalizePartnerFeeBps
currently uses Number.isInteger and allows unsafe integers and values >10000;
update it to use Number.isSafeInteger(partnerFeeBps) and validate 0 <=
partnerFeeBps <= 10000, otherwise throw the same VeloraApiError (include
partnerFeeBps in metadata) so out-of-range or unsafe integers are rejected; keep
the function signature and return the validated partnerFeeBps.
---
Nitpick comments:
In `@src/features/swap/components/SwapModal.tsx`:
- Around line 389-435: The Est/Min receive blocks in SwapModal are duplicated;
create a small reusable component (e.g., ReceiveAmountRow or AmountRow) that
takes props like label:string, previewValue (receivePreview |
minReceivePreview), zeroPreview (zeroReceivePreview), isQuoting:boolean, and
targetToken, and move the shared JSX (tooltip with .full/.compact, TokenIcon
usage, and the conditional branches for missing token / quoting) into that
component; then replace both duplicated blocks in SwapModal with calls to this
new component, ensuring you pass the correct preview prop (receivePreview vs
minReceivePreview) and preserve existing classNames and Tooltip/TokenIcon props.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 55928b78-bcc1-49f1-9251-b87d25f2c167
📒 Files selected for processing (8)
src/features/swap/api/velora.tssrc/features/swap/components/SwapModal.tsxsrc/features/swap/constants.tssrc/features/swap/hooks/useVeloraSwap.tssrc/hooks/useDeleverageQuote.tssrc/hooks/useDeleverageTransaction.tssrc/hooks/useLeverageQuote.tssrc/hooks/useLeverageTransaction.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/swap/constants.ts
antoncoding
force-pushed
the
feat/velora-partner-fee
branch
from
8be21ef to
55c76d2
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/features/swap/components/SwapModal.tsx (1)
391-410:⚠️ Potential issue | 🟡 MinorFallback to zero preview can mislead.
When
receivePreviewis null (error/chain mismatch), UI showszeroReceivePreviewinstead of "-". Users might think they're getting zero tokens rather than "quote unavailable".Consider showing "-" when preview is null:
- <span className="tabular-nums inline-flex items-center gap-1.5"> - <Tooltip content={<span className="text-xs">{(receivePreview ?? zeroReceivePreview).full}</span>}> - <span className="cursor-help border-b border-dotted border-white/40"> - {(receivePreview ?? zeroReceivePreview).compact} - </span> - </Tooltip> + ) : !receivePreview ? ( + <span className="text-right">-</span> + ) : ( + <span className="tabular-nums inline-flex items-center gap-1.5"> + <Tooltip content={<span className="text-xs">{receivePreview.full}</span>}> + <span className="cursor-help border-b border-dotted border-white/40"> + {receivePreview.compact} + </span> + </Tooltip>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/swap/components/SwapModal.tsx` around lines 391 - 410, The current render falls back to zeroReceivePreview when receivePreview is null, which can mislead users; update the conditional in SwapModal (the JSX block using receivePreview, zeroReceivePreview, isQuoting, and targetToken) to treat a null receivePreview as "quote unavailable" by showing "-" instead of using zeroReceivePreview: specifically, change the predicate so it checks for receivePreview presence (e.g., !targetToken || !receivePreview) before deciding to render "-" and only use zeroReceivePreview when you explicitly have a guaranteed zero-preview object, while preserving the isQuoting branch and the TokenIcon/rendering behavior when a real receivePreview exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/features/swap/components/SwapModal.tsx`:
- Around line 391-410: The current render falls back to zeroReceivePreview when
receivePreview is null, which can mislead users; update the conditional in
SwapModal (the JSX block using receivePreview, zeroReceivePreview, isQuoting,
and targetToken) to treat a null receivePreview as "quote unavailable" by
showing "-" instead of using zeroReceivePreview: specifically, change the
predicate so it checks for receivePreview presence (e.g., !targetToken ||
!receivePreview) before deciding to render "-" and only use zeroReceivePreview
when you explicitly have a guaranteed zero-preview object, while preserving the
isQuoting branch and the TokenIcon/rendering behavior when a real receivePreview
exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1fbfb795-8b90-4f27-bf74-11cba1454193
📒 Files selected for processing (8)
src/features/swap/api/velora.tssrc/features/swap/components/SwapModal.tsxsrc/features/swap/constants.tssrc/features/swap/hooks/useVeloraSwap.tssrc/hooks/useDeleverageQuote.tssrc/hooks/useDeleverageTransaction.tssrc/hooks/useLeverageQuote.tssrc/hooks/useLeverageTransaction.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/swap/hooks/useVeloraSwap.ts
- src/hooks/useDeleverageQuote.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fee semantics
Validation
Summary by CodeRabbit
New Features
Improvements