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

Optimize proposer attestation inclusion #4507

Closed
terencechain opened this issue Jan 12, 2020 · 6 comments · Fixed by #7267
Closed

Optimize proposer attestation inclusion #4507

terencechain opened this issue Jan 12, 2020 · 6 comments · Fixed by #7267
Assignees
Labels
Priority: High High priority item

Comments

@terencechain
Copy link
Member

terencechain commented Jan 12, 2020

As specified in the spec:
The attestations added must satisfy the verification conditions found in attestation processing. To maximize profit, the validator should attempt to gather aggregate attestations that include singular attestations from the largest number of validators whose signatures from the same epoch have not previously been added on chain.

This has became a bottleneck in RPC get block request to verify all the candidate attestations signature before including them into the block. Opening this issue for investigation

In v1.0 of the spec on verifying attestation signature, attestation signature is verified via bls.FastAggregateVerify, I can imagine that's going to be whole lot faster than what we currently have. I suggest to resolve our current testnet, given complete validator honesty assumption. We can remove signature verification for attestation inclusion for block

@prestonvanloon
Copy link
Member

Assigning to @farazdagi as Q2 OKR

@prestonvanloon
Copy link
Member

Related: #5980

@rauljordan
Copy link
Contributor

Can we close this one @farazdagi ?

@farazdagi
Copy link
Contributor

Not yet, I will re-visit attestations at the end of the week (I have semi-ready branch, that optimizes attestation selection by proposer), and file one more PR, to wrap this functionality up.

@rauljordan
Copy link
Contributor

Update on this @farazdagi ?

@farazdagi
Copy link
Contributor

Sorry for the delay, got stuck with next round of updates to init-sync. Will produce an update on this, right after I get my peer scoring related PRs merged (should be next couple of days).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High High priority item
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants