-
Notifications
You must be signed in to change notification settings - Fork 210
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
[Merged by Bors] - use atx grading for block proposal #4717
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4717 +/- ##
=======================================
Coverage 77.1% 77.2%
=======================================
Files 255 257 +2
Lines 28886 28975 +89
=======================================
+ Hits 22294 22371 +77
- Misses 5200 5211 +11
- Partials 1392 1393 +1
|
"github.com/spacemeshos/go-spacemesh/sql/certificates" | ||
) | ||
|
||
func ActiveSetFromBlock(db sql.Executor, epoch types.EpochID) ([]types.ATXID, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the naming doesn't fit. It generates the ActiveSet from the first block in the provided epoch. Maybe EpochActiveSet
or similar is a better name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EpochActiveSet means many things. it can mean the active set is derived from
- hare active set (determined by hare rules)
- tortoise active set (all epoch atx ids)
- block active set (derived from block)
- fallback (overwritten manually)
i think the naming fits here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is confusing to me because instead of passing in a Block ID, I have to pass in an Epoch ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess i can rename it to ActiveSetFromFirstBlockInEpoch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I cannot think of a better name and you already stated that EpochActiveSet
would be confusing for other reasons.
"github.com/spacemeshos/go-spacemesh/sql/certificates" | ||
) | ||
|
||
func ActiveSetFromBlock(db sql.Executor, epoch types.EpochID) ([]types.ATXID, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to have a dedicated test for this function? It is indirectly covered by other tests, but maybe crucial enough to have a few test cases directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
bors merge |
bors merge |
1 similar comment
bors merge |
## Motivation <!-- Please mention the issue fixed by this PR or detailed motivation --> part of #4089 <!-- `Closes #XXXX, closes #XXXX, ...` links mentioned issues to this PR and automatically closes them when this it's merged --> ## Changes - grade atxs as described in https://community.spacemesh.io/t/grading-atxs-for-the-active-set/335, let s be the start of the epoch, and δ the network propagation time. - grade 0/evil: ATX was received at time t >= s-3δ, or an equivocation proof was received by time s-δ. - grade 1/acceptable: ATX was received at time t < s-3δ before the start of the epoch, and no equivocation proof was received by time s-δ. - grade 2/good: ATX was received at time t < s-4δ, and no equivocation proof was received for that id until time s. - miner only include grade 2/good ATXs in its active set for its first ballot of the epoch - newly sync'ed node uses the first block of epoch for active set in ref ballot, since it doesn't have accurate received timestamp for atxs or malfeasance proof
Pull request successfully merged into develop. Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Motivation
part of #4089
Changes
grade atxs as described in https://community.spacemesh.io/t/grading-atxs-for-the-active-set/335, let s be the start of the epoch, and δ the network propagation time.
miner only include grade 2/good ATXs in its active set for its first ballot of the epoch
newly sync'ed node uses the first block of epoch for active set in ref ballot, since it doesn't have accurate received timestamp for atxs or malfeasance proof