feat: add positive output vault balance order filter#2562
Open
feat: add positive output vault balance order filter#2562
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR introduces a new SQLite scalar function Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/quote/src/order_quotes.rs`:
- Around line 161-168: The zero-balance check is using the live snapshot
`order.outputs` instead of the outputs for the requested quote block, which can
drop valid historical pairs; update the call site that invokes
`output_vault_has_zero_orderbook_balance(&order.outputs, output.token,
output_vault_id)` to pass the outputs snapshot for the requested `block_number`
(the historical outputs/order state used to build the quote) instead of
`order.outputs` so the zero-balance decision is based on the quote's block
state; change the argument to the appropriate variable representing outputs at
the quote block (the historical/order-at-block outputs snapshot used elsewhere
in the quote-generation flow) in the same conditional.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 739414e0-4ca1-449e-83d2-63658485c754
📒 Files selected for processing (2)
crates/quote/ARCHITECTURE.mdcrates/quote/src/order_quotes.rs
findolor
force-pushed
the
t3code/safe-zero-balance-filter
branch
from
07f0af9 to
56699f4
Compare
findolor
changed the title
Contributor
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)
crates/common/src/raindex_client/orders.rs (1)
1366-1376:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
fetch_orders_for_pairnever turns the new balance filter on.This helper still builds
GetOrdersFilterswithhas_positive_output_vault_balance: None. If it is part of quote candidate discovery, both the local-db and SG paths will bypass the new filtering entirely.#!/bin/bash set -euo pipefail # Find every caller of fetch_orders_for_pair. rg -n -C3 '\bfetch_orders_for_pair\s*\(' --type=rust # Check whether those callers are in quote/candidate-building flows. rg -n -C3 'fetch_orders_for_pair|get_quotes|order_quotes|build_candidate_from_quote' crates/common/src/raindex_client🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/raindex_client/orders.rs` around lines 1366 - 1376, The GetOrdersFilters constructed in fetch_orders_for_pair currently leaves has_positive_output_vault_balance as None, so the new balance-based filtering is never applied; change the construction in fetch_orders_for_pair (the GetOrdersFilters creation) to set has_positive_output_vault_balance: Some(true) when this helper is used for quote candidate discovery, or alternatively add a boolean parameter to fetch_orders_for_pair (e.g., require_positive_output_vault_balance) and propagate it to set has_positive_output_vault_balance accordingly; update all callers of fetch_orders_for_pair involved in quote/candidate-building flows (those found by searching for fetch_orders_for_pair/get_quotes/order_quotes/build_candidate_from_quote) to pass the flag so the local-db and SG paths actually enable the filter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/common/src/local_db/query/fetch_orders_common.rs`:
- Around line 49-66: The current POSITIVE_OUTPUT_VAULT_BALANCE_CLAUSE
(POSITIVE_OUTPUT_VAULT_BALANCE_EXISTS_BODY) only checks at order scope and still
exempts vault_id 0; change it to enforce pair-level semantics and drop the
special-case vault 0 exclusion. Update the EXISTS subquery so it correlates to
the specific pair (match io_balance and vb_balance to the pair identifiers used
in the outer query — e.g., token / pair id columns used by aliases l and la or
whatever pair key is present) instead of only matching the order, and remove the
"AND io_balance.vault_id != {zero_vault_id}" predicate; ensure
vb_balance.balance is tested per that same pair
(FLOAT_GT_ZERO(vb_balance.balance)) so only output vaults for the exact pair are
considered.
In `@crates/common/src/raindex_client/orders.rs`:
- Around line 1435-1468: sg_order_has_positive_output_vault_balance currently
uses any(...) so mixed-balance orders slip through and
sg_vault_has_positive_orderbook_balance short-circuits on vault_id ==
U256::ZERO, preserving the zero-vault exception; change
sg_order_has_positive_output_vault_balance to require all outputs be positive
(use all instead of any) and remove the early return for vault_id == U256::ZERO
inside sg_vault_has_positive_orderbook_balance so the zero vault is treated like
any other vault (only return false on parse failures or when the balance is not
> zero).
---
Outside diff comments:
In `@crates/common/src/raindex_client/orders.rs`:
- Around line 1366-1376: The GetOrdersFilters constructed in
fetch_orders_for_pair currently leaves has_positive_output_vault_balance as
None, so the new balance-based filtering is never applied; change the
construction in fetch_orders_for_pair (the GetOrdersFilters creation) to set
has_positive_output_vault_balance: Some(true) when this helper is used for quote
candidate discovery, or alternatively add a boolean parameter to
fetch_orders_for_pair (e.g., require_positive_output_vault_balance) and
propagate it to set has_positive_output_vault_balance accordingly; update all
callers of fetch_orders_for_pair involved in quote/candidate-building flows
(those found by searching for
fetch_orders_for_pair/get_quotes/order_quotes/build_candidate_from_quote) to
pass the flag so the local-db and SG paths actually enable the filter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1664e9a1-35aa-4a29-bbd6-49c02007f9c7
📒 Files selected for processing (9)
crates/common/src/local_db/functions/float_gt_zero.rscrates/common/src/local_db/functions/mod.rscrates/common/src/local_db/query/fetch_orders/mod.rscrates/common/src/local_db/query/fetch_orders/query.sqlcrates/common/src/local_db/query/fetch_orders_common.rscrates/common/src/local_db/query/fetch_orders_count/mod.rscrates/common/src/local_db/query/fetch_orders_count/query.sqlcrates/common/src/raindex_client/local_db/query/fetch_orders.rscrates/common/src/raindex_client/orders.rs
✅ Files skipped from review due to trivial changes (1)
- crates/common/src/local_db/query/fetch_orders_count/mod.rs
Comment on lines
+49
to
+66
| pub(crate) const POSITIVE_OUTPUT_VAULT_BALANCE_CLAUSE: &str = | ||
| "/*POSITIVE_OUTPUT_VAULT_BALANCE_CLAUSE*/"; | ||
| const POSITIVE_OUTPUT_VAULT_BALANCE_EXISTS_BODY: &str = "EXISTS ( | ||
| SELECT 1 FROM order_ios io_balance | ||
| JOIN running_vault_balances vb_balance | ||
| ON vb_balance.chain_id = io_balance.chain_id | ||
| AND vb_balance.orderbook_address = io_balance.orderbook_address | ||
| AND vb_balance.owner = l.order_owner | ||
| AND vb_balance.token = io_balance.token | ||
| AND vb_balance.vault_id = io_balance.vault_id | ||
| WHERE io_balance.chain_id = l.chain_id | ||
| AND io_balance.orderbook_address = l.orderbook_address | ||
| AND io_balance.transaction_hash = la.transaction_hash | ||
| AND io_balance.log_index = la.log_index | ||
| AND lower(io_balance.io_type) = 'output' | ||
| AND io_balance.vault_id != {zero_vault_id} | ||
| AND FLOAT_GT_ZERO(vb_balance.balance) | ||
| )"; |
Contributor
There was a problem hiding this comment.
This clause still works at order scope and still exempts vault id 0.
EXISTS (...) only proves that an order has some positive output vault, so mixed orders still keep their zero-balance output pairs. On top of that, io_balance.vault_id != {zero_vault_id} preserves the exact vault-id-0 bypass the PR is trying to remove. This needs pair-level filtering semantics, and the special-case exclusion for vault 0 should go away.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/common/src/local_db/query/fetch_orders_common.rs` around lines 49 -
66, The current POSITIVE_OUTPUT_VAULT_BALANCE_CLAUSE
(POSITIVE_OUTPUT_VAULT_BALANCE_EXISTS_BODY) only checks at order scope and still
exempts vault_id 0; change it to enforce pair-level semantics and drop the
special-case vault 0 exclusion. Update the EXISTS subquery so it correlates to
the specific pair (match io_balance and vb_balance to the pair identifiers used
in the outer query — e.g., token / pair id columns used by aliases l and la or
whatever pair key is present) instead of only matching the order, and remove the
"AND io_balance.vault_id != {zero_vault_id}" predicate; ensure
vb_balance.balance is tested per that same pair
(FLOAT_GT_ZERO(vb_balance.balance)) so only output vaults for the exact pair are
considered.
Collaborator
Author
There was a problem hiding this comment.
False positive for this use case.
outputs_ applies token_in, balance_gt: 0, and vaultId_not: 0 to the same output vault relation. With an output token filter, orders whose
requested output token vault has zero balance are excluded.
Returned data is still order-level, but pair pruning is not needed here.
Contributor
There was a problem hiding this comment.
@findolor, thanks for the clarification — that makes sense. The EXISTS subquery is intentionally order-scoped (does this order have any output vault with a positive balance and non-zero vault id?), and the vault_id != {zero_vault_id} condition deliberately mirrors the vaultId_not: 0 predicate on the subgraph side to keep both paths consistent. Pair-level pruning isn't needed here, so the implementation is correct. I'll mark this as a false positive.
✏️ Learnings added
Learnt from: findolor
Repo: rainlanguage/raindex PR: 2562
File: crates/common/src/local_db/query/fetch_orders_common.rs:49-66
Timestamp: 2026-04-30T10:27:57.541Z
Learning: In `crates/common/src/local_db/query/fetch_orders_common.rs`, the `POSITIVE_OUTPUT_VAULT_BALANCE_EXISTS_BODY` SQL clause is intentionally order-scoped (EXISTS checks whether the order has at least one output vault with a positive balance). The `io_balance.vault_id != {zero_vault_id}` predicate is deliberate, mirroring the subgraph-side `vaultId_not: 0` filter. Pair-level filtering is not required here; order-level semantics are the intended behavior.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2399
File: crates/rest_api/src/routes/take_orders.rs:74-95
Timestamp: 2026-01-16T12:07:15.521Z
Learning: In Rust projects using utoipa, when you use #[schema(...)] attributes (for example #[schema(example = json!(...))]), the json! macro is handled internally by utoipa's proc-macro at compile time. You do not need to import serde_json::json! in the module for these attributes. Do not add a serde_json::json! import solely for these schema attributes; ensure serde_json is present only if used elsewhere."
findolor
force-pushed
the
t3code/safe-zero-balance-filter
branch
from
56699f4 to
014016c
Compare
This was referenced Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Quote-layer zero-balance filtering was tied to the order snapshot returned by the indexer, which can be wrong for historical block quotes. Consumers only need a way to request orders that have usable indexed output vault liquidity before they build quote requests.
Solution
hasPositiveOutputVaultBalanceto order list filters as an optional feature flag.falseas no filter.trueas requiring at least one output vault withvaultId != 0and indexed balance greater than zero.outputs_nested filter.FLOAT_GT_ZEROfor local-db Rain Float balance comparison.Checks
By submitting this for review, I'm confirming I've done the following:
Verified locally:
nix develop -c cargo fmt --allnix develop -c cargo test -p rain_orderbook_subgraph_client positive_output_vault_balancenix develop -c cargo test -p rain_orderbook_common positive_output_vault_balancenix develop -c cargo test -p rain_orderbook_subgraph_clientnix develop -c cargo test -p rain_orderbook_commonnix develop -c ob-rs-testnix develop -c rainix-wasm-testnix develop -c rainix-rs-static