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

avoid packing attestations from other forks #3893

Merged
merged 1 commit into from
Jul 21, 2022
Merged

Conversation

etan-status
Copy link
Contributor

When there is heavy forking, proposals may get missed due to including
attestations from different forks that later fail verification.
Checking attestation signatures when building blocks should fix this.

When there is heavy forking, proposals may get missed due to including
attestations from different forks that later fail verification.
Checking attestation signatures when building blocks should fix this.
@etan-status
Copy link
Contributor Author

DBG 2022-07-20 13:53:00.015+00:00 Packed attestations for block              topics="attpool" newBlockSlot=240 packingDur=2ms566us314ns totalCandidates=69 attestations=57
ERR 2022-07-20 13:53:00.019+00:00 Cannot create block for proposal           topics="beacval" slot=240 head=0d13c2df:238 error="indexed attestation: signature verification failure"

@tersec
Copy link
Contributor

tersec commented Jul 20, 2022

It would be useful to have some measurements performancewise of how much this matters. Hitting proposer boosting is useful.

@etan-status
Copy link
Contributor Author

without this:

Running state_sim --validators=8000 --slots=160

Loaded genesis_mainnet_8000_1.2.0-rc.1.ssz...
Starting simulation...
:............................... slot: 32 epoch: 1
:............................... slot: 64 epoch: 2
:............................... slot: 96 epoch: 3
:............................... slot: 128 epoch: 4
:............................... slot: 160 epoch: 5
:Done!
Validators: 8000, epoch length: 32
Validators per attestation (mean): 250.0
All time are ms
     Average,       StdDev,          Min,          Max,      Samples,         Test
       0.827,        0.162,        0.727,        2.442,          155, Process non-epoch slot with block
       3.398,        0.689,        2.268,        4.110,            5, Process epoch slot with block
       0.018,        0.005,        0.008,        0.051,          160, Tree-hash block
      84.659,        3.710,       78.661,       98.758,          160, Combine committee attestations

Running block_sim --validators=8000 --slots=160

Loaded genesis_mainnet_8000_1.2.0-rc.1.ssz...
Starting simulation...
................................ slot: 32 epoch: 1
................................ slot: 64 epoch: 2
................................ slot: 96 epoch: 3
................................ slot: 128 epoch: 4
................................ slot: 160 epoch: 5
Done!
Validators: 8000, epoch length: 32
Validators per attestation (mean): 0.0
All time are ms
     Average,       StdDev,          Min,          Max,      Samples,         Test
      16.805,        7.705,       12.502,      109.302,          155, Process non-epoch slot with block
     162.210,      193.915,       28.946,      461.087,            5, Process epoch slot with block
       0.026,        0.005,        0.013,        0.052,          160, Tree-hash block
       0.335,        0.056,        0.294,        0.688,          160, Sign block
      72.598,        4.716,       64.517,       92.929,          160, Have committee attest to block
     272.536,      135.019,        0.000,      403.813,          160, Produce sync committee actions
      54.321,        0.000,       54.321,       54.321,            1, Replay all produced blocks

with this:

Running state_sim --validators=8000 --slots=160

Loaded genesis_mainnet_8000_1.2.0-rc.1.ssz...
Starting simulation...
:............................... slot: 32 epoch: 1
:............................... slot: 64 epoch: 2
:............................... slot: 96 epoch: 3
:............................... slot: 128 epoch: 4
:............................... slot: 160 epoch: 5
:Done!
Validators: 8000, epoch length: 32
Validators per attestation (mean): 250.0
All time are ms
     Average,       StdDev,          Min,          Max,      Samples,         Test
       0.823,        0.155,        0.727,        2.330,          155, Process non-epoch slot with block
       3.253,        0.454,        2.448,        3.528,            5, Process epoch slot with block
       0.017,        0.002,        0.009,        0.038,          160, Tree-hash block
      81.231,        2.815,       78.258,       96.779,          160, Combine committee attestations

Running block_sim --validators=8000 --slots=160

Loaded genesis_mainnet_8000_1.2.0-rc.1.ssz...
Starting simulation...
................................ slot: 32 epoch: 1
................................ slot: 64 epoch: 2
................................ slot: 96 epoch: 3
................................ slot: 128 epoch: 4
................................ slot: 160 epoch: 5
Done!
Validators: 8000, epoch length: 32
Validators per attestation (mean): 0.0
All time are ms
     Average,       StdDev,          Min,          Max,      Samples,         Test
      41.949,        7.694,       17.563,      106.825,          155, Process non-epoch slot with block
     188.216,      186.439,       58.099,      467.096,            5, Process epoch slot with block
       0.028,        0.011,        0.013,        0.093,          160, Tree-hash block
       0.328,        0.052,        0.294,        0.655,          160, Sign block
      70.252,        2.788,       64.091,       78.638,          160, Have committee attest to block
     264.344,      130.261,        0.000,      355.739,          160, Produce sync committee actions
      54.099,        0.000,       54.099,       54.099,            1, Replay all produced blocks

Not sure if those benchmarks are the correct ones to measure this.
And it's true that in periods of heavy forking, it is quite questionable whether the extra rewards offset any penalties from potentially missing proposer boost cutoff.
A hybrid could be made that limits the time to collect attestations to an upper bound, but not sure how high that one should be set.

@github-actions
Copy link

Unit Test Results

       12 files  ±0       860 suites  ±0   1h 1m 39s ⏱️ - 12m 28s
  1 908 tests ±0    1 760 ✔️ ±0  148 💤 ±0  0 ±0 
10 349 runs  ±0  10 143 ✔️ ±0  206 💤 ±0  0 ±0 

Results for commit 338ba37. ± Comparison against base commit df66029.

@zah zah merged commit 5dcfb0c into unstable Jul 21, 2022
@zah zah deleted the dev/etan/fc-checksigs branch July 21, 2022 11:04
zah added a commit that referenced this pull request Jul 27, 2022
etan-status added a commit that referenced this pull request Oct 31, 2022
Revisit #3893 using method based on Lighthouse (less heavy computation).
zah pushed a commit that referenced this pull request Nov 1, 2022
* avoid packing attestations from other forks

Revisit #3893 using method based on Lighthouse (less heavy computation).

* fix comment
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