use AtStake to dispatch rewards instead of AwardedPts #1896
Conversation
nbaztec
added
B7-runtimenoteworthy
nbaztec
force-pushed
the
nish-staking-at-stake-awarded
branch
from
0d50af2
to
361ca58
Compare
| @@ -9061,7 +9121,7 @@ fn test_on_initialize_weights() { | |||
| // | |||
| // following this assertion, we add individual weights together to show that we can | |||
| // derive this number independently. | |||
| let expected_on_init = 3_017_871_000; | |||
| let expected_on_init = 2_506_497_000; | |||
There was a problem hiding this comment.
@notlesh This is the second time I've had to touch this test and honestly the only thing I do is make the number match the expectation. Could we somehow improve the test so it doesn't require us to update the magic number all the time? I'm afraid that we'd just ignore the failure and always update numbers without realizing what actually went down.
In my case, admittedly, I'd started adding the weight from auto-compounding to extra_weight, and here changed the execution behavior. Even though I knew in both cases what changed I couldn't come up with a good magic number value. :(
There was a problem hiding this comment.
I agree that it's annoying. The point of it is so that we don't accidentally change the overall weight of on_init without realizing it. If you can think of a better way to accomplish that, I'm all for it.
Otherwise I think we could discuss whether it is worth having it at all. I like it for the reason above, but not enough that it should annoy everyone :)
While we're on the subject, I'm surprised that the amount decreased so much...
There was a problem hiding this comment.
Yeah I wondered about that, my guess was the removal of clear_prefix here.
| @@ -1557,22 +1559,25 @@ pub mod pallet { | |||
| // 2. we called pay_one_collator_reward when we were actually done with deferred | |||
| // payouts | |||
| log::warn!("pay_one_collator_reward called with no <Points<T>> for the round!"); | |||
| return (None, Weight::zero()); | |||
| return (RewardPayment::Finished, Weight::zero()); | |||
There was a problem hiding this comment.
I think this weight fails to capture the <Points<T>> read above, and that may be true for weights returned in other places in this fn.
There was a problem hiding this comment.
...it looks like this was the case before this PR...
notlesh
added
D1-audited👍
What does it do?
uses
AtStakestorage to dispatch rewards instead ofAwardedPts.What important points reviewers should know?
AtStakestorage is a superset ofAwardedPtshence guaranteed to be cleaned up if round length is >=TopSelectedcandidates.Is there something left for follow-up PRs?
What alternative implementations were considered?
#1923 was considered to not change the existing behavior but we went with #1896 as it avoids an
elseblock that is responsible for cleanup and easily missed.Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?
What value does it bring to the blockchain users?