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.

Sign up for GitHub

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

Jump to bottom

Add extra tests / examples of lockable ranges #58

Merged
merged 4 commits into from
Jun 19, 2023
Merged

Add extra tests / examples of lockable ranges #58

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ecf6b6
Add extra tests / examples of lockable struct
maurolacy Jun 17, 2023
8d71260
Idiomatic filter
maurolacy Jun 17, 2023
5ce8463
Idiomatic maps
maurolacy Jun 17, 2023
f2d6d8e
Improve syntax / add comments
maurolacy Jun 17, 2023
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
160 changes: 148 additions & 12 deletions packages/sync/src/locks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,13 +264,24 @@ mod tests_plus {
}

#[derive(Error, Debug, PartialEq)]
pub enum PersonError {
pub enum TestsError {
#[error("{0}")]
Std(#[from] StdError),
#[error("{0}")]
Lock(#[from] LockError),
}

#[cw_serde]
#[derive(Default)]
pub struct UserInfo {
// User collateral
pub collateral: Uint128,
// Highest user lien
pub max_lien: Uint128,
// Total slashable amount for user
pub total_slashable: Uint128,
}

#[test]
fn modify_item_with_locks() {
let mut store = MockStorage::new();
Expand All @@ -285,7 +296,7 @@ mod tests_plus {
PERSON
.update(&mut store, |mut p| {
p.write()?.age += 1;
Ok::<_, PersonError>(p)
Ok::<_, TestsError>(p)
})
.unwrap();

Expand All @@ -294,7 +305,7 @@ mod tests_plus {
.update(&mut store, |mut p| {
assert_eq!(p.read()?.age, 33);
p.lock_read()?;
Ok::<_, PersonError>(p)
Ok::<_, TestsError>(p)
})
.unwrap();

Expand All @@ -312,7 +323,7 @@ mod tests_plus {
.update(&mut store, |mut p| {
p.write()?.age += 1;
assert_eq!(p.read()?.age, 34);
Ok::<_, PersonError>(p)
Ok::<_, TestsError>(p)
})
.unwrap();
}
Expand All @@ -332,23 +343,23 @@ mod tests_plus {
AGES.update(&mut store, "John", |p| {
let mut p = p.unwrap_or_default();
*p.write()? += Uint128::new(1);
Ok::<_, PersonError>(p)
Ok::<_, TestsError>(p)
})
.unwrap();

// Update works on new values, setting to unlocked by default
AGES.update(&mut store, "Wilber", |p| {
let mut p = p.unwrap_or_default();
*p.write()? += Uint128::new(2);
Ok::<_, PersonError>(p)
Ok::<_, TestsError>(p)
})
.unwrap();

// We can range over them well
let total_age = AGES
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.fold(Ok(Uint128::zero()), |sum, item| {
Ok::<_, PersonError>(sum? + *item?.1.read()?)
Ok::<_, TestsError>(sum? + *item?.1.read()?)
})
.unwrap();
assert_eq!(total_age, Uint128::new(33 + 47 + 2));
Expand All @@ -369,16 +380,16 @@ mod tests_plus {
.update(&mut store, "John", |p| {
let mut p = p.unwrap_or_default();
*p.write()? += Uint128::new(1);
Ok::<_, PersonError>(p)
Ok::<_, TestsError>(p)
})
.unwrap_err();
assert_eq!(err, PersonError::Lock(LockError::ReadLocked));
assert_eq!(err, TestsError::Lock(LockError::ReadLocked));

// We can still range over all
let total_age = AGES
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.fold(Ok(Uint128::zero()), |sum, item| {
Ok::<_, PersonError>(sum? + *item?.1.read()?)
Ok::<_, TestsError>(sum? + *item?.1.read()?)
})
.unwrap();
assert_eq!(total_age, Uint128::new(33 + 47 + 2));
Expand All @@ -398,15 +409,140 @@ mod tests_plus {
let err = AGES
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.fold(Ok(Uint128::zero()), |sum, item| {
Ok::<_, PersonError>(sum? + *item?.1.read()?)
Ok::<_, TestsError>(sum? + *item?.1.read()?)
})
.unwrap_err();
assert_eq!(err, PersonError::Lock(LockError::WriteLocked));
assert_eq!(err, TestsError::Lock(LockError::WriteLocked));

// We can get count (kind of edge case bug, but I don't think we can change this or it matters)
let num_people = AGES
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.count();
assert_eq!(num_people, 3);
}

#[test]
fn map_methods_with_locked_struct() {
let mut store = MockStorage::new();
const USERS: Map<&str, Lockable<UserInfo>> = Map::new("users");

// add a few people
USERS
.save(
&mut store,
"John",
&Lockable::new(UserInfo {
collateral: Default::default(),
max_lien: Default::default(),
total_slashable: Default::default(),
}),
)
.unwrap();
USERS
.save(
&mut store,
"Maria",
&Lockable::new(UserInfo {
collateral: Uint128::new(1),
max_lien: Uint128::new(2),
total_slashable: Uint128::new(3),
}),
)
.unwrap();

// Update works on new values, setting to unlocked by default
USERS
.update(&mut store, "Wilber", |p| {
let mut p = p.unwrap_or_default();
*p.write()? = UserInfo {
collateral: Uint128::new(4),
max_lien: Uint128::new(5),
total_slashable: Uint128::new(6),
};
Ok::<_, TestsError>(p)
})
.unwrap();

// Read-lock John
let mut j = USERS.load(&store, "John").unwrap();
j.lock_read().unwrap();
USERS.save(&mut store, "John", &j).unwrap();

// We can still range over all
let total_collateral = USERS
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.fold(Ok(Uint128::zero()), |sum, item| {
Ok::<_, TestsError>(sum? + item?.1.read()?.collateral)
})
.unwrap();
assert_eq!(total_collateral, Uint128::new(1 + 4));

// We can get count
let num_users = USERS
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.count();
assert_eq!(num_users, 3);

// Write-lock Wilber
let mut w = USERS.load(&store, "Wilber").unwrap();
w.lock_write().unwrap();
USERS.save(&mut store, "Wilber", &w).unwrap();

// We cannot range over all
let err = USERS
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.fold(Ok(Uint128::zero()), |sum, item| {
Ok::<_, TestsError>(sum? + item?.1.read()?.max_lien)
})
.unwrap_err();
assert_eq!(err, TestsError::Lock(LockError::WriteLocked));

// We cannot range and map over all values either
let err = USERS
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.map(|item| {
let (_, user_lock) = item?;
let user = user_lock.read()?;
Ok(user.collateral)
})
.collect::<Result<Vec<_>, TestsError>>()
.unwrap_err();
assert_eq!(err, TestsError::Lock(LockError::WriteLocked));

// But we can re-map (perhaps not a good idea) the write-locked values
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to explain what is and is not possible here

let collaterals: Vec<_> = USERS
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.map(|item| {
item.map(|(_, user_lock)| {
user_lock
.read()
.map(|user| Ok(user.collateral))
.unwrap_or(Ok(Uint128::zero())) // Re-map locked collateral
})? // Surface errors
})
.collect::<Result<_, TestsError>>()
.unwrap();
assert_eq!(collaterals.len(), 3);

// Or we can skip (perhaps not a good idea either) the write-locked values
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reflecting more on this, I think the best would be to return:

enum MaybeAccount {
  Account(Account),
  Locked{ user: String },
}

Or something Similar. (Not sure how you use the keys).
I can make an example as follow up PR (unless this makes sense to you and you want to try)

Copy link
Collaborator Author

@maurolacy maurolacy Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼 Will implement this for reference / as an example.

let collaterals: Vec<_> = USERS
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.filter(|item| {
item.as_ref()
.map(|(_, user_lock)| user_lock.read().map(|_| true).unwrap_or(false)) // Filter locked values
.unwrap_or(true) // Surface other errors
})
.map(|item| {
item.map(|(_, user_lock)| user_lock.read().map(|user| Ok(user.collateral))?)?
})
.collect::<Result<_, TestsError>>()
.unwrap();
assert_eq!(collaterals.len(), 2);

// We can get count (kind of edge case bug, but I don't think we can change this or it matters)
let num_users = USERS
.range(&store, None, None, cosmwasm_std::Order::Ascending)
.count();
assert_eq!(num_users, 3);
}
}
Loading