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] - Sync malfeasance proofs continuously #5718

Closed
wants to merge 18 commits into from

Conversation

ivan4th
Copy link
Contributor

@ivan4th ivan4th commented Mar 16, 2024

Motivation

Need to sync malfeasance proofs continuously to facilitate distributed verification. See #5306

Description

Implemented the proposed protocol from #5306 (comment)

proposed protocol:

  • on node startup check how long ago proofs were synced, if it was in a previous epoch or a quarter of epoch ago download all malfeasance identities and download missing proofs synchronously from N peers. N can be 2-3, just to avoid downloading data from not very well synced node
  • otherwise spin up a background loop to download malfeasance identities every M minutes from N peers. and then sync missing malfeasance proofs for those identities. the interval and number of peers can be parametrized, but should be chosen such that overhead is negligible.

for example, we have ~600 malfeasance/canceled identities, if this number grows to 3000 then every query will have to download 10KB of data. if we ask 10 peers every 30 minutes (not concurrently), thats 55 bytes per second.

For now malfeasance proof IDs are requested simultaneously from several peers, should the number of malfeasant identities grow substantially, we can make these requests sequential.

Test Plan

Verified on mainnet. Checked sync during startup and also by deleting malfeasance from db while go-spacemesh is running to make sure they're restored during resync.

TODO

  • Explain motivation or link existing issue(s)
  • Test changes and document test plan
  • Update changelog as needed

@ivan4th ivan4th changed the title Feature/better malfeasance sync Contui Mar 16, 2024
@ivan4th ivan4th changed the title Contui Sync malfeasance proofs continuously Mar 16, 2024
Copy link

codecov bot commented Mar 16, 2024

Codecov Report

Attention: Patch coverage is 85.44304% with 46 lines in your changes are missing coverage. Please review.

Project coverage is 80.1%. Comparing base (3649c2d) to head (afefcf7).
Report is 2 commits behind head on develop.

Files Patch % Lines
syncer/malsync/syncer.go 87.0% 19 Missing and 13 partials ⚠️
sql/malsync/malsync.go 76.0% 3 Missing and 3 partials ⚠️
syncer/syncer.go 80.9% 3 Missing and 1 partial ⚠️
checkpoint/recovery.go 50.0% 1 Missing and 1 partial ⚠️
fetch/mesh_data.go 80.0% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #5718    +/-   ##
========================================
  Coverage     80.1%   80.1%            
========================================
  Files          283     285     +2     
  Lines        29410   29668   +258     
========================================
+ Hits         23575   23784   +209     
- Misses        4209    4243    +34     
- Partials      1626    1641    +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ivan4th
Copy link
Contributor Author

ivan4th commented Mar 17, 2024

bors try

spacemesh-bors bot added a commit that referenced this pull request Mar 17, 2024
@spacemesh-bors
Copy link

try

Build failed:

@ivan4th
Copy link
Contributor Author

ivan4th commented Mar 18, 2024

bors try

spacemesh-bors bot added a commit that referenced this pull request Mar 18, 2024
@spacemesh-bors
Copy link

try

Build failed:

@ivan4th
Copy link
Contributor Author

ivan4th commented Mar 19, 2024

bors try

spacemesh-bors bot added a commit that referenced this pull request Mar 19, 2024
@spacemesh-bors
Copy link

try

Build succeeded:

Copy link
Contributor

@dshulyak dshulyak left a comment

Choose a reason for hiding this comment

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

it would be also better to clear malfeasence sync timestamp when loading checkpoint

syncer/syncer.go Outdated
@@ -267,6 +273,20 @@ func (s *Syncer) Start() {
}
}
})
if !s.cfg.Standalone {
s.eg.Go(func() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

i think it will be better if background jobs were spawned in the same place. in the syncAtx method we do synchronous requests and then spawn background job to check atxs from peers, maybe spawn background job for malfeasance sync there as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@ivan4th
Copy link
Contributor Author

ivan4th commented Mar 20, 2024

it would be also better to clear malfeasence sync timestamp when loading checkpoint

done

@ivan4th ivan4th force-pushed the feature/better-malfeasance-sync branch from 97f8228 to b356f1f Compare March 21, 2024 20:45
@ivan4th
Copy link
Contributor Author

ivan4th commented Mar 22, 2024

bors merge

@spacemesh-bors
Copy link

Canceled.

@ivan4th
Copy link
Contributor Author

ivan4th commented Mar 22, 2024

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Mar 22, 2024
## Motivation

Need to sync malfeasance proofs continuously to facilitate distributed verification. See #5306
@spacemesh-bors
Copy link

Pull request successfully merged into develop.

Build succeeded:

@spacemesh-bors spacemesh-bors bot changed the title Sync malfeasance proofs continuously [Merged by Bors] - Sync malfeasance proofs continuously Mar 22, 2024
@spacemesh-bors spacemesh-bors bot closed this Mar 22, 2024
@spacemesh-bors spacemesh-bors bot deleted the feature/better-malfeasance-sync branch March 22, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants