Skip to content

Conversation

@brice-stacks
Copy link
Contributor

The signer is currently sending an approval for every pre-commit that it receives after reaching the threshold weight. This change just makes sure it only sends its approval when it first reaches that threshold, and then ignores the rest of the pre-commits.

@brice-stacks brice-stacks marked this pull request as draft December 10, 2025 14:31
This allows us to differentiate between a signer that has sent a
pre-commit and that has sent its approval.
@brice-stacks brice-stacks marked this pull request as ready for review December 10, 2025 17:51
@brice-stacks brice-stacks marked this pull request as draft December 10, 2025 21:45
@brice-stacks
Copy link
Contributor Author

It turns out this is not as simple as I hoped.

This allows the signer to differentiate between a block that it has
pre-committed to and one that it has actually signed.

Fixes: stacks-network#6743
@brice-stacks brice-stacks force-pushed the fix/avoid-extra-approvals branch from 5cddcc4 to 143fa6d Compare December 11, 2025 19:47
@codecov
Copy link

codecov bot commented Dec 12, 2025

Codecov Report

❌ Patch coverage is 42.85714% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.63%. Comparing base (c9003aa) to head (a30265c).
⚠️ Report is 25 commits behind head on develop.

Files with missing lines Patch % Lines
stacks-node/src/tests/signer/v0.rs 0.00% 25 Missing ⚠️
stacks-signer/src/signerdb.rs 85.71% 2 Missing ⚠️
stacks-signer/src/v0/signer.rs 85.71% 1 Missing ⚠️

❌ Your project status has failed because the head coverage (77.63%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6750      +/-   ##
===========================================
- Coverage    77.90%   77.63%   -0.27%     
===========================================
  Files          580      580              
  Lines       361187   361131      -56     
===========================================
- Hits        281374   280371    -1003     
- Misses       79813    80760     +947     
Files with missing lines Coverage Δ
stacks-common/src/util/macros.rs 88.54% <ø> (ø)
...tacks-node/src/tests/signer/commands/block_wait.rs 23.76% <ø> (+23.76%) ⬆️
stackslib/src/net/api/postblock_proposal.rs 67.03% <100.00%> (+0.15%) ⬆️
stacks-signer/src/v0/signer.rs 74.03% <85.71%> (-0.57%) ⬇️
stacks-signer/src/signerdb.rs 92.99% <85.71%> (+0.35%) ⬆️
stacks-node/src/tests/signer/v0.rs 16.46% <0.00%> (+1.82%) ⬆️

... and 86 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 c9003aa...a30265c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@brice-stacks brice-stacks marked this pull request as ready for review December 12, 2025 02:46
jacinta-stacks
jacinta-stacks previously approved these changes Dec 12, 2025
@brice-stacks brice-stacks added this pull request to the merge queue Dec 16, 2025
Merged via the queue into stacks-network:develop with commit b1c0de6 Dec 16, 2025
308 of 312 checks passed
@brice-stacks brice-stacks deleted the fix/avoid-extra-approvals branch December 16, 2025 16:08
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.

3 participants