Conversation
Introduce a ChainSettings struct that holds Ethereum RPC tuning parameters (timeouts, retries, batch sizes, block ranges, etc.) per chain instead of reading them from global ENV_VARS. Settings are parsed from [chains.<name>] TOML sections with serde defaults falling back to the existing environment variables, preserving full backwards compatibility. Thread ChainSettings through Chain, EthereumAdapter, and the free receipt-fetching functions so each chain can be independently tuned.
Add the full list of per-chain Ethereum RPC tuning settings to docs/config.md with env var fallback defaults, and add a cross-reference note in docs/environment-variables.md.
Reject invalid ChainSettings values at startup: max_block_range_size and max_event_only_range must be positive (i32), and block_batch_size, block_ptr_batch_size, block_ingestor_max_concurrent_json_rpc_calls, and get_logs_max_contracts must be non-zero (used as .buffered() args or filter batch limits where 0 would panic or silently break).
| impl From<ConfigChainSettings> for ChainSettings { | ||
| fn from(c: ConfigChainSettings) -> Self { | ||
| let ConfigChainSettings { | ||
| polling_interval, | ||
| json_rpc_timeout, | ||
| request_retries, | ||
| max_block_range_size, | ||
| block_batch_size, | ||
| block_ptr_batch_size, | ||
| max_event_only_range, | ||
| target_triggers_per_block_range, | ||
| get_logs_max_contracts, | ||
| block_ingestor_max_concurrent_json_rpc_calls, | ||
| genesis_block_number, | ||
| } = c; | ||
| ChainSettings { | ||
| polling_interval, | ||
| json_rpc_timeout, | ||
| request_retries, | ||
| max_block_range_size, | ||
| block_batch_size, | ||
| block_ptr_batch_size, | ||
| max_event_only_range, | ||
| target_triggers_per_block_range, | ||
| get_logs_max_contracts, | ||
| block_ingestor_max_concurrent_json_rpc_calls, | ||
| genesis_block_number, | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Minor question: ethereum::ChainSettings and config::ChainSettings seems to be exactly the same thing, could we reuse the struct? since this chain settings is very specific to ethereum should we just use the from ethereum crate. what do you think? ChainSettings seems to be just ethereum specific.
There was a problem hiding this comment.
Yeah, I don't like this as well, but I left them separate for separation of concern. The config:: one to deal with the parsing, etc, the ethereum:: one for all the rest. To combine them i need to move all the serde stuff and defaults in ethereum:: which is new concern and not related to it's logic. The other way around will create a circular dependency.
There was a problem hiding this comment.
Ah, alright. I was thinking maybe moving it to graph crate but that also doesnt work. Lets keep it as it is then
There was a problem hiding this comment.
Moving it to graph crate could work, I'll look into it.
| self.settings | ||
| .validate() | ||
| .map_err(|e| anyhow!("chain {}: {}", name, e))?; |
There was a problem hiding this comment.
nit: Currently this runs for near too. Should just be ethereum ?
There was a problem hiding this comment.
Yeah, it will run for all chains regardless of protocol, but validations should pass as the values will default to the ENV var defaults, so unless someone manually sets any of the setting to 0 for non ethereum chains it should be fine. But I can wrap it in
if self.protocol == BlockchainKind::Ethereum {
...
}
| #[serde(default = "default_blockchain_kind")] | ||
| pub protocol: BlockchainKind, | ||
| pub struct ChainSettings { | ||
| /// Set by `polling_interval` (milliseconds). Defaults to `GRAPH_ETHEREUM_POLLING_INTERVAL`. |
There was a problem hiding this comment.
| /// Set by `polling_interval` (milliseconds). Defaults to `GRAPH_ETHEREUM_POLLING_INTERVAL`. | |
| /// Set by `polling_interval` (milliseconds). Defaults to `ETHEREUM_POLLING_INTERVAL`. |
| details): | ||
|
|
||
| - `polling_interval`: block ingestor polling interval in milliseconds. | ||
| Default: `ETHEREUM_POLLING_INTERVAL` (500ms). |
There was a problem hiding this comment.
minor: Default is actually 1000ms, its a pre-existing doc bug
| fn validate(&self) -> Result<()> { | ||
| anyhow::ensure!( | ||
| self.max_block_range_size > 0, | ||
| "max_block_range_size must be > 0" | ||
| ); | ||
| anyhow::ensure!( | ||
| self.max_event_only_range > 0, | ||
| "max_event_only_range must be > 0" | ||
| ); | ||
| anyhow::ensure!(self.block_batch_size > 0, "block_batch_size must be > 0"); | ||
| anyhow::ensure!( | ||
| self.block_ptr_batch_size > 0, | ||
| "block_ptr_batch_size must be > 0" | ||
| ); | ||
| anyhow::ensure!( | ||
| self.block_ingestor_max_concurrent_json_rpc_calls > 0, | ||
| "block_ingestor_max_concurrent_json_rpc_calls must be > 0" | ||
| ); | ||
| anyhow::ensure!( | ||
| self.get_logs_max_contracts > 0, | ||
| "get_logs_max_contracts must be > 0" | ||
| ); | ||
| Ok(()) |
There was a problem hiding this comment.
While we're at it, request_retries, json_rpc_timeout, target_triggers_per_block_range, and polling_interval also need zero-value validation. These weren't validated before either, but since validate() exists now it'd be good to add it too
2 participants
Per-chain RPC settings via TOML config
What this PR does:
Backwards compatibility
Fully backwards compatible. All 11 settings default to their corresponding environment variable values, so no TOML changes are required. The env vars remain the global baseline.