fix: 2 blocks dead time between 2.5 -> 3.0 transition

[jbencin]

Description

Fix 2 block downtime by passing LeaderKeyRegistrationState from Neon runloop to Nakamoto runloop

Applicable issues

• fixes Nakamoto Miner[3.0] - Fix the 2 blocks dead time between 2.5 -> 3.0 transition #4710

Additional info (benefits, drawbacks, caveats)

Checklist

• Test coverage for new or modified code paths
• Changelog is updated
• Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
• New clarity functions have corresponding PR in clarity-benchmarking repo
• New integration test(s) added to bitcoin-tests.yml

[codecov]

Codecov Report

Attention: Patch coverage is 34.48276% with 38 lines in your changes missing coverage. Please review.

Project coverage is 81.19%. Comparing base (0256637) to head (376ed5a).
Report is 44 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #4883       +/-   ##
============================================
+ Coverage    25.92%   81.19%   +55.26%     
============================================
  Files          482      490        +8     
  Lines       347616   351570     +3954     
  Branches         0      323      +323     
============================================
+ Hits         90134   285460   +195326     
+ Misses      257482    66102   -191380     
- Partials         0        8        +8     

Files	Coverage Δ
stackslib/src/chainstate/stacks/miner.rs	81.08% <100.00%> (+15.99%)	⬆️
testnet/stacks-node/src/globals.rs	65.75% <100.00%> (-7.01%)	⬇️
testnet/stacks-node/src/nakamoto_node/relayer.rs	0.00% <ø> (-82.45%)	⬇️
testnet/stacks-node/src/node.rs	88.36% <100.00%> (+79.70%)	⬆️
testnet/stacks-node/src/run_loop/mod.rs	99.18% <ø> (+63.41%)	⬆️
testnet/stacks-node/src/neon_node.rs	81.91% <90.00%> (+4.75%)	⬆️
testnet/stacks-node/src/run_loop/nakamoto.rs	0.00% <0.00%> (-86.68%)	⬇️
testnet/stacks-node/src/run_loop/neon.rs	78.97% <28.57%> (-3.38%)	⬇️
testnet/stacks-node/src/nakamoto_node.rs	0.00% <0.00%> (-88.76%)	⬇️
testnet/stacks-node/src/run_loop/boot_nakamoto.rs	59.18% <5.88%> (-24.15%)	⬇️

... and 436 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 9254679...376ed5a. Read the comment docs.

[jbencin]

Passing leader_key_registration_state into the Nakamoto run loop may not be the best approach. It only works in one of 3 possible scenarios:

• ✔️ Neon and Nakamoto miner using same private key
• ❌ Neon and Nakamoto miner using different private key
• ❌ Neon miner using no private key

Maybe almost all miners are doing the first, so the other two don't really matter? Or maybe I just need to explicitly register the VRF pubkey with the Nakamoto miner pubkey hash before Epoch 3.0

EDIT: Added a warning for third case. Second case probably has negligible real-world usage. So I think this is okay as-is

[obycode]

The tests in testnet/stacks-node/src/tests/nakamoto_integrations.rs are going to need to be updated, as most (all?) of them expect to wait for this key registration. It would be great to also add a case to test for your scenarios 1, 2, and 3 in there.

[jbencin]

Somehow this broke the Nakamoto integration tests. They timing out on next_block_and():

called `Result::unwrap()` on an `Err` value: "Timed out"

Not sure how these changes broke bitcoin block production, but looking into it now

[kantai]

@jbencin -- this patch fixed nakamoto_integrations::simple_neon_integration:

diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs
index c630776d5..22cf8cd3c 100644
--- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs
+++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs
@@ -1133,14 +1133,7 @@ fn simple_neon_integration() {
     info!("Nakamoto miner started...");
     blind_signer(&naka_conf, &signers, proposals_submitted);
 
-    // first block wakes up the run loop, wait until a key registration has been submitted.
-    next_block_and(&mut btc_regtest_controller, 60, || {
-        let vrf_count = vrfs_submitted.load(Ordering::SeqCst);
-        Ok(vrf_count >= 1)
-    })
-    .unwrap();
-
-    // second block should confirm the VRF register, wait until a block commit is submitted
+    // first block should allow miner to submit a block commit, wait until a block commit is submitted
     next_block_and(&mut btc_regtest_controller, 60, || {
         let commits_count = commits_submitted.load(Ordering::SeqCst);
         Ok(commits_count >= 1)

My guess is a similar patch applied to the other tests would fix things with those.

[jbencin]

I've fixed all the integration tests except for signer::v1, but those are failing on develop as well right now, so I don't think I broke them here

[kantai]

LGTM!

[obycode]

LGTM!

[blockstack-devops]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.