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: start of ability for miner to keep track of active signers bitvec #4627

Merged
merged 6 commits into from
Apr 24, 2024

Conversation

hstove
Copy link
Contributor

@hstove hstove commented Apr 2, 2024

This PR adds two things:

  • When a new Nakamoto block is formed, it starts with a BitVec of 1's with the length of the signer set.
  • In the miner's coordinator thread, a next_signer_bitvec property is added. After a miner has collected all block responses from the signers, it updates this BitVec to match the signers who gave a response.

Note that, from a functional perspective, all this changes is that the length of the BitVec will match the signer's length. However, the BitVec will be all 1's, even if not all signers respond. Future work is required to construct a new block in the case that a block is rejected, and in that work we can use this next_signer_bitvec in that new block.

@hstove hstove linked an issue Apr 2, 2024 that may be closed by this pull request
@hstove hstove changed the title feat: start of ability for miner to keep track of active signers feat: start of ability for miner to keep track of active signers bitvec Apr 3, 2024
kantai
kantai previously approved these changes Apr 3, 2024
jcnelson
jcnelson previously approved these changes Apr 3, 2024
@hstove hstove dismissed stale reviews from jcnelson and kantai via 222d766 April 3, 2024 18:17
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

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

Project coverage is 81.27%. Comparing base (c383bdd) to head (222d766).
Report is 2 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #4627      +/-   ##
==========================================
- Coverage   83.15%   81.27%   -1.89%     
==========================================
  Files         470      465       -5     
  Lines      332698   331526    -1172     
  Branches      317        0     -317     
==========================================
- Hits       276663   269444    -7219     
- Misses      56027    62082    +6055     
+ Partials        8        0       -8     
Files Coverage Δ
stacks-common/src/bitvec.rs 92.05% <94.11%> (+0.34%) ⬆️
stackslib/src/chainstate/nakamoto/mod.rs 81.38% <0.00%> (-1.41%) ⬇️
testnet/stacks-node/src/nakamoto_node/miner.rs 0.00% <0.00%> (-83.47%) ⬇️
.../stacks-node/src/nakamoto_node/sign_coordinator.rs 0.00% <0.00%> (-77.09%) ⬇️

... and 79 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c383bdd...222d766. Read the comment docs.

kantai
kantai previously approved these changes Apr 3, 2024
jcnelson
jcnelson previously approved these changes Apr 4, 2024
jferrant
jferrant previously approved these changes Apr 5, 2024
@hstove hstove dismissed stale reviews from jferrant, jcnelson, and kantai via d26e5a6 April 8, 2024 15:14
@hstove
Copy link
Contributor Author

hstove commented Apr 8, 2024

Updated to add an error log based on a comment from Jude in d26e5a6. Requesting re-reviews

jferrant
jferrant previously approved these changes Apr 9, 2024
jcnelson
jcnelson previously approved these changes Apr 9, 2024
@hstove
Copy link
Contributor Author

hstove commented Apr 10, 2024

FYI - I fixed an issue with how the block was formed by the miner, which was breaking integration tests. I've also opened #4669 , which is the same code but off of develop. I'm happy to close this, but wanted to double check if that's the right move.

@hstove hstove requested a review from jcnelson April 15, 2024 18:08
@hstove hstove changed the base branch from next to develop April 19, 2024 13:37
@hstove
Copy link
Contributor Author

hstove commented Apr 19, 2024

This PR is now based off of develop. Would love re-reviews please 😄

@hstove hstove removed the request for review from jcnelson April 24, 2024 14:41
@hstove hstove merged commit d56e256 into develop Apr 24, 2024
1 check passed
@hstove hstove deleted the feat/miner-bitvec branch April 24, 2024 21:37
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.

Construct signer_bitvec based on who signed each block
4 participants