feat: implement fallback rpcs and assets to hold redistribution feature - #27
Draft
khomiakmaxim wants to merge 9 commits into
Draft
khomiakmaxim wants to merge 9 commits into
khomiakmaxim wants to merge 9 commits into
Conversation
khomiakmaxim
changed the title
There was a problem hiding this comment.
Pull request overview
This PR introduces RPC failover support and expands the keeper’s “assets to hold” concept into a weighted portfolio rebalancing model. It updates the keeper to route RPC calls through a new failover client, and refactors the Balancer from “swap everything into one target” into a hub-based, thresholded, multi-leg rebalance batcher.
Changes:
- Add
FailoverClientand wire it throughGateway, executor, and collectors to support primary + fallback RPC endpoints with a per-call duration budget. - Redesign Balancer to rebalance toward per-asset target weights (bps) using a stablecoin hub asset, packing sells+buys into a single atomic batch with leg caps and impact gating.
- Improve capital accounting by introducing “available balance” (net of in-flight reservations) and using it for sizing.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates Balancer config naming in docs. |
| keeper/src/strategy/liquidator.rs | Sizes actions against available (unreserved) capital; updates settle hook to release multiple reservations. |
| keeper/src/strategy/balancer.rs | Major Balancer redesign: hub-based portfolio rebalance, multi-leg batching, new sizing/impact logic, new unit tests. |
| keeper/src/stellar/client.rs | Switches Gateway to use FailoverClient and updates constructor signature. |
| keeper/src/metrics/catalog.rs | Adds Balancer batch-leg metric and expands Balancer outcome labels. |
| keeper/src/main.rs | Wires new config fields (hub, fallbacks, call duration), adjusts asset-to-hold handling, and updates collectors to use Gateway. |
| keeper/src/liquidator_capital.rs | Adds try_get_available_balance and improves cache invalidation on reservation release. |
| keeper/src/execute/stellar_tx.rs | Updates executor to use FailoverClient and settle hook to release multiple reservation IDs. |
| keeper/src/execute/mod.rs | Exposes new execute::failover module. |
| keeper/src/execute/failover.rs | New failover wrapper around stellar_rpc_client::Client. |
| keeper/src/config.rs | Adds hub + distribution config and validation; introduces failover-related config fields; adds example-config tests. |
| keeper/src/collect/stellar_ledger.rs | Switches ledger polling to use Gateway (and thus failover). |
| keeper/src/collect/stellar_event.rs | Switches event polling to use Gateway (and thus failover). |
| docs/configuration.md | Documents hub + distribution-based Balancer behavior and new config fields. |
| config.example.toml | Updates example config for hub + distribution targets and failover fields. |
| config.example.json | Adds JSON example config for the new schema. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
801
to
+805
| return 0; | ||
| } | ||
| (token_amount.saturating_mul(info.oracle_price) / denom).max(0) | ||
|
|
||
| token_amount.saturating_mul(info.oracle_price) / denom | ||
| } |
Comment on lines
+344
to
+346
| let target_bps = | ||
| i128::from(self.config.assets_to_hold.get(&h.address).copied().unwrap_or(0)); // WARN: swallowing error here | ||
|
|
| let target_bps = | ||
| i128::from(self.config.assets_to_hold.get(&h.address).copied().unwrap_or(0)); // WARN: swallowing error here | ||
|
|
||
| let current_bps = h.value_cents * BPS_FACTOR / total_value_cents; // safe? |
Comment on lines
+699
to
704
| info!( | ||
| %provider, %asset_in, amount_in, amount_out, | ||
| price_impact_bps = impact.bps, | ||
| execution_price_scaled = impact.execution_price_scaled, | ||
| "probe within max price impact" | ||
| "probe within max execution impact" | ||
| ); |
Comment on lines
+709
to
+713
| warn!( | ||
| %provider, %asset_in, amount_in, amount_out, | ||
| price_impact_bps = impact.bps, | ||
| max = self.config.max_price_impact_bps, | ||
| "probe exceeds max price impact; halving amount_in" | ||
| max = self.config.max_execution_impact_bps, | ||
| "probe exceeds max execution impact; halving amount_in" |
Comment on lines
216
to
219
| // engine.add_strategy(Box::new(bad_debt_request_initiator)); | ||
| // engine.add_strategy(Box::new(liquidator)); | ||
| // engine.add_strategy(Box::new(withdrawer)); | ||
| engine.add_strategy(Box::new(balancer)); |
| @@ -0,0 +1,46 @@ | |||
| { | |||
| "rpc_url": "https://sorobn-testnet.stellar.org", | |||
Comment on lines
+19
to
+22
| # Wall-clock budget for a single logical RPC call across all failover attempts. | ||
| # Once exceeded, the failover loop stops trying further endpoints and returns | ||
| # the last transport error. Optional; defaults to 30. | ||
| rpc_max_call_duration_secs = 30 |
Comment on lines
+36
to
+38
| /// Optional fallback RPC endpoints, tried in order whenever the primary | ||
| /// [`Self::rpc_url`] is on cooldown or fails with a transport error. | ||
| pub fallback_rpc_urls: Vec<Url>, |
Comment on lines
+24
to
+27
| max_call_duration: Duration, | ||
| current_rpc_index: AtomicUsize, | ||
| uncallable_untill: Vec<AtomicU64>, | ||
| } |
…ation types to cli args
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (4)
keeper/src/strategy/balancer.rs:913
compute_value_centscan return a negative value whentoken_amountis negative, but the tests (and the caller logic) expect non-positive inputs to be treated as zero. As written,non_positive_inputs_are_safewill fail and portfolio valuation can go negative if available balance dips below zero.
fn compute_value_cents(token_amount: i128, info: &AssetInfo) -> i128 {
if info.oracle_price <= 0 {
return 0;
}
let denom_pow = info.decimals + info.oracle_decimals;
if denom_pow < 2 {
return 0;
}
let denom = 10_i128.pow(denom_pow - 2);
if denom == 0 {
return 0;
}
token_amount.saturating_mul(info.oracle_price) / denom
}
keeper/src/strategy/balancer.rs:188
- This
ifcontains commented-out logic and an inline note ("WRONG, right?") that makes the control flow hard to read and leaves uncertainty in production code. Since the conditional is just checking whether the keeper is the liquidator, simplify it and remove the commented code.
if liquidator != self.pkey
/*|| self.config.assets_to_hold.contains_key(&collateral_pool)*/ // WRONG, right?
{
return None;
}
config.example.json:2
- The example config's
rpc_urlhas a typo (sorobn-testnet) which makes the example unusable as-is.
"rpc_url": "https://sorobn-testnet.stellar.org",
keeper/src/config.rs:105
fallback_rpc_urlsandrpc_max_call_duration_secsare documented as optional, but the config schema requires them (no#[serde(default)]). This will break loading existing config files that don't include the new fields and contradicts the comments inconfig.example.toml. Consider adding#[serde(default)]forfallback_rpc_urlsand a#[serde(default = "...")]forrpc_max_call_duration_secs(e.g. 30s).
pub rpc_url: Url,
/// Optional fallback RPC endpoints, tried in order whenever the primary
/// [`Self::rpc_url`] is on cooldown or fails with a transport error.
pub fallback_rpc_urls: Vec<Url>,
/// Wall-clock budget for a single logical RPC call across all failover
/// attempts. Once exceeded, the failover loop stops trying further
/// endpoints and returns the last transport error.
#[validate(range(min = 1))]
pub rpc_max_call_duration_secs: u64,
Comment on lines
+86
to
+105
| let n = self.clients.len(); | ||
| let start = Instant::now(); | ||
|
|
||
| let start_from = self.current_rpc_index.load(Ordering::Relaxed); | ||
|
|
||
| let mut last_err: Option<Error> = None; | ||
| let mut attempted = false; | ||
|
|
||
| for pass in 0..2 { | ||
| if pass == 1 && attempted { | ||
| break; | ||
| } | ||
|
|
||
| for offset in 0..n { | ||
| let idx = (start_from + offset) % n; | ||
|
|
||
| if pass == 0 && self.is_uncallable(idx) { | ||
| continue; | ||
| } | ||
|
|
| ## Balancer | ||
|
|
||
| Converts non-target assets in the keeper's wallet back into the target asset (`assets_to_hold[0]`) via on-chain AMMs, so collateral seized in liquidations doesn't sit around as odd tokens. **Operates on `markets[0]` only.** Runs each interval and also reacts to the keeper's own `Liquidate`/`Withdraw` events. | ||
| Rebalances the held portfolio back toward the `assets_to_hold` target weights, using `hub_address` as the swap counterparty. Each cycle it values the whole portfolio in the hub's numeraire, finds assets that have drifted past the threshold, **sells** surpluses into the hub then **buys** deficits with the hub — all in a single atomic `submit_requests_batch` of up to `balancer_max_swaps_per_batch` legs. **Operates on `markets[0]` only.** Runs each interval and also reacts to the keeper's own `Liquidate`/`Withdraw` events. |
…ebt_results' into obligations' constructing events
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (4)
keeper/src/strategy/balancer.rs:913
compute_value_centscurrently returns negative values for negativetoken_amount(and it is relied on to be safe for non-positive inputs; the unit tests below assertcompute_value_cents(-100, ...) == 0). This can also skew portfolio weights if any balance math goes negative. Clamp the result to 0.
fn compute_value_cents(token_amount: i128, info: &AssetInfo) -> i128 {
if info.oracle_price <= 0 {
return 0;
}
let denom_pow = info.decimals + info.oracle_decimals;
if denom_pow < 2 {
return 0;
}
let denom = 10_i128.pow(denom_pow - 2);
if denom == 0 {
return 0;
}
token_amount.saturating_mul(info.oracle_price) / denom
}
keeper/src/strategy/balancer.rs:187
- This condition contains commented-out code and an inline "WRONG, right?" note, which makes the logic unclear and leaves dead/commented code in production. If you want to suppress rebalances when the seized collateral is one of the held assets, add a clear, correct check; otherwise remove the commented fragment.
let (liquidator, collateral_pool) =
(self.gateway.decode_topic(event, 1), self.gateway.decode_topic(event, 4));
if liquidator != self.pkey
/*|| self.config.assets_to_hold.contains_key(&collateral_pool)*/ // WRONG, right?
{
return None;
docs/configuration.md:88
- Docs say the Balancer "operates on markets[0] only", but the implementation now reads all configured markets'
MarketDatato reconcile oracle prices / spread-gate before submitting a batch on the first market. Update this sentence to match the current behavior.
Rebalances the held portfolio back toward the `assets_to_hold` target weights, using `hub_address` as the swap counterparty. Each cycle it values the whole portfolio in the hub's numeraire, finds assets that have drifted past the threshold, **sells** surpluses into the hub then **buys** deficits with the hub — all in a single atomic `submit_requests_batch` of up to `balancer_max_swaps_per_batch` legs. **Operates on `markets[0]` only.** Runs each interval and also reacts to the keeper's own `Liquidate`/`Withdraw` events.
config.example.toml:22
- The example says
rpc_max_call_duration_secsis optional with a default, butCliConfigrequires this field (no serde default). Adjust the comment so operators don't omit it in their own configs.
# Wall-clock budget for a single logical RPC call across all failover attempts.
# Once exceeded, the failover loop stops trying further endpoints and returns
# the last transport error. Optional; defaults to 30.
rpc_max_call_duration_secs = 30
| ) -> anyhow::Result<i128> { | ||
| let raw = self.try_get_balance(token_address, ledger_reader).await?; | ||
| let reserved = self.reserved_amount(token_address); | ||
| let available = raw.saturating_sub(reserved); |
Comment on lines
+105
to
+107
| let obligations = store.obligations().load_all(&markets[0])?; | ||
|
|
||
| info!(?obligations, %pkey, %version, ?enabled_strategies, ?enabled_liquidation_types, "starting keeper..."); |
Comment on lines
+109
to
+111
| if actions.is_empty() { | ||
| info!("no withdrawal opportunities"); | ||
| } |
Comment on lines
+148
to
+151
| # Optional: which strategies to run. Omit or leave empty to run all. | ||
| # Valid values: "liquidator", "withdrawer", "balancer", "bad-debt". | ||
| # A `--strategies` CLI flag overrides this. | ||
| # strategies = ["liquidator", "balancer"] |
| @@ -0,0 +1,47 @@ | |||
| { | |||
| "rpc_url": "https://soroban-testnet.stellar.org", | |||
| "fallback_rpc_urls": ["https://sorban-testnet.stellar.org"], | |||
khomiakmaxim
force-pushed
the
feat/fallback_rpcs_assets_to_hold_distribution
branch
from
July 24, 2026 12:12
4fb8fa1 to
fbc9435
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (7)
keeper/src/strategy/balancer.rs:187
try_parse_asset_from_liquidate_eventcurrently contains a commented-out condition and an in-code note ("WRONG, right?"). This leaves dead code in the hot path and can confuse future readers about intended behavior. If the only required filter is “event is for this keeper”, remove the commented check entirely and keep the condition explicit.
let (liquidator, collateral_pool) =
(self.gateway.decode_topic(event, 1), self.gateway.decode_topic(event, 4));
if liquidator != self.pkey
/*|| self.config.assets_to_hold.contains_key(&collateral_pool)*/ // WRONG, right?
{
return None;
keeper/src/strategy/balancer.rs:912
compute_value_centscan return a negative value whentoken_amountis negative, but the unit test in this file expects non-positive inputs to yield0. This will make the new tests fail and can also break portfolio-weight math if a negative value ever leaks in.
token_amount.saturating_mul(info.oracle_price) / denom
keeper/src/main.rs:107
- Logging the full
obligationsHashMap at info level can be extremely noisy (and potentially sensitive), especially on nodes with a large cached state. It’s usually enough to log the count at info level and keep full contents for debug-only troubleshooting.
info!(?obligations, %pkey, %version, ?enabled_strategies, ?enabled_liquidation_types, "starting keeper...");
keeper/src/strategy/withdrawer.rs:110
info!("no withdrawal opportunities")will fire on every refresh tick when there’s nothing to do, which can flood logs in normal steady-state. This message is more appropriate atdebuglevel (or behind a rate limit).
info!("no withdrawal opportunities");
config.example.json:3
- The fallback RPC URL in the example config has a typo (
sorban-testnet), which will cause confusing connection failures if copied as-is.
"fallback_rpc_urls": ["https://sorban-testnet.stellar.org"],
config.example.toml:151
- The example
strategiescomment lists"liquidator", butliquidatoris not a selectableStrategyKind(it always runs). Keeping this in the example will lead users to a config/CLI parse error.
# Optional: which strategies to run. Omit or leave empty to run all.
# Valid values: "liquidator", "withdrawer", "balancer", "bad-debt".
# A `--strategies` CLI flag overrides this.
# strategies = ["liquidator", "balancer"]
keeper/src/execute/failover.rs:27
- Field name
uncallable_untillis misspelled (should beuncallable_until). Since this is a new public struct, it’s a good time to fix the typo before it becomes part of the project’s stable vocabulary.
pub struct FailoverClient {
clients: Vec<Client>,
base: Instant,
urls: Vec<String>,
max_call_duration: Duration,
current_rpc_index: AtomicUsize,
uncallable_untill: Vec<AtomicU64>,
}
| **`liquidator`** models obligations and oracle prices per market, estimates net profit after gas, swap costs, and `liquidator_min_profit_margin_cents`, and picks the most profitable (borrow, deposit) pair to liquidate. It chooses between three execution modes based on the keeper's on-hand liquidity: `Direct` (the keeper already holds enough of the repayment asset), `PreSwap` (swap a held asset into the repayment asset first, then liquidate), or `Flash` (flash-borrow the repayment asset from the pool, seize the collateral, swap it back, and repay the flash loan atomically). Non-target collateral received from liquidations is later swapped by the rebalancer; assets in `assets_to_hold` are kept. | ||
|
|
||
| **`rebalancer`** (the `Balancer` strategy, config prefix `balancer_*`) runs every `balancer_refresh_interval_blocks` ledgers. It walks the wallet and swaps each non-target asset whose dollar value exceeds `balancer_min_swap_amount_value_cents` into the rebalancer target (the first entry of `assets_to_hold`). Trade size is capped so on-chain price impact stays under `balancer_max_price_impact_bps`, probing progressively smaller sizes up to `balancer_max_swap_provider_probes` times per provider; `balancer_max_allowed_swap_slippage_bps` is applied on top when constructing `min_amount_out`. Retries up to `balancer_max_retries` times on failure. | ||
| **`rebalancer`** (the `Balancer` strategy, config prefix `balancer_*`) runs every `balancer_refresh_interval_blocks` ledgers. It walks the wallet and swaps each non-target asset whose dollar value exceeds `balancer_min_swap_amount_value_cents` into the rebalancer target (the first entry of `assets_to_hold`). Trade size is capped so on-chain price impact stays under `balancer_max_price_impact_bps`, probing progressively smaller sizes up to `balancer_max_swap_provider_halving_probes` times per provider; `balancer_max_allowed_swap_slippage_bps` is applied on top when constructing `min_amount_out`. Retries up to `balancer_max_retries` times on failure. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.