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

Worst case redeem benchmark #1056

Merged
merged 2 commits into from
May 16, 2023
Merged

Worst case redeem benchmark #1056

merged 2 commits into from
May 16, 2023

Conversation

daniel-savu
Copy link
Contributor

Uses lend tokens as collateral in the redeem benchmarks, and re-generates the weights. This PR depends on #1052.

@@ -1,3 +1,5 @@
#![cfg(feature = "runtime-benchmarks")]
Copy link
Member

Choose a reason for hiding this comment

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

Not required since it is already specified in lib.rs

Comment on lines +355 to +358
&loans::GenesisConfig {
max_exchange_rate: Rate::from_inner(DEFAULT_MAX_EXCHANGE_RATE),
min_exchange_rate: Rate::from_inner(DEFAULT_MIN_EXCHANGE_RATE),
},
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we can implement Default for this at source to use some sensible values then we don't need to re-declare this everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default is already implemented but with different values than the ones in the tests. Instead of the 0.02 initial (min) exchange rate, tests use 1 because they break otherwise (at least when multiple collateral currencies are being checked).

@@ -174,6 +180,7 @@ pub mod benchmarks {
mint_wrapped::<T>(&caller, amount.into());

mint_collateral::<T>(&vault_id.account_id, 100_000u32.into());
mint_vault_collateral::<T>(&vault_id);
Copy link
Member

Choose a reason for hiding this comment

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

Does this not replace the call in the preceding line?

if vault_id.collateral_currency().is_lend_token() {
mint_lend_tokens::<T>(&vault_id.account_id, vault_id.collateral_currency());
} else {
mint_collateral::<T>(&vault_id.account_id, 100_000u32.into());
Copy link
Member

Choose a reason for hiding this comment

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

Actually, this is not required right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not required right now, I just wanted to make it easy to change the collateral used by the vault in the future

@gregdhill gregdhill merged commit 51b67e4 into master May 16, 2023
2 checks passed
@gregdhill gregdhill deleted the dan/worst-case-bench-redeem branch May 16, 2023 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants