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

increase after-block attestation delay #3518

Merged
merged 1 commit into from
Mar 18, 2022
Merged

Conversation

arnetheduck
Copy link
Member

Recently, block processing times have been going up as the network grows
making early attestation riskier. Since blocks are big and attestations
are small (though numerous and therefore bandwidth-intense), it seems
better to wait a little bit longer after receiving a block, before we
publish the attestation.

Recently, block processing times have been going up as the network grows
making early attestation riskier. Since blocks are big and attestations
are small (though numerous and therefore bandwidth-intense), it seems
better to wait a little bit longer after receiving a block, before we
publish the attestation.
@arnetheduck arnetheduck enabled auto-merge (squash) March 18, 2022 08:19
@arnetheduck arnetheduck merged commit 8395f7d into unstable Mar 18, 2022
@arnetheduck arnetheduck deleted the inc-attest-delay branch March 18, 2022 11:02
@github-actions
Copy link

Unit Test Results

     12 files  ±0     834 suites  ±0   37m 49s ⏱️ +6s
1 686 tests ±0  1 637 ✔️ ±0    49 💤 ±0  0 ±0 
9 825 runs  ±0  9 709 ✔️ ±0  116 💤 ±0  0 ±0 

Results for commit fb9e3d3. ± Comparison against base commit 12dc427.

# Regardless, because we "just" received the block, we'll impose the
# delay.

const afterBlockDelay = 1000
const afterBlockDelay = millis(2000)
Copy link
Contributor

@etan-status etan-status Mar 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should probably be scaled with SECONDS_PER_SLOT, or take into account the attestation_deadline. But those constants seem to be untouched on most networks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh, that's a good point actually .. gnosis runs a different slot time

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the deadline would be used on those networks as an upper limit, so the impact of the change is .. limited .. but still

Copy link
Contributor

@dapplion dapplion Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But those constants seem to be untouched on most networks.

Gnosis chain changes SECONDS_PER_SLOT

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.

4 participants