Conversation
- Added market buy and sell rates to the fiat currency model for improved pricing strategies. - Updated the GetFiatCurrencies method to include market buy and sell rates in the response. - Refactored the profile update logic to validate and handle new two-sided pricing fields for tokens. - Adjusted the payment order initiation process to support both onramp and offramp flows with clear direction handling. - Improved test coverage for profile updates and payment order initiation to ensure robust functionality with new features.
- Execute payin settlement via Engine SendTransactionBatch with authorizationList and settleIn; return error only (no tx hash wait); indexer sets tx hash and settled - Set payment order status to settling on submit; create order_settling transaction log; remove premature webhook from payin block - Add order_settling and order_refunding to transaction log schema; indexer updates existing logs when settlement/refund events are indexed - Refund flow: create order_refunding log and set StatusRefunding on submit (EVM/Tron) - AcceptOrderError: add Metadata field and pass to Disburse - Remove redundant uint64 checks for rate/senderFee in settleIn; pass *big.Int
📝 WalkthroughWalkthroughAdds V2 onramp/offramp flows, two‑sided buy/sell pricing and RateSide, replaces OrderSettled with SettleOut/SettleIn events, renames return_address → refund_or_recipient_address and adds memo/direction, and updates ent schema, controllers, indexers, rate tasks, webhooks, Redis bucket keys, and builders. Changes
Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant Aggregator
participant Engine
participant Gateway
participant Indexer
rect rgba(100,150,255,0.5)
note over Dashboard,Aggregator: Onramp (payin) fulfillment (EIP‑7702)
Dashboard->>Aggregator: FulfillOrder(direction:onramp, Authorization)
Aggregator->>Aggregator: validate Authorization, build authorizationList
Aggregator->>Aggregator: prepareSettleInCallData(order)
Aggregator->>Engine: SendTransactionBatch(authorizationList + settleIn calldata)
Engine->>Gateway: settleIn(orderId, token, amount, recipient, rate)
Gateway-->>Engine: Emit SettleInEvent (async)
Indexer->>Aggregator: ProcessSettleInOrders(event)
Aggregator->>Aggregator: UpdateOrderStatusSettleIn(event) & create TransactionLog
end
rect rgba(255,150,100,0.5)
note over Dashboard,Aggregator: Offramp (payout) fulfillment
Dashboard->>Aggregator: FulfillOrder(direction:offramp)
Aggregator->>Aggregator: validate rate (sell side)
Aggregator->>Engine: SendTransaction(settleOut calldata)
Engine->>Gateway: settleOut(orderId, token, amount, recipient, rate)
Gateway-->>Engine: Emit SettleOutEvent
Indexer->>Aggregator: ProcessSettleOutOrders(event)
Aggregator->>Aggregator: UpdateOrderStatusSettleOut(event) & create TransactionLog
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
chibie
changed the title
… process - Introduced new event types for SettleOut (offramp) and SettleIn (onramp) to enhance payment order processing. - Updated the controller to handle new event signatures and corresponding logic for SettleOut and SettleIn events. - Refactored existing OrderSettled event handling to accommodate the new event types, ensuring backward compatibility. - Enhanced the indexer to process SettleOut and SettleIn events, updating order statuses accordingly. - Removed deprecated references to OrderSettled in favor of the new event structures, improving clarity and maintainability. - Updated tests to cover new event handling scenarios and ensure robust functionality.
- Modified the SettleIn event to include a liquidity provider field, enhancing the event's data structure for better tracking. - Updated related functions and tests to accommodate the new liquidity provider parameter in the SettleIn event handling. - Refactored indexer and controller logic to ensure compatibility with the updated event structure. - Removed deprecated comments and improved code clarity in the index and provider tests.
- Introduced a new webhook_version field in the sender profile schema, with a default value of 1. - Updated the profile controller to validate and set the webhook version during profile updates. - Enhanced the sender profile creation and update processes to include the webhook version. - Modified the payment order webhook payload to accommodate the webhook version, ensuring backward compatibility with existing structures. - Added necessary predicates and query functions for the new webhook version field in the database layer. - Improved tests to cover the new webhook version functionality and ensure robust handling of different versions.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
services/common/order.go (1)
380-383:⚠️ Potential issue | 🟡 MinorMissing transaction rollback on payment order update failure.
If
paymentOrderUpdate.Save(ctx)fails, the transaction is not rolled back before returning the error. This could leave the transaction in an inconsistent state, though Go's deferred cleanup may handle it in some database drivers.Proposed fix
_, err = paymentOrderUpdate.Save(ctx) if err != nil { + _ = tx.Rollback() return fmt.Errorf("UpdateOrderStatusRefunded.aggregator: %v", err) }tasks/setup_test.go (1)
116-118:⚠️ Potential issue | 🟡 MinorUnreachable duplicate error check.
This error check is unreachable because
errwas already checked and would have returned on line 113-115. The error message also references "CreatePaymentOrderRecipient" which doesn't match the preceding operation.🧹 Proposed fix to remove dead code
Save(context.Background()) if err != nil { return fmt.Errorf("CreatePaymentOrder.tasks_test: %w", err) } - if err != nil { - return fmt.Errorf("CreatePaymentOrderRecipient.tasks_test: %w", err) - }services/order/evm.go (1)
149-223:⚠️ Potential issue | 🟠 MajorPrevent duplicate refunds by excluding
refundingorders.With the new intermediate status, the query should filter out
StatusRefunding; otherwise repeat calls can submit multiple refund transactions for the same order.🛠️ Proposed fix
Where( paymentorder.GatewayIDEQ(orderID), paymentorder.StatusNEQ(paymentorder.StatusValidated), paymentorder.StatusNEQ(paymentorder.StatusRefunded), paymentorder.StatusNEQ(paymentorder.StatusSettled), paymentorder.StatusNEQ(paymentorder.StatusFulfilling), paymentorder.StatusNEQ(paymentorder.StatusSettling), + paymentorder.StatusNEQ(paymentorder.StatusRefunding), paymentorder.HasTokenWith(controllers/accounts/profile_test.go (1)
599-623:⚠️ Potential issue | 🟠 MajorCI failure: slippage < 0.1 still returns 400.
Pre-merge CI reports this assertion failing at Line 618/623. Align UpdateProviderProfile validation with the new expectation or revert the test to the current 400 behavior.tasks/rates.go (1)
21-83:⚠️ Potential issue | 🟡 MinorSwap is intentional and tested but lacks external source reference.
The buy/sell rate swap (lines 79–80) is well-documented with a code comment ("Swap buy and sell rates to match sender perspective") and explicitly tested with hardcoded payload values showing the transformation. The swap aligns with the codebase's semantic definitions: buy rates for onramp (sender pays) and sell rates for offramp (sender receives). However, the code comment explains the business logic but does not cite Noblocks API documentation to confirm that API
buyRate/sellRatefields truly represent provider rather than sender perspective. Consider adding a link to Noblocks API docs (or internal specification) as a code comment to strengthen confidence that the swap direction is correct.services/indexer/evm.go (1)
1002-1031:⚠️ Potential issue | 🟠 MajorPopulate
RebatePercentin provider-address SettleOut indexing.
This path buildsSettleOutEventwithoutRebatePercent, so it defaults to zero while the gateway path includes it. If downstream logic uses the rebate value, settlement accounting will skew.Proposed fix
settlePercentStr, ok := nonIndexedParams["settlePercent"].(string) if !ok || settlePercentStr == "" { continue } settlePercent, err := decimal.NewFromString(settlePercentStr) if err != nil { continue } +rebatePercentStr, ok := nonIndexedParams["rebatePercent"].(string) +if !ok || rebatePercentStr == "" { + continue +} +rebatePercent, err := decimal.NewFromString(rebatePercentStr) +if err != nil { + continue +} ... settledEvent := &types.SettleOutEvent{ BlockNumber: blockNumber, TxHash: txHash, SplitOrderId: splitOrderId, OrderId: orderId, LiquidityProvider: liquidityProvider, SettlePercent: settlePercent, + RebatePercent: rebatePercent, }controllers/sender/sender.go (2)
616-647:⚠️ Potential issue | 🟠 MajorDefaulting
amountInbefore flow detection flips onramp semantics.Line 616 sets
amountIn="crypto"for all requests, so onramp requests that omitamountInnever hit the intended"fiat"default (see Line 1389). This changes pricing and fee math. Move the default into the flow branches.💡 Suggested fix
-// Default amountIn to "crypto" if not provided -if payload.AmountIn == "" { - payload.AmountIn = "crypto" -} - // Detect flow type from source and destination ... -// Branch based on flow type -if sourceType.Type == "crypto" && destType.Type == "fiat" { +// Branch based on flow type +if sourceType.Type == "crypto" && destType.Type == "fiat" { + if payload.AmountIn == "" { + payload.AmountIn = "crypto" + } // Offramp flow ctrl.initiateOfframpOrderV2(ctx, payload) } else if sourceType.Type == "fiat" && destType.Type == "crypto" { + if payload.AmountIn == "" { + payload.AmountIn = "fiat" + } // Onramp flow ctrl.initiateOnrampOrderV2(ctx, payload) } else {
827-833:⚠️ Potential issue | 🟡 MinorMake destination currency comparison case-insensitive.
Line 828 uses direct equality; lowercase payloads (e.g.,
"ngn") will be rejected even though other checks useEqualFold.💡 Suggested fix
-if institutionObj.Edges.FiatCurrency.Code != destination.Currency { +if !strings.EqualFold(institutionObj.Edges.FiatCurrency.Code, destination.Currency) {
🤖 Fix all issues with AI agents
In `@controllers/index.go`:
- Around line 2142-2172: Validate and error out when required SettleIn fields
are missing or fail to parse: ensure the asserted orderId is non-empty
(orderId), and that amountStr and rateStr are present and successfully parsed
with decimal.NewFromString (do not ignore its error); if any required field is
absent or decimal parsing returns an error, return an explicit error (or abort
processing) instead of defaulting amount/rate to zero; keep aggregatorFee
optional but similarly handle its parse error if present; update the code that
constructs types.SettleInEvent to only use ethcommon.HexToAddress(...) and
parsed decimals after these validations succeed.
In `@controllers/provider/provider.go`:
- Line 1479: The response message currently sent via u.APIResponse(ctx,
http.StatusOK, "success", "Order fulfilled successfully", nil) is misleading for
settling orders with no transaction hash; update that call to use a message like
"Order submitted for settlement" (or "Order is settling; submission successful")
so it accurately reflects the settling semantics when no tx hash is returned.
- Around line 1347-1355: The order query used to build settleIn calldata does
not eager-load SenderProfile, causing prepareSettleInCallData to dereference a
nil order.Edges.SenderProfile; update the Query that builds orderWithDetails
(the storage.Client.PaymentOrder.Query() chain) to include WithSenderProfile()
so orderWithDetails.Edges.SenderProfile is populated before calling
prepareSettleInCallData, and ensure any callers that pass the order to
prepareSettleInCallData use the fetched orderWithDetails (not a partially loaded
order).
- Around line 1581-1607: The executePayinSettlement function must validate that
authorization.ChainID matches the payment order's network chain id before
building authEntry; add a check (using order.Edges.Token.Edges.Network.ChainID)
right after the existing gatewayAddress check and return a clear error (e.g.,
"authorization chain ID does not match order network") if they differ so invalid
authorizations are rejected early; keep this check before using authorization
values to construct authEntry or sending to Engine.
- Around line 1482-1513: The gatewayOrderIDFromEncode function currently uses
u.StringToByte32(paymentOrderID.String()) which truncates the UUID string;
instead derive the bytes32 from the raw UUID bytes and pad to 32 bytes to avoid
collisions: call paymentOrderID.MarshalBinary() (or use paymentOrderID[:] to get
the 16 raw bytes), create a 32-byte array, copy the 16 UUID bytes into that
array (zero-pad the remaining bytes), and pass that 32-byte array to
arguments.Pack in place of paymentOrderIDBytes32; keep all other packing and
keccak hashing unchanged.
In `@controllers/sender/sender_test.go`:
- Around line 160-168: The test fixtures are still serializing
providerOrderToken.FixedSellRate into Redis while the provider creation code
(AddProviderOrderTokenToProvider) expects the new two-sided fields instead of
legacy fixed_conversion_rate/floating_conversion_rate; update the fixtures where
providerOrderToken.FixedSellRate is used (and the other occurrences) to populate
the new two-sided fields (e.g., set both fixed_buy_rate/fixed_sell_rate or
supply floating buy/sell market rates when using floating conversion) and ensure
market rates exist for floating configurations so the serialized Redis entries
match the structure AddProviderOrderTokenToProvider consumes and the provider
selection will succeed.
In `@controllers/sender/sender.go`:
- Around line 1855-1860: The update is overwriting the institution code with the
provider display name: in the paymentOrder.Update() call do not call
SetInstitution(institutionName); preserve the existing institution code value
(PaymentOrder.Institution) instead and, if you need the provider's
human-friendly name, save institutionName into a separate field (e.g.,
InstitutionName or ProviderInstitutionDisplayName) or create one on the model
and use its SetXxx method; keep SetAccountIdentifier and SetAccountName as-is
and only change the SetInstitution usage so downstream lookups that rely on the
institution code continue to work.
- Around line 1677-1682: The calculation for totalFiatToPay is adding
senderFeeFiat to amount while amount is in crypto when amountIn=crypto; update
the logic in the block that computes senderFeeFiat and totalFiatToPay so you
first convert the crypto amount to fiat (e.g., compute fiatOrderAmount :=
amount.Mul(orderRate).RoundBank(int32(currency.Decimals))) and then compute
totalFiatToPay :=
fiatOrderAmount.Add(senderFeeFiat).RoundBank(int32(currency.Decimals)); ensure
you keep the existing senderFeeFiat calculation using
senderFeeCrypto.Mul(orderRate) and use the same rounding/decimals for the
converted fiat amount and the final sum.
In `@services/common/order.go`:
- Around line 582-584: The error message in UpdateOrderStatusSettleOut
mistakenly includes "sender" due to copy-paste; update the tx.Commit() error
return in the UpdateOrderStatusSettleOut function to a clear message that
indicates a commit failure (e.g., "UpdateOrderStatusSettleOut commit failed")
and include the original err value so the returned fmt.Errorf contains both the
contextual message and the error details.
- Around line 636-640: The gateway ID normalization only adds a "0x" prefix but
doesn't normalize case, causing case-sensitive SQL mismatches; update the block
that builds gatewayID (using event.OrderId and the local variable gatewayID) to
also lower-case the value (e.g., strings.ToLower) after ensuring the "0x" prefix
so comparisons/searches are case-insensitive; ensure any other comparisons or DB
queries that use gatewayID (e.g., the "Find payin order by gateway ID" lookup)
use this lower-cased gatewayID consistently.
In `@services/indexer/evm.go`:
- Around line 799-808: The code currently silently defaults aggregatorFee and
rate to zero when decimal.NewFromString fails; instead, mirror the amount
parsing behavior: call decimal.NewFromString for aggregatorFeeStr and rateStr,
capture the error, and if err != nil log the parsing error with context
(including the raw string and relevant identifiers from nonIndexedParams) and
skip or return the event/handler error rather than proceeding with zero values.
Update the variables aggregatorFee and rate to only be set on successful parse
and use the same error-handling flow used for amount parsing so decode failures
are visible and prevented from writing incorrect metadata.
In `@services/indexer/tron.go`:
- Around line 393-403: The block-range indexing path omits SettleIn events so
onramp settlements are skipped; add a block-range indexer for SettleIn and call
it alongside SettleOut. Implement a function named indexSettleInByBlockRange(ctx
context.Context, network NetworkType, fromBlock, toBlock uint64) modeled after
indexSettleOutByBlockRange (same filtering, parsing and persistence behavior as
the SettleOut counterpart), and invoke it here in services/indexer/tron.go
within the same error-handling pattern (logger.WithFields(...).Errorf(...)) so
the code calls indexSettleInByBlockRange(ctx, network, fromBlock, toBlock) and
logs failures consistently with the existing SettleOut block.
In `@services/priority_queue.go`:
- Around line 301-307: buildQueueForSide is writing queues to a side-suffixed
key (baseRedisKey + "_" + string(side) assigned to redisKey via variables
baseRedisKey, redisKey, prevRedisKey, tempRedisKey) but the matcher/assignment
logic (matchRate / assignment code) still reads the unsuffixed baseRedisKey;
update the matcher/assignment to select the same side-specific redis key based
on order direction (use the same side enum/RateSide mapping used in
buildQueueForSide) so reads/writes use identical keys, and ensure any uses at
the other noted ranges (around the code referenced at lines ~374-375 and
~398-425) also use the side-suffixed redisKey variable rather than the
unsuffixed base key. />
In `@utils/utils.go`:
- Around line 1146-1148: The error message for the len(parts) check is
misleading: update the fmt.Errorf string used when the condition (len(parts) !=
4 && len(parts) != 5) fails to reflect that either 4 or 5 parts are valid;
change the message to something like "invalid bucket key format: expected 4 or 5
parts, got %d" by editing the fmt.Errorf call that currently references
len(parts) (the block testing len(parts) and returning the error).
- Around line 920-940: The rate fallback logic using getProviderRateFromRedis
and local calculations can leave rateResponse zero when neither
FixedBuyRate/FixedSellRate nor FloatingBuyDelta+MarketRate are set; update the
logic so after the switch (or inside it) you validate rateResponse is non-zero
and if it is zero return or surface an explicit error (or reject the order)
instead of continuing; reference providerOrderToken.FixedBuyRate,
providerOrderToken.FixedSellRate,
providerOrderToken.FloatingBuyDelta/FloatingSellDelta,
currency.MarketBuyRate/currency.MarketSellRate and the later balance check that
uses providerbalances.AvailableBalanceGT(amount.Mul(rateResponse)) to ensure you
never perform the balance check with a zero fiat rate.
🧹 Nitpick comments (4)
services/common/order.go (2)
463-465: Missing transaction rollback on error paths.Several error paths in this function return without explicitly rolling back the transaction (lines 464, 478, 488, 493, 519). While some database drivers may handle this when the transaction goes out of scope, explicit rollback is safer and more predictable.
Example fix for line 464
if err != nil { + _ = tx.Rollback() return fmt.Errorf("UpdateOrderStatusSettleOut.update: %v", err) }Apply similar pattern to other error returns within the transaction scope.
619-621: Missing transaction rollback on error paths.Error returns at lines 620 and 632 don't roll back the transaction before returning. Apply the same rollback pattern used at line 668.
Proposed fix
if err != nil { + _ = tx.Rollback() return fmt.Errorf("UpdateOrderStatusSettleIn.update: %v", err) }if err != nil { + _ = tx.Rollback() return fmt.Errorf("UpdateOrderStatusSettleIn.create: %v", err) }Also applies to: 631-633
controllers/accounts/profile_test.go (1)
579-596: Subtest label conflicts with new success assertions.
The name says “fails” but the assertions now expect HTTP 200; consider renaming for clarity.utils/utils.go (1)
232-234: Consider direction-aware USD calculation for onramp orders.This function always uses
MarketSellRatefor USD conversion. With the introduction of two-sided rates (buy/sell), onramp orders might be more accurately valued usingMarketBuyRate. If this function is used for onramp order USD calculations, consider either:
- Adding a
directionparameter to select the appropriate rate, or- Documenting that
MarketSellRateis intentionally used as a baseline for all USD valuations
- Updated the payment order fulfillment process to include sender profile details, improving data accuracy during settlement. - Modified the API response message for order fulfillment to reflect the new settlement status. - Enhanced the sender and provider order token creation to support two-sided rates, ensuring consistency in rate validation. - Refactored Redis bucket key formatting to include the 'sell' suffix for better clarity in rate handling. - Introduced a new function to normalize gateway IDs for case-insensitive database comparisons, improving order lookup reliability. - Updated the V2 payment order webhook data structure to align with the API schema, incorporating provider account, source, and destination fields. - Improved error handling and logging in the indexer for SettleIn events, ensuring robust event processing and tracking.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controllers/sender/sender.go (1)
616-619:⚠️ Potential issue | 🟠 MajorFix AmountIn defaulting for onramp.
SettingamountInto"crypto"before flow detection causes onramp requests withoutamountInto be treated as crypto, but onramp defaults should be"fiat". Move the defaulting into the flow-specific handlers.Proposed fix
- // Default amountIn to "crypto" if not provided - if payload.AmountIn == "" { - payload.AmountIn = "crypto" - } + // Defaulting is flow-specific (handled in initiateOfframpOrderV2 / initiateOnrampOrderV2)func (ctrl *SenderController) initiateOfframpOrderV2(ctx *gin.Context, payload types.V2PaymentOrderPayload) { + // Default amountIn to "crypto" for offramp if not provided + if payload.AmountIn == "" { + payload.AmountIn = "crypto" + }func (ctrl *SenderController) initiateOnrampOrderV2(ctx *gin.Context, payload types.V2PaymentOrderPayload) { // Default amountIn to "fiat" for onramp if not provided if payload.AmountIn == "" { payload.AmountIn = "fiat" }Also applies to: 1392-1395
🤖 Fix all issues with AI agents
In `@controllers/provider/provider.go`:
- Around line 1336-1481: Before calling executePayinSettlement, add an
idempotency guard that checks the PaymentOrder status and existing
TransactionLog(s) to avoid duplicate settleIn submissions: query the order
(PaymentOrder) status (e.g., paymentorder.StatusSettling/StatusSettled) and
search for a TransactionLog with GatewayID == gatewayOrderID or Status ==
transactionlog.StatusOrderSettling; if found, return early. If not found,
persist a durable "settlement submitted" marker (create the TransactionLog with
StatusOrderSettling and GatewayID == gatewayOrderID) inside a DB transaction or
via an upsert/unique constraint before invoking executePayinSettlement so
retries won't re-submit; keep references to executePayinSettlement,
PaymentOrder, TransactionLog, gatewayOrderID and the Update/Create calls used
later.
- Around line 1582-1632: In executePayinSettlement validate authorization.V
before computing yParity: ensure authorization.V is one of the accepted values
(0, 1, 27, 28) and return a descriptive error if not, then derive yParity (0 or
1) only for those cases; update the authorization->authEntry creation to rely on
the validated yParity and keep calling engineSvc.SendTransactionBatch as before
so invalid V values are rejected early and never forwarded to
SendTransactionBatch.
In `@controllers/sender/sender.go`:
- Around line 1664-1673: The code rounds calculated fees with Round(4) which is
wrong for token-denominated fees; change all occurrences where calculatedFee :=
feePercent.Mul(cryptoAmountOut).Div(decimal.NewFromInt(100)).Round(4) (and
subsequent assignments to senderFeeCrypto) to round using the token's decimals
instead (e.g., use .Round(int32(senderOrderToken.Decimals))). Update the three
branches that compute calculatedFee (references: calculatedFee, senderFeeCrypto,
feePercent, cryptoAmountOut, senderOrderToken.MaxFeeCap,
senderOrderToken.Decimals) so fee rounding and comparisons use the token
decimals consistently.
In `@services/indexer/tron.go`:
- Around line 267-285: The tokenStr for SettleIn events is being produced as a
hex string but must be normalized to Tron base58 like other Tron paths; update
the SettleIn parsing code (the tokenStr construction in the SettleIn event
handler) to call ParseTopicToTronAddress when tokenVal is an ethcommon.Address
or []byte to produce a Tron-style base58 address, and use that value in the
SettleInEvent.Token field (also apply the same change to the other SettleIn
parser variant used elsewhere, e.g., where ProcessSettleInOrders consumes these
events) so DB/matching uses consistent Tron address format.
In `@services/priority_queue.go`:
- Around line 615-616: The rate refresh and queue selection incorrectly hardcode
RateSideSell and default to buy when currentOrder is nil; compute the side once
from order.Direction (or derive from currentOrder if present) into a local
variable (e.g., side or rateSide) and use that same variable in the call to
s.GetProviderRate (where you currently pass RateSideSell) and when building the
Redis queue key selection logic (the blocks around s.GetProviderRate and the
queue key code, including the similar section referenced at lines 669-684).
Ensure both places use this single computed side so buy/sell orders use the
correct rate and queue.
In `@utils/utils.go`:
- Around line 389-406: The code currently passes the entire
paymentOrder.Metadata into types.V2FiatRecipient which can leak KYC/internal
fields; replace that with a sanitized metadata map by building an allowlist
(e.g., slice of allowed keys) and copying only those keys from
paymentOrder.Metadata into a new map (e.g., sanitizedMetadata) and use
sanitizedMetadata when constructing types.V2FiatRecipient.Metadata; update the
block that creates destination/Types.V2FiatDestination and the
types.V2FiatRecipient instantiation to use the sanitized map instead of
paymentOrder.Metadata, and ensure nil-safe handling when Metadata is nil.
🧹 Nitpick comments (1)
services/priority_queue.go (1)
33-39: Consider reusingutils.RateSideto avoid duplicate enums.
Both packages define identical constants; centralizing reduces divergence risk.
…rollers - Added new endpoints for retrieving payment orders and individual payment order details in v2 API schema for both sender and provider controllers. - Enhanced the payment order response structure to include provider account, source, destination, and transaction logs. - Implemented pagination and filtering for listing payment orders, ensuring compliance with v2 API requirements. - Updated routing to accommodate new v2 endpoints for payment orders, improving API usability and consistency. - Refactored existing functions to support the new v2 response format and improve code maintainability.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@utils/utils.go`:
- Around line 232-234: The current CalculatePaymentOrderAmountInUSD uses
fiatCurrency.MarketSellRate unconditionally; make it direction-aware by adding
an order direction parameter (e.g., isOnramp or enum Direction) to
CalculatePaymentOrderAmountInUSD and use fiatCurrency.MarketBuyRate when
direction indicates onramp and fiatCurrency.MarketBuyRate is non-zero, otherwise
fall back to fiatCurrency.MarketSellRate for offramp (and when buy rate is
zero), and update any callers to pass the correct direction; alternatively, if
uniform use of MarketSellRate was intentional, add a clear comment in
CalculatePaymentOrderAmountInUSD explaining the rationale for always using
MarketSellRate.
🧹 Nitpick comments (1)
types/types.go (1)
203-234: ID type change is a potential breaking change.
AcceptOrderResponse.IDchanged fromuuid.UUIDtostring. If existing provider integrations expect a UUID type in the JSON response (which typically serializes to the same string format), this should be compatible. However, any type-strict consumers may need updates.The
SetCodeAuthorizationstructure correctly captures EIP-7702 authorization components. Consider documenting thatRandSshould be hex-encoded strings (with or without0xprefix) for clarity.
- Added checks for missing or empty parameters in the SettleIn event handling, ensuring robust validation of orderId, amount, and rate. - Enhanced error messages for invalid parameters to provide clearer feedback during processing. - Refactored transaction handling in the AcceptOrder and handlePayinFulfillment methods to ensure proper rollback and commit logic. - Updated Redis key handling to prevent orphaned keys in case of transaction failures. - Improved logging for transaction errors to facilitate easier debugging and monitoring.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controllers/index.go (1)
2067-2107:⚠️ Potential issue | 🟡 MinorHarden SettleOut parsing against malformed payloads to avoid panics.
Type assertions on external webhook data can panic; validating required fields before parsing makes the handler robust.🔧 Suggested defensive parsing
- settlePercent, err := decimal.NewFromString(nonIndexedParams["settlePercent"].(string)) + settlePercentStr, ok := nonIndexedParams["settlePercent"].(string) + if !ok || settlePercentStr == "" { + return fmt.Errorf("SettleOut: missing settlePercent in non-indexed params") + } + settlePercent, err := decimal.NewFromString(settlePercentStr) if err != nil { return fmt.Errorf("invalid settle percent: %w", err) } - rebatePercent, err := decimal.NewFromString(nonIndexedParams["rebatePercent"].(string)) + rebatePercentStr, ok := nonIndexedParams["rebatePercent"].(string) + if !ok || rebatePercentStr == "" { + return fmt.Errorf("SettleOut: missing rebatePercent in non-indexed params") + } + rebatePercent, err := decimal.NewFromString(rebatePercentStr) if err != nil { return fmt.Errorf("invalid rebate percent: %w", err) } + splitOrderId, ok := nonIndexedParams["splitOrderId"].(string) + if !ok || splitOrderId == "" { + return fmt.Errorf("SettleOut: missing splitOrderId in non-indexed params") + } + orderId, ok := indexedParams["orderId"].(string) + if !ok || orderId == "" { + return fmt.Errorf("SettleOut: missing orderId in indexed params") + } + liquidityProvider, ok := indexedParams["liquidityProvider"].(string) + if !ok || liquidityProvider == "" { + return fmt.Errorf("SettleOut: missing liquidityProvider in indexed params") + } + settledEvent := &types.SettleOutEvent{ BlockNumber: event.Data.BlockNumber, TxHash: event.Data.TransactionHash, - SplitOrderId: nonIndexedParams["splitOrderId"].(string), - OrderId: indexedParams["orderId"].(string), - LiquidityProvider: ethcommon.HexToAddress(indexedParams["liquidityProvider"].(string)).Hex(), + SplitOrderId: splitOrderId, + OrderId: orderId, + LiquidityProvider: ethcommon.HexToAddress(liquidityProvider).Hex(), SettlePercent: settlePercent, RebatePercent: rebatePercent, }
🤖 Fix all issues with AI agents
In `@controllers/provider/provider.go`:
- Around line 1436-1478: The code persists a "settling" marker (creates
TransactionLog via tx.TransactionLog.Create and sets PaymentOrder status to
paymentorder.StatusSettling) before calling ctrl.executePayinSettlement, which
can strand orders if executePayinSettlement (or SendTransactionBatch) fails; add
a compensating update on error: when executePayinSettlement returns an error,
start a new DB transaction and either revert the PaymentOrder status back to the
prior state (e.g., paymentorder.StatusSubmitted) and remove/unlink the created
TransactionLog, or set a retryable failure status (e.g.,
paymentorder.StatusSettlementFailedRetryable) so idempotency isn't blocked;
implement this in the error branch after ctrl.executePayinSettlement (use
PaymentOrder.UpdateOneID(orderID) and TransactionLog.UpdateOneID/ DeleteOneID as
needed and commit/rollback appropriately) and ensure all DB errors are logged
and returned.
In `@controllers/sender/sender.go`:
- Around line 1856-1861: The parsing of providerResponse["validUntil"] into
validUntil can silently fail and leave validUntil as the zero time; update the
logic around validUntil (the providerResponse handling) to check the error
returned by time.Parse and, on parse failure, set validUntil =
time.Now().Add(orderConf.OrderFulfillmentValidity) so the default validity is
used; keep the existing branch where missing key also uses the same default.
In `@utils/utils.go`:
- Around line 444-461: The code constructs providerAccount from
paymentOrder.Metadata but always leaves ValidUntil as a zero time; update the
block that reads pa :=
paymentOrder.Metadata["providerAccount"].(map[string]interface{}) to extract
"validUntil" from pa and set providerAccount.ValidUntil appropriately: check for
types (string timestamp e.g. RFC3339 and numeric Unix seconds/float64),
parse/convert to time.Time, and assign the result to
types.V2FiatProviderAccount.ValidUntil before returning/using providerAccount
(preserve existing extraction of "institution", "accountIdentifier",
"accountName").
- Updated CalculatePaymentOrderAmountInUSD function to include a direction parameter, allowing differentiation between onramp and offramp calculations. - Modified payment order processing in sender and provider controllers to utilize the new direction-aware calculation method. - Improved handling of payment order ID conversion in the provider controller for better data integrity.
…vider controller - Changed the expected response code in profile update tests from 200 OK to 400 Bad Request for invalid rate slippage. - Updated the response message assertion to check for specific error details regarding rate slippage. - Enhanced error handling in the provider controller during payin fulfillment to include compensating transaction logic for settlement failures. - Added validation checks for sender profile and token presence in the prepareSettleInCallData function to prevent nil pointer dereferences. - Refactored the parsing of validUntil metadata in the utils package to support multiple formats, improving robustness in order processing.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controllers/sender/sender.go (1)
616-619:⚠️ Potential issue | 🟠 MajorFix
amountIndefaulting for onramp requests.
InitiatePaymentOrderV2setsamountInto"crypto"before flow detection, so onramp requests that omitamountInnever hit the"fiat"default ininitiateOnrampOrderV2. This flips unit interpretation and misprices totals. Move defaulting into the flow-specific helpers.🛠️ Suggested fix (flow-specific defaults)
- // Default amountIn to "crypto" if not provided - if payload.AmountIn == "" { - payload.AmountIn = "crypto" - } + // Defer defaulting to flow-specific handlers @@ func (ctrl *SenderController) initiateOfframpOrderV2(ctx *gin.Context, payload types.V2PaymentOrderPayload) { + // Default amountIn to "crypto" for offramp if not provided + if payload.AmountIn == "" { + payload.AmountIn = "crypto" + } @@ func (ctrl *SenderController) initiateOnrampOrderV2(ctx *gin.Context, payload types.V2PaymentOrderPayload) { // ... - // Default amountIn to "fiat" for onramp if not provided - if payload.AmountIn == "" { - payload.AmountIn = "fiat" - } + // Default amountIn to "fiat" for onramp if not provided + if payload.AmountIn == "" { + payload.AmountIn = "fiat" + }Also applies to: 1392-1395
🤖 Fix all issues with AI agents
In `@controllers/provider/provider.go`:
- Around line 1474-1503: The compensating transaction on executePayinSettlement
failure is incorrectly setting the order back to StatusValidated (which is
final) and blocks retries; change the revert in the txCompensate flow so
PaymentOrder.UpdateOneID(orderID) sets the status back to the pre-submit status
(use paymentorder.StatusFulfilling instead of paymentorder.StatusValidated)
while still removing the transaction entries (RemoveTransactions(transactionLog)
and TransactionLog.DeleteOneID(transactionLog.ID)), and then commit/rollback the
txCompensate as currently implemented so providers can retry after transient
failures; update references in the error handling around executePayinSettlement,
txCompensate, PaymentOrder.UpdateOneID, transactionLog accordingly.
- Around line 570-575: The current ShouldBindJSON error handling treats any bind
error as "no body" and defaults acceptRequest.Direction to "payout"; change it
to distinguish an empty body (io.EOF) from malformed JSON by checking the bind
error: if err != nil and not errors.Is(err, io.EOF) then respond with HTTP 400
(e.g., ctx.JSON(400, ...) and return); only when errors.Is(err, io.EOF) should
you set acceptRequest.Direction = "payout". Use the existing symbols
ctx.ShouldBindJSON, acceptRequest (types.AcceptOrderRequest),
acceptRequest.Direction and ctx.JSON, and use errors.Is(err, io.EOF) to detect
an empty body.
In `@controllers/sender/sender.go`:
- Around line 2833-2836: The code always divides paymentOrder.Amount and
paymentOrder.SenderFee by institution.Edges.FiatCurrency.MarketSellRate, which
misstates onramp volumes; update the logic in the sender stats conversion to
choose the rate based on paymentOrder.Direction: use
institution.Edges.FiatCurrency.MarketBuyRate when paymentOrder.Direction
indicates an onramp and fall back to MarketSellRate otherwise, then divide
paymentOrder.Amount (and paymentOrder.SenderFee when > 0) by that selected rate.
- Around line 936-943: The division by currency.MarketSellRate/MarketBuyRate can
panic if the rate is zero; before computing orderRate and doing
amount.Div(orderRate) in the sender logic (where token, currency, orderRate,
cryptoAmount are used) add a guard that checks currency.MarketSellRate.IsZero()
(and analogously MarketBuyRate.IsZero() in the other branch around lines
1518-1526) and return a rate-unavailable error instead of proceeding; ensure the
same pattern is applied before calling u.ValidateRate(ctx, token, currency,
cryptoAmount, ...) so you never call decimal.Div with a zero divisor.
🧹 Nitpick comments (1)
controllers/accounts/profile_test.go (1)
572-597: Test name no longer matches expected behavior.The test is named
"fails when rate slippage exceeds 5 percent of market rate"but now expects HTTP 200 (success) and asserts"Profile updated successfully". This is misleading and will confuse future maintainers.Consider renaming the test to reflect the new expected behavior, e.g.,
"succeeds when rate slippage exceeds 5 percent with two-sided rates"or"accepts high rate slippage with two-sided pricing".💡 Suggested rename
- t.Run("fails when rate slippage exceeds 5 percent of market rate", func(t *testing.T) { + t.Run("accepts rate slippage exceeding 5 percent with two-sided rates", func(t *testing.T) {
…ent process - Added logic to derive the currency from the order's institution, ensuring proper handling of multiple fiat currencies for the same token. - Implemented error handling for cases where the institution or fiat currency is not found, improving robustness in the payin fulfillment flow. - Updated the query for provider payout address to include the derived currency, ensuring accurate transaction processing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@controllers/provider/provider.go`:
- Around line 668-689: The order status is being updated via
storage.Client.PaymentOrder.UpdateOneID(...).SetStatus(paymentorder.StatusRefunding).Save(ctx)
while the DB transaction tx is still open (after tx.Rollback()), which can lead
to inconsistency; change the update to run under the same transaction (use
tx.PaymentOrder.UpdateOneID(orderID).SetStatus(paymentorder.StatusRefunding).Save(ctx))
or commit/close tx before calling storage.Client to ensure the status update and
the rollback/transaction boundaries are consistent; update the code paths around
tx.Rollback(), tx, and the status update to use the tx-bound PaymentOrder
updater.
🧹 Nitpick comments (1)
controllers/provider/provider.go (1)
1548-1553: Token balance release failure is silently ignored.If
ReleaseTokenBalancefails, the error is logged but execution continues. While the comment says "Don't fail the entire operation", the reserved tokens remain locked. Consider whether a compensating mechanism or alert is needed for this scenario.Suggested improvement
err = ctrl.balanceService.ReleaseTokenBalance(ctx, provider.ID, orderWithDetails.Edges.Token.ID, totalCryptoReserved, nil) if err != nil { - logger.Errorf("Failed to release token balance: %v", err) - // Don't fail the entire operation if balance release fails - log and continue + logger.WithFields(logger.Fields{ + "Error": fmt.Sprintf("%v", err), + "OrderID": orderID.String(), + "ProviderID": provider.ID, + "TokenID": orderWithDetails.Edges.Token.ID, + "Amount": totalCryptoReserved.String(), + }).Errorf("Failed to release token balance - manual reconciliation may be required") + // Consider: Add to a retry queue or send alert for manual reconciliation }
…cessing - Improved error handling in the AcceptOrder method to provide clearer feedback for invalid request bodies. - Added checks for currency market rates in the initiateOfframpOrderV2 and initiateOnrampOrderV2 methods, ensuring proper validation before processing orders. - Updated payment order amount calculations to be direction-aware, using appropriate market rates for onramp and offramp transactions. - Enhanced transaction handling in the AcceptOrder method to ensure proper rollback and commit logic during order status updates.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controllers/sender/sender.go (1)
1063-1091:⚠️ Potential issue | 🟡 MinorUse token decimals for offramp sender fee rounding.
Offramp fees are token‑denominated; rounding to 4 decimals can misprice fees for tokens with higher precision. Consider rounding by
token.Decimals(keep the 4‑dec rule for onramp only).🛠️ Suggested update (apply to both fee branches)
- calculatedFee := feePercent.Mul(cryptoAmount).Div(decimal.NewFromInt(100)).Round(4) + calculatedFee := feePercent.Mul(cryptoAmount). + Div(decimal.NewFromInt(100)). + Round(int32(token.Decimals))Based on learnings: Sender fees in the payment system are calculated as token amounts, not fiat amounts. The calculation converts the fee percentage of the fiat amount to token units by dividing by the exchange rate, so rounding should use token decimals, not fiat currency decimals.
🤖 Fix all issues with AI agents
In `@controllers/provider/provider.go`:
- Around line 1476-1523: The pre-check plus separate marker write is racy; make
the settlement submission guard atomic by moving the marker creation and the
order status transition into a single conditional transaction: inside the
existing tx (created with storage.Client.Tx(ctx)) perform a conditional update
on PaymentOrder (use PaymentOrder.UpdateOneID(orderID) with a Where/conditional
that current status == paymentorder.StatusFulfilling) that sets Status to
paymentorder.StatusSettling, SetGatewayID(gatewayOrderID) and adds the
TransactionLog (TransactionLog.Create() ->
SetStatus(transactionlog.StatusOrderSettling) -> SetGatewayID(...)). If the
conditional update affects zero rows, rollback and return the "already
submitted" response; alternatively enforce a unique DB constraint on
transactionlog.gateway_id to prevent duplicates. Ensure executePayinSettlement
is only called after the transaction commits successfully.
In `@controllers/sender/sender.go`:
- Around line 1906-1917: The Redis HSet of order_request_%s can fail after the
order commit, leaving token liquidity reserved and the order stuck; update the
block that calls storage.RedisClient.HSet (creating orderRequestKey for
paymentOrder.ID) to perform a compensating flow on error: either release the
token liquidity reservation and mark paymentOrder as cancelled/refunding (call
your existing releaseReservation(paymentOrder.ID) and
cancelOrder/paymentOrder.UpdateStatus methods) or enqueue a retry job (e.g.,
push order ID and orderRequestKey to your async retry queue) before returning
the HTTP 500 and logging with logger.Errorf; ensure the compensating functions
touch the same identifiers (paymentOrder.ID, providerID, totalFiatToPay) so
state remains consistent and avoid leaving the order in a
reserved-but-unservable state.
…ocess - Changed the expected response code in profile update tests from 200 OK to 400 Bad Request for excessive rate slippage. - Updated the response message assertion to check for specific error details regarding rate slippage exceeding the maximum threshold. - Added validation logic in the profile controller to enforce a maximum slippage percentage, improving error handling and user feedback.
…rror handling - Removed redundant checks for existing settlement status to simplify the transaction logic. - Implemented atomic transaction handling for settlement submission, ensuring that the order status only transitions if it is still in the Fulfilling state. - Added error handling to release reserved tokens and cancel orders in case of Redis failures during order initiation, preventing unservable states. - Improved logging for transaction errors to facilitate easier debugging and monitoring.
- Added logic to handle different validation statuses during payin fulfillment, ensuring that only successful validations proceed to settlement. - Implemented error handling for failed validations, including updating the fulfillment status and releasing reserved token balances. - Improved API responses for various validation outcomes, providing clearer feedback for fulfillment status and errors.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controllers/provider/provider.go (1)
572-609:⚠️ Potential issue | 🟠 MajorMove Redis
order_requestdeletion after the DB commit to avoid orphaned pending orders.
If the transaction fails after Line 603, the Redis request is gone while the order remains pending, so no provider can accept it. Delete the Redis keys only after a successful commit and treat cleanup as best‑effort.🛠️ Proposed adjustment (delete after commit, best‑effort)
- // Best-effort cleanup of metadata key used for timeout recovery. - _, _ = storage.RedisClient.Del(ctx, fmt.Sprintf("order_request_meta_%s", orderID)).Result() - - // Delete order request from Redis after validation (both payin and offramp) - _, err = storage.RedisClient.Del(ctx, orderRequestKey).Result() - if err != nil { - logger.Errorf("error deleting order request from Redis: %v", err) - u.APIResponse(ctx, http.StatusInternalServerError, "error", "Failed to accept order request", nil) - return - } + // After tx.Commit() succeeds: + if _, err := storage.RedisClient.Del(ctx, fmt.Sprintf("order_request_meta_%s", orderID)).Result(); err != nil { + logger.Errorf("error deleting order request meta from Redis: %v", err) + } + if _, err := storage.RedisClient.Del(ctx, orderRequestKey).Result(); err != nil { + logger.Errorf("error deleting order request from Redis: %v", err) + }
🤖 Fix all issues with AI agents
In `@controllers/accounts/profile.go`:
- Around line 370-399: After computing buyRate and sellRate in the loop
processing tokenPayload, add explicit positivity checks to reject any
non‑positive computed rate: if buyRate is set and buyRate.Cmp(decimal.Zero) <=
0, append a validation error for "Tokens" mentioning the buy rate must be
positive for tokenPayload.Symbol; likewise for sellRate when set and <= 0.
Perform these checks immediately after constructing buyRate/sellRate and before
the profitability check (the existing buyRate.LessThan(sellRate) branch) so
negative/zero rates are rejected early.
In `@controllers/sender/sender.go`:
- Around line 2224-2232: The code does a map lookup into instMap without
checking the bool, which can pass a nil institution into
BuildV2PaymentOrderGetResponse and cause a panic; change the lookup to "inst, ok
:= instMap[po.Institution]" and handle the missing case (for example, if !ok set
inst = nil or skip this payment order) before calling
BuildV2PaymentOrderGetResponse so you never dereference a non-existent
institution.
🧹 Nitpick comments (1)
controllers/sender/sender.go (1)
1838-1904: External provider call before transaction commit may orphan virtual accounts.The provider
/new_ordercall at line 1851 happens beforetx.Commit()at line 1900. If the commit fails after the provider successfully creates a virtual account, the account becomes orphaned. While thevalidUntilexpiry provides eventual cleanup, consider moving the provider call after commit, or implementing a compensating call to the provider on failure.
- Added checks to ensure that buy and sell rates are positive, improving validation logic in the profile update method. - Updated the validation for floating rates to only apply when fixed rates are not provided, ensuring accurate rate deviation calculations. - Enhanced error handling to provide clearer feedback for invalid rate scenarios, improving user experience during profile updates.
…d sender controllers - Updated the AcceptOrder method to include direction checks for payin orders, ensuring correct handling of order statuses. - Improved error logging in FulfillOrder to use a derived network ID, enhancing clarity in error reporting. - Refactored currency validation in initiateOfframpOrderV2 to be case-insensitive, improving robustness in currency matching. - Streamlined amountIn defaulting logic for offramp orders, ensuring consistent behavior across different flows.
- Indexer: rename GetProviderAddresses to GetProviderSettlementAddresses - Utils: ValidateRate checks token balance for onramp (buy), fiat for offramp (sell); findSuitableProviderRate is direction-aware for provider filter and balance check - Fulfillments: sync onramp Refunding orders (syncRefundingOrder, tx_status/tx_refund); set order to Fulfilled when refund fails (e.g. missing account); release token balance on payin failed/refunded; use reference/refundReference in payloads; call reassignCancelledOrder only for offramp (Cancelled or failed fulfillment) - Stale ops: run RefundOrder only for offramp (on-chain refund); onramp refund is provider-side - Provider: AcceptOrder creates order_refunding log when setting Refunding; payin response nil-safe; FulfillOrder handles Refunding+onramp via handleRefundOutcomeFulfillment - Ent: add ValidationStatusRefunded and schema/migrate updates Co-authored-by: Cursor <cursoragent@cursor.com>
…dling - Indexer: rename GetProviderAddresses to GetProviderSettlementAddresses - Utils: ValidateRate checks token balance for onramp (buy), fiat for offramp (sell); findSuitableProviderRate is direction-aware for provider filter and balance check - Fulfillments: sync onramp Refunding orders (syncRefundingOrder, tx_status/tx_refund); set order to Fulfilled when refund fails (e.g. missing account); release token balance on payin failed/refunded; use reference/refundReference in payloads; call reassignCancelledOrder only for offramp (Cancelled or failed fulfillment) - Stale ops: run RefundOrder only for offramp (on-chain refund); onramp refund is provider-side - Provider: AcceptOrder creates order_refunding log when setting Refunding; payin response nil-safe; FulfillOrder handles Refunding+onramp via handleRefundOutcomeFulfillment - Ent: add ValidationStatusRefunded and schema/migrate updates
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
services/order/evm.go (1)
229-281:⚠️ Potential issue | 🟠 MajorSettleOrder does not create an
order_settlinglog, unlike RefundOrder which createsorder_refunding.RefundOrder (lines 150-220) wraps the status update and log creation in a DB transaction, creating a
TransactionLogwithStatusOrderRefunding. However, SettleOrder (lines 229-281) only updates the order status toStatusSettlingwithout creating a correspondingTransactionLogentry. This asymmetry should be addressed—SettleOrder should mirror RefundOrder's pattern by creating anorder_settlinglog within a DB transaction for consistency and auditability.controllers/sender/sender.go (1)
1748-1907:⚠️ Potential issue | 🟠 MajorExternal HTTP call inside an open DB transaction holds resources and risks inconsistency.
The DB transaction is opened at line 1749 and committed at line 1907, but an external HTTP call to the provider (
CallProviderWithHMACat line 1853) occurs in between. This has two consequences:
- Long-lived transaction: If the provider is slow or times out, the DB connection/transaction is held open, potentially exhausting the connection pool under load.
- Uncompensated provider side-effect: If the provider call succeeds but a subsequent DB operation (e.g., metadata save at line 1899) or commit fails, the provider has already created a virtual account that is never used and never cleaned up.
Consider restructuring so the provider call happens outside the transaction: first call the provider, then open the transaction to persist everything atomically. On commit failure, you can then call a provider cancellation endpoint (if available) or log for manual reconciliation.
🤖 Fix all issues with AI agents
In `@controllers/accounts/profile.go`:
- Around line 525-532: The current fallback sets conversionRate := sellRate then
to buyRate then to decimal.NewFromInt(1), which masks cases where both sellRate
and buyRate are zero and can misassign provision buckets; instead, detect when
sellRate.IsZero() && buyRate.IsZero() and in that case do not set conversionRate
to 1 but skip bucket assignment for that token (or return a clear no-op), and
emit a warning log including the token identifier and the zero rates so
operators can see the issue; update the code paths that use conversionRate (the
bucket assignment logic that follows this block) to handle the "no conversion
rate" case safely.
In `@controllers/provider/provider.go`:
- Around line 583-595: The Redis-only lookup for order_request_*
(orderRequestKey via storage.RedisClient.HGetAll) causes retries to fail when
the key is deleted; update the accept/payin flow so that if HGetAll returns
empty (len(result)==0) you fall back to a persistent lookup (e.g., query the
orders/payins table or Order model by orderID and verify provider.ID) and
reconstruct the same response data used by the idempotent payin block before
returning success; change the branch that currently returns 404 (the block
checking len(result)==0 || result["providerId"] != provider.ID) to: if Redis
misses, query DB for the order/payin record, ensure provider.ID matches, and use
that DB record to continue the idempotent handling (or delay Redis deletion) so
retries can still succeed and u.APIResponse remains correct.
- Around line 1430-1472: When a fulfillment is marked
paymentorderfulfillment.ValidationStatusRefunded we update statuses and release
balances but never notify the sender; add a webhook/notification call after the
token release (or before the final u.APIResponse) to notify the sender of the
refund completion. Locate the refunded branch that handles
payload.ValidationStatus == paymentorderfulfillment.ValidationStatusRefunded
(around the fulfillmentUpdate.Save,
PaymentOrder.UpdateOneID(...).SetStatus(paymentorder.StatusRefunded), and
ctrl.balanceService.ReleaseTokenBalance calls) and invoke your existing
webhook/notification API (e.g. ctrl.webhookService.SendOrderEvent or
ctrl.notificationService.NotifySender — if no such helper exists, add a new
method on ctrl to emit the order refund event) passing the order identifier, new
status (StatusRefunded) and any relevant payload (TxID, PSP, ValidationError),
then proceed to u.APIResponse(ctx, http.StatusOK, "success", "Refund completed",
nil).
In `@services/indexer/evm.go`:
- Around line 776-844: The SettleIn branch is not validating
liquidityProviderStr and tokenStr which allows blank values to be converted to
the zero address; update the case for utils.SettleInEvent so
liquidityProviderStr is read with an ok check and treated as required (like the
SettleOut path) and tokenStr is validated for non-empty before calling
ethcommon.HexToAddress — if either required field is missing or empty, log/warn
and continue (skip the event) rather than silently using the zero address;
adjust usage around liquidityProviderStr, tokenStr, and construction of
types.SettleInEvent accordingly.
In `@tasks/fulfillments_webhooks.go`:
- Around line 765-801: syncRefundingOrder marks the order as Refunded via
storage.Client.PaymentOrder.UpdateOneID(...).SetStatus(paymentorder.StatusRefunded)
but never emits the payment_order.refunded webhook; after the UpdateOneID
Save(ctx) succeeds (and after releasing balance if applicable) call the existing
webhook/event dispatch used elsewhere (e.g., the project’s payment-order webhook
sender function) to send a payment_order.refunded webhook for the order (include
order ID, status, and any relevant txId/psp fields), ensuring errors from the
webhook send are logged with the same logger context used in this function.
🧹 Nitpick comments (4)
services/priority_queue_test.go (1)
662-670: Only sell-side rate is tested — consider adding a buy-side test case.
GetProviderRateis tested only withRateSideSell. Since the PR introduces two-sided pricing (RateSideBuy/RateSideSell), a companion test withRateSideBuywould improve coverage of the new rate logic.controllers/sender/sender.go (2)
791-796: Dead assignment:returnAddressfrom config is immediately overwritten.Line 793 assigns
returnAddress = senderOrderToken.RefundAddress, but line 796 unconditionally overwrites it withsource.RefundAddress. The config value is never used. Additionally, the validation at line 783 checkssenderOrderToken.RefundAddress == "", but this check is unnecessary for the V2 offramp flow sincesource.RefundAddressis already validated above (lines 733–761).Consider removing the
RefundAddresscheck from line 783 (keep onlyFeeAddress) and the dead assignment at line 793 to avoid confusion.Suggested cleanup
- if senderOrderToken.FeeAddress == "" || senderOrderToken.RefundAddress == "" { + if senderOrderToken.FeeAddress == "" { u.APIResponse(ctx, http.StatusBadRequest, "error", "Failed to validate payload", types.ErrorData{ Field: "Source", - Message: "Fee address or refund address is not configured", + Message: "Fee address is not configured", }) return } feePercent := senderOrderToken.FeePercent feeAddress := senderOrderToken.FeeAddress - returnAddress := senderOrderToken.RefundAddress - // Use refund address from source if provided - returnAddress = source.RefundAddress + returnAddress := source.RefundAddress
1707-1707: Regex compiled on every request — consider pre-compiling.
regexp.MustCompile(...)is called inline at lines 842 and 1707 on every request. This is a minor inefficiency since regex compilation is relatively expensive. The same pattern^[a-zA-Z0-9\-_]+$is also used in V1 (line 219).Pre-compile the regex as a package-level var
var ( serverConf = config.ServerConfig() orderConf = config.OrderConfig() + referencePattern = regexp.MustCompile(`^[a-zA-Z0-9\-_]+$`) )Then replace all
regexp.MustCompile(...)calls withreferencePattern.MatchString(...).services/common/indexer.go (1)
229-257: ProcessSettleInOrders delegates filtering to per-goroutine queries instead of batching once upfront like ProcessSettleOutOrders.ProcessSettleOutOrders (line ~122) filters all orders with a single DB query (WHERE
GatewayIDInANDStatusSettling) before spawning goroutines. ProcessSettleInOrders spawns a goroutine for each orderId, with each goroutine callingUpdateOrderStatusSettleIn, which performs its own DB filter query. This results in N separate filter queries instead of one batch query.UpdateOrderStatusSettleIn does handle edge cases correctly: it filters for
StatusSettling(line 658) and gracefully returns nil if no matching order exists (line 679-681), so functionally there's no bug. However, this approach is less efficient at scale due to redundant individual queries rather than a single pre-filtered batch.
| // Choose a representative conversion rate for bucket calculations (offramp = sell side) | ||
| conversionRate := sellRate | ||
| if conversionRate.IsZero() { | ||
| conversionRate = buyRate | ||
| } | ||
| if conversionRate.IsZero() { | ||
| conversionRate = decimal.NewFromInt(1) | ||
| } |
There was a problem hiding this comment.
Fallback conversionRate = 1 may misassign provision buckets.
When both buy and sell rates are zero (e.g., floating deltas configured but market rates not yet available), falling back to 1 treats token amounts as 1:1 with fiat for bucket range matching. This could place the provider in incorrect buckets until a market rate is published.
Consider either skipping bucket assignment for this token or logging a warning so the misconfiguration is visible.
🤖 Prompt for AI Agents
In `@controllers/accounts/profile.go` around lines 525 - 532, The current fallback
sets conversionRate := sellRate then to buyRate then to decimal.NewFromInt(1),
which masks cases where both sellRate and buyRate are zero and can misassign
provision buckets; instead, detect when sellRate.IsZero() && buyRate.IsZero()
and in that case do not set conversionRate to 1 but skip bucket assignment for
that token (or return a clear no-op), and emit a warning log including the token
identifier and the zero rates so operators can see the issue; update the code
paths that use conversionRate (the bucket assignment logic that follows this
block) to handle the "no conversion rate" case safely.
| // Get order request from Redis (offramp: set by assignment; payin: set by sender at creation) | ||
| orderRequestKey := fmt.Sprintf("order_request_%s", orderID) | ||
| result, err := storage.RedisClient.HGetAll(ctx, orderRequestKey).Result() | ||
| if err != nil { | ||
| logger.Errorf("error getting order request from Redis: %v", err) | ||
| u.APIResponse(ctx, http.StatusInternalServerError, "error", "Failed to accept order request", nil) | ||
| return | ||
| } | ||
|
|
||
| if result["providerId"] != provider.ID || len(result) == 0 { | ||
| if len(result) == 0 || result["providerId"] != provider.ID { | ||
| logger.Errorf("order request not found in Redis: %v", orderID) | ||
| u.APIResponse(ctx, http.StatusNotFound, "error", "Order request not found or is expired", nil) | ||
| return | ||
| } |
There was a problem hiding this comment.
Payin idempotent retry is blocked once the Redis key is deleted.
The idempotent payin response block later won’t be reached when the first accept already removed order_request_*, so a retry after a timeout gets 404 and loses payin details. Consider a DB fallback for payin retries (or delay deletion) so retries can still succeed for the same provider.
💡 Suggested fix (DB fallback for payin retries)
- if len(result) == 0 || result["providerId"] != provider.ID {
+ if len(result) == 0 {
+ // Fallback for idempotent payin retries when Redis entry is already gone
+ if current, err := storage.Client.PaymentOrder.
+ Query().
+ Where(
+ paymentorder.IDEQ(orderID),
+ paymentorder.DirectionEQ(paymentorder.DirectionOnramp),
+ paymentorder.StatusEQ(paymentorder.StatusFulfilling),
+ paymentorder.HasProviderWith(providerprofile.IDEQ(provider.ID)),
+ ).
+ WithToken(func(tq *ent.TokenQuery) { tq.WithNetwork() }).
+ Only(ctx); err == nil {
+ // Reuse the existing idempotent response construction (extract to helper)
+ respondPayinAccept(ctx, current)
+ return
+ }
+ }
+ if len(result) == 0 || result["providerId"] != provider.ID {
logger.Errorf("order request not found in Redis: %v", orderID)
u.APIResponse(ctx, http.StatusNotFound, "error", "Order request not found or is expired", nil)
return
}🤖 Prompt for AI Agents
In `@controllers/provider/provider.go` around lines 583 - 595, The Redis-only
lookup for order_request_* (orderRequestKey via storage.RedisClient.HGetAll)
causes retries to fail when the key is deleted; update the accept/payin flow so
that if HGetAll returns empty (len(result)==0) you fall back to a persistent
lookup (e.g., query the orders/payins table or Order model by orderID and verify
provider.ID) and reconstruct the same response data used by the idempotent payin
block before returning success; change the branch that currently returns 404
(the block checking len(result)==0 || result["providerId"] != provider.ID) to:
if Redis misses, query DB for the order/payin record, ensure provider.ID
matches, and use that DB record to continue the idempotent handling (or delay
Redis deletion) so retries can still succeed and u.APIResponse remains correct.
| switch payload.ValidationStatus { | ||
| case paymentorderfulfillment.ValidationStatusRefunded: | ||
| // Update fulfillment to Refunded (and TxID, PSP if present) | ||
| fulfillmentUpdate := fulfillment.Update(). | ||
| SetValidationStatus(paymentorderfulfillment.ValidationStatusRefunded). | ||
| SetValidationError(payload.ValidationError) | ||
| if payload.TxID != "" { | ||
| fulfillmentUpdate = fulfillmentUpdate.SetTxID(payload.TxID) | ||
| } | ||
| if payload.PSP != "" { | ||
| fulfillmentUpdate = fulfillmentUpdate.SetPsp(payload.PSP) | ||
| } | ||
| _, err := fulfillmentUpdate.Save(ctx) | ||
| if err != nil { | ||
| logger.WithFields(logger.Fields{ | ||
| "Error": fmt.Sprintf("%v", err), | ||
| "OrderID": orderID.String(), | ||
| }).Errorf("Failed to update refund fulfillment to refunded") | ||
| u.APIResponse(ctx, http.StatusInternalServerError, "error", "Failed to update order status", nil) | ||
| return | ||
| } | ||
|
|
||
| // Note: TransactionLog status is immutable in schema; order_refunding log remains. Order status Refunded is the source of truth. | ||
| // Set order status to Refunded | ||
| _, err = storage.Client.PaymentOrder.UpdateOneID(order.ID).SetStatus(paymentorder.StatusRefunded).Save(ctx) | ||
| if err != nil { | ||
| logger.WithFields(logger.Fields{ | ||
| "Error": fmt.Sprintf("%v", err), | ||
| "OrderID": orderID.String(), | ||
| }).Errorf("Failed to update order status to refunded") | ||
| u.APIResponse(ctx, http.StatusInternalServerError, "error", "Failed to update order status", nil) | ||
| return | ||
| } | ||
|
|
||
| // Release reserved token balance | ||
| if order.Edges.Token != nil && order.Edges.Provider != nil { | ||
| totalCryptoReserved := order.Amount.Add(order.SenderFee) | ||
| if relErr := ctrl.balanceService.ReleaseTokenBalance(ctx, order.Edges.Provider.ID, order.Edges.Token.ID, totalCryptoReserved, nil); relErr != nil { | ||
| logger.Errorf("Failed to release token balance for refunded order (order %s): %v", orderID, relErr) | ||
| } | ||
| } | ||
| u.APIResponse(ctx, http.StatusOK, "success", "Refund completed", nil) | ||
| return |
There was a problem hiding this comment.
Send webhook when refund outcome is marked Refunded.
Refund completion currently updates the order status but never notifies the sender.
💡 Suggested fix
_, err = storage.Client.PaymentOrder.UpdateOneID(order.ID).SetStatus(paymentorder.StatusRefunded).Save(ctx)
if err != nil {
logger.WithFields(logger.Fields{
"Error": fmt.Sprintf("%v", err),
"OrderID": orderID.String(),
}).Errorf("Failed to update order status to refunded")
u.APIResponse(ctx, http.StatusInternalServerError, "error", "Failed to update order status", nil)
return
}
+ if whErr := u.SendPaymentOrderWebhook(ctx, order); whErr != nil {
+ logger.WithFields(logger.Fields{
+ "OrderID": orderID.String(),
+ "Error": whErr,
+ }).Errorf("handleRefundOutcomeFulfillment: webhook failed")
+ }🤖 Prompt for AI Agents
In `@controllers/provider/provider.go` around lines 1430 - 1472, When a
fulfillment is marked paymentorderfulfillment.ValidationStatusRefunded we update
statuses and release balances but never notify the sender; add a
webhook/notification call after the token release (or before the final
u.APIResponse) to notify the sender of the refund completion. Locate the
refunded branch that handles payload.ValidationStatus ==
paymentorderfulfillment.ValidationStatusRefunded (around the
fulfillmentUpdate.Save,
PaymentOrder.UpdateOneID(...).SetStatus(paymentorder.StatusRefunded), and
ctrl.balanceService.ReleaseTokenBalance calls) and invoke your existing
webhook/notification API (e.g. ctrl.webhookService.SendOrderEvent or
ctrl.notificationService.NotifySender — if no such helper exists, add a new
method on ctrl to emit the order refund event) passing the order identifier, new
status (StatusRefunded) and any relevant payload (TxID, PSP, ValidationError),
then proceed to u.APIResponse(ctx, http.StatusOK, "success", "Refund completed",
nil).
| case utils.SettleInEventSignature: | ||
| // SettleIn (onramp) event | ||
| orderIdStr, ok := indexedParams["orderId"].(string) | ||
| if !ok || orderIdStr == "" { | ||
| continue | ||
| } | ||
| liquidityProviderStr, _ := indexedParams["liquidityProvider"].(string) | ||
| recipientStr, ok := indexedParams["recipient"].(string) | ||
| if !ok || recipientStr == "" { | ||
| continue | ||
| } | ||
| amountStr, ok := nonIndexedParams["amount"].(string) | ||
| if !ok || amountStr == "" { | ||
| continue | ||
| } | ||
| amount, err := decimal.NewFromString(amountStr) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| tokenStr, ok := nonIndexedParams["token"].(string) | ||
| if !ok { | ||
| tokenStr = "" | ||
| } | ||
| aggregatorFeeStr, _ := nonIndexedParams["aggregatorFee"].(string) | ||
| var aggregatorFee decimal.Decimal | ||
| if aggregatorFeeStr == "" { | ||
| aggregatorFee = decimal.Zero | ||
| } else { | ||
| var parseErr error | ||
| aggregatorFee, parseErr = decimal.NewFromString(aggregatorFeeStr) | ||
| if parseErr != nil { | ||
| logger.WithFields(logger.Fields{ | ||
| "Error": fmt.Sprintf("%v", parseErr), | ||
| "aggregatorFee": aggregatorFeeStr, | ||
| "orderId": orderIdStr, | ||
| "txHash": txHashFromEvent, | ||
| }).Warnf("SettleIn: failed to parse aggregatorFee, skipping event") | ||
| continue | ||
| } | ||
| } | ||
| rateStr, _ := nonIndexedParams["rate"].(string) | ||
| var rate decimal.Decimal | ||
| if rateStr == "" { | ||
| rate = decimal.Zero | ||
| } else { | ||
| var parseErr error | ||
| rate, parseErr = decimal.NewFromString(rateStr) | ||
| if parseErr != nil { | ||
| logger.WithFields(logger.Fields{ | ||
| "Error": fmt.Sprintf("%v", parseErr), | ||
| "rate": rateStr, | ||
| "orderId": orderIdStr, | ||
| "txHash": txHashFromEvent, | ||
| }).Warnf("SettleIn: failed to parse rate, skipping event") | ||
| continue | ||
| } | ||
| } | ||
| settleInEvent := &types.SettleInEvent{ | ||
| BlockNumber: blockNumber, | ||
| TxHash: txHashFromEvent, | ||
| OrderId: orderIdStr, | ||
| LiquidityProvider: ethcommon.HexToAddress(liquidityProviderStr).Hex(), | ||
| Amount: amount, | ||
| Recipient: ethcommon.HexToAddress(recipientStr).Hex(), | ||
| Token: ethcommon.HexToAddress(tokenStr).Hex(), | ||
| AggregatorFee: aggregatorFee, | ||
| Rate: rate, | ||
| } | ||
| settleInEvents = append(settleInEvents, settleInEvent) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual code in context
head -900 services/indexer/evm.go | tail -200Repository: paycrest/aggregator
Length of output: 6159
🏁 Script executed:
# Search for SettleIn event signature/definition
rg "SettleIn" services/indexer/ --type go -B 2 -A 2 | head -100Repository: paycrest/aggregator
Length of output: 6293
🏁 Script executed:
# Look for the event type definition
rg "type SettleInEvent" -A 20 --type goRepository: paycrest/aggregator
Length of output: 908
🏁 Script executed:
# Check the SettleOut path mentioned in the review
rg -n "SettleOut\|case utils.SettleOut" services/indexer/evm.go -A 30 | head -80Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Search for Gateway contract ABI or SettleIn event definition
rg "GatewayMetaData\|SettleIn" --type go -A 5 | grep -A 30 "SettleIn"Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Look for contracts package with ABI definitions
fd -e go -e json -e abi "contracts" | head -20Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Check if there's any contract ABI JSON files
fd -e json . | grep -i "gateway\|abi" | head -10Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Look for contracts directory structure
find . -type d -name "contracts" | head -5Repository: paycrest/aggregator
Length of output: 83
🏁 Script executed:
# Search for Gateway ABI or event signature in any file
rg "SettleIn.*event\|event.*SettleIn" -i --type goRepository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Check if there's documentation about SettleIn event fields
rg "liquidityProvider.*recipient\|indexed.*SettleIn" --type go -B 2 -A 2Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Look for the actual event signature or comments about required fields
rg "orderId.*liquidityProvider.*recipient\|indexed orderId" --type go -B 3 -A 3Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# List contents of contracts directory
ls -la services/contracts/Repository: paycrest/aggregator
Length of output: 521
🏁 Script executed:
# Look for Go files in contracts that might define the event
find services/contracts -type f -name "*.go" | head -20Repository: paycrest/aggregator
Length of output: 237
🏁 Script executed:
# Search for any comment or documentation about SettleIn event structure
rg "SettleIn" services/contracts --type go -B 5 -A 5 2>/dev/null | head -100Repository: paycrest/aggregator
Length of output: 50375
🏁 Script executed:
# Check TRON implementation to see how it handles these fields
rg -n "liquidityProviderStr :=" services/indexer/tron.go -B 2 -A 10 | grep -A 15 "SettleIn"Repository: paycrest/aggregator
Length of output: 1001
🏁 Script executed:
# Extract and format the SettleIn event from the ABI
python3 << 'EOF'
import json
# The ABI is massive, let's search for just the SettleIn event definition
with open('services/contracts/Gateway.go', 'r') as f:
content = f.read()
# Find the ABI string and extract it
start = content.find('ABI: "[')
end = content.find(']",', start) + 2
abi_str = content[start+6:end]
# Parse it as JSON
try:
abi = json.loads(abi_str)
# Find SettleIn event
for item in abi:
if item.get('type') == 'event' and item.get('name') == 'SettleIn':
print(json.dumps(item, indent=2))
except Exception as e:
print(f"Error: {e}")
EOFRepository: paycrest/aggregator
Length of output: 145
🏁 Script executed:
# Let's use a simpler approach - search for the SettleIn event details in the ABI string
rg '"name":"SettleIn"' services/contracts/Gateway.go -A 15 | head -50Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Or check what the TRON indexer tells us about SettleIn indexed fields
rg "indexed.*liquidityProvider\|liquidityProvider.*indexed" services/indexer/tron.go -B 5 -A 5Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Let's look at the SettleIn comment in TRON file more carefully
rg "SettleIn.*indexed\|indexed.*SettleIn" services/indexer/tron.go -B 2 -A 10Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Let's extract the actual ABI JSON more directly
sed -n '5,10p' services/contracts/Gateway.go | grep -o '"name":"SettleIn"[^}]*"type":"event"' | head -1Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Try to get the SettleIn event definition by searching for the exact pattern
rg -o '"indexed":[^,}]*,"internalType":"[^"]*","name":"liquidityProvider"' services/contracts/Gateway.go -A 2 -B 2Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Check what the Gateway.go ABI says about indexed fields in SettleIn
python3 << 'EOF'
import re
with open('services/contracts/Gateway.go', 'r') as f:
content = f.read()
# Find the ABI JSON string - it's on multiple lines
# Let's search for the SettleIn event section
match = re.search(r'\{"anonymous":false,"inputs":\[.*?"name":"SettleIn".*?\],"type":"event"\}', content, re.DOTALL)
if match:
event_str = match.group(0)
# Count indexed parameters
indexed_count = event_str.count('"indexed":true')
print(f"Found SettleIn event with {indexed_count} indexed parameters")
# Print first 500 chars
print(event_str[:500])
else:
print("SettleIn event not found in expected format")
# Try simpler search
if 'SettleIn' in content:
idx = content.find('SettleIn')
print("Found SettleIn at position, context:")
print(content[max(0,idx-200):min(len(content),idx+300)])
EOFRepository: paycrest/aggregator
Length of output: 644
🏁 Script executed:
# Now let's get more context around SettleIn to see all its parameters
python3 << 'EOF'
import re
with open('services/contracts/Gateway.go', 'r') as f:
content = f.read()
# Find SettleIn event - look backwards from "name":"SettleIn" to find the opening brace with inputs
settle_in_pos = content.find('"name":"SettleIn"')
if settle_in_pos > 0:
# Go backwards to find the opening of this event's inputs array
search_back = content[:settle_in_pos]
# Find the last occurrence of "inputs":[ before SettleIn
inputs_match = re.search(r'"inputs":\[([^\]]*)\]', search_back[::-1])
# Better approach: look for the full event definition
# SettleIn should be preceded by aggregatorFee parameter
start_search = max(0, settle_in_pos - 500)
event_section = content[start_search:settle_in_pos + 200]
print("Context around SettleIn:")
print(event_section[-400:])
EOFRepository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Let's search for the indexed parameters before SettleIn event
python3 << 'EOF'
with open('services/contracts/Gateway.go', 'r') as f:
content = f.read()
# Find SettleIn and work backwards
settle_in_idx = content.find('"name":"SettleIn"')
# Go back 800 chars to capture the full inputs array
context = content[max(0, settle_in_idx-800):settle_in_idx+100]
# Find the inputs array that leads to SettleIn
import re
# Look for indexed fields before SettleIn
indexed_fields = re.findall(r'"indexed":(true|false),"internalType":"[^"]*","name":"([^"]*)"', context)
print("Indexed fields before SettleIn:")
for is_indexed, name in indexed_fields[-7:]: # Last 7 fields should cover SettleIn
print(f" {name}: indexed={is_indexed}")
EOFRepository: paycrest/aggregator
Length of output: 94
🏁 Script executed:
# Let's dump a larger section of the Gateway.go file to see the ABI clearly
head -50 services/contracts/Gateway.go | tail -30Repository: paycrest/aggregator
Length of output: 942
🏁 Script executed:
# Extract the ABI string more carefully
python3 << 'EOF'
with open('services/contracts/Gateway.go', 'r') as f:
lines = f.readlines()
# The ABI is on line 5 (after the GatewayMetaData = &bind.MetaData{)
# Find the line with ABI
for i, line in enumerate(lines[:100]):
if 'ABI:' in line:
print(f"Line {i}: Found ABI definition")
# The ABI is a long JSON string, get a portion around SettleIn
# It might span multiple lines
content = ''.join(lines[i:i+20])
# Find SettleIn in this section
if 'SettleIn' in content:
idx = content.find('SettleIn')
print(content[max(0, idx-600):min(len(content), idx+400)])
break
EOFRepository: paycrest/aggregator
Length of output: 1093
🏁 Script executed:
# Direct search in the ABI: find all indexed parameters for SettleIn
python3 << 'EOF'
import re
with open('services/contracts/Gateway.go', 'r') as f:
full_content = f.read()
# Find the SettleIn event definition in the ABI - look for the full event object
# Pattern: {"inputs":[...], "name":"SettleIn", ...}
pattern = r'\{"inputs":\[(.*?)\],"(anonymous":false,")?name":"SettleIn"'
match = re.search(pattern, full_content, re.DOTALL)
if match:
inputs_str = match.group(1)
# Find all parameters with their indexed status
params = re.findall(r'"indexed":(true|false),"internalType":"[^"]*","name":"([^"]*)"', inputs_str)
print("SettleIn Event Parameters:")
for indexed, name in params:
print(f" {name}: indexed={indexed}")
else:
# Try alternate pattern
if 'SettleIn' in full_content:
# Find the section containing SettleIn
pos = full_content.find('"name":"SettleIn"')
# Extract surrounding context
start = max(0, pos - 1000)
end = min(len(full_content), pos + 200)
section = full_content[start:end]
# Look for indexed fields in this range
params = re.findall(r'"indexed":(true|false),"internalType":"[^"]*","name":"([^"]*)"', section)
print("Found parameters near SettleIn:")
for indexed, name in params:
print(f" {name}: indexed={indexed}")
EOFRepository: paycrest/aggregator
Length of output: 94
🏁 Script executed:
# Extract just the SettleIn event parameters to be crystal clear
python3 << 'EOF'
abiLine = """\"orderId\",\"type\":\"bytes32\"},{\"indexed\":true,\"internalType\":\"address\",\"name\":\"liquidityProvider\",\"type\":\"address\"},{\"indexed\":true,\"internalType\":\"address\",\"name\":\"recipient\",\"type\":\"address\"},{\"indexed\":false,\"internalType\":\"uint256\",\"name\":\"amount\",\"type\":\"uint256\"},{\"indexed\":false,\"internalType\":\"address\",\"name\":\"token\",\"type\":\"address\"},{\"indexed\":false,\"internalType\":\"uint256\",\"name\":\"aggregatorFee\",\"type\":\"uint256\"},{\"indexed\":false,\"internalType\":\"uint96\",\"name\":\"rate\",\"type\":\"uint96\"}],\"name\":\"SettleIn\""""
print("SettleIn Event Parameters from ABI:")
print("=" * 60)
print("INDEXED (from topics):")
print(" - orderId: indexed=true")
print(" - liquidityProvider: indexed=true")
print(" - recipient: indexed=true")
print()
print("NON-INDEXED (from data):")
print(" - amount: indexed=false")
print(" - token: indexed=false")
print(" - aggregatorFee: indexed=false")
print(" - rate: indexed=false")
EOFRepository: paycrest/aggregator
Length of output: 406
liquidityProviderStr and tokenStr are not validated, creating inconsistency and silent zero-address entries.
Per the contract ABI, liquidityProvider is an indexed parameter (required), yet line 782 uses a blank _ for the ok check, unlike the SettleOut path (lines 760-763) which properly validates it as non-empty. If liquidityProviderStr is missing or empty, ethcommon.HexToAddress("") on line 837 silently stores the zero address.
Similarly, tokenStr (lines 795-798) allows empty values, producing a zero address on line 840.
Validate both fields consistently:
Proposed fix
- liquidityProviderStr, _ := indexedParams["liquidityProvider"].(string)
+ liquidityProviderStr, ok := indexedParams["liquidityProvider"].(string)
+ if !ok || liquidityProviderStr == "" {
+ continue
+ }- tokenStr, ok := nonIndexedParams["token"].(string)
- if !ok {
- tokenStr = ""
- }
+ tokenStr, ok := nonIndexedParams["token"].(string)
+ if !ok || tokenStr == "" {
+ continue
+ }📝 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.
| case utils.SettleInEventSignature: | |
| // SettleIn (onramp) event | |
| orderIdStr, ok := indexedParams["orderId"].(string) | |
| if !ok || orderIdStr == "" { | |
| continue | |
| } | |
| liquidityProviderStr, _ := indexedParams["liquidityProvider"].(string) | |
| recipientStr, ok := indexedParams["recipient"].(string) | |
| if !ok || recipientStr == "" { | |
| continue | |
| } | |
| amountStr, ok := nonIndexedParams["amount"].(string) | |
| if !ok || amountStr == "" { | |
| continue | |
| } | |
| amount, err := decimal.NewFromString(amountStr) | |
| if err != nil { | |
| continue | |
| } | |
| tokenStr, ok := nonIndexedParams["token"].(string) | |
| if !ok { | |
| tokenStr = "" | |
| } | |
| aggregatorFeeStr, _ := nonIndexedParams["aggregatorFee"].(string) | |
| var aggregatorFee decimal.Decimal | |
| if aggregatorFeeStr == "" { | |
| aggregatorFee = decimal.Zero | |
| } else { | |
| var parseErr error | |
| aggregatorFee, parseErr = decimal.NewFromString(aggregatorFeeStr) | |
| if parseErr != nil { | |
| logger.WithFields(logger.Fields{ | |
| "Error": fmt.Sprintf("%v", parseErr), | |
| "aggregatorFee": aggregatorFeeStr, | |
| "orderId": orderIdStr, | |
| "txHash": txHashFromEvent, | |
| }).Warnf("SettleIn: failed to parse aggregatorFee, skipping event") | |
| continue | |
| } | |
| } | |
| rateStr, _ := nonIndexedParams["rate"].(string) | |
| var rate decimal.Decimal | |
| if rateStr == "" { | |
| rate = decimal.Zero | |
| } else { | |
| var parseErr error | |
| rate, parseErr = decimal.NewFromString(rateStr) | |
| if parseErr != nil { | |
| logger.WithFields(logger.Fields{ | |
| "Error": fmt.Sprintf("%v", parseErr), | |
| "rate": rateStr, | |
| "orderId": orderIdStr, | |
| "txHash": txHashFromEvent, | |
| }).Warnf("SettleIn: failed to parse rate, skipping event") | |
| continue | |
| } | |
| } | |
| settleInEvent := &types.SettleInEvent{ | |
| BlockNumber: blockNumber, | |
| TxHash: txHashFromEvent, | |
| OrderId: orderIdStr, | |
| LiquidityProvider: ethcommon.HexToAddress(liquidityProviderStr).Hex(), | |
| Amount: amount, | |
| Recipient: ethcommon.HexToAddress(recipientStr).Hex(), | |
| Token: ethcommon.HexToAddress(tokenStr).Hex(), | |
| AggregatorFee: aggregatorFee, | |
| Rate: rate, | |
| } | |
| settleInEvents = append(settleInEvents, settleInEvent) | |
| case utils.SettleInEventSignature: | |
| // SettleIn (onramp) event | |
| orderIdStr, ok := indexedParams["orderId"].(string) | |
| if !ok || orderIdStr == "" { | |
| continue | |
| } | |
| liquidityProviderStr, ok := indexedParams["liquidityProvider"].(string) | |
| if !ok || liquidityProviderStr == "" { | |
| continue | |
| } | |
| recipientStr, ok := indexedParams["recipient"].(string) | |
| if !ok || recipientStr == "" { | |
| continue | |
| } | |
| amountStr, ok := nonIndexedParams["amount"].(string) | |
| if !ok || amountStr == "" { | |
| continue | |
| } | |
| amount, err := decimal.NewFromString(amountStr) | |
| if err != nil { | |
| continue | |
| } | |
| tokenStr, ok := nonIndexedParams["token"].(string) | |
| if !ok || tokenStr == "" { | |
| continue | |
| } | |
| aggregatorFeeStr, _ := nonIndexedParams["aggregatorFee"].(string) | |
| var aggregatorFee decimal.Decimal | |
| if aggregatorFeeStr == "" { | |
| aggregatorFee = decimal.Zero | |
| } else { | |
| var parseErr error | |
| aggregatorFee, parseErr = decimal.NewFromString(aggregatorFeeStr) | |
| if parseErr != nil { | |
| logger.WithFields(logger.Fields{ | |
| "Error": fmt.Sprintf("%v", parseErr), | |
| "aggregatorFee": aggregatorFeeStr, | |
| "orderId": orderIdStr, | |
| "txHash": txHashFromEvent, | |
| }).Warnf("SettleIn: failed to parse aggregatorFee, skipping event") | |
| continue | |
| } | |
| } | |
| rateStr, _ := nonIndexedParams["rate"].(string) | |
| var rate decimal.Decimal | |
| if rateStr == "" { | |
| rate = decimal.Zero | |
| } else { | |
| var parseErr error | |
| rate, parseErr = decimal.NewFromString(rateStr) | |
| if parseErr != nil { | |
| logger.WithFields(logger.Fields{ | |
| "Error": fmt.Sprintf("%v", parseErr), | |
| "rate": rateStr, | |
| "orderId": orderIdStr, | |
| "txHash": txHashFromEvent, | |
| }).Warnf("SettleIn: failed to parse rate, skipping event") | |
| continue | |
| } | |
| } | |
| settleInEvent := &types.SettleInEvent{ | |
| BlockNumber: blockNumber, | |
| TxHash: txHashFromEvent, | |
| OrderId: orderIdStr, | |
| LiquidityProvider: ethcommon.HexToAddress(liquidityProviderStr).Hex(), | |
| Amount: amount, | |
| Recipient: ethcommon.HexToAddress(recipientStr).Hex(), | |
| Token: ethcommon.HexToAddress(tokenStr).Hex(), | |
| AggregatorFee: aggregatorFee, | |
| Rate: rate, | |
| } | |
| settleInEvents = append(settleInEvents, settleInEvent) |
🤖 Prompt for AI Agents
In `@services/indexer/evm.go` around lines 776 - 844, The SettleIn branch is not
validating liquidityProviderStr and tokenStr which allows blank values to be
converted to the zero address; update the case for utils.SettleInEvent so
liquidityProviderStr is read with an ok check and treated as required (like the
SettleOut path) and tokenStr is validated for non-empty before calling
ethcommon.HexToAddress — if either required field is missing or empty, log/warn
and continue (skip the event) rather than silently using the zero address;
adjust usage around liquidityProviderStr, tokenStr, and construction of
types.SettleInEvent accordingly.
| switch statusVal { | ||
| case "success": | ||
| _, err = storage.Client.PaymentOrderFulfillment. | ||
| Create(). | ||
| SetOrderID(order.ID). | ||
| SetPsp(pspVal). | ||
| SetTxID(txIdVal). | ||
| SetValidationStatus(paymentorderfulfillment.ValidationStatusRefunded). | ||
| Save(ctx) | ||
| if err != nil { | ||
| logger.WithFields(logger.Fields{ | ||
| "OrderID": order.ID.String(), | ||
| "ProviderID": order.Edges.Provider.ID, | ||
| "Error": err, | ||
| }).Errorf("SyncPaymentOrderFulfillments.syncRefundingOrder: create fulfillment") | ||
| return | ||
| } | ||
| _, err = storage.Client.PaymentOrder.UpdateOneID(order.ID).SetStatus(paymentorder.StatusRefunded).Save(ctx) | ||
| if err != nil { | ||
| logger.WithFields(logger.Fields{ | ||
| "OrderID": order.ID.String(), | ||
| "ProviderID": order.Edges.Provider.ID, | ||
| "Error": err, | ||
| }).Errorf("SyncPaymentOrderFulfillments.syncRefundingOrder: set Refunded") | ||
| return | ||
| } | ||
| if order.Edges.Token != nil && order.Edges.Provider != nil { | ||
| balanceService := balance.New() | ||
| totalCryptoReserved := order.Amount.Add(order.SenderFee) | ||
| if relErr := balanceService.ReleaseTokenBalance(ctx, order.Edges.Provider.ID, order.Edges.Token.ID, totalCryptoReserved, nil); relErr != nil { | ||
| logger.WithFields(logger.Fields{ | ||
| "OrderID": order.ID.String(), | ||
| "ProviderID": order.Edges.Provider.ID, | ||
| "Error": relErr, | ||
| }).Errorf("SyncPaymentOrderFulfillments.syncRefundingOrder: release balance") | ||
| } | ||
| } |
There was a problem hiding this comment.
Send refund-completed webhook after marking an onramp order as refunded.
Right now syncRefundingOrder flips the order to Refunded without notifying the sender, so payment_order.refunded never fires for this path.
💡 Suggested fix
_, err = storage.Client.PaymentOrder.UpdateOneID(order.ID).SetStatus(paymentorder.StatusRefunded).Save(ctx)
if err != nil {
logger.WithFields(logger.Fields{
"OrderID": order.ID.String(),
"ProviderID": order.Edges.Provider.ID,
"Error": err,
}).Errorf("SyncPaymentOrderFulfillments.syncRefundingOrder: set Refunded")
return
}
+ if whErr := utils.SendPaymentOrderWebhook(ctx, order); whErr != nil {
+ logger.WithFields(logger.Fields{
+ "OrderID": order.ID.String(),
+ "Error": whErr,
+ }).Errorf("SyncPaymentOrderFulfillments.syncRefundingOrder: webhook failed")
+ }🤖 Prompt for AI Agents
In `@tasks/fulfillments_webhooks.go` around lines 765 - 801, syncRefundingOrder
marks the order as Refunded via
storage.Client.PaymentOrder.UpdateOneID(...).SetStatus(paymentorder.StatusRefunded)
but never emits the payment_order.refunded webhook; after the UpdateOneID
Save(ctx) succeeds (and after releasing balance if applicable) call the existing
webhook/event dispatch used elsewhere (e.g., the project’s payment-order webhook
sender function) to send a payment_order.refunded webhook for the order (include
order ID, status, and any relevant txId/psp fields), ensuring errors from the
webhook send are logged with the same logger context used in this function.
| "EventType": "Gateway", | ||
| "OrderCreated": counts.OrderCreated, | ||
| "OrderSettled": counts.OrderSettled, | ||
| "SettleOut": counts.SettleOut, |
There was a problem hiding this comment.
Missing SettleIn count in gateway event log output
Low Severity
When indexing gateway events by txHash, eventCounts.SettleIn is accumulated (line 2433) but omitted from the subsequent log statement (lines 2437–2447). The address-based indexing path correctly logs "SettleIn": counts.SettleIn at line 2514. This inconsistency means SettleIn events indexed via txHash are silently uncounted in logs, making on-ramp settlement debugging harder.
Additional Locations (1)
| rate, err := decimal.NewFromString(rateStr) | ||
| if err != nil { | ||
| return fmt.Errorf("SettleIn: invalid rate %q: %w", rateStr, err) | ||
| } |
There was a problem hiding this comment.
handleSettleInEvent may not convert rate from contract format
High Severity
In handleSettleInEvent, the rate from the webhook event is read as a raw string without converting from the contract's scaled format. The contract stores rate as rate * 100 (uint96 where 100 = 1.00), and the Tron indexer uses FromSubunit(rate, 2) to unscale it. The webhook handler passes the raw value (e.g., 95000 instead of 950), causing UpdateOrderStatusSettleIn to receive a rate 100× too large.
| return | ||
| } | ||
| localStablecoinVolume[0].Sum = localStablecoinVolume[0].Sum.Div(fiatCurrency.MarketRate) | ||
| localStablecoinVolume[0].Sum = localStablecoinVolume[0].Sum.Div(fiatCurrency.MarketSellRate) |
There was a problem hiding this comment.
Potential division by zero with MarketSellRate in Stats
Medium Severity
Dividing by fiatCurrency.MarketSellRate can panic if the value is zero. The old code divided by a single MarketRate that was always populated. With the new two-sided rate model, MarketSellRate can independently be zero (e.g., currency only has buy rates configured, or rates task hasn't run yet), causing a runtime panic in the provider Stats endpoint.
- Added ExpireStaleOrders function to handle the expiration of orders past their validity, including onramp Pending orders and offramp initiated orders with expired receive addresses. - Updated the cron job to call ExpireStaleOrders every 6 minutes, replacing the previous receive address validity check. - Enhanced error handling and logging for expired orders and token balance release during expiration. - Refactored utility function for parsing validUntil metadata to improve clarity and consistency.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| "data": fmt.Sprintf("0x%x", settleInData), | ||
| "value": "0", | ||
| }, | ||
| } |
There was a problem hiding this comment.
EIP-7702 authorizationList sent as separate batch element
High Severity
The authorizationList is placed as a separate element in the params array rather than as a field on the transaction object. Every other call to SendTransactionBatch in the codebase structures each param element with to, data, and value. Here, the first element only contains authorizationList (with no transaction fields) and the second element contains the actual settleIn transaction (without the authorization list). The Engine API will treat these as two independent transactions — the first will be invalid, and the second will lack the EIP-7702 delegation. The authorizationList needs to be a property of the same map as to, data, and value for the settlement transaction to carry the delegation.
| Message: "EIP-7702 SetCodeAuthorization is required for payin fulfillment", | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
Authorization required even for failed validation reports
Medium Severity
The payload.Authorization == nil check at the top of handlePayinFulfillment blocks providers from reporting failed fiat validations. When ValidationStatus is Failed, no on-chain settlement occurs and the authorization is never used — the function just updates the fulfillment status, transitions the order, and releases reserved balance. Requiring an EIP-7702 SetCodeAuthorization for this path is unnecessary and prevents providers from declining payin orders they can't fulfill. The nil check belongs inside the ValidationStatusSuccess case only.
| "EventType": "Gateway", | ||
| "OrderCreated": counts.OrderCreated, | ||
| "OrderSettled": counts.OrderSettled, | ||
| "SettleOut": counts.SettleOut, |
There was a problem hiding this comment.
SettleIn count missing from first gateway event log
Low Severity
The first gateway event log (txHash-based indexing) accumulates SettleIn counts at line 2433 but omits it from the log fields at lines 2437-2447. The second occurrence (address-based indexing at lines 2506-2514) correctly includes "SettleIn": counts.SettleIn. This inconsistency means onramp settlement event counts won't appear in logs for transaction-hash-based indexing, making debugging harder.
Additional Locations (1)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controllers/sender/sender.go (1)
1747-1860:⚠️ Potential issue | 🟠 MajorDatabase transaction held open during external HTTP call to provider.
The DB transaction
txis opened at line 1749 (for balance reservation), but the provider/new_orderHTTP call at line 1854 happens inside this same transaction scope. The HTTP client has a 30-second timeout (CallProviderWithHMACusesfastshotwith 30s timeout). If the provider is slow, this holds a DB connection from the pool for up to 30+ seconds, risking connection pool exhaustion under load.Consider restructuring so the external call happens before the transaction, or splitting into two transactions: one for reservation and order creation, and a separate update for the provider account metadata after the external call returns.
Sketch of restructured flow
- Validate inputs, compute amounts/fees (as today)
- Call provider
/new_order(outside any DB tx) — get VA details- Open DB tx → reserve balance + create order + set metadata → commit
- Seed Redis key (with existing compensating logic)
This avoids holding a DB transaction open during the network round-trip.
🤖 Fix all issues with AI agents
In `@utils/utils.go`:
- Around line 610-617: The fallback that sets resp.AmountIn =
paymentOrder.Amount.String() is wrong; change the fallback to derive AmountIn
from paymentOrder.Direction (e.g., set resp.AmountIn = "fiat" for onramp orders
and "crypto" for offramp orders) so the GET response matches the create
responses (see resp.AmountIn and paymentOrder.Direction), or alternatively
persist the "amountIn" key into paymentOrder.Metadata during
initiateOnrampOrderV2 / initiateOfframpOrderV2 so the original metadata value is
available instead of falling back to the numeric amount string.
- Around line 572-619: The response currently recalculates sender percentage in
BuildV2PaymentOrderGetResponse by dividing paymentOrder.SenderFee by
paymentOrder.Amount; instead use the stored paymentOrder.FeePercent value
instead to avoid phantom percentages and rounding precision loss — set
SenderFeePercent from paymentOrder.FeePercent (format as the existing string
field expects, e.g., FeePercent.String() or equivalent) and keep the existing
fallback behavior only if FeePercent is nil/empty; update references around
SenderFeePercent and remove the reverse-calculation block that computes
senderFeePercentStr from SenderFee and Amount.
🧹 Nitpick comments (4)
tasks/refunds.go (1)
65-112: Consider bounding the onramp pending query to avoid scanning the full table over time.The offramp stream is naturally bounded by
ReceiveAddressExpiryLTE(time.Now()), but the onramp stream queries allDirectionOnramp + StatusPendingorders with no time constraint. As the table grows, this will scan an increasing number of rows every 6 minutes. Most stale orders will haveCreatedAtwithin a recent window (e.g.,OrderFulfillmentValidityago). Adding aCreatedAtLTEupper bound or an index-friendly time predicate would keep performance stable.♻️ Proposed optimization
pendingOnramp, err := storage.Client.PaymentOrder. Query(). Where( paymentorder.DirectionEQ(paymentorder.DirectionOnramp), paymentorder.StatusEQ(paymentorder.StatusPending), + // Only check orders old enough to possibly be expired + paymentorder.CreatedAtLTE(time.Now().Add(-orderConf.OrderFulfillmentValidity/2)), ).Also, the status update and balance release on lines 95–111 are not atomic—if
SetStatus(Expired)succeeds butReleaseTokenBalancefails, the balance stays reserved. The error is logged, so a subsequent run won't retry (the order is alreadyExpired). Consider wrapping both in a transaction, or adding a separate reconciliation/cleanup task for orphaned reservations.controllers/sender/sender.go (1)
791-796: Dead assignment:returnAddressis immediately overwritten.Line 793 assigns
senderOrderToken.RefundAddress, but line 796 unconditionally overwrites it withsource.RefundAddress. Unlike V1 (which conditionally overrides), V2 always uses the source-provided address. The first assignment is dead code.♻️ Simplify
- feePercent := senderOrderToken.FeePercent - feeAddress := senderOrderToken.FeeAddress - returnAddress := senderOrderToken.RefundAddress - - // Use refund address from source if provided - returnAddress = source.RefundAddress + feePercent := senderOrderToken.FeePercent + feeAddress := senderOrderToken.FeeAddress + returnAddress := source.RefundAddressutils/utils.go (2)
1429-1461: Significant query duplication between network-filtered and unfiltered branches.The two branches (lines 1429-1473 and 1476-1519) differ only in the network filter and
.Only(ctx)vs.First(ctx). The side-dependent balance predicates (buy/sell) are identically duplicated in both branches. Consider extracting the common query-building logic into a helper to reduce the ~80 lines of near-identical code.Also applies to: 1476-1507
1009-1015: Consolidate duplicateRateSidetype definition.
RateSideis defined identically in bothutils/utils.go(lines 1010–1015) andservices/priority_queue.go(lines 34–39). Go treats these as distinct types—passingutils.RateSideSellto a function expectingservices.RateSiderequires explicit conversion. Move the definition to a single location (e.g.,utils) and import it inservices/priority_queue.goto eliminate the type distinction and prevent conversion bugs.
| // BuildV2PaymentOrderGetResponse builds a full V2PaymentOrderGetResponse from payment order and optional provider fields. | ||
| func BuildV2PaymentOrderGetResponse( | ||
| paymentOrder *ent.PaymentOrder, | ||
| institution *ent.Institution, | ||
| transactionLogs []types.TransactionLog, | ||
| cancellationReasons []string, | ||
| otcExpiry *time.Time, | ||
| ) *types.V2PaymentOrderGetResponse { | ||
| source, destination, providerAccount := BuildV2OrderSourceDestinationProviderAccount(paymentOrder, institution) | ||
| txFee := paymentOrder.NetworkFee.Add(paymentOrder.ProtocolFee) | ||
| senderFeePercentStr := "" | ||
| if !paymentOrder.Amount.IsZero() { | ||
| senderFeePercentStr = paymentOrder.SenderFee.Div(paymentOrder.Amount).Mul(decimal.NewFromInt(100)).String() | ||
| } | ||
| resp := &types.V2PaymentOrderGetResponse{ | ||
| ID: paymentOrder.ID, | ||
| Status: string(paymentOrder.Status), | ||
| Direction: string(paymentOrder.Direction), | ||
| CreatedAt: paymentOrder.CreatedAt, | ||
| UpdatedAt: paymentOrder.UpdatedAt, | ||
| Amount: paymentOrder.Amount.String(), | ||
| AmountInUsd: paymentOrder.AmountInUsd.String(), | ||
| AmountPaid: paymentOrder.AmountPaid.String(), | ||
| AmountReturned: paymentOrder.AmountReturned.String(), | ||
| PercentSettled: paymentOrder.PercentSettled.String(), | ||
| Rate: paymentOrder.Rate.String(), | ||
| SenderFee: paymentOrder.SenderFee.String(), | ||
| SenderFeePercent: senderFeePercentStr, | ||
| TransactionFee: txFee.String(), | ||
| Reference: paymentOrder.Reference, | ||
| ProviderAccount: providerAccount, | ||
| Source: source, | ||
| Destination: destination, | ||
| TxHash: paymentOrder.TxHash, | ||
| TransactionLogs: transactionLogs, | ||
| CancellationReasons: cancellationReasons, | ||
| OTCExpiry: otcExpiry, | ||
| } | ||
| if paymentOrder.Metadata != nil { | ||
| if amountIn, ok := paymentOrder.Metadata["amountIn"].(string); ok { | ||
| resp.AmountIn = amountIn | ||
| } | ||
| } | ||
| if resp.AmountIn == "" { | ||
| resp.AmountIn = paymentOrder.Amount.String() | ||
| } | ||
| return resp | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if FeePercent field exists on PaymentOrder entity
fd paymentorder.go | xargs cat -n | head -100Repository: paycrest/aggregator
Length of output: 6208
🏁 Script executed:
# Search for FeePercent references in the entire codebase
rg "FeePercent" -n | head -30Repository: paycrest/aggregator
Length of output: 2545
🏁 Script executed:
# Check how SenderFee and fees are set during order creation
rg -n "SetSenderFee|SenderFee.*=" controllers/sender/sender.go | head -20Repository: paycrest/aggregator
Length of output: 621
🏁 Script executed:
# Check how SenderFee and FeePercent are set in relation to each other
rg -B3 -A3 "SetSenderFee" controllers/sender/sender.go | head -40Repository: paycrest/aggregator
Length of output: 689
🏁 Script executed:
# Check the V2 payment order creation flow to understand fee handling
rg -B5 -A5 "initiateOnrampOrderV2|initiatePaymentOrderV2" controllers/sender/sender.go | head -60Repository: paycrest/aggregator
Length of output: 1222
🏁 Script executed:
# Look for how FeePercent is used in response building
rg -B2 -A2 "FeePercent" utils/utils.goRepository: paycrest/aggregator
Length of output: 698
🏁 Script executed:
# Get more context around the SetSenderFee calls to see if FeePercent is set
rg -B10 "SetSenderFee" controllers/sender/sender.go | grep -A10 "SetSenderFee"Repository: paycrest/aggregator
Length of output: 731
🏁 Script executed:
# Check if there are different code paths for V1 vs V2 APIs
rg -n "func (ctrl \*SenderController) initiate" controllers/sender/sender.go | head -10Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Look at the V2 payment order flow more closely, specifically around FeePercent
sed -n '1600,1850p' controllers/sender/sender.go | cat -nRepository: paycrest/aggregator
Length of output: 10655
🏁 Script executed:
# Check initiateOfframpOrderV2 to see if it also sets FeePercent
sed -n '1400,1600p' controllers/sender/sender.go | cat -nRepository: paycrest/aggregator
Length of output: 8479
🏁 Script executed:
# Look at test cases to understand expected SenderFeePercent behavior
rg -B5 -A5 "SenderFeePercent.*:" controllers/sender/sender_test.go | head -50Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Look for SenderFeePercent in test expectations
rg "SenderFeePercent" controllers/sender/sender_test.go -B2 -A2Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Check the V1 API initiate function to see how it handles this
sed -n '400,600p' controllers/sender/sender.go | cat -nRepository: paycrest/aggregator
Length of output: 8847
🏁 Script executed:
# Look at the offramp flow (initiateOfframpOrderV2) around line 1200-1300
sed -n '1200,1400p' controllers/sender/sender.go | cat -nRepository: paycrest/aggregator
Length of output: 8246
🏁 Script executed:
# See where BuildV2PaymentOrderGetResponse is called
rg "BuildV2PaymentOrderGetResponse" -B3 -A3 controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 612
🏁 Script executed:
# Compare the two response building patterns more carefully
rg -n "senderFeePercentStr|SenderFeePercent" utils/utils.go controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 1563
🏁 Script executed:
# Check if there are any tests verifying the SenderFeePercent in GET response
rg -B5 -A5 "SenderFeePercent.*:" controllers/sender/sender_test.go | grep -A5 -B5 "GetResponse\|get.*response"Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Look at lines around 1311-1325 to see the offramp response pattern
sed -n '1310,1330p' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 844
🏁 Script executed:
# Look at lines around 1937-1951 to see the onramp response pattern
sed -n '1935,1955p' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 821
🏁 Script executed:
# Check if FeePercent could be zero legitimately in other cases
rg -n "FeePercent.*Zero|IsZero.*FeePercent" controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 101
🏁 Script executed:
# Verify the exact check pattern used in controller responses
sed -n '1311,1315p' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 164
🏁 Script executed:
sed -n '1311,1315p' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 164
Use paymentOrder.FeePercent directly instead of reverse-calculating.
Line 584 reverse-calculates SenderFee / Amount * 100, which breaks in two scenarios: (1) when a fixed fee is provided, FeePercent is zero but reverse-calculation produces a phantom percentage, and (2) when fees are rounded to 4 decimals for UX (per the V2 API design), reverse-calculation from the rounded fee loses precision. The controllers already set and store FeePercent correctly during order creation; the GET response should use that stored value directly instead of recalculating.
♻️ Proposed fix
- senderFeePercentStr := ""
- if !paymentOrder.Amount.IsZero() {
- senderFeePercentStr = paymentOrder.SenderFee.Div(paymentOrder.Amount).Mul(decimal.NewFromInt(100)).String()
- }
+ senderFeePercentStr := ""
+ if !paymentOrder.FeePercent.IsZero() {
+ senderFeePercentStr = paymentOrder.FeePercent.String()
+ }🤖 Prompt for AI Agents
In `@utils/utils.go` around lines 572 - 619, The response currently recalculates
sender percentage in BuildV2PaymentOrderGetResponse by dividing
paymentOrder.SenderFee by paymentOrder.Amount; instead use the stored
paymentOrder.FeePercent value instead to avoid phantom percentages and rounding
precision loss — set SenderFeePercent from paymentOrder.FeePercent (format as
the existing string field expects, e.g., FeePercent.String() or equivalent) and
keep the existing fallback behavior only if FeePercent is nil/empty; update
references around SenderFeePercent and remove the reverse-calculation block that
computes senderFeePercentStr from SenderFee and Amount.
| if paymentOrder.Metadata != nil { | ||
| if amountIn, ok := paymentOrder.Metadata["amountIn"].(string); ok { | ||
| resp.AmountIn = amountIn | ||
| } | ||
| } | ||
| if resp.AmountIn == "" { | ||
| resp.AmountIn = paymentOrder.Amount.String() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the utils file and check the specific lines
fd -type f -name "utils.go" | grep -E "utils/utils\.go"Repository: paycrest/aggregator
Length of output: 233
🏁 Script executed:
# Once found, read utils/utils.go around lines 610-617
cat -n utils/utils.go | sed -n '600,625p'Repository: paycrest/aggregator
Length of output: 1050
🏁 Script executed:
# Search for amountIn in the codebase and its metadata usage
rg -n '"amountIn"' --type goRepository: paycrest/aggregator
Length of output: 545
🏁 Script executed:
# Check sender.go for lines 1323 and 1949
fd -type f -name "sender.go" | head -5Repository: paycrest/aggregator
Length of output: 233
🏁 Script executed:
# Find sender.go and read lines around 1323 and 1949
fd -type f -name "sender.go"Repository: paycrest/aggregator
Length of output: 233
🏁 Script executed:
# Read sender.go around line 1323
cat -n controllers/sender/sender.go | sed -n '1315,1335p'Repository: paycrest/aggregator
Length of output: 947
🏁 Script executed:
# Read sender.go around line 1949
cat -n controllers/sender/sender.go | sed -n '1940,1960p'Repository: paycrest/aggregator
Length of output: 964
🏁 Script executed:
# Search for where "amountIn" is set in metadata during order creation
rg -n 'Metadata\[.*amountIn' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Search for initiateOnrampOrderV2 and initiateOfframpOrderV2 to see if they set amountIn
rg -n 'initiateOnrampOrderV2\|initiateOfframpOrderV2' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Search for where amountIn is set in metadata (fix the rg command)
rg -n 'amountIn' controllers/sender/sender.go | grep -i 'metadata'Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Find initiateOnrampOrderV2 function
rg -n 'func.*initiateOnrampOrderV2' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 176
🏁 Script executed:
# Find initiateOfframpOrderV2 function
rg -n 'func.*initiateOfframpOrderV2' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 176
🏁 Script executed:
# Check what values AmountIn can take in the response
rg -n 'AmountIn.*=' controllers/sender/sender.go | head -20Repository: paycrest/aggregator
Length of output: 196
🏁 Script executed:
# Look for paymentorder.Direction enum to understand directions
fd -name "*.go" | xargs grep -l "DirectionOnramp\|DirectionOfframp" | head -5Repository: paycrest/aggregator
Length of output: 293
🏁 Script executed:
# Look at initiateOnrampOrderV2 to see how metadata is set during order creation
cat -n controllers/sender/sender.go | sed -n '1346,1700p' | head -80Repository: paycrest/aggregator
Length of output: 3233
🏁 Script executed:
# Look at initiateOfframpOrderV2 to see how metadata is set during order creation
cat -n controllers/sender/sender.go | sed -n '654,900p' | head -80Repository: paycrest/aggregator
Length of output: 3258
🏁 Script executed:
# Search for Direction enum definition
rg -n 'DirectionOnramp\|DirectionOfframp' --max-count 10Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Search for where paymentOrder.Metadata is assigned during creation
rg -n 'paymentOrder.Metadata\s*=' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Search for Direction type definition
rg -n 'Direction.*=' models/paymentorder/paymentorder.go 2>/dev/null || rg -n 'type Direction' --max-count 5Repository: paycrest/aggregator
Length of output: 121
🏁 Script executed:
# Look at how paymentOrder is created during order initiation
rg -n 'paymentorder\.Create\|InsertPaymentOrder' controllers/sender/sender.go | head -5Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Check more of initiateOnrampOrderV2 to see where paymentOrder is created and metadata set
cat -n controllers/sender/sender.go | sed -n '1500,1700p' | head -100Repository: paycrest/aggregator
Length of output: 4559
🏁 Script executed:
# Look at sender_test.go to understand how metadata is used in tests
cat -n controllers/sender/sender_test.go | sed -n '2010,2025p'Repository: paycrest/aggregator
Length of output: 716
🏁 Script executed:
# Search for where paymentOrder is created and saved with metadata in initiateOnrampOrderV2
cat -n controllers/sender/sender.go | sed -n '1700,1800p'Repository: paycrest/aggregator
Length of output: 4194
🏁 Script executed:
# Search for all occurrences of paymentorder.Create or similar in initiateOnrampOrderV2
rg -n 'Create.*paymentorder\|paymentOrder.*Create' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Look at the entire create payload to see if amountIn is stored in metadata
cat -n controllers/sender/sender.go | sed -n '1750,1850p'Repository: paycrest/aggregator
Length of output: 4456
🏁 Script executed:
# Check if Direction enum has DirectionOnramp value
cat -n ent/paymentorder/paymentorder.go | sed -n '315,350p'Repository: paycrest/aggregator
Length of output: 1223
🏁 Script executed:
# Check initiateOfframpOrderV2 to see if it stores amountIn in metadata
cat -n controllers/sender/sender.go | sed -n '800,950p' | grep -A 20 'Build metadata'Repository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Verify the complete metadata building for offramp
cat -n controllers/sender/sender.go | sed -n '850,1000p' | head -80Repository: paycrest/aggregator
Length of output: 3373
🏁 Script executed:
# Find where metadata is built and paymentOrder is created in initiateOfframpOrderV2
cat -n controllers/sender/sender.go | sed -n '1000,1150p' | head -100Repository: paycrest/aggregator
Length of output: 4322
🏁 Script executed:
# Check if amountIn is ever stored in metadata anywhere in the codebase
rg -n 'metadata\[.*amountIn' controllers/sender/sender.goRepository: paycrest/aggregator
Length of output: 45
🏁 Script executed:
# Find metadata building section in initiateOfframpOrderV2
cat -n controllers/sender/sender.go | sed -n '1100,1250p' | head -100Repository: paycrest/aggregator
Length of output: 4532
🏁 Script executed:
# Search for all SetMetadata calls to see what gets stored
rg -n 'SetMetadata' controllers/sender/sender.go -A 2 -B 2Repository: paycrest/aggregator
Length of output: 998
🏁 Script executed:
# Find the metadata building section in initiateOfframpOrderV2 (around line 1200-1250)
cat -n controllers/sender/sender.go | sed -n '1200,1250p'Repository: paycrest/aggregator
Length of output: 2003
🏁 Script executed:
# Look for where the metadata variable is created in initiateOfframpOrderV2
cat -n controllers/sender/sender.go | sed -n '1195,1245p'Repository: paycrest/aggregator
Length of output: 2044
AmountIn fallback returns numeric amount instead of "crypto"/"fiat".
Line 616 sets resp.AmountIn = paymentOrder.Amount.String(), which produces a numeric string like "100.5". But AmountIn must be "crypto" or "fiat" (matching the create responses at sender.go:1323 and 1949). The metadata key "amountIn" is never stored in paymentOrder.Metadata during order creation in either initiateOnrampOrderV2 or initiateOfframpOrderV2, so this fallback always fires and produces incorrect output.
Fix by deriving from paymentOrder.Direction:
Proposed fix
if paymentOrder.Metadata != nil {
if amountIn, ok := paymentOrder.Metadata["amountIn"].(string); ok {
resp.AmountIn = amountIn
}
}
if resp.AmountIn == "" {
- resp.AmountIn = paymentOrder.Amount.String()
+ if paymentOrder.Direction == paymentorder.DirectionOnramp {
+ resp.AmountIn = "fiat"
+ } else {
+ resp.AmountIn = "crypto"
+ }
}Alternatively, persist amountIn in metadata during order creation so the get response matches the create response exactly.
📝 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.
| if paymentOrder.Metadata != nil { | |
| if amountIn, ok := paymentOrder.Metadata["amountIn"].(string); ok { | |
| resp.AmountIn = amountIn | |
| } | |
| } | |
| if resp.AmountIn == "" { | |
| resp.AmountIn = paymentOrder.Amount.String() | |
| } | |
| if paymentOrder.Metadata != nil { | |
| if amountIn, ok := paymentOrder.Metadata["amountIn"].(string); ok { | |
| resp.AmountIn = amountIn | |
| } | |
| } | |
| if resp.AmountIn == "" { | |
| if paymentOrder.Direction == paymentorder.DirectionOnramp { | |
| resp.AmountIn = "fiat" | |
| } else { | |
| resp.AmountIn = "crypto" | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@utils/utils.go` around lines 610 - 617, The fallback that sets resp.AmountIn
= paymentOrder.Amount.String() is wrong; change the fallback to derive AmountIn
from paymentOrder.Direction (e.g., set resp.AmountIn = "fiat" for onramp orders
and "crypto" for offramp orders) so the GET response matches the create
responses (see resp.AmountIn and paymentOrder.Direction), or alternatively
persist the "amountIn" key into paymentOrder.Metadata during
initiateOnrampOrderV2 / initiateOfframpOrderV2 so the original metadata value is
available instead of falling back to the numeric amount string.
1 participant
Description
This PR implements the onramp (payin) payment flow end-to-end and introduces direction-aware offramp/onramp handling, two-sided pricing (buy/sell), webhook versioning, and aligned settling/refunding flows. Settlement and refund statuses are set when a transaction is submitted (settling / refunding); the indexer sets the final settled / refunded status and
tx_hashwhen events are indexed.Breaking changes / API and data model: Payment order gains
direction(offramp/onramp) andrefund_or_recipient_address(rename fromreturn_address). Provider order token moves from singlefixed_conversion_rate/floating_conversion_ratetofixed_buy_rate,fixed_sell_rate,floating_buy_delta,floating_sell_delta. Fiat currency moves frommarket_ratetomarket_buy_rate/market_sell_rate. Indexer method renamed fromIndexProviderAddresstoIndexProviderSettlementAddress. Accept order API extended with direction and payin-specific fields (SetCodeAuthorization, chainId, rpcUrl, delegationAddress). Sender profile gainswebhook_version; webhook payloads includewebhookVersionand v2 addsdirection,recipient/refundAccountwith metadata (no KYC in webhook).Implementation details:
executePayinSettlementbuildsauthorizationListfrom providerSetCodeAuthorization(V → yParity) and sends EIP-7702 tx plussettleInvia EngineSendTransactionBatch. Returns onlyerror; indexer setstx_hashand settled when SettleIn is indexed. On submit: order → settling, log order_settling (no tx hash yet), API "Settlement submitted". Webhook when indexer marks settled.ProcessSettleOutOrders/UpdateOrderStatusSettleOut. SettleIn (onramp, EVM/Tron):ProcessSettleInOrders/UpdateOrderStatusSettleIn(event: orderId, liquidityProvider, amount, token, aggregatorFee, rate, recipient). Starknet: keeps OrderSettled;ProcessOrderSettledOrdersmaps to SettleOut shape; onramp not supported on Starknet here.webhook_version"1" (default) or "2". All payloads include top-levelwebhookVersion. v2:direction,recipient/refundAccountwith metadata; no KYC.ValidateRateuse RateSide (buy/sell). Redis bucket keys include side suffix (e.g.bucket_{currency}_{min}_{max}_sell). Offramp = sell; onramp = buy. tasks/rates and provider token config use buy/sell rates.executePayinSettlement, sets settling and order_settling log. AcceptOrderResponse includes Metadata passed to Disburse. Removed redundant uint64 conversion for rate/senderFee in settleIn; pass*big.Intto Gateway ABI._sellsuffix for rate validation.References
Testing
Manual/QA: Verify payin fulfillment submits EIP-7702 + settleIn tx and order moves to settling; indexer sets tx hash and settled on SettleIn. Verify offramp SettleOut and refund flows (order_refunding → order_refunded). Verify webhook v1/v2 payloads and
direction. Verify two-sided rates (buy/sell) for onramp and offramp.Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
Improvements
Changes