Migrate autopilot native price estimation config from CLI args to config file#4194
Migrate autopilot native price estimation config from CLI args to config file#4194jmg-duarte merged 8 commits intomainfrom
Conversation
Move banned_users, order_events_cleanup_interval/threshold, and S3 upload settings from CLI arguments to the TOML config file, following the same pattern as prior fee_policy and trusted_tokens migrations. - Add config modules: banned_users, order_events_cleanup, s3 - Remove infra::persistence::cli (S3 CLI args) - Delete corresponding CLI arguments and Display impl entries - Wire config values through run.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move native price estimator settings (estimators, api_estimators, results_required, cache_refresh_interval, native_price_prefetch_time) from CLI arguments to the TOML config file. Add Serialize/Deserialize derives to NativePriceEstimators and related types. Update e2e test setup to construct NativePriceConfig programmatically. Add unit tests for deserialization and defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
The pull request successfully migrates the autopilot's native price estimation settings from CLI arguments to the TOML configuration file. This change improves configuration management by grouping related parameters (estimators, API estimators, results required, cache refresh interval, and prefetch time) within the native-price-estimation section of the config. The migration preserves existing logic, maintains sensible defaults, and includes comprehensive updates to the E2E test suite to reflect the new configuration structure. No critical issues were found during the review.
There was a problem hiding this comment.
Code Review
The pull request successfully migrates native price estimation configuration from CLI arguments to a TOML config file. This improves maintainability and centralizes configuration. The changes are well-tested and reflect the intended migration. No critical issues were found.
MartinquaXD
left a comment
There was a problem hiding this comment.
Actually left another comment on the format when I saw it fully in the services PR here. Given that the full format wasn't really easily visible in this PR I wonder if the next step should be to introduce another test that deserializes an example config with all formats inside. That way we have a reference that at least has the possibility to stay up-to-date and new formats that get introduced are easier to spot.
|
Let's add a migration guide entry to this PR: Migration GuideAutopilot native price estimator cli arguments have been moved to the TOML config file, under the table `[native-price-estimation]:
[native-price-estimation]
estimators = []Important note, the values for specific estimator were previously separated by
[native-price-estimation]
api-estimators = []
[native-price-estimation]
results-required =
[native-price-estimation]
cache-refresh-interval =
[native-price-estimation]
native-price-prefetch-time = |
Description
Move native price estimator settings (estimators, api_estimators, results_required, cache_refresh_interval, native_price_prefetch_time) from CLI arguments to the TOML config file.
Changes
Migration Guide
Autopilot native price estimator CLI arguments have been moved to the TOML config file under the
[native-price-estimation]table.Estimator format
Previously, estimator variants were specified as
|-separated strings. Now they are inline tables with atypefield:CoinGecko{ type = "CoinGecko" }OneInchSpotPriceApi{ type = "OneInchSpotPriceApi" }Driver|solver1|http://localhost:8080{ type = "Driver", name = "solver1", url = "http://localhost:8080" }Forwarder|http://localhost:12088{ type = "Forwarder", url = "http://localhost:12088" }Stages (previously separated by
;) become separate inner arrays. Estimators within a stage (previously separated by,) are elements of the same inner array.Old (CLI):
New (TOML):
Arguments
--native-price-estimators(CLI) /NATIVE_PRICE_ESTIMATORS(env):--api-native-price-estimators(CLI) /API_NATIVE_PRICE_ESTIMATORS(env):--native-price-estimation-results-required(CLI) /NATIVE_PRICE_ESTIMATION_RESULTS_REQUIRED(env):--native-price-cache-refresh(CLI) /NATIVE_PRICE_CACHE_REFRESH(env):--native-price-prefetch-time(CLI) /NATIVE_PRICE_PREFETCH_TIME(env):How to test
Staging