Skip to content

Commit 6eaebdb

Browse files
author
peteroche
committed
refactor: make FeeDistribution fields and nested stats optional to handle incomplete data safely
1 parent 6292c69 commit 6eaebdb

3 files changed

Lines changed: 66 additions & 58 deletions

File tree

packages/devkit/src/profiling/report.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,7 @@ impl ProfilingReportBuilder {
173173
}
174174

175175
/// Add multiple `CpuSample` instances to the builder.
176-
pub fn add_cpu_samples(
177-
&mut self,
178-
samples: impl IntoIterator<Item = CpuSample>,
179-
) -> &mut Self {
176+
pub fn add_cpu_samples(&mut self, samples: impl IntoIterator<Item = CpuSample>) -> &mut Self {
180177
self.cpu_samples.extend(samples);
181178
self
182179
}
@@ -188,10 +185,7 @@ impl ProfilingReportBuilder {
188185
}
189186

190187
/// Add multiple `MemSample` instances to the builder.
191-
pub fn add_mem_samples(
192-
&mut self,
193-
samples: impl IntoIterator<Item = MemSample>,
194-
) -> &mut Self {
188+
pub fn add_mem_samples(&mut self, samples: impl IntoIterator<Item = MemSample>) -> &mut Self {
195189
self.mem_samples.extend(samples);
196190
self
197191
}
@@ -331,12 +325,22 @@ mod tests {
331325

332326
let t1 = Utc::now();
333327
builder.add_cpu_sample(
334-
CpuSample::new("fee_estimator", Duration::from_millis(10), Duration::from_millis(10), 100.0)
335-
.with_timestamp(t1),
328+
CpuSample::new(
329+
"fee_estimator",
330+
Duration::from_millis(10),
331+
Duration::from_millis(10),
332+
100.0,
333+
)
334+
.with_timestamp(t1),
336335
);
337336
builder.add_cpu_sample(
338-
CpuSample::new("fee_estimator", Duration::from_millis(20), Duration::from_millis(20), 100.0)
339-
.with_timestamp(t1),
337+
CpuSample::new(
338+
"fee_estimator",
339+
Duration::from_millis(20),
340+
Duration::from_millis(20),
341+
100.0,
342+
)
343+
.with_timestamp(t1),
340344
);
341345
builder.add_mem_sample(MemSample::new("fee_estimator", 2048, 1024).with_timestamp(t1));
342346
builder.add_mem_sample(MemSample::new("fee_estimator", 4096, 2048).with_timestamp(t1));

packages/devkit/src/protocol/fee_stats.rs

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -112,34 +112,36 @@ where
112112
/// Statistical percentile distribution of Stellar transaction fees.
113113
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)]
114114
pub struct FeeDistribution {
115-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
116-
pub min: u64,
117-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
118-
pub max: u64,
119-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
120-
pub mode: u64,
121-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
122-
pub p10: u64,
123-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
124-
pub p20: u64,
125-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
126-
pub p30: u64,
127-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
128-
pub p40: u64,
129-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
130-
pub p50: u64,
131-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
132-
pub p60: u64,
133-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
134-
pub p70: u64,
135-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
136-
pub p80: u64,
137-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
138-
pub p90: u64,
139-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
140-
pub p95: u64,
141-
#[serde(default, deserialize_with = "deserialize_u64_lenient")]
142-
pub p99: u64,
115+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
116+
pub min: Option<u64>,
117+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
118+
pub max: Option<u64>,
119+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
120+
pub mode: Option<u64>,
121+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
122+
pub p10: Option<u64>,
123+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
124+
pub p20: Option<u64>,
125+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
126+
pub p30: Option<u64>,
127+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
128+
pub p40: Option<u64>,
129+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
130+
pub p50: Option<u64>,
131+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
132+
pub p60: Option<u64>,
133+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
134+
pub p70: Option<u64>,
135+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
136+
pub p80: Option<u64>,
137+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
138+
pub p90: Option<u64>,
139+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
140+
pub p95: Option<u64>,
141+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
142+
pub p99: Option<u64>,
143+
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]
144+
pub transaction_count: Option<u64>,
143145
}
144146

145147
/// Type alias for backward compatibility.
@@ -155,9 +157,9 @@ pub struct HorizonFeeStats {
155157
#[serde(default, deserialize_with = "deserialize_f64_lenient")]
156158
pub ledger_capacity_usage: f64,
157159
#[serde(default)]
158-
pub fee_charged: FeeDistribution,
160+
pub fee_charged: Option<FeeDistribution>,
159161
#[serde(default)]
160-
pub max_fee: FeeDistribution,
162+
pub max_fee: Option<FeeDistribution>,
161163

162164
// Legacy/auxiliary top-level fields returned by Horizon
163165
#[serde(default, deserialize_with = "deserialize_opt_u64_lenient")]

packages/devkit/src/protocol/parser.rs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ pub fn parse_fee_stats(json: &str) -> Result<HorizonFeeStats, DevkitError> {
77
let value: Value = serde_json::from_str(json)
88
.map_err(|e| DevkitError::Protocol(format!("JSON parse error: {e}")))?;
99

10-
// If json is empty object or invalid, check required fields
11-
if !value.is_object() || value.as_object().map_or(true, |o| o.is_empty()) {
10+
let Some(obj) = value.as_object() else {
11+
return Err(DevkitError::Protocol("expected JSON object".to_string()));
12+
};
13+
14+
if obj.is_empty() {
1215
return Err(DevkitError::Protocol("empty JSON object".to_string()));
1316
}
1417

@@ -42,21 +45,20 @@ pub fn validate_fee_stats(stats: &HorizonFeeStats) -> Result<(), DevkitError> {
4245
)));
4346
}
4447

45-
let fl = &stats.fee_charged;
46-
if fl.p10 > 0 || fl.p50 > 0 || fl.p90 > 0 || fl.p99 > 0 {
47-
if !(fl.p10 <= fl.p50 && fl.p50 <= fl.p90 && fl.p90 <= fl.p99) {
48-
return Err(DevkitError::Protocol(format!(
49-
"fee_charged percentiles must be monotonic: p10={} p50={} p90={} p99={}",
50-
fl.p10, fl.p50, fl.p90, fl.p99
51-
)));
48+
if let Some(ref fl) = stats.fee_charged {
49+
if let (Some(p10), Some(p50), Some(p90), Some(p99)) = (fl.p10, fl.p50, fl.p90, fl.p99) {
50+
if !(p10 <= p50 && p50 <= p90 && p90 <= p99) {
51+
return Err(DevkitError::Protocol(format!(
52+
"fee_charged percentiles must be monotonic: p10={p10} p50={p50} p90={p90} p99={p99}"
53+
)));
54+
}
5255
}
53-
}
54-
if fl.min > 0 || fl.mode > 0 || fl.max > 0 {
55-
if !(fl.min <= fl.mode && fl.mode <= fl.max) {
56-
return Err(DevkitError::Protocol(format!(
57-
"fee_charged min/mode/max must be ordered: min={} mode={} max={}",
58-
fl.min, fl.mode, fl.max
59-
)));
56+
if let (Some(min), Some(mode), Some(max)) = (fl.min, fl.mode, fl.max) {
57+
if !(min <= mode && mode <= max) {
58+
return Err(DevkitError::Protocol(format!(
59+
"fee_charged min/mode/max must be ordered: min={min} mode={mode} max={max}"
60+
)));
61+
}
6062
}
6163
}
6264

0 commit comments

Comments
 (0)