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

Cleanup baseline power check #459

Closed
LesnyRumcajs opened this issue Jun 10, 2022 · 4 comments · Fixed by #1107
Closed

Cleanup baseline power check #459

LesnyRumcajs opened this issue Jun 10, 2022 · 4 comments · Fixed by #1107
Assignees
Labels

Comments

@LesnyRumcajs
Copy link
Contributor

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 equal BaselinePowerFromPrev(InitBaselinePower()) even though both are theoretically equal to Baseline(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) where effective_epoch <= epoch
taking equality of epoch and effective_epoch as the worst case we have
err = 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

@anorth
Copy link
Member

anorth commented Aug 11, 2022

@ZenGround0 @LesnyRumcajs is this still relevant?

@LesnyRumcajs
Copy link
Contributor Author

@sudo-shashank sudo-shashank self-assigned this Jan 4, 2023
@sudo-shashank
Copy link
Contributor

@LesnyRumcajs and @ZenGround0 does this ticket track cleanup of

state.effective_baseline_power <= next_epoch_baseline_power,
from check_state_invariants only or is it something else?

@LesnyRumcajs
Copy link
Contributor Author

@sudo-shashank I believe it tracks only this particular piece of logic.

@anorth anorth added the cleanup label May 27, 2024
@anorth anorth added this to FilOz May 28, 2024
@rjan90 rjan90 moved this to 📌 Triage in FilOz Jun 4, 2024
@rjan90 rjan90 moved this from 📌 Triage to 🐱Todo in FilOz Jun 4, 2024
@rjan90 rjan90 moved this from 🐱Todo to 🎉 Done in FilOz Jun 5, 2024
@rjan90 rjan90 moved this from 🎉 Done to ☑️Done (Archive) in FilOz Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging a pull request may close this issue.

4 participants