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

Fix atterstation effectivnes #493

Merged
merged 8 commits into from
Feb 10, 2021

Conversation

BeroBurny
Copy link
Collaborator

closes #485

@BeroBurny BeroBurny self-assigned this Feb 9, 2021
src/renderer/ducks/validator/actions.ts Outdated Show resolved Hide resolved
payload.block === toHex(data.beaconBlockRoot) &&
data.index === payload.index &&
data.slot === payload.slot &&
payload.bits === JSON.stringify(aggregationBits),
Copy link
Member

Choose a reason for hiding this comment

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

You need to check if bit on validator position in committee is set to true. I think you should also check source and target root.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

source and target contains epoch and root of slot as i check source slot in parent object of source and think checking for source and target can be bit a overkill and add unnecessary complexity

Copy link
Member

Choose a reason for hiding this comment

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

it contains epoch and epoch boundary root (it's basically validator vote) that are unreleated to slot. That said since, you are checking committee index, it's highly unlikely that someone created attestation with different source and target outside of chainguardian (in that case more pressing issue is slashing anyways).

@BeroBurny BeroBurny marked this pull request as ready for review February 10, 2021 07:58
@BeroBurny BeroBurny requested a review from a team as a code owner February 10, 2021 07:58
@BeroBurny BeroBurny requested a review from mpetrunic February 10, 2021 07:58
@codecov
Copy link

codecov bot commented Feb 10, 2021

Codecov Report

Merging #493 (faff647) into master (d12329c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #493   +/-   ##
=======================================
  Coverage   56.98%   56.98%           
=======================================
  Files          73       73           
  Lines        1160     1160           
  Branches      135      135           
=======================================
  Hits          661      661           
  Misses        473      473           
  Partials       26       26           
Flag Coverage Δ
unit 56.98% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d12329c...faff647. Read the comment docs.

@BeroBurny BeroBurny requested a review from mpetrunic February 10, 2021 09:17
@BeroBurny BeroBurny merged commit 23a6deb into master Feb 10, 2021
@BeroBurny BeroBurny deleted the beroburny/fix-atterstation-effectivnes branch February 10, 2021 14:08
@BeroBurny BeroBurny added bug Something isn't working and removed bug Something isn't working labels Feb 18, 2021
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.

Reorganize validator and beacon node tabs
2 participants