Tests for external staking #42
Conversation
hashedone
force-pushed
the
external-staking-test
branch
from
a170058 to
eb1a3b6
Compare
| @@ -36,6 +36,9 @@ thiserror = { workspace = true } | |||
| sylvia = { workspace = true, features = ["mt"] } | |||
| cw-multi-test = { workspace = true } | |||
| anyhow = { workspace = true } | |||
| mesh-vault = { workspace = true, features = ["mt"] } | |||
| mesh-native-staking-proxy = { workspace = true, features = ["mt"] } | |||
There was a problem hiding this comment.
Cool you test those, but do you need native staking in the external staking tests?
Or is this just for initialization of the vault?
There was a problem hiding this comment.
Only vault initialization. I was looking for a way to fake it somehow, but I need to pass the proper message and so on, so I decided to add them and just instantiate - doesn't hurt for tests.
contracts/vault/src/contract.rs
Outdated
| @@ -263,6 +263,22 @@ impl VaultContract<'_> { | |||
| Ok(resp) | |||
| } | |||
|
|
|||
| /// Returns a singl claim between the user and lienholder | |||
|
|
||
| use anyhow::Result as AnyResult; | ||
|
|
||
| const OSMO: &str = "OSMO"; |
There was a problem hiding this comment.
Nit: if you want to be closer to reality, use uosmo here (OSMO only exists in UIs not on the chain)
There was a problem hiding this comment.
It completely does not matter, but will change just for consistency.
| fn staking() { | ||
| let users = ["user1", "user2"]; | ||
|
|
||
| let app = MtApp::new(|router, _api, storage| { |
There was a problem hiding this comment.
Minor note for Sylvia:
It would be nice to add some helper for the simple setup of just setting balances (and only use closure for more important cases). Like keep MtApp:::new as is, but something like this, which takes balances: &[(&str, Vec<Coin>])
let app = MtApp::with_balances(&[
(users[0], coins(300, OSMO)),
(users[1], coins(300, OSMO))
];There was a problem hiding this comment.
In general, what would happen for sure:
App::new()would be changed to match theApp::new(...)from multitest - will take instantiation closureAppBuilderwould be created to mimic the one from MT - allowing setting up custom modules- There would be an additional layer between the builder and the app -
AppConfigwhich would have some additional functions knowing what modules are used. Eg. when defaultBankKeeperis used it would have function likeinitial_balances(...)- exactly what you said. It would be another builder state, so when there is another config option it doesn't add a dozen of functions passing it in different combinations. ObviouslyBuilderwould still be able to return anAppdirectly if we don't want to config it. - There would be added
From<cw_multi_test::App> fo sylvia::Appimplementation, so it would still be possible to create mt app manually and then convert to sylvia one - in case some MT feature would be added, and we will not update sylvia automatically.
| .call(users[1]) | ||
| .unwrap(); | ||
|
|
||
| vault |
There was a problem hiding this comment.
Great stuff.
For completeness, I would add one more stake, and ensure it returns an error (no more collateral left for lien)
| .call(users[1]) | ||
| .unwrap(); | ||
|
|
||
| // Properly unstake some tokens |
There was a problem hiding this comment.
nice cases and error checking here
| .unwrap(); | ||
| assert_eq!(stake.stake.u128(), 0); | ||
|
|
||
| // Another timetravel - just enough for first bathc of stakes to release, |
There was a problem hiding this comment.
Nice setup for the withdrawing process and ensuring this works with multiple batches pending.
Small typo: bathc -> batch
| @@ -49,13 +49,13 @@ impl Stake { | |||
| }; | |||
|
|
|||
| // First item is still not ready for release | |||
| if self.pending_unbonds[0].release_at < info.time { | |||
| if self.pending_unbonds[0].release_at > info.time { | |||
There was a problem hiding this comment.
That's what tests are for.
There was a problem hiding this comment.
Yup, that is why I really didn't check it 100 times before, as it would be burned time - I knew I am writing tests for it in next PR and it would be verified there.
| assert_eq!(stake.stake.u128(), 0); | ||
|
|
||
| // Another timetravel - just enough for first bathc of stakes to release, | ||
| // to early for second batch |
| contract.withdraw_unbonded().call(users[0]).unwrap(); | ||
| contract.withdraw_unbonded().call(users[1]).unwrap(); | ||
|
|
||
| // Now claims on vauld got reduced, but only for first batch amount |
| .unwrap(); | ||
| assert_eq!(claim.amount.u128(), 240); | ||
|
|
||
| // Moving forward more, passign through second bath pending duration |
Follow up of #39
Closes #13
Implements only the third task mentioned in #13 (comment).
Next - distribution.
Multitests + minor bugfixes found by tests.