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

[Merged by Bors] - ballots: request active set if reference ballot was submitted with empty active set #4956

Closed
wants to merge 16 commits into from

Conversation

dshulyak
Copy link
Contributor

@dshulyak dshulyak commented Sep 4, 2023

closes: #4672

starting from layer X reference ballots will be gossiped with empty active sets.
any updated node will request active set by its hash from the ballot sender, and save it in a separate table. unless it wasn't saved before, in that case we won't download anything.

reference ballot will be stored with full active set, so that sync protocol remains compatible with nodes that didn't upgrade. for them reference ballot without active set will look syntactically invalid. but they will be able to download it later and not get stuck this way.

@dshulyak dshulyak marked this pull request as ready for review September 6, 2023 09:11
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #4956 (b581be0) into develop (430408c) will decrease coverage by 0.1%.
The diff coverage is 76.0%.

@@            Coverage Diff            @@
##           develop   #4956     +/-   ##
=========================================
- Coverage     77.1%   77.1%   -0.1%     
=========================================
  Files          254     255      +1     
  Lines        30159   30266    +107     
=========================================
+ Hits         23281   23341     +60     
- Misses        5373    5412     +39     
- Partials      1505    1513      +8     
Files Changed Coverage Δ
common/types/activation.go 56.7% <ø> (ø)
tortoise/algorithm.go 78.3% <ø> (ø)
miner/proposal_builder.go 80.2% <38.0%> (-3.5%) ⬇️
proposals/handler.go 86.0% <78.9%> (-2.0%) ⬇️
sql/activesets/activesets.go 83.3% <83.3%> (ø)
config/mainnet.go 95.7% <100.0%> (+<0.1%) ⬆️
datastore/store.go 71.3% <100.0%> (+0.2%) ⬆️
fetch/fetch.go 78.1% <100.0%> (+<0.1%) ⬆️
fetch/mesh_data.go 65.0% <100.0%> (+0.4%) ⬆️
node/node.go 63.1% <100.0%> (+<0.1%) ⬆️

... and 6 files with indirect coverage changes

📢 Have feedback on the report? Share it here.

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 6, 2023

bors try

bors bot added a commit that referenced this pull request Sep 6, 2023
@bors
Copy link

bors bot commented Sep 6, 2023

try

Build failed:

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 6, 2023

bors try

bors bot added a commit that referenced this pull request Sep 6, 2023
@bors
Copy link

bors bot commented Sep 6, 2023

try

Build failed:

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 6, 2023

bors try

bors bot added a commit that referenced this pull request Sep 6, 2023
@bors
Copy link

bors bot commented Sep 6, 2023

try

Build failed:

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 6, 2023

bors try

bors bot added a commit that referenced this pull request Sep 6, 2023
@bors
Copy link

bors bot commented Sep 6, 2023

try

Build failed:

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 6, 2023

bors try

bors bot added a commit that referenced this pull request Sep 6, 2023
@bors
Copy link

bors bot commented Sep 6, 2023

try

Build failed:

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 6, 2023

bors try

bors bot added a commit that referenced this pull request Sep 6, 2023
@bors
Copy link

bors bot commented Sep 6, 2023

try

Build failed:

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 6, 2023

bors try

bors bot added a commit that referenced this pull request Sep 6, 2023
@bors
Copy link

bors bot commented Sep 6, 2023

try

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.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 6, 2023

bors try

bors bot added a commit that referenced this pull request Sep 6, 2023
@bors
Copy link

bors bot commented Sep 6, 2023

try

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.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

Copy link
Contributor

@countvonzero countvonzero left a comment

Choose a reason for hiding this comment

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

so after the layer takes effect, the new-version nodes will see blob store not found activeset from old-version nodes. but the old-version nodes will continue to operate fine. this seems to punish miners who upgraded...

sql/migrations/0003_next.sql Show resolved Hide resolved
sql/activesets/activesets.go Outdated Show resolved Hide resolved
sql/activesets/activesets.go Show resolved Hide resolved
proposals/handler.go Outdated Show resolved Hide resolved
proposals/handler.go Outdated Show resolved Hide resolved
miner/proposal_builder.go Show resolved Hide resolved
proposals/handler.go Show resolved Hide resolved
proposals/handler.go Show resolved Hide resolved
proposals/handler.go Show resolved Hide resolved
@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 7, 2023

so after the layer takes effect, the new-version nodes will see blob store not found activeset from old-version nodes. but the old-version nodes will continue to operate fine. this seems to punish miners who upgraded...

they will ask peer that sent this data. unless someone maliciously sends ballot with empty active set then it is ok.

as for dos it makes no sense, it is very simple work (query one key). at the same time you can ask sync for atx ids, which is a range scan

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 7, 2023

bors try

bors bot added a commit that referenced this pull request Sep 7, 2023
@bors
Copy link

bors bot commented Sep 7, 2023

try

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.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 7, 2023

spacemeshos/go-spacemesh-dev:b22e6d9

@@ -139,6 +141,12 @@ func WithMinGoodAtxPct(pct int) Opt {
}
}

func WithEmitEmptyActiveSet(lid types.LayerID) Opt {
Copy link
Contributor

Choose a reason for hiding this comment

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

i still don't see this invoked in node.go. what am i missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh sorry :) i thought you are suggesting to rename it

proposals/handler.go Show resolved Hide resolved
@dshulyak
Copy link
Contributor Author

dshulyak commented Sep 8, 2023

bors merge

bors bot pushed a commit that referenced this pull request Sep 8, 2023
…pty active set (#4956)

closes: #4672

starting from layer X reference ballots will be gossiped with empty active sets.
any updated node will request active set by its hash from the ballot sender, and save it in a separate table. unless  it wasn't saved before, in that case we won't download anything.

reference ballot will be stored with full active set, so that sync protocol remains compatible with nodes that didn't upgrade. for them reference ballot without active set will look syntactically invalid. but they will be able to download it later and not get stuck this way.
@bors
Copy link

bors bot commented Sep 8, 2023

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.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title ballots: request active set if reference ballot was submitted with empty active set [Merged by Bors] - ballots: request active set if reference ballot was submitted with empty active set Sep 8, 2023
@bors bors bot closed this Sep 8, 2023
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.

fetcher, ballots: request active set using active set hash
2 participants