Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: return error when filtering with invalid event id #405

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/many-ledger/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ define_attribute_many_error!(
}
);

define_attribute_many_error!(
attribute 4 => {
1: pub fn event_not_found(id) => "Event not found: {id}.",
}
);

define_attribute_many_error!(
attribute 11 => {
1: pub fn token_info_not_found(symbol) => "Token information not found in persistent storage: {symbol}.",
Expand Down
25 changes: 20 additions & 5 deletions src/many-ledger/src/module/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use many_modules::events::{
EventFilterAttributeSpecific, EventFilterAttributeSpecificIndex, EventInfo, EventLog,
};
use many_types::{CborRange, Timestamp, VecOrSingle};
use std::collections::BTreeMap;
use std::collections::{BTreeMap, Bound};
use std::ops::RangeBounds;

const MAXIMUM_EVENT_COUNT: usize = 100;

Expand Down Expand Up @@ -121,10 +122,24 @@ impl events::EventsModuleBackend for LedgerModuleImpl {

let storage = &self.storage;
let nb_events = storage.nb_events()?;
let iter = storage.iter_events(
filter.id_range.unwrap_or_default(),
order.unwrap_or_default(),
);

// Check that the id range is valid.
// Return an error if any key doesn't exist
let id_range = filter.id_range.unwrap_or_default();
match id_range.start_bound() {
Bound::Included(x) | Bound::Excluded(x) => {
storage.check_event_id(x)?;
}
Bound::Unbounded => {}
}
match id_range.end_bound() {
Bound::Included(x) | Bound::Excluded(x) => {
storage.check_event_id(x)?;
}
Bound::Unbounded => {}
}

let iter = storage.iter_events(id_range, order.unwrap_or_default());

let iter = Box::new(iter.map(|item| {
let (_k, v) = item.map_err(ManyError::unknown)?;
Expand Down
11 changes: 11 additions & 0 deletions src/many-ledger/src/storage/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ impl LedgerStorage {
self.latest_tid.clone()
}

pub(crate) fn check_event_id(&self, id: &events::EventId) -> Result<(), ManyError> {
match self
.persistent_store
.get(&key_for_event(id.clone()))
.map_err(error::storage_get_failed)?
{
None => Err(error::event_not_found(hex::encode(id.as_ref()))),
Some(_) => Ok(()),
}
}

pub fn nb_events(&self) -> Result<u64, ManyError> {
self.persistent_store
.get(EVENT_COUNT_ROOT)
Expand Down
194 changes: 193 additions & 1 deletion src/many-ledger/tests/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use many_modules::account::features::multisig::{
self, AccountMultisigModuleBackend, MultisigTransactionState,
};
use many_modules::events::{
self, EventFilterAttributeSpecific, EventFilterAttributeSpecificIndex, EventsModuleBackend,
self, EventFilterAttributeSpecific, EventFilterAttributeSpecificIndex, EventId,
EventsModuleBackend,
};
use many_modules::ledger;
use many_modules::ledger::LedgerCommandsModuleBackend;
Expand Down Expand Up @@ -228,6 +229,197 @@ fn list_filter_kind() {
assert!(list_return.events[0].is_about(id));
}

#[test]
fn list_filter_id() {
let Setup {
mut module_impl,
id,
..
} = setup();

// Create 5 events
for i in 0..5 {
send(&mut module_impl, id, identity(i));
}

// Collect all events ids
let events_ids = get_all_events_ids(&mut module_impl);

let tests = vec![
// Unbounded bounds
(Bound::Unbounded, Bound::Unbounded, 5, 0..5),
// Included bounds
(
Bound::Unbounded,
Bound::Included(events_ids[3].clone()),
4,
0..4,
),
(
Bound::Included(events_ids[1].clone()),
Bound::Unbounded,
4,
1..5,
),
(
Bound::Included(events_ids[1].clone()),
Bound::Included(events_ids[3].clone()),
3,
1..4,
),
// Excluded bounds
(
Bound::Unbounded,
Bound::Excluded(events_ids[4].clone()),
4,
0..4,
),
(
Bound::Excluded(events_ids[0].clone()),
Bound::Unbounded,
4,
1..5,
),
(
Bound::Excluded(events_ids[0].clone()),
Bound::Excluded(events_ids[4].clone()),
3,
1..4,
),
// Mixed Included/Excluded bounds
(
Bound::Included(events_ids[1].clone()),
Bound::Excluded(events_ids[4].clone()),
3,
1..4,
),
(
Bound::Excluded(events_ids[0].clone()),
Bound::Included(events_ids[3].clone()),
3,
1..4,
),
];

for (start_bound, end_bound, expected_len, id_range) in tests {
let list_return = filter_events(&mut module_impl, start_bound, end_bound);
assert_eq!(list_return.nb_events, 5);
assert_eq!(list_return.events.len(), expected_len);

list_return
.events
.into_iter()
.zip(events_ids[id_range].iter())
.for_each(|(event, id)| {
assert_eq!(event.id, *id);
});
}
}

fn get_all_events_ids(module_impl: &mut LedgerModuleImpl) -> Vec<EventId> {
let result = module_impl
.list(events::ListArgs {
count: None,
order: None,
filter: None,
})
.unwrap();

assert_eq!(result.nb_events, 5);
assert_eq!(result.events.len(), 5);

result.events.iter().map(|e| e.id.clone()).collect()
}

fn filter_events(
module_impl: &mut LedgerModuleImpl,
start_bound: Bound<EventId>,
end_bound: Bound<EventId>,
) -> events::ListReturns {
let result = module_impl.list(events::ListArgs {
count: None,
order: None,
filter: Some(events::EventFilter {
id_range: Some(CborRange {
start: start_bound,
end: end_bound,
}),
..events::EventFilter::default()
}),
});

assert!(result.is_ok());

result.unwrap()
}

#[test]
fn list_invalid_filter_id() {
let Setup {
mut module_impl,
id,
..
} = setup();

// Create 5 events
for i in 0..5 {
send(&mut module_impl, id, identity(i));
}
let invalid_tests = vec![
// Hypothetical error case - included range out of bounds
(
Bound::Included(vec![6, 7, 8].into()),
Bound::Included(vec![9, 10, 11].into()),
),
// Hypothetical error case - excluded range out of bounds
(
Bound::Excluded(vec![6, 7, 8].into()),
Bound::Excluded(vec![9, 10, 11].into()),
),
// Hypothetical error case - start included and end excluded, but range is out of bounds
(
Bound::Included(vec![0].into()),
Bound::Excluded(vec![10].into()),
),
// Hypothetical error case - start excluded and end included, but range is out of bounds
(
Bound::Excluded(vec![0].into()),
Bound::Included(vec![10].into()),
),
// Hypothetical error case - start unbounded and end included, but range is out of bounds
(Bound::Unbounded, Bound::Included(vec![6].into())),
// Hypothetical error case - start included and end unbounded, but range is out of bounds
(Bound::Included(vec![0].into()), Bound::Unbounded),
// Hypothetical error case - start unbounded and end excluded, but range is out of bounds
(Bound::Unbounded, Bound::Excluded(vec![6].into())),
// Hypothetical error case - start excluded and end unbounded, but range is out of bounds
(Bound::Excluded(vec![0].into()), Bound::Unbounded),
];

for (start_bound, end_bound) in invalid_tests {
assert_invalid_filter(&mut module_impl, start_bound, end_bound);
}
}

fn assert_invalid_filter(
module_impl: &mut LedgerModuleImpl,
start_bound: Bound<EventId>,
end_bound: Bound<EventId>,
) {
let result = module_impl.list(events::ListArgs {
count: None,
order: None,
filter: Some(events::EventFilter {
id_range: Some(CborRange {
start: start_bound,
end: end_bound,
}),
..events::EventFilter::default()
}),
});

assert!(result.is_err());
}
#[test]
fn list_filter_date() {
let Setup {
Expand Down
2 changes: 1 addition & 1 deletion src/many-modules/src/_4_events/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub struct ListArgs {
pub filter: Option<events::EventFilter>,
}

#[derive(Encode, Decode)]
#[derive(Debug, Encode, Decode)]
#[cbor(map)]
pub struct ListReturns {
#[n(0)]
Expand Down