builtin-actors
builtin-actors copied to clipboard
Cleanup baseline power check
Baseline power checks come with a rounding error which is visible for epoch zero, like here (same in Rust here).
(copied from discussion with @ZenGround0 regarding reward actor invariant checks that were failing for prior_epoch == -1
)
The issue is that
BaselineInitialValue
is not exactly equalBaselinePowerFromPrev(InitBaselinePower())
even though both are theoretically equal toBaseline(0)
.Error is introduced because we use fixed point numbers InitBaselinePower() starts with initial value, divides by the exponent but then multiplying back precisely the same exponent does not give back the same number
I want to figure out exactly how error grows so we can have high confidence invariant that takes error into consideration Sketching this out --
st.BaselinePower = (init_val - epsilon) * x^(epoch)
st.EffectiveBaselinePower = init_val*x^(effective_epoch)
whereeffective_epoch <= epoch
taking equality ofepoch
andeffective_epoch
as the worst case we haveerr = st.EffectiveBaselinePower - st.BaselinePower = init_valx^y - (init_val - epsilon)x^y
err = epsilon*x^y
So we want
let epsilon = BASELINE_INITIAL_VALUE - baseline_power_from_prev(INIT_BASELINE_POWER) let err = epsilon * exp(BASELINE_EXPONENT, st.epoch)
If you reimplement this in rust: https://github.com/filecoin-project/specs-actors/blob/0bcea9e3fa074b875135563cdaba6e786428c669/actors/util/math/exp.go#L6 we can do exp efficiently Now that I know what's going on I can confidently say it's a better use of our limited time for you to delete this baseline check entirely. If you can copy the above discussion into an issue for later so we can track this as a low priority cleanup that would be amazing. Instead of deleting a reasonable compromise hack is to compare
st.EffectiveBaselinePower < baseline_power_from_prev(st.BaselinePower)
under the assumption that errors are much smaller than per epoch updates. This won't work for very very large epochs but its more or less good enough forever especially with the above issue filed
Given the above, I'll go with the easy solution for now but it would be nice to improve the check with the suggestions above.
Related invariant check
@ZenGround0 @LesnyRumcajs is this still relevant?
@anorth I believe so: https://github.com/filecoin-project/builtin-actors/blob/master/actors/reward/src/testing.rs#L58
@LesnyRumcajs and @ZenGround0 does this ticket track cleanup of https://github.com/filecoin-project/builtin-actors/blob/a600986f67ac5bd3f33edf544b3c50114045d03a/actors/reward/src/testing.rs#L60 from check_state_invariants only or is it something else?
@sudo-shashank I believe it tracks only this particular piece of logic.