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

Ignore aggregate if falling behind #9190

Merged
merged 2 commits into from
Mar 3, 2025
Merged

Conversation

rolfyone
Copy link
Contributor

@rolfyone rolfyone commented Mar 3, 2025

Currently if we're falling behind and receive an aggregate we attempt to validate it but our state is too old in some circumstances.

We can't validate this aggregate, but currently we're rejecting it, which is worse than ignoring it for gossip rules, so we're better off just ignoring it.

fixes #9187

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

rolfyone added 2 commits March 3, 2025 11:16
Currently if we're falling behind and receive an aggregate we attempt to validate it but our state is too old in some circumstances.

We can't validate this aggregate, but currently we're rejecting it, which is worse than ignoring it for gossip rules, so we're better off just ignoring it.

fixes Consensys#9187

Signed-off-by: Paul Harris <[email protected]>
Signed-off-by: Paul Harris <[email protected]>
@@ -174,14 +176,18 @@ SafeFuture<InternalValidationResultWithState> singleOrAggregateAttestationChecks

if (!attestation.isSingleAttestation()) {
// [REJECT] The number of aggregation bits matches the committee size
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd take the opportunity here to move the comment to the if check so it continues to matche what the code is doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but this whole block only does that... i was just making it not go boom... not sure it'll be v. clear if its down 2 levels?

@rolfyone rolfyone merged commit e889eaa into Consensys:master Mar 3, 2025
16 checks passed
@rolfyone rolfyone deleted the noisy-miners branch March 3, 2025 04:14
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.

[HOLESKY PECTRA] noisy stacktrace message
2 participants