[Merged by Bors] - simplify calculate_committee_fraction

[eserilev]

Issue Addressed

#4211

Proposed Changes

This PR conforms the helper function calculate_committee_fraction to the v1.3.0 spec

Additional Info

the old definition of calculate_committee_fraction is almost identical, but the new definition is simpler.

[eserilev]

fork_choice_test_definition::execution_status::test_03 is failing

line 1003 mentions a "magic number" generated from calculate_committee_fraction. I might need to generate a new magic number here for the test to pass

[michaelsproul]

line 1003 mentions a "magic number" generated from calculate_committee_fraction

Huh, that is strange. I wouldn't have thought the values calculated by calculate_committee_fraction would actually be any different due to the simplification. We may want to dig into this a bit to understand exactly what's going on

[eserilev]

line 1003 mentions a "magic number" generated from calculate_committee_fraction

Huh, that is strange. I wouldn't have thought the values calculated by calculate_committee_fraction would actually be any different due to the simplification. We may want to dig into this a bit to understand exactly what's going on

Digging into whats going on

the update to calculate_committee_fraction is causing issues with test_03. I attempted to debug the issue by running test_03 locally. When executing the operations in test_03, I added a call to calculate_committee_fraction each time a new op is iterated through, and logged the results.

In the previous version of calculate_committee_fraction, calling the function in test_03 returns 31,000 at each op iteration

In the new version of calculate_committee_fraction, calling the function in test_03 returns 31,250 at each op iteration.

so the previous vs new iterations return a slightly different value.

Furthermore, in test_03, the magic numbers chosen on lines 990, 994, and 1003 only work for the older version of calculate_committee_fraction. In my most recent push I updated the magic numbers to reflect what they should be for the new version of calculate_committee_fraction. This update allows test_03 to pass. To be honest, I have no idea what the implications of this are. I'll try to continue digging into fork choice rules and some of the discussions around this specific spec update to gain additional context.

[eserilev]

ok I thought about this some more and I think I understand whats going on here

in Rust the following calculations could return different results, even though they are mathematically equivalent

a * (b /c) vs (a * b) / c

that's because w/ integer division, the remainder gets thrown away, so the order in which we divide and multiply plays a factor.

so even though the old vs new committee_weight calculations are mathematically equivalent:

(total_effective_balance / num_active_validators) * (num_active_validators / slots_per epoch) = total_effective_balance / slots_per_epoch

they dont return the exact same results due to the fact that the remainder gets thrown away during integer division

[michaelsproul]

@eserilev Do you mind rebasing on/merging the latest unstable? That will fix the outstanding CI failures.

We'll try to review this ASAP and get it merged for 4.2.0

[eserilev]

@michaelsproul i merged the latest unstable. thank you!

[michaelsproul]

Looks good. Happy to merge.

[michaelsproul]

bors r+

[bors]

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• arbitrary-check
• beacon-chain-tests
• cargo-audit
• cargo-fmt
• cargo-udeps
• cargo-vendor
• check-benchmarks
• check-msrv
• clippy
• compile-with-beta-compiler
• debug-tests-ubuntu
• dockerfile-ubuntu
• doppelganger-protection-test
• ef-tests-ubuntu
• eth1-simulator-ubuntu
• execution-engine-integration-ubuntu
• merge-transition-ubuntu
• no-eth1-simulator-ubuntu
• op-pool-tests
• release-tests-ubuntu
• release-tests-windows
• slasher-tests
• state-transition-vectors-ubuntu
• syncing-simulator-ubuntu