Skip to content

Commit d023666

Browse files
committed
fix(aggregator): CSD epoch missing offset when expanding latest
Cardano Stake Distribution signed entities are created with an offset of `-1` to the epoch, this must be reflected to the epoch expansion when using `latest` (with or without additional offset). Else querying with `latest` will always returns a `404`.
1 parent dbcf6c1 commit d023666

File tree

4 files changed

+116
-34
lines changed

4 files changed

+116
-34
lines changed

mithril-aggregator/src/http_server/routes/artifact_routes/cardano_database.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ mod handlers {
115115
http_message_service: Arc<dyn MessageService>,
116116
max_artifact_epoch_offset: u32,
117117
) -> Result<impl warp::Reply, Infallible> {
118-
let (expanded_epoch, offset) = match parameters::expand_epoch(&epoch, epoch_service).await {
118+
let expanded_epoch = match parameters::expand_epoch(&epoch, epoch_service).await {
119119
Ok(epoch) => epoch,
120120
Err(err) => {
121121
warn!(logger,"list_by_epoch_artifacts_cardano_database::invalid_epoch"; "error" => ?err);
@@ -126,7 +126,7 @@ mod handlers {
126126
}
127127
};
128128

129-
if offset.is_some_and(|o| o > max_artifact_epoch_offset as u64) {
129+
if expanded_epoch.has_offset_greater_than(max_artifact_epoch_offset as u64) {
130130
return Ok(reply::bad_request(
131131
"invalid_epoch".to_string(),
132132
format!(
@@ -136,7 +136,7 @@ mod handlers {
136136
}
137137

138138
match http_message_service
139-
.get_cardano_database_list_message_by_epoch(LIST_MAX_ITEMS, expanded_epoch)
139+
.get_cardano_database_list_message_by_epoch(LIST_MAX_ITEMS, *expanded_epoch)
140140
.await
141141
{
142142
Ok(message) => Ok(reply::json(&message, StatusCode::OK)),

mithril-aggregator/src/http_server/routes/artifact_routes/cardano_stake_distribution.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ pub mod handlers {
130130
client_metadata.client_type.as_deref().unwrap_or_default(),
131131
]);
132132

133-
let (expanded_epoch, offset) = match parameters::expand_epoch(&epoch, epoch_service).await {
133+
let expanded_epoch = match parameters::expand_epoch(&epoch, epoch_service).await {
134134
Ok(epoch) => epoch,
135135
Err(err) => {
136136
warn!(logger, "get_artifact_by_epoch::invalid_epoch"; "error" => ?err);
@@ -141,7 +141,7 @@ pub mod handlers {
141141
}
142142
};
143143

144-
if offset.is_some_and(|o| o > max_artifact_epoch_offset as u64) {
144+
if expanded_epoch.has_offset_greater_than(max_artifact_epoch_offset as u64) {
145145
return Ok(reply::bad_request(
146146
"invalid_epoch".to_string(),
147147
format!(
@@ -151,7 +151,10 @@ pub mod handlers {
151151
}
152152

153153
match http_message_service
154-
.get_cardano_stake_distribution_message_by_epoch(expanded_epoch)
154+
.get_cardano_stake_distribution_message_by_epoch(
155+
// Reminder: Cardano stake distribution is recorded with a `-1` epoch offset
156+
expanded_epoch.apply_offset_for_latest(1),
157+
)
155158
.await
156159
{
157160
Ok(Some(message)) => Ok(reply::json(&message, StatusCode::OK)),

mithril-aggregator/src/http_server/routes/middlewares.rs

Lines changed: 94 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -210,41 +210,84 @@ pub mod validators {
210210

211211
pub mod parameters {
212212
use anyhow::Context;
213+
use std::ops::Deref;
213214

214215
use mithril_common::StdResult;
215216

216217
use super::*;
217218

219+
/// An epoch parsed from a http request parameter
220+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
221+
pub enum ExpandedEpoch {
222+
/// Epoch was explicitly provided as a number (e.g., "123")
223+
Parsed(Epoch),
224+
/// Epoch was provided as "latest" (e.g., "latest")
225+
Latest(Epoch),
226+
/// Epoch was provided as "latest-{offset}" (e.g., "latest-100")
227+
LatestMinusOffset(Epoch, u64),
228+
}
229+
230+
impl Deref for ExpandedEpoch {
231+
type Target = Epoch;
232+
233+
fn deref(&self) -> &Self::Target {
234+
match self {
235+
ExpandedEpoch::Parsed(epoch)
236+
| ExpandedEpoch::Latest(epoch)
237+
| ExpandedEpoch::LatestMinusOffset(epoch, _) => epoch,
238+
}
239+
}
240+
}
241+
242+
impl ExpandedEpoch {
243+
/// Returns true if this epoch was expanded from 'latest-{offset}' and the offset
244+
/// is greater than the given value.
245+
pub fn has_offset_greater_than(&self, value: u64) -> bool {
246+
match self {
247+
ExpandedEpoch::Parsed(_) | ExpandedEpoch::Latest(_) => false,
248+
ExpandedEpoch::LatestMinusOffset(_, offset) => offset > &value,
249+
}
250+
}
251+
252+
/// Apply an additional negative offset to the epoch, but only if it was derived from 'latest'.
253+
pub fn apply_offset_for_latest(&self, additional_offset: u64) -> Epoch {
254+
match self {
255+
ExpandedEpoch::Parsed(epoch) => *epoch,
256+
ExpandedEpoch::Latest(epoch) | ExpandedEpoch::LatestMinusOffset(epoch, ..) => {
257+
Epoch(epoch.saturating_sub(additional_offset))
258+
}
259+
}
260+
}
261+
}
262+
218263
/// Parse the string into an [Epoch] if it's a number, or if it's 'latest{-{offset}}' expand
219264
/// into the actual epoch minus the optional given offset.
220265
///
221266
/// Return the expanded epoch and the eventual offset that was applied
222267
pub async fn expand_epoch(
223268
epoch_str: &str,
224269
epoch_service: EpochServiceWrapper,
225-
) -> StdResult<(Epoch, Option<u64>)> {
270+
) -> StdResult<ExpandedEpoch> {
226271
let epoch_str = epoch_str.to_lowercase();
227272
if epoch_str == "latest" {
228273
epoch_service
229274
.read()
230275
.await
231276
.epoch_of_current_data()
232-
.map(|epoch| (epoch, None))
277+
.map(ExpandedEpoch::Latest)
233278
} else if epoch_str.starts_with("latest-") {
234279
let (_, offset_str) = epoch_str.split_at(7);
235280
let offset = offset_str
236281
.parse::<u64>()
237282
.with_context(|| "Invalid epoch offset: must be a number")?;
238283

239-
epoch_service
240-
.read()
241-
.await
242-
.epoch_of_current_data()
243-
.map(|epoch| (Epoch(epoch.saturating_sub(offset)), Some(offset)))
284+
epoch_service.read().await.epoch_of_current_data().map(|epoch| {
285+
ExpandedEpoch::LatestMinusOffset(Epoch(epoch.saturating_sub(offset)), offset)
286+
})
244287
} else {
245288
epoch_str
246289
.parse::<u64>()
247-
.map(|epoch| (Epoch(epoch), None))
290+
.map(|epoch| ExpandedEpoch::Parsed(Epoch(epoch)))
248291
.with_context(|| "Invalid epoch: must be a number or 'latest'")
249292
}
250293
}
@@ -409,7 +452,7 @@ mod tests {
409452

410453
use crate::services::FakeEpochServiceBuilder;
411454

412-
use super::*;
455+
use super::{parameters::ExpandedEpoch, *};
413456

414457
fn fake_epoch_service(returned_epoch: Epoch) -> EpochServiceWrapper {
415458
Arc::new(RwLock::new(
@@ -422,15 +465,15 @@ mod tests {
422465
let epoch_service = fake_epoch_service(Epoch(300));
423466
let expanded_epoch = parameters::expand_epoch("456", epoch_service).await.unwrap();
424467

425-
assert_eq!(expanded_epoch, (Epoch(456), None));
468+
assert_eq!(expanded_epoch, ExpandedEpoch::Parsed(Epoch(456)));
426469
}
427470

428471
#[tokio::test]
429472
async fn use_epoch_service_current_epoch_if_latest() {
430473
let epoch_service = fake_epoch_service(Epoch(89));
431474
let expanded_epoch = parameters::expand_epoch("latest", epoch_service).await.unwrap();
432475

433-
assert_eq!(expanded_epoch, (Epoch(89), None));
476+
assert_eq!(expanded_epoch, ExpandedEpoch::Latest(Epoch(89)));
434477
}
435478

436479
#[tokio::test]
@@ -439,10 +482,16 @@ mod tests {
439482
let expanded_epoch = parameters::expand_epoch("latest-13", epoch_service.clone())
440483
.await
441484
.unwrap();
442-
assert_eq!(expanded_epoch, (Epoch(76), Some(13)));
485+
assert_eq!(
486+
expanded_epoch,
487+
ExpandedEpoch::LatestMinusOffset(Epoch(76), 13)
488+
);
443489

444490
let expanded_epoch = parameters::expand_epoch("latest-0", epoch_service).await.unwrap();
445-
assert_eq!(expanded_epoch, (Epoch(89), Some(0)));
491+
assert_eq!(
492+
expanded_epoch,
493+
ExpandedEpoch::LatestMinusOffset(Epoch(89), 0)
494+
);
446495
}
447496

448497
#[tokio::test]
@@ -472,7 +521,38 @@ mod tests {
472521
.await
473522
.unwrap();
474523

475-
assert_eq!(expanded_epoch, (Epoch(0), Some(u64::MAX)));
524+
assert_eq!(
525+
expanded_epoch,
526+
ExpandedEpoch::LatestMinusOffset(Epoch(0), u64::MAX)
527+
);
528+
}
529+
530+
#[test]
531+
fn test_expanded_epoch_is_parsed_offset_greater_than() {
532+
assert!(!ExpandedEpoch::Parsed(Epoch(90)).has_offset_greater_than(15));
533+
assert!(!ExpandedEpoch::Latest(Epoch(90)).has_offset_greater_than(15));
534+
assert!(!ExpandedEpoch::LatestMinusOffset(Epoch(90), 10).has_offset_greater_than(10));
535+
assert!(ExpandedEpoch::LatestMinusOffset(Epoch(90), 11).has_offset_greater_than(10));
536+
}
537+
538+
#[test]
539+
fn test_expanded_epoch_apply_additional_offset() {
540+
assert_eq!(
541+
ExpandedEpoch::Parsed(Epoch(90)).apply_offset_for_latest(15),
542+
Epoch(90)
543+
);
544+
assert_eq!(
545+
ExpandedEpoch::Latest(Epoch(90)).apply_offset_for_latest(15),
546+
Epoch(75)
547+
);
548+
assert_eq!(
549+
ExpandedEpoch::LatestMinusOffset(Epoch(90), 13).apply_offset_for_latest(15),
550+
Epoch(75)
551+
);
552+
assert_eq!(
553+
ExpandedEpoch::Latest(Epoch(90)).apply_offset_for_latest(u64::MAX),
554+
Epoch(0)
555+
);
476556
}
477557
}
478558
}

mithril-aggregator/src/http_server/routes/signer_routes.rs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -178,26 +178,25 @@ mod handlers {
178178
epoch_service: EpochServiceWrapper,
179179
verification_key_store: Arc<dyn VerificationKeyStorer>,
180180
) -> Result<impl warp::Reply, Infallible> {
181-
let (registered_at_epoch, _offset) =
182-
match parameters::expand_epoch(&registered_at, epoch_service).await {
183-
Ok(epoch) => epoch,
184-
Err(err) => {
185-
warn!(logger,"registered_signers::invalid_epoch"; "error" => ?err);
186-
return Ok(reply::bad_request(
187-
"invalid_epoch".to_string(),
188-
err.to_string(),
189-
));
190-
}
191-
};
181+
let expanded_epoch = match parameters::expand_epoch(&registered_at, epoch_service).await {
182+
Ok(epoch) => epoch,
183+
Err(err) => {
184+
warn!(logger,"registered_signers::invalid_epoch"; "error" => ?err);
185+
return Ok(reply::bad_request(
186+
"invalid_epoch".to_string(),
187+
err.to_string(),
188+
));
189+
}
190+
};
192191

193192
// The given epoch is the epoch at which the signer registered, the store works on
194-
// the recording epoch so we need to offset.
193+
// the recording epoch, so we need to offset.
195194
match verification_key_store
196-
.get_signers(registered_at_epoch.offset_to_recording_epoch())
195+
.get_signers(expanded_epoch.offset_to_recording_epoch())
197196
.await
198197
{
199198
Ok(Some(signers)) => {
200-
let message = SignerRegistrationsMessage::new(registered_at_epoch, signers);
199+
let message = SignerRegistrationsMessage::new(*expanded_epoch, signers);
201200
Ok(reply::json(&message, StatusCode::OK))
202201
}
203202
Ok(None) => {

0 commit comments

Comments
 (0)