Charge and collect vortex/partner fees on Alfredpay corridors#1288
Open
ebma wants to merge 6 commits into
Open
Charge and collect vortex/partner fees on Alfredpay corridors#1288ebma wants to merge 6 commits into
ebma wants to merge 6 commits into
Conversation
The Alfredpay onramp/offramp routes build no distributeFees transaction, so vortex and partner-markup quote components could never be collected on-chain: the onramp deducted them from the user's output and stranded the difference on the ephemeral, while the offramp displayed them without charging anyone. Force both components to zero in the Alfredpay fee engines so displayed fees match what is actually charged; only the Alfredpay provider fee applies. Re-introducing these fees requires a fee-distribution phase for the routes first.
✅ Deploy Preview for vortexfi ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vrtx-dashboard canceled.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Zeros uncollectable Vortex and partner fees for Alfredpay corridors so quotes match transaction behavior.
Changes:
- Forces both fee components to zero in Alfredpay fee engines.
- Adds MXN corridor regression tests.
- Documents the fee-distribution limitation and follow-up path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
docs/security-spec/03-ramp-engine/fee-integrity.md |
Documents Alfredpay fee handling. |
apps/api/src/tests/corridors/mxn-onramp.scenario.test.ts |
Tests onramp fee output. |
apps/api/src/tests/corridors/mxn-offramp.scenario.test.ts |
Tests offramp fee output. |
apps/api/src/api/services/quote/engines/fee/onramp-alfredpay-to-evm.ts |
Zeros onramp platform fees. |
apps/api/src/api/services/quote/engines/fee/offramp-evm-to-alfredpay.ts |
Zeros offramp platform fees. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Both Alfredpay directions now charge vortex/partner-markup fees and collect them on-chain, replacing the interim zeroing: - onramp deducts the components from the Polygon mint (user-facing legs move mint minus fees); offramp deducts them from the bridged USD leg before pricing the Alfredpay payout - the fee residual on the Polygon ephemeral is paid out by a new distributeFees phase that runs only after the user-facing leg succeeded, as plain sequential ephemeral-signed USDT transfers (Multicall3 aggregate3 cannot move the caller's tokens); per-nonce hashes make retries idempotent - failure paths never collect: the mint fallback stays full-mint and the offramp refund fallback is sized deposit plus fees - both discount engines subtract the charged fee from the subsidy's expected-output target so a discount subsidy cannot refill the fee - offramp finalSettlementSubsidy tops the ephemeral up to deposit plus fees so the fee transfers are always covered
ebma
changed the title
… base Multicall3.aggregate3 executes calls with the Multicall3 contract as msg.sender, so the partner-split path's batched ERC-20 transfers would move the contract's (empty) balance, not the ephemeral's — the batch reverts and any Base ramp with nonzero partner markup plus a configured partner payout address fails at distributeFees. The Base builder now emits one plain ephemeral-signed transfer per fee recipient at consecutive nonces via the same implementation as the Alfredpay/Polygon builder, and the distribute-fees handler broadcasts EVM fee transfers with per-nonce hash idempotency on both networks (the legacy single distributeFeeHash is honored for in-flight ramps). Payout addresses are now only required when a quote carries nonzero fees.
… partner payout Review findings on the sequential fee distribution: - updateRamp merged presigned transactions by phase/network/signer only, so the two distributeFees transfers of a split-fee quote overwrote each other and /start rejected the ramp as incomplete. The nonce is now part of the replacement identity, and the Base split-fee corridor test drives presigns through the real /v1/ramp/update and /v1/ramp/start flow. - Alfredpay corridors charged partner markup even when the pricing partner had no payout_address_evm, silently stranding the charged portion on the Polygon ephemeral. requiresEvmPartnerPayout now covers the Alfredpay fiats so such quotes fail closed at creation, and the fee-distribution builder throws instead of warn-dropping a charged markup as defense in depth.
…lize amounts Review findings on the sequential fee distribution: - the distribute-fees handler required the FULL original fee total on the ephemeral before consulting per-nonce hashes, so any retry after a partial success (or a crash after all transfers) waited on a balance that could never return and wedged the phase forever. Paid transfers are now reconciled first and only the unpaid amount — decoded from the pending presigned transfers — is required (legacy non-transfer presigns fall back to the full total). - the offramp deducted the full-precision preNabla fee while distribution moved metadata-derived amounts rounded to 2 fiat decimals, so deposit, fallback, settlement target and transfers could disagree by dust. All consumers now derive the charged total from one component-wise raw computation, and the offramp deduction applies the same rounding and price-feed conversion as the persisted metadata: bridged amount equals Alfredpay deposit plus distributed fees in exact raw units. - fee transfers carried the destination-transfer builder's 3x-buffered gas fields and the SDK multiplies by 3 again at signing (9x); the builder now emits unbuffered estimates so only the SDK buffer applies.
Comment on lines
+72
to
+76
| const feeFiatRounded = (ctx.preNabla?.deductibleFeeAmountInFeeCurrency ?? new Big(0)).round(2); | ||
| const feeUsd = await priceFeedService.convertCurrency( | ||
| feeFiatRounded.toString(), | ||
| ctx.preNabla?.feeCurrency ?? (outputCurrency as RampCurrency), | ||
| EvmToken.USDC as RampCurrency |
Comment on lines
+201
to
+207
| const status = await this.checkEvmTransactionStatus(existingHash, network); | ||
| if (status === ExtrinsicStatus.Success) { | ||
| logger.info(`Fee distribution transfer at nonce ${feeTx.nonce} already succeeded for ramp ${state.id}, skipping.`); | ||
| continue; | ||
| } | ||
| } | ||
| pendingTxs.push(feeTx); |
Comment on lines
+302
to
+306
| if (request.rampType === RampDirection.SELL && isAlfredpayToken(request.outputCurrency as FiatToken)) { | ||
| return true; | ||
| } | ||
| if (request.rampType === RampDirection.BUY && isAlfredpayToken(request.inputCurrency as FiatToken)) { | ||
| return true; |
…arkup quotes Copilot review findings: - the offramp deduction rounded the aggregate preNabla fee while the persisted metadata rounds vortex and partner components separately, so two sub-cent components could deduct half of what distribution moves. preNabla now exposes the unrounded components and the Alfredpay deduction rounds, converts, and floors each one exactly like computeFeeComponentRaws. - a mined-but-reverted fee transfer consumed its nonce, yet the handler rebroadcast the same primary presign forever. A Fail receipt now fails the phase unrecoverably for manual intervention via the presigned backups; only unknown hashes are rebroadcast. - the EVM-payout quote guard fired on markupType alone, rejecting legal zero-value markup configs that charge nothing; it now also requires a positive markupValue (Big-compared — Sequelize DECIMALs are strings).
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
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
The Alfredpay corridors (USDT-based MXN/COP/ARS/USD on- and off-ramps) displayed vortex and partner-markup fees but never collected them on-chain — their routes built no
distributeFeestransaction. This PR lands the fix in two commits:How collection works
distributeFeesphase (last beforecomplete, entered fromdestinationTransfer/alfredpayOfframpTransferonly when fee transfers exist) broadcasts plain sequential ephemeral-signed USDT transfers: network+vortex to the vortexpayout_address_evm(with the env fallback), partner markup to the pricing partner's address. Per-nonce hashes make retries idempotent. The SDK signs the new unsigned txs generically — no SDK changes.finalSettlementSubsidy(SELL) tops the ephemeral up to deposit + fees so the fee transfers are always covered.finalSettlementSubsidy.aggregate3executes calls with the Multicall3 contract asmsg.sender, so a batched ERC-20transfercannot move the ephemeral's tokens. This also means the existing Base partner-split path is broken — documented as a new open checklist item infee-integrity.md(separate fix).Notes for reviewers
nextPhaseSelector); theseed:phase-metadatascript references a deleted file and the CLAUDE.md claim about DB-seeded transitions is stale.validation.tsnow classifiesdistributeFeeson Polygon as an EVM tx (was Substrate-by-default outside Base).world.squidRouter.toTokenDecimals = 6; the previous 18-decimal default shrank the bridged amount to ~0 and the unbounded-subsidy bug silently padded it back, masking fee math.codex/security-spec-review-2026-07-24rewrite offee-integrity.md— port the Alfredpay section + the new Base Multicall3 item when the branches meet.Follow-up: broken Multicall3 partner split on Base (fixed here too)
While implementing the Polygon distribution it turned out the existing Base partner-split path was broken: it batched plain ERC-20
transfercalls throughMulticall3.aggregate3, which executes with the Multicall3 contract asmsg.sender— the batch could only move the contract's (empty) balance and would revert, failing any Base ramp with nonzero partner markup plus a configured partner payout address. Third commit:contracts/Multicall3.tsis deleted (no remaining users).distributeFeeHashis honored for in-flight ramps.brl-offramp.scenario.test.ts: a profile-priced partner (markup + vortex fee, 1 USDC each) — both payout addresses receive their exact fee on the fake ledger. The old aggregate3 batch credits nobody, so the assertions fail without the fix.fee-integrity.md: open checklist item resolved; the "Distribution Mechanisms" section now describes the sequential-transfer scheme.Test plan
distributeFeesin the phase history, and the vortex payout address receives the exact fee on the fake ledger.bun typecheckand Biome clean.