-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(validator): session signatures storage #428
base: master
Are you sure you want to change the base?
Conversation
24eeec1
to
9d624fb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #428 +/- ##
==========================================
- Coverage 43.03% 43.00% -0.03%
==========================================
Files 262 262
Lines 45556 45554 -2
Branches 45556 45554 -2
==========================================
- Hits 19603 19591 -12
- Misses 24898 24905 +7
- Partials 1055 1058 +3 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
9d624fb
to
c2051a4
Compare
d15519c
to
161a241
Compare
161a241
to
e0f45af
Compare
.build(block_id, state.weight_threshold); | ||
let block_signatures = { | ||
match &cached { | ||
Some(cached) => self.reuse_signatures(block_id, cached.clone()).await, |
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.
entry
lives across the await
here, it can deadlock
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.
refactored
let cached = state | ||
.cached_signatures | ||
.peek(&block_id.seqno, &scc::ebr::Guard::new()) | ||
.map(Arc::clone); | ||
.remove(&block_id.seqno) |
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.
We were intentionally overlapping cached_signatures
item lifetime with block signatures lifetime. You can't delete a value here since it can miss some signatures
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.
refactored
1a6b32e
to
e5c8645
Compare
e5c8645
to
34cc696
Compare
No description provided.