Add disable-balances config flag to driver#4271
Add disable-balances config flag to driver#4271felixjff wants to merge 3 commits intocowprotocol:mainfrom
disable-balances config flag to driver#4271Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Code Review
This pull request introduces a disable-balances configuration flag to prevent the driver from making RPC calls for token balances, which is useful for local development. The implementation correctly adds the flag and applies it to the tokens::Fetcher to skip background updates and on-demand fetching. However, the account_balances fetcher is not affected by this flag and will continue to make RPC calls. This should be addressed to fully meet the PR's goal of disabling all balance-related RPC traffic.
|
recheck |
When developing or testing a colocated solver locally, the driver's token balance fetcher makes continuous RPC calls on every new block. This adds a `disable-balances` TOML config option (defaults to false) that skips both the background update task and on-demand balance fetching, returning zero-balance metadata instead. This avoids unnecessary RPC costs during local development. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d6df27c to
6eb6060
Compare
Instead of scattering `if disable_balances` checks inside the Fetcher, use an enum with Rpc and Disabled variants so the branch happens once at construction time.
Clarifies that this flag disables settlement contract token balance fetching, not user account balance fetching.
|
|
||
| /// Provides metadata of tokens. | ||
| struct Inner { | ||
| pub struct Inner { |
There was a problem hiding this comment.
pub and Inner should never go in the "same line" as Inner implies internal implementation and should not be exposed.
I like the enum approach as it has some advantages in terms of speed, but it brings more complexities in maintenance; since we're not chasing sub-ms speeds yet, this is unnecessary.
One reason it doesn't scale well in terms of maintenance is the burden of handling different branches, for example, in get we'd need matching on every enum variant we add. Leading to a big match statement that to be eventually be made ergonomic will require more "inner structs" that implement a given trait/interface, which is just a less ergonomic trait implementation.
I think the best way to go about this right now is:
- Split the enum into two structs —
RpcFetcher&DisabledFetcher - Extract the
getinto a trait calledSettlementBalanceFetching - Implement the trait on the previous structs
- Update the type of
let tokensin thecrates/driver/src/infra/api/mod.rsto beArc<dyn SettlementBalanceFetching> - Update where necessary to
Arctoo (old tokens::Fetcher basically)
| /// on-demand). Balances default to 0. Useful for local testing to | ||
| /// avoid RPC calls. | ||
| #[serde(default)] | ||
| disable_settlement_balance_fetcher: bool, |
There was a problem hiding this comment.
This is applied wholesale to the driver, as the driver is intended to handle different solvers this is a setting that should be handled on a per-solver level.
|
This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed. |
Summary
disable-balancesTOML config option to the driver (defaults tofalse)Motivation
When developing or testing a colocated solver locally, the driver's token balance fetcher continuously queries the RPC node on every new block. This is unnecessary during local development and adds RPC costs. The
disable-balancesflag lets developers skip all balance-related RPC traffic.Usage
Test plan
cargo build -p driverdisable-balances = true, confirm no balance RPC calls are made