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

Single stateroot sender #1953

Merged
merged 8 commits into from
May 7, 2021
Merged

Single stateroot sender #1953

merged 8 commits into from
May 7, 2021

Conversation

roman-khimov
Copy link
Member

Problem

#1922 originally, but some additional fixes were also needed.

We're modifying it here, so there can be a race between this method and
AddSignature().
Makes more sense because it's not just roots that we're relaying.
And don't add/resend it multiple times.

1. We can be in a setup with one SV only and no AddSignature() called at all.
2. AddSignature() might add M-1 signatures and our signature should be the
   last one to complete MPTRoot, but we'll never do that.
_ = time.AfterFunc(delay, func() {
s.sendVote(ir)
})
ir.retries++
Copy link
Contributor

Choose a reason for hiding this comment

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

The mutex is taken on read only.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

ir.isSent = true
ir.Unlock()
}
s.srMtx.Unlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this unlock before the if?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

These are arbitrary numbers, but that's what C# node uses. maxRetries could be
set lower because we have exponential backoff anyway, but this works too.
Drop old incomplete roots from the map.
@roman-khimov roman-khimov force-pushed the single-stateroot-sender branch from a10ee0d to 3957174 Compare May 7, 2021 11:31
@codecov
Copy link

codecov bot commented May 7, 2021

Codecov Report

Merging #1953 (3957174) into master (ba52739) will increase coverage by 0.06%.
The diff coverage is 72.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1953      +/-   ##
==========================================
+ Coverage   82.71%   82.77%   +0.06%     
==========================================
  Files         290      290              
  Lines       23328    23371      +43     
==========================================
+ Hits        19295    19345      +50     
+ Misses       2803     2790      -13     
- Partials     1230     1236       +6     
Impacted Files Coverage Δ
pkg/services/stateroot/signature.go 72.22% <63.63%> (+1.85%) ⬆️
pkg/services/stateroot/service.go 75.80% <66.66%> (-3.04%) ⬇️
pkg/services/stateroot/validators.go 74.35% <67.85%> (-7.31%) ⬇️
pkg/services/stateroot/network.go 81.03% <83.33%> (+7.95%) ⬆️
pkg/network/discovery.go 98.30% <0.00%> (+0.84%) ⬆️
pkg/services/notary/notary.go 92.46% <0.00%> (+1.00%) ⬆️
pkg/services/oracle/request.go 53.23% <0.00%> (+4.31%) ⬆️
pkg/services/oracle/oracle.go 90.90% <0.00%> (+11.36%) ⬆️

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 ba52739...3957174. Read the comment docs.

@roman-khimov roman-khimov merged commit bde4e6a into master May 7, 2021
@roman-khimov roman-khimov deleted the single-stateroot-sender branch May 7, 2021 11:44
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.

2 participants