Skip to content

Add StateMachineUpdateContent::V1 which inculdes a vector of replay_transactions #6048

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

Conversation

jferrant
Copy link
Contributor

Pulled the state machine version upgrade to a sep PR for replay transactions.

@jferrant jferrant requested a review from a team as a code owner April 29, 2025 16:13
@jferrant jferrant requested review from hstove, kantai and obycode and removed request for a team April 29, 2025 16:13
@wileyj wileyj moved this to Status: 💻 In Progress in Stacks Core Eng Apr 30, 2025
… into feat/signer-sends-replay-set-in-update
@jferrant jferrant moved this from Status: 💻 In Progress to Status: In Review in Stacks Core Eng Apr 30, 2025
… into feat/signer-sends-replay-set-in-update
obycode
obycode previously approved these changes Apr 30, 2025
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@kantai kantai left a comment

Choose a reason for hiding this comment

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

This LGTM, but I think this should have some more test coverage:

  1. The state_machine_update parsing in the integration test should be explicit about which version it expects.
  2. If possible, it'd be good to test the protocol version switch over. i.e., pin the protocol version at "0" for 3 out of 5 signers, then assert that all the signers use V0 when submitting state machine updates. Then unpin 2 of the signers so that they're at version "1", and assert that 4 of the signers switch to using V1.

@jferrant jferrant requested a review from kantai May 1, 2025 16:57
jferrant added 3 commits May 1, 2025 10:20
…r state machine update version number

Signed-off-by: Jacinta Ferrant <[email protected]>
…grading signer protocol versions

Signed-off-by: Jacinta Ferrant <[email protected]>
… into feat/signer-sends-replay-set-in-update
Copy link
Contributor

@kantai kantai 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 a couple superficial comments

obycode
obycode previously approved these changes May 2, 2025
@hstove hstove added this to the 3.1.0.0.9 milestone May 2, 2025
@jferrant jferrant requested review from kantai and obycode May 2, 2025 15:55
@github-project-automation github-project-automation bot moved this from Status: In Review to Status: 💻 In Progress in Stacks Core Eng May 2, 2025
@obycode obycode added this pull request to the merge queue May 2, 2025
Merged via the queue into stacks-network:develop with commit 4a7b908 May 2, 2025
205 of 208 checks passed
@github-project-automation github-project-automation bot moved this from Status: 💻 In Progress to Status: ✅ Done in Stacks Core Eng May 2, 2025
Copy link

codecov bot commented May 2, 2025

Codecov Report

Attention: Patch coverage is 26.78133% with 298 lines in your changes missing coverage. Please review.

Project coverage is 53.94%. Comparing base (032d33e) to head (dc5982c).
Report is 22 commits behind head on develop.

Files with missing lines Patch % Lines
testnet/stacks-node/src/tests/signer/v0.rs 0.61% 161 Missing ⚠️
libsigner/src/v0/messages.rs 21.05% 75 Missing ⚠️
stacks-signer/src/v0/signer_state.rs 60.00% 48 Missing ⚠️
stacks-signer/src/tests/signer_state.rs 10.00% 9 Missing ⚠️
stackslib/src/chainstate/stacks/mod.rs 0.00% 3 Missing ⚠️
stacks-signer/src/v0/tests.rs 80.00% 2 Missing ⚠️

❌ Your project status has failed because the head coverage (53.94%) 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    #6048       +/-   ##
============================================
+ Coverage    36.40%   53.94%   +17.53%     
============================================
  Files            5      538      +533     
  Lines         1269   387714   +386445     
  Branches       323      323               
============================================
+ Hits           462   209149   +208687     
- Misses         799   178557   +177758     
  Partials         8        8               
Files with missing lines Coverage Δ
stacks-signer/src/v0/signer.rs 76.66% <100.00%> (ø)
stacks-signer/src/v0/tests.rs 87.64% <80.00%> (ø)
stackslib/src/chainstate/stacks/mod.rs 71.66% <0.00%> (ø)
stacks-signer/src/tests/signer_state.rs 28.61% <10.00%> (ø)
stacks-signer/src/v0/signer_state.rs 80.99% <60.00%> (ø)
libsigner/src/v0/messages.rs 54.82% <21.05%> (ø)
testnet/stacks-node/src/tests/signer/v0.rs 29.03% <0.61%> (ø)

... and 526 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 a5b5137...dc5982c. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants