Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix staking rebond weight refund #9508

Merged
3 commits merged into from
Aug 9, 2021
Merged

Fix staking rebond weight refund #9508

3 commits merged into from
Aug 9, 2021

Conversation

emostov
Copy link
Contributor

@emostov emostov commented Aug 6, 2021

This PR updates the weight refund for pallet-staking's rebond. The rebond benchmark is variant over the number of unlocking chunks iterated over; so in this PR we calculate the max amount of "relocked" chunks and pass that too WeightInfo::rebond to get the actual weight.

@emostov emostov requested a review from kianenigma August 6, 2021 03:59
)
.into())

let removed_chunks = 1 + // account the case where last iterated chunk is not removed
Copy link
Contributor

Choose a reason for hiding this comment

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

better use safe arithmetic

Copy link
Contributor

Choose a reason for hiding this comment

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

and FYI the dependency of this function on unlocking is super small (substrate weight: .saturating_add((35_000 as Weight).saturating_mul(l as Weight)), which is very small)

@kianenigma kianenigma marked this pull request as ready for review August 6, 2021 13:31
@kianenigma kianenigma added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Aug 6, 2021
@bkchr bkchr requested a review from gui1117 August 6, 2021 14:49
let ledger = ledger.rebond(value);
// Last check: the new active amount of ledger must be more than ED.
ensure!(ledger.active >= T::Currency::minimum_balance(), Error::<T>::InsufficientBond);

Self::deposit_event(Event::<T>::Bonded(ledger.stash.clone(), value));
Self::update_ledger(&controller, &ledger);
Ok(Some(
35 * WEIGHT_PER_MICROS +
50 * WEIGHT_PER_NANOS * (ledger.unlocking.len() as Weight) +
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems the previous formula is wrong as it just look at the final unlocking len.

@gui1117
Copy link
Contributor

gui1117 commented Aug 9, 2021

bot merge

@ghost
Copy link

ghost commented Aug 9, 2021

Trying merge.

@ghost ghost merged commit 91061a7 into master Aug 9, 2021
@ghost ghost deleted the zeke-fix-rebond-weight-refund branch August 9, 2021 11:59
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants