[Altair] Remove duplicate vars, panic on u64::pow overflow

[paulhauner]

Issue Addressed

NA

Proposed Changes

• Remove duplicates of INACTIVITY_SCORE_BIAS and INACTIVITY_PENALTY_QUOTIENT_ALTAIR as per https://github.com/sigp/lighthouse/pull/2279/files#r634154482
• Panic instead of silently saturating when computing ChainSpec values.

Additional Info

Adding the panics to ChainSpec likely provides marginal safety in reality, but it feels more consistent than using pow (overflowing) with a saturating_mul. It's also kinda nice cause the integer_arithmetic should probably fail those pow calls.

Instead of a panic we could return a Result/Option, but that seems like overkill. After all, they're constant values that will cause tests to fail if they overflow.

[michaelsproul]

Looks fine. Happy to merge with or without the pow methods added to the disallowed methods 👍

[michaelsproul]

Agree that this should be on the ChainSpec rather than a constant, as per: https://github.com/ethereum/eth2.0-specs/blob/48e1ef193a3c640ddd3df264f25470989908f1c7/configs/minimal_config.yaml

[michaelsproul]

Oops thought I posted this earlier:

It's also kinda nice cause the integer_arithmetic should probably fail those pow calls.

I don't think the integer_arith lint will get them, but we could add pow to our list of banned methods here:

https://github.com/sigp/lighthouse/blob/stable/consensus/types/clippy.toml

[paulhauner]

I don't think the integer_arith lint will get them, but we could add pow to our list of banned methods here:

I'm struggling to get clippy to ban u64::pow.

I have this in the consensus/types/clippy.toml:

# Disallow sum and product methods which are prone to overflow.
disallowed-methods = [
    "core::iter::traits::iterator::Iterator::sum",
    "core::iter::traits::iterator::Iterator::product",
    "core::primitive::u64::pow"
]

But I can't get clippy to fail on

seconds_per_slot: core::primitive::u64::pow(4, 2),

Anything obvious you can see @michaelsproul? I recall you spending some time working on these types of lints.

[michaelsproul]

Anything obvious you can see @michaelsproul? I recall you spending some time working on these types of lints.

I tried a few things but nothing worked. I reckon we merge this PR and come back to it. I suspect it will require some hacking on the lint itself. My guesses were:

• Maybe the lint doesn't work for bare functions? No. Bare functions are supported since 1.52.0 via disallowed_methods: Support functions in addition to methods rust-lang/rust-clippy#6674
• Maybe the lint doesn't work well with const functions because they get const-evaluated before the lint runs? But it seems that even in situations where the compiler shouldn't do const evaluation, the lint still doesn't fire, e.g.

#![deny(clippy::disallowed_method)]

fn thing() -> u64 {
    let mut x: u64 = 0;
    for i in 0..3 {
        x += i;
    }
    x.pow(2)
}

fn main() {
    let y = thing();
    assert_eq!(y, 9);
}