Refactor provider selection to use ProviderOrderToken queries and add helper utilities#732
Refactor provider selection to use ProviderOrderToken queries and add helper utilities#732
Conversation
📝 WalkthroughWalkthroughThis PR migrates provider selection from a Redis bucket-based matching system to direct database queries of ProviderOrderToken records. It removes associated cron jobs for bucket queue processing and introduces helper functions for currency resolution and per-provider volume aggregation. Provider assignment now filters candidates by network, token, currency, settlement address, and provider activity, then selects based on trust score and recent successful volume. Changes
Sequence DiagramsequenceDiagram
participant Order as Payment Order
participant Service as AssignPaymentOrder
participant DB as Database (ProviderOrderToken)
participant Rate as Rate Computation
participant Select as Provider Selection
Order->>Service: Request provider assignment
Service->>Service: Resolve institution currency
Service->>Service: Derive network from order
Service->>DB: Query eligible ProviderOrderToken candidates<br/>(filtered by network, token, currency,<br/>settlement address, active providers)
DB-->>Service: Return candidate list
Service->>Rate: Compute buy/sell rates for each candidate<br/>(fixed rates or market + delta)
Rate-->>Service: Return rates (rounded to 2 decimals)
Service->>DB: Query recent successful volume by provider<br/>(Validated/Settled orders, 24h window)
DB-->>Service: Return volume map
Service->>Select: Select provider by trust score<br/>+ recent successful volume
Select-->>Service: Return selected provider
Service-->>Order: Assign selected provider or fallback
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65163ac2e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| providerordertoken.SettlementAddressNEQ(""), | ||
| providerordertoken.HasProviderWith( | ||
| providerprofile.IsActive(true), | ||
| providerprofile.VisibilityModeEQ(providerprofile.VisibilityModePublic), | ||
| ), |
There was a problem hiding this comment.
validateBucketRate now queries any active public ProviderOrderToken and then returns the best nominal rate, but it no longer verifies ProviderBalances or the stuck-fulfillment threshold before picking a provider. ValidateRate feeds the quote/create-order paths in controllers/sender/sender.go and controllers/index.go, and sender-created orders persist rateResult.ProviderID, so a provider with no available liquidity (or one already over the stuck threshold) can now be quoted and locked onto an order even though AssignPaymentOrder will reject it later. In practice this turns formerly-valid quotes into orders that can only fail over or refund.
Useful? React with 👍 / 👎.
| institution, err := utils.GetInstitutionByCode(ctx, institutionCode, true) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if institution == nil || institution.Edges.FiatCurrency == nil { |
There was a problem hiding this comment.
This helper always re-resolves the institution with enabledFiatCurrency=true, and AssignPaymentOrder/sendOrderRequest now use it instead of the order's stored ProvisionBucket currency. The reassignment paths in tasks/order_requests.go and tasks/stale_ops.go operate on already-created orders that may sit around for hours; if the institution is later disabled or its fiat currency mapping changes, those retries now fail or reserve the wrong currency even though the order was created against the original bucket.
Useful? React with 👍 / 👎.
| providerordertoken.HasProviderWith( | ||
| providerprofile.IsActive(true), | ||
| providerprofile.VisibilityModeEQ(providerprofile.VisibilityModePublic), | ||
| providerprofile.HasProviderBalancesWith( | ||
| providerbalances.HasFiatCurrencyWith(fiatcurrency.IDEQ(currency.ID)), |
There was a problem hiding this comment.
When FallbackProviderID points to a public provider, this candidate filter now includes it in the primary assignment pool. That regresses the existing semantics in buildQueueForSide, which still skips the fallback provider so TryFallbackAssignment is only used after no normal provider matches. With the new ranking loop, a high-trust fallback can be selected on the very first assignment attempt for ordinary orders, bypassing FallbackTriedAt and sending regular traffic to the emergency node.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/priority_queue.go`:
- Around line 852-854: The DB query that builds the first-pass candidate set
(the providerordertoken.HasProviderWith(...) call using providerprofile.IsActive
and providerprofile.VisibilityModeEQ) must also exclude
orderConf.FallbackProviderID so the explicit fallback path
(TryFallbackAssignment) retains control; update the predicate passed to
providerordertoken.HasProviderWith to add an exclusion for the fallback provider
(e.g., a provider ID != orderConf.FallbackProviderID predicate) so
AssignPaymentOrder/matchRate candidate construction omits the fallback node; do
not alter fallback bookkeeping—only add the exclusion predicate to the existing
query.
- Around line 343-359: The helper providerOrderTokenRate currently derives
floating rates even when the currency market rate is zero; restore the same
guard used in GetProviderRate/tokenRateForBucket: keep the existing
FixedBuyRate/FixedSellRate checks, but before computing a floating rate verify
the corresponding MarketBuyRate/MarketSellRate is non-zero and if it is zero
return decimal.Zero; otherwise compute Market*.Add(Floating*Delta).RoundBank(2).
Update providerOrderTokenRate to reference
orderToken.Edges.Currency.MarketBuyRate/MarketSellRate and
orderToken.FloatingBuyDelta/FloatingSellDelta and return decimal.Zero when the
market rate is zero to avoid using the delta as the full quote.
- Around line 855-859: The current balance filter always checks fiat inventory
(providerprofile.HasProviderBalancesWith(providerbalances.HasFiatCurrencyWith(...),
...)), which is correct for RateSideSell but wrong for RateSideBuy; update the
match in AssignPaymentOrder/matchRate to choose side-aware balance predicates:
when order.RateSide == RateSideSell keep the existing fiat checks
(AvailableBalanceGTE(order.Amount * order.Rate)), and when order.RateSide ==
RateSideBuy switch to token-asset predicates (e.g.,
providerbalances.HasTokenCurrencyWith(token.ID) and
AvailableTokenBalanceGTE(order.Amount) or the equivalent token-available
predicate used elsewhere). Mirror the branching logic used in
utils.validateProviderRate so the same balance checks are applied and don’t
alter exclude-list, bucket-key, or other provider-selection logic.
In `@utils/utils.go`:
- Around line 1289-1304: The current providerordertoken query in
GetTokenRateFromQueue (built into q using providerordertoken.HasTokenWith,
HasCurrencyWith, SettlementAddressNEQ and HasProviderWith) does not enforce the
token-vs-fiat spendable-balance checks that validateProviderRate (and
validateBucketRate) apply; add the same availability predicates used by
validateProviderRate/validateBucketRate (the token-side and fiat-side balance
predicates comparing spendable inventory to the requested amount/side) to this
query so GetTokenRateFromQueue only returns providerordertokens that actually
have spendable inventory for the given amount and networkIdentifier; keep the
existing WithProvider(...).WithCurrency() usage and only append the additional
balance predicates (and still conditionally apply providerordertoken.NetworkEQ
when networkIdentifier != "").
- Around line 1309-1315: The loop currently only skips regular orders
out-of-range; add a symmetric guard for OTC orders: after calling
DetermineOrderType(orderToken, amount), if orderType ==
paymentorder.OrderTypeOtc then check the OTC-specific bounds on the token (e.g.,
orderToken.MinOrderAmountOtc and orderToken.MaxOrderAmountOtc) and continue the
loop when amount is below the OTC min or above the OTC max so out-of-range OTC
amounts are rejected early (retain the existing regular-order guard for
OrderTypeRegular).
- Around line 978-987: The token lookup in GetTokenRateFromQueue (the
tokenEntity =
storage.Client.Token.Query().Where(tokenEnt.SymbolEQ(tokenSymbol)).Only(...))
wrongly assumes symbol is unique; change the lookup to disambiguate by network
or handle multiple matches: either add a network parameter to
GetTokenRateFromQueue and update the call sites, then query with
.Where(tokenEnt.SymbolEQ(tokenSymbol), tokenEnt.NetworkEQ(network)).Only(...),
or keep the current signature and replace .Only(...) with a non-unique query
(.Where(tokenEnt.SymbolEQ(tokenSymbol)).All(...)) and explicitly handle multiple
results (return a clear error or select the correct token by additional
criteria) before calling validateBucketRate; update any affected callers
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e196e29-553d-4a54-b973-b2926a6fbe79
📒 Files selected for processing (5)
services/priority_queue.goservices/provider_selection_constants.goservices/provider_selection_helpers.gotasks/startup.goutils/utils.go
💤 Files with no reviewable changes (1)
- tasks/startup.go
| func providerOrderTokenRate(orderToken *ent.ProviderOrderToken, side RateSide) decimal.Decimal { | ||
| if orderToken == nil || orderToken.Edges.Currency == nil { | ||
| return decimal.Zero | ||
| } | ||
| switch side { | ||
| case RateSideBuy: | ||
| if !orderToken.FixedBuyRate.IsZero() { | ||
| return orderToken.FixedBuyRate | ||
| } | ||
| return orderToken.Edges.Currency.MarketBuyRate.Add(orderToken.FloatingBuyDelta).RoundBank(2) | ||
| default: | ||
| if !orderToken.FixedSellRate.IsZero() { | ||
| return orderToken.FixedSellRate | ||
| } | ||
| return orderToken.Edges.Currency.MarketSellRate.Add(orderToken.FloatingSellDelta).RoundBank(2) | ||
| } | ||
| } |
There was a problem hiding this comment.
Restore the market-rate guard in this helper.
GetProviderRate and tokenRateForBucket both refuse to derive floating rates when the market rate is zero. This copy doesn't, so a missing market rate turns Floating*Delta into the whole quote and can even produce negative rates during candidate matching.
Suggested fix
func providerOrderTokenRate(orderToken *ent.ProviderOrderToken, side RateSide) decimal.Decimal {
if orderToken == nil || orderToken.Edges.Currency == nil {
return decimal.Zero
}
switch side {
case RateSideBuy:
if !orderToken.FixedBuyRate.IsZero() {
return orderToken.FixedBuyRate
}
+ if orderToken.Edges.Currency.MarketBuyRate.IsZero() {
+ return decimal.Zero
+ }
return orderToken.Edges.Currency.MarketBuyRate.Add(orderToken.FloatingBuyDelta).RoundBank(2)
- default:
+ case RateSideSell:
if !orderToken.FixedSellRate.IsZero() {
return orderToken.FixedSellRate
}
+ if orderToken.Edges.Currency.MarketSellRate.IsZero() {
+ return decimal.Zero
+ }
return orderToken.Edges.Currency.MarketSellRate.Add(orderToken.FloatingSellDelta).RoundBank(2)
+ default:
+ return decimal.Zero
}
}📝 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.
| func providerOrderTokenRate(orderToken *ent.ProviderOrderToken, side RateSide) decimal.Decimal { | |
| if orderToken == nil || orderToken.Edges.Currency == nil { | |
| return decimal.Zero | |
| } | |
| switch side { | |
| case RateSideBuy: | |
| if !orderToken.FixedBuyRate.IsZero() { | |
| return orderToken.FixedBuyRate | |
| } | |
| return orderToken.Edges.Currency.MarketBuyRate.Add(orderToken.FloatingBuyDelta).RoundBank(2) | |
| default: | |
| if !orderToken.FixedSellRate.IsZero() { | |
| return orderToken.FixedSellRate | |
| } | |
| return orderToken.Edges.Currency.MarketSellRate.Add(orderToken.FloatingSellDelta).RoundBank(2) | |
| } | |
| } | |
| func providerOrderTokenRate(orderToken *ent.ProviderOrderToken, side RateSide) decimal.Decimal { | |
| if orderToken == nil || orderToken.Edges.Currency == nil { | |
| return decimal.Zero | |
| } | |
| switch side { | |
| case RateSideBuy: | |
| if !orderToken.FixedBuyRate.IsZero() { | |
| return orderToken.FixedBuyRate | |
| } | |
| if orderToken.Edges.Currency.MarketBuyRate.IsZero() { | |
| return decimal.Zero | |
| } | |
| return orderToken.Edges.Currency.MarketBuyRate.Add(orderToken.FloatingBuyDelta).RoundBank(2) | |
| case RateSideSell: | |
| if !orderToken.FixedSellRate.IsZero() { | |
| return orderToken.FixedSellRate | |
| } | |
| if orderToken.Edges.Currency.MarketSellRate.IsZero() { | |
| return decimal.Zero | |
| } | |
| return orderToken.Edges.Currency.MarketSellRate.Add(orderToken.FloatingSellDelta).RoundBank(2) | |
| default: | |
| return decimal.Zero | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/priority_queue.go` around lines 343 - 359, The helper
providerOrderTokenRate currently derives floating rates even when the currency
market rate is zero; restore the same guard used in
GetProviderRate/tokenRateForBucket: keep the existing FixedBuyRate/FixedSellRate
checks, but before computing a floating rate verify the corresponding
MarketBuyRate/MarketSellRate is non-zero and if it is zero return decimal.Zero;
otherwise compute Market*.Add(Floating*Delta).RoundBank(2). Update
providerOrderTokenRate to reference
orderToken.Edges.Currency.MarketBuyRate/MarketSellRate and
orderToken.FloatingBuyDelta/FloatingSellDelta and return decimal.Zero when the
market rate is zero to avoid using the delta as the full quote.
| providerordertoken.HasProviderWith( | ||
| providerprofile.IsActive(true), | ||
| providerprofile.VisibilityModeEQ(providerprofile.VisibilityModePublic), |
There was a problem hiding this comment.
Keep the fallback provider out of the first-pass candidate set.
Lines 433-435 still skip orderConf.FallbackProviderID during normal queue construction, but this new DB query no longer does. That lets the fallback node absorb regular traffic before TryFallbackAssignment runs, which bypasses the explicit fallback path and its bookkeeping.
As per coding guidelines, "Ask before changing provider assignment logic in services/priority_queue.go (AssignPaymentOrder, matchRate); do not change without considering exclude list, balance, and bucket keys".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/priority_queue.go` around lines 852 - 854, The DB query that builds
the first-pass candidate set (the providerordertoken.HasProviderWith(...) call
using providerprofile.IsActive and providerprofile.VisibilityModeEQ) must also
exclude orderConf.FallbackProviderID so the explicit fallback path
(TryFallbackAssignment) retains control; update the predicate passed to
providerordertoken.HasProviderWith to add an exclusion for the fallback provider
(e.g., a provider ID != orderConf.FallbackProviderID predicate) so
AssignPaymentOrder/matchRate candidate construction omits the fallback node; do
not alter fallback bookkeeping—only add the exclusion predicate to the existing
query.
| providerprofile.HasProviderBalancesWith( | ||
| providerbalances.HasFiatCurrencyWith(fiatcurrency.IDEQ(currency.ID)), | ||
| providerbalances.IsAvailableEQ(true), | ||
| providerbalances.AvailableBalanceGTE(order.Amount.Mul(order.Rate).RoundBank(0)), | ||
| ), |
There was a problem hiding this comment.
Use side-aware balance filters here.
Lines 855-859 always require fiat balance sized as order.Amount * order.Rate. That is right for RateSideSell, but RateSideBuy still needs token inventory; the old queue path and utils.validateProviderRate both branch that way. As written, onramp orders can skip providers that can actually fill them and accept providers with no token balance at all.
As per coding guidelines, "Ask before changing provider assignment logic in services/priority_queue.go (AssignPaymentOrder, matchRate); do not change without considering exclude list, balance, and bucket keys".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/priority_queue.go` around lines 855 - 859, The current balance
filter always checks fiat inventory
(providerprofile.HasProviderBalancesWith(providerbalances.HasFiatCurrencyWith(...),
...)), which is correct for RateSideSell but wrong for RateSideBuy; update the
match in AssignPaymentOrder/matchRate to choose side-aware balance predicates:
when order.RateSide == RateSideSell keep the existing fiat checks
(AvailableBalanceGTE(order.Amount * order.Rate)), and when order.RateSide ==
RateSideBuy switch to token-asset predicates (e.g.,
providerbalances.HasTokenCurrencyWith(token.ID) and
AvailableTokenBalanceGTE(order.Amount) or the equivalent token-available
predicate used elsewhere). Mirror the branching logic used in
utils.validateProviderRate so the same balance checks are applied and don’t
alter exclude-list, bucket-key, or other provider-selection logic.
| tokenEntity, err := storage.Client.Token.Query().Where(tokenEnt.SymbolEQ(tokenSymbol)).Only(context.Background()) | ||
| if err != nil { | ||
| return decimal.Decimal{}, err | ||
| } | ||
|
|
||
| rateResponse := marketRate | ||
| highestMaxAmount := decimal.NewFromInt(0) | ||
|
|
||
| // Scan through the buckets to find a suitable rate | ||
| for _, key := range keys { | ||
| bd, err := parseBucketKey(key) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| minAmount, maxAmount := bd.MinAmount, bd.MaxAmount | ||
|
|
||
| for index := 0; ; index++ { | ||
| // Get the topmost provider in the priority queue of the bucket | ||
| providerData, err := storage.RedisClient.LIndex(ctx, key, int64(index)).Result() | ||
| if err != nil { | ||
| break | ||
| } | ||
| parts := strings.Split(providerData, ":") | ||
| if len(parts) != 6 { | ||
| logger.WithFields(logger.Fields{ | ||
| "Error": fmt.Sprintf("%v", err), | ||
| "ProviderData": providerData, | ||
| "Token": tokenSymbol, | ||
| "Currency": fiatCurrency, | ||
| "MinAmount": minAmount, | ||
| "MaxAmount": maxAmount, | ||
| }).Errorf("GetTokenRate.InvalidProviderData: %v", providerData) | ||
| continue | ||
| } | ||
|
|
||
| // Skip entry if token doesn't match | ||
| if parts[1] != tokenSymbol { | ||
| continue | ||
| } | ||
|
|
||
| // Skip entry if order amount is not within provider's min and max order amount | ||
| minOrderAmount, err := decimal.NewFromString(parts[4]) | ||
| if err != nil { | ||
| continue | ||
| } | ||
|
|
||
| maxOrderAmount, err := decimal.NewFromString(parts[5]) | ||
| if err != nil { | ||
| continue | ||
| } | ||
|
|
||
| if orderAmount.LessThan(minOrderAmount) || orderAmount.GreaterThan(maxOrderAmount) { | ||
| continue | ||
| } | ||
|
|
||
| // Get fiat equivalent of the token amount | ||
| rate, _ := decimal.NewFromString(parts[3]) | ||
| fiatAmount := orderAmount.Mul(rate) | ||
|
|
||
| // Check if fiat amount is within the bucket range and set the rate | ||
| if fiatAmount.GreaterThanOrEqual(minAmount) && fiatAmount.LessThanOrEqual(maxAmount) { | ||
| rateResponse = rate | ||
| break | ||
| } else if maxAmount.GreaterThan(highestMaxAmount) { | ||
| // Get the highest max amount | ||
| highestMaxAmount = maxAmount | ||
| rateResponse = rate | ||
| } | ||
| } | ||
| currency, err := storage.Client.FiatCurrency.Query().Where(fiatcurrency.CodeEQ(fiatCurrency)).Only(context.Background()) | ||
| if err != nil { | ||
| return decimal.Decimal{}, err | ||
| } | ||
|
|
||
| return rateResponse, nil | ||
| result, err := validateBucketRate(context.Background(), tokenEntity, currency, orderAmount, "", side) | ||
| if err != nil { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Current lookup =="
sed -n '975,990p' utils/utils.go
echo
echo "== Evidence that symbol+network is already treated as the unique key =="
rg -n -C2 'symbol:network|same token symbol|USDT on Ethereum vs Tron' services utils
echo
echo "== Token schema/index definitions =="
fd 'token.go$' ent . -x rg -n -C2 'field\.String\("symbol"\)|field\.String\("network"\)|index|Unique' {}
echo
echo "== Other symbol-only token lookups =="
rg -n -C2 'Token\.Query\(\)\.Where\([^)]*SymbolEQ\(|HasTokenWith\([^)]*SymbolEQ\(' --type=goRepository: paycrest/aggregator
Length of output: 17595
Don't use Only() for symbol-only token lookup.
The Only() call at line 978 assumes tokenSymbol uniquely identifies a token, but the Token schema has no unique constraint on symbol. The codebase already handles same-symbol multi-network tokens (e.g., USDT on Ethereum vs Tron), as evidenced by services/priority_queue.go lines 463–464 and ProviderOrderToken's unique index including a network field. This lookup will fail at runtime once duplicate symbols exist on different networks.
Since GetTokenRateFromQueue() receives only the token symbol with no network context, either:
- Add a network parameter to disambiguate the token lookup, or
- Query by a composite key that includes network information instead of symbol alone.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/utils.go` around lines 978 - 987, The token lookup in
GetTokenRateFromQueue (the tokenEntity =
storage.Client.Token.Query().Where(tokenEnt.SymbolEQ(tokenSymbol)).Only(...))
wrongly assumes symbol is unique; change the lookup to disambiguate by network
or handle multiple matches: either add a network parameter to
GetTokenRateFromQueue and update the call sites, then query with
.Where(tokenEnt.SymbolEQ(tokenSymbol), tokenEnt.NetworkEQ(network)).Only(...),
or keep the current signature and replace .Only(...) with a non-unique query
(.Where(tokenEnt.SymbolEQ(tokenSymbol)).All(...)) and explicitly handle multiple
results (return a clear error or select the correct token by additional
criteria) before calling validateBucketRate; update any affected callers
accordingly.
| q := storage.Client.ProviderOrderToken.Query(). | ||
| Where( | ||
| providerordertoken.HasTokenWith(tokenEnt.IDEQ(token.ID)), | ||
| providerordertoken.HasCurrencyWith(fiatcurrency.CodeEQ(currency.Code)), | ||
| providerordertoken.SettlementAddressNEQ(""), | ||
| providerordertoken.HasProviderWith( | ||
| providerprofile.IsActive(true), | ||
| providerprofile.VisibilityModeEQ(providerprofile.VisibilityModePublic), | ||
| ), | ||
| ). | ||
| WithProvider(func(pq *ent.ProviderProfileQuery) { pq.WithProviderRating() }). | ||
| WithCurrency() | ||
| if networkIdentifier != "" { | ||
| q = q.Where(providerordertoken.NetworkEQ(networkIdentifier)) | ||
| } | ||
| orderTokens, err := q.All(ctx) |
There was a problem hiding this comment.
Keep validateBucketRate availability-aware.
Lines 1289-1304 now filter only on provider activity/visibility and a settlement address. The old queue path and validateProviderRate still enforce side-aware balances before returning a provider, so this DB path can now quote a rate/provider that has no spendable inventory for amount. Add the same token-vs-fiat balance predicates here so GetTokenRateFromQueue and assignment stay aligned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/utils.go` around lines 1289 - 1304, The current providerordertoken
query in GetTokenRateFromQueue (built into q using
providerordertoken.HasTokenWith, HasCurrencyWith, SettlementAddressNEQ and
HasProviderWith) does not enforce the token-vs-fiat spendable-balance checks
that validateProviderRate (and validateBucketRate) apply; add the same
availability predicates used by validateProviderRate/validateBucketRate (the
token-side and fiat-side balance predicates comparing spendable inventory to the
requested amount/side) to this query so GetTokenRateFromQueue only returns
providerordertokens that actually have spendable inventory for the given amount
and networkIdentifier; keep the existing WithProvider(...).WithCurrency() usage
and only append the additional balance predicates (and still conditionally apply
providerordertoken.NetworkEQ when networkIdentifier != "").
| for _, orderToken := range orderTokens { | ||
| if amount.LessThan(orderToken.MinOrderAmount) { | ||
| continue | ||
| } | ||
|
|
||
| // Get all providers in this bucket to find the first suitable one (priority queue order) | ||
| providers, err := storage.RedisClient.LRange(ctx, key, 0, -1).Result() | ||
| if err != nil { | ||
| logger.WithFields(logger.Fields{ | ||
| "Key": key, | ||
| "Error": err, | ||
| }).Errorf("ValidateRate.FailedToGetProviders: failed to get providers from bucket") | ||
| orderType := DetermineOrderType(orderToken, amount) | ||
| if orderType == paymentorder.OrderTypeRegular && amount.GreaterThan(orderToken.MaxOrderAmount) { | ||
| continue |
There was a problem hiding this comment.
Reject out-of-range OTC amounts in this loop.
DetermineOrderType returns OrderTypeOtc for any amount above MaxOrderAmount once OTC is configured. Because Lines 1313-1315 only skip the regular && amount > max case, amounts below MinOrderAmountOtc or above MaxOrderAmountOtc now pass here, even though validateProviderRate still rejects them.
Suggested guard
for _, orderToken := range orderTokens {
if amount.LessThan(orderToken.MinOrderAmount) {
continue
}
- orderType := DetermineOrderType(orderToken, amount)
- if orderType == paymentorder.OrderTypeRegular && amount.GreaterThan(orderToken.MaxOrderAmount) {
- continue
- }
+ orderType := paymentorder.OrderTypeRegular
+ if amount.GreaterThan(orderToken.MaxOrderAmount) {
+ if orderToken.MinOrderAmountOtc.IsZero() || orderToken.MaxOrderAmountOtc.IsZero() ||
+ amount.LessThan(orderToken.MinOrderAmountOtc) || amount.GreaterThan(orderToken.MaxOrderAmountOtc) {
+ continue
+ }
+ orderType = paymentorder.OrderTypeOtc
+ }📝 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.
| for _, orderToken := range orderTokens { | |
| if amount.LessThan(orderToken.MinOrderAmount) { | |
| continue | |
| } | |
| // Get all providers in this bucket to find the first suitable one (priority queue order) | |
| providers, err := storage.RedisClient.LRange(ctx, key, 0, -1).Result() | |
| if err != nil { | |
| logger.WithFields(logger.Fields{ | |
| "Key": key, | |
| "Error": err, | |
| }).Errorf("ValidateRate.FailedToGetProviders: failed to get providers from bucket") | |
| orderType := DetermineOrderType(orderToken, amount) | |
| if orderType == paymentorder.OrderTypeRegular && amount.GreaterThan(orderToken.MaxOrderAmount) { | |
| continue | |
| for _, orderToken := range orderTokens { | |
| if amount.LessThan(orderToken.MinOrderAmount) { | |
| continue | |
| } | |
| orderType := paymentorder.OrderTypeRegular | |
| if amount.GreaterThan(orderToken.MaxOrderAmount) { | |
| if orderToken.MinOrderAmountOtc.IsZero() || orderToken.MaxOrderAmountOtc.IsZero() || | |
| amount.LessThan(orderToken.MinOrderAmountOtc) || amount.GreaterThan(orderToken.MaxOrderAmountOtc) { | |
| continue | |
| } | |
| orderType = paymentorder.OrderTypeOtc | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/utils.go` around lines 1309 - 1315, The loop currently only skips
regular orders out-of-range; add a symmetric guard for OTC orders: after calling
DetermineOrderType(orderToken, amount), if orderType ==
paymentorder.OrderTypeOtc then check the OTC-specific bounds on the token (e.g.,
orderToken.MinOrderAmountOtc and orderToken.MaxOrderAmountOtc) and continue the
loop when amount is below the OTC min or above the OTC max so out-of-range OTC
amounts are rejected early (retain the existing regular-order guard for
OrderTypeRegular).
1 participant
Description
This PR begins the provider-selection redesign by moving the main assignment and rate-validation flow away from Redis bucket queue scans and toward direct database queries over
ProviderOrderToken.The main goal is to make provider selection more deterministic and easier to reason about by sourcing candidate eligibility from persisted provider token configuration and institution-derived fiat currency, instead of relying on provision-bucket queue state in Redis.
Key implementation details:
AssignPaymentOrdernow resolves the fiat currency from the order institution, queries eligibleProviderOrderTokenrecords directly, and filters/sorts candidates in code before attempting assignment.sendOrderRequestand fallback assignment paths now resolve currency from institution data instead of expectingProvisionBucketcurrency on the order.This is an incremental step toward the broader redesign in #714. It does not yet remove the bucket schema or complete Ent/migration updates for bucket removal. There are no intended API shape changes in this PR, but provider-selection behavior for assignment and rate lookup is affected.
References
Testing
Developed in the provided Codex container.
Commands run:
gofmt -w services/provider_selection_constants.go services/provider_selection_helpers.go services/priority_queue.go tasks/startup.go utils/utils.gogit diff --checktime go build ./servicestime go build ./...Additional environment/tooling checks:
time go generate ./entFailed in this environment because the Go proxy is blocked from downloading Ent CLI transitive dependencies.
apt-get update && apt-get install -y golang-golang-x-tools-dev golang-github-spf13-cobra-dev golang-github-olekukonko-tablewriter-dev golang-ariga-atlas-devFailed because apt repository access is also blocked by HTTP 403 responses in this environment.
time atlas migrate diff remove_provision_bucket --dir "file://ent/migrate/migrations" --to "ent://ent/schema?globalid=1" --dev-url "docker://postgres/15/test?search_path=public"Failed because
atlasis not installed and could not be installed in this environment.Not tested:
End-to-end runtime verification against live provider assignment flows in a deployed environment.
Ent regeneration and versioned migration generation, due the environment restrictions above.
This change adds test coverage for new/changed/fixed functionality
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.