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

Feat: trigger stackerdb refresh when .signers is written #4330

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

kantai
Copy link
Member

@kantai kantai commented Feb 1, 2024

Description

This implements #4328 by adding a signaling bool to the coordinator channels structs. It's a bit cludgier than I would otherwise want, but I think it's the best we can do without majorly refactoring the coordinator's signatures (which is probably a good idea, but one best left to the future).

This is a draft for now, because it depends on feat/larger-stackerdb, but it could be cherry-picked/rebased to next instead.

@kantai kantai force-pushed the feat/signer-trigger-stackerdb branch from 6d8a952 to fb9c54d Compare February 1, 2024 19:42
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7c7a527) 61.25% compared to head (645a931) 76.99%.

Files Patch % Lines
testnet/stacks-node/src/nakamoto_node/peer.rs 92.30% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             next    #4330       +/-   ##
===========================================
+ Coverage   61.25%   76.99%   +15.74%     
===========================================
  Files         445      445               
  Lines      317161   317217       +56     
===========================================
+ Hits       194276   244247    +49971     
+ Misses     122885    72970    -49915     

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

@@ -202,6 +202,21 @@ impl PeerThread {
}
}

fn check_stackerdb_reload(&mut self) {
Copy link
Member

Choose a reason for hiding this comment

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

Minor nit: This is doing a check and then maybe doing some work based on that check. Maybe this should be called stackerdb_refresh() or something?

Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

LGTM; just one minor nit

@jferrant
Copy link
Collaborator

jferrant commented Feb 3, 2024

Thank you for doing this!

@jferrant
Copy link
Collaborator

jferrant commented Feb 5, 2024

Having a really hard time rebasing on this XD (couldn't cherry pick it out)

@kantai kantai changed the base branch from feat/larger-stackerdb to next February 6, 2024 19:53
@kantai kantai force-pushed the feat/signer-trigger-stackerdb branch from fb9c54d to 645a931 Compare February 6, 2024 20:05
@saralab saralab merged commit 4f175f5 into next Feb 7, 2024
2 checks passed
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.

None yet

4 participants