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

handle rewards overflow #1635

Merged
merged 4 commits into from Mar 10, 2020
Merged

handle rewards overflow #1635

merged 4 commits into from Mar 10, 2020

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented Mar 3, 2020

  • Add EFFECTIVE_BALANCE_INCREMENT to avoid overflow in reward calculations
  • Create bit analysis to show safety and precision
  • Hand audit all multiplications in spec for overflows. The culprit in the two we make adjustments for were due to multiplying by the total active balance. I recommend a couple others do a similar audit

Analysis generally looks good. Even at 10billion ETH deposited, the calculation only hits ~40 bits.

Because total_balance and attesting_balance are normalized with EFFECTIVE_BALANCE_INCREMENT, no precision in calculations is lost.


todo (maybe future PR):

Figure out a way to test this... running a single rewards test in minimal config with 50k vals took 1 hour. Might look into using a config with "weird" values such as a MAX_EFFECTIVE_BALANCE equal to 32000 eth instead of 32.

@djrtwo
Copy link
Contributor Author

@djrtwo djrtwo commented Mar 3, 2020

cc: @hermanjunge

@djrtwo djrtwo changed the title [wip] handle rewards overflow handle rewards overflow Mar 3, 2020
@djrtwo djrtwo requested a review from protolambda Mar 3, 2020
specs/phase0/beacon-chain.md Outdated Show resolved Hide resolved
@ghost
Copy link

@ghost ghost commented Mar 4, 2020

Took the new computations into this simulator https://github.com/hermanjunge/eth2-reward-simulation/blob/154130f7c712211c16483343916b70159681dade/src/process_epoch/get_attestation_deltas.rs#L56

Compiled using the --release flag. The latter is relevant, as rust won't yell at you about overflows in release mode, only on debug.

If we run the simulation at an online probability of .95 and 5 epochs against the initial stakes of 500k, 1M, 5M, 10M, and 100M and compare the results gotten (below the horizontal line, to improve readability) with a back-of-the-envelope calculation in a spreadsheet, we don't see major discrepancies in the total FFG rewards nor the total FFG penalties.

Screenshot from 2020-03-04 00-20-12


[ ubuntu: reward ]$ ./target/release/simulation -i 500000 -e 5 -p 0.95
epoch number,FFG rewards,FFG penalties,proposer rewards,attester rewards,total staked balance,total effective balance,max balance,min balance,total validators,total active validatos,time μs
0,970160952,52067778,39753180,290278175,500001248124529,499242000000000,32001409887,31999931309,15625,15625,2618
1,965361000,56282520,42433024,288851720,500002488487753,499242000000000,32001495657,31999864715,15625,15625,597
2,968483352,52995792,41106992,289785980,500003734868285,499202000000000,32002906533,31999798121,15625,15625,641
3,968183043,53275236,41023652,289697659,500004980497403,499202000000000,32002991381,31999880312,15625,15625,559
4,970476948,50860569,41065322,290384034,500006231563138,499198000000000,32003076229,31999813715,15625,15625,617

[ ubuntu: reward ]$ ./target/release/simulation -i 1000000 -e 5 -p 0.95
epoch number,FFG rewards,FFG penalties,proposer rewards,attester rewards,total staked balance,total effective balance,max balance,min balance,total validators,total active validatos,time μs
0,1374141480,71300760,58134951,411170292,1000001772145963,998532000000000,32001935267,31999951430,31250,31250,2855
1,1368506826,76094286,54379674,409486464,1000003528424641,998532000000000,32001997110,31999904342,31250,31250,1252
2,1370202267,74309526,59777595,409993768,1000005294088745,998458000000000,32002057099,31999857254,31250,31250,1308
3,1368681216,75896964,58192425,409541606,1000007054607028,998458000000000,32002117092,31999915368,31250,31250,1105
4,1367702880,76926861,60069600,409248857,1000008814701504,998452000000000,32002177085,31999868284,31250,31250,1141

[ ubuntu: reward ]$ ./target/release/simulation -i 5000000 -e 5 -p 0.95
epoch number,FFG rewards,FFG penalties,proposer rewards,attester rewards,total staked balance,total effective balance,max balance,min balance,total validators,total active validatos,time μs
0,3061879260,170719200,109108610,916011470,5000003916280140,4992140000000000,32004223292,31999978280,156250,156250,6798
1,3059543778,170325912,130091035,915536950,5000007851125991,4992140000000000,32004250120,31999957220,156250,156250,6549
2,3062176530,167554296,129961199,916324774,5000011792034198,4991753000000000,32004276948,31999936160,156250,156250,7101
3,3059941368,169644618,129831363,915655928,5000015727818239,4991753000000000,32004303776,31999962150,156250,156250,5873
4,3059775531,169819200,125764714,915606303,5000019659145587,4991706000000000,32004330604,31999941090,156250,156250,5912

[ ubuntu: reward ]$ ./target/release/simulation -i 10000000 -e 5 -p 0.95
epoch number,FFG rewards,FFG penalties,proposer rewards,attester rewards,total staked balance,total effective balance,max balance,min balance,total validators,total active validatos,time μs
0,4334305536,237573120,189972480,1296846078,10000005583550974,9984533000000000,32005955598,31999984640,312500,312500,14281
1,4324605510,242375595,184035840,1294292985,10000011144109714,9984533000000000,32005974565,31999969751,312500,312500,14529
2,4325408868,241529802,177913680,1294533387,10000016700435847,9983781000000000,32005993532,31999954862,312500,312500,15261
3,4331683017,235547844,183293760,1296159441,10000022276024221,9983781000000000,32006012502,31999939970,312500,312500,13097
4,4327471080,239982192,183664800,1294899112,10000027842077021,9983699000000000,32006031472,31999958341,312500,312500,13277

[ ubuntu: reward ]$ ./target/release/simulation -i 100000000 -e 5 -p 0.95
epoch number,FFG rewards,FFG penalties,proposer rewards,attester rewards,total staked balance,total effective balance,max balance,min balance,total validators,total active validatos,time μs
0,13696331406,760465347,562204380,4096432020,100000017594502459,99843429000000000,32018746140,31999995143,3125000,3125000,149188
1,13686732822,756199224,521384260,4094009858,100000035140430175,99843429000000000,32018752138,31999990436,3125000,3125000,154040
2,13686842307,756083961,542350958,4094042615,100000052707582094,99835981000000000,32018758136,31999985729,3125000,3125000,153285
3,13682894079,759087837,580387888,4092868691,100000070304644915,99835981000000000,32018764134,31999981022,3125000,3125000,150937
4,13680966606,761117112,598571396,4092291962,100000087915357767,99835290000000000,32018770132,31999976315,3125000,3125000,148122

@djrtwo djrtwo requested a review from hwwhww Mar 6, 2020
hwwhww
hwwhww approved these changes Mar 10, 2020
Copy link
Contributor

@hwwhww hwwhww left a comment

The analysis looks solid. LGTM! 👍

specs/phase0/beacon-chain.md Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants