feat(metering): add DA size getter and dynamic DA config#1114
feat(metering): add DA size getter and dynamic DA config#1114niran wants to merge 1 commit intoniran/priority-fee-2-ingestionbase/base:niran/priority-fee-2-ingestionfrom niran/priority-fee-3-dynamic-dabase/base:niran/priority-fee-3-dynamic-daCopy head branch name to clipboard
Conversation
| Ok(()) => info!( | ||
| url = %rpc_url, | ||
| tx_hash = %tx_hash, | ||
| "Forwarded metering information" |
There was a problem hiding this comment.
nit: This fires on every successfully forwarded metering event. At high transaction throughput this will be very noisy. Consider downgrading to debug! — the error/warn paths already cover the cases operators need to monitor.
| Ok(()) => info!( | |
| url = %rpc_url, | |
| tx_hash = %tx_hash, | |
| "Forwarded metering information" | |
| Ok(()) => debug!( | |
| url = %rpc_url, | |
| tx_hash = %tx_hash, | |
| "Forwarded metering information" | |
| ), |
| Ok(n) => info!( | ||
| receivers = n, | ||
| bundle_hash = %bundle_hash, | ||
| "Broadcast metering data to builder connectors" | ||
| ), |
There was a problem hiding this comment.
Same concern: this logs at info! on every successful broadcast, which fires per-transaction. Consider debug! to avoid log noise in production.
| Ok(n) => info!( | |
| receivers = n, | |
| bundle_hash = %bundle_hash, | |
| "Broadcast metering data to builder connectors" | |
| ), | |
| Ok(n) => debug!( | |
| receivers = n, | |
| bundle_hash = %bundle_hash, | |
| "Broadcast metering data to builder connectors" | |
| ), |
| /// Sets the shared DA configuration. | ||
| pub fn with_da_config(mut self, da_config: OpDAConfig) -> Self { | ||
| self.da_config = Some(da_config); | ||
| self | ||
| } |
There was a problem hiding this comment.
This method and the da_config field are added here and threaded through launch_node, but no caller in this PR invokes with_da_config. Is this intended as scaffolding for a subsequent PR? If so, consider adding a brief doc comment noting that, or deferring the addition until there's a caller to avoid dead code.
2401403 to
f2bbd3f
Compare
72cf5bd to
118bff9
Compare
| /// Sets the shared DA configuration. | ||
| pub fn with_da_config(mut self, da_config: OpDAConfig) -> Self { | ||
| self.da_config = Some(da_config); | ||
| self | ||
| } |
There was a problem hiding this comment.
with_da_config is defined on impl BaseNodeRunner<DefaultPayloadServiceBuilder> but da_config is a field on the generic struct. If a caller chains with_service_builder() before with_da_config(), the method disappears because the SB parameter is no longer DefaultPayloadServiceBuilder.
Consider moving it to the generic impl<SB: PayloadServiceBuilder> BaseNodeRunner<SB> block (alongside with_service_builder, install_ext, and run) so it composes correctly regardless of call order.
| /// Sets the shared DA configuration. | |
| pub fn with_da_config(mut self, da_config: OpDAConfig) -> Self { | |
| self.da_config = Some(da_config); | |
| self | |
| } | |
| impl<SB: PayloadServiceBuilder> BaseNodeRunner<SB> { | |
| /// Sets the shared DA configuration. | |
| pub fn with_da_config(mut self, da_config: OpDAConfig) -> Self { | |
| self.da_config = Some(da_config); | |
| self | |
| } | |
Review SummaryPR adds Findings
Observations (no action required)
|
PR Stack
This is PR 5 of 5 in the metered priority fee stack:
base_meteredPriorityFeePerGasRPC endpointSummary
miner_getMaxDASizeRPC toMinerApiExttrait and implement inOpMinerExtApida_configthroughMeteringExtension/MeteringConfigso the node can query the sequencer's current DA budgetTest Plan
set_max_da_size; getter follows the same pattern