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.

Sign up for GitHub

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

Jump to bottom

Fix/5642 #5655

Open
wants to merge 34 commits into
base: develop
Choose a base branch
from
Open

Fix/5642 #5655

wants to merge 34 commits into from
+273 −59

Conversation

jcnelson
Copy link
Member

@jcnelson jcnelson commented Jan 4, 2025 •
edited
Loading

This fixes #5642 by adding a dedicated IBD inference rule for Nakamoto. The Stacks node is in IBD mode when either of the following conditions are true:

  • The sortition height is less than the Bitcoin block height
  • The highest available tenure (as determined by the Nakamoto block downloader) is higher than the ongoing Stacks tenure

This (hopefully) fixes some edge cases we've seen on testnet whereby a node can erroneously believe it is not sync'ed when it really is. This had impacted the affected node's ability to participate in StackerDB replication. I intend to test this on naka3.sh, on testnet, and on a single mainnet signer.

Writing the code for that second criterion led to the discoveries of #5649 and #5650, since I had been using rc_consensus_hash as the ongoing Stacks tenure when in fact it was not.

EDIT: this also contains #5667, so let's merge that first.

jcnelson added 19 commits January 3, 2025 15:03
@jcnelson
feat: add a way to query the highest available tenure
cedbfd6
@jcnelson
chore: improve documentation on downloader state
8e283e6
@jcnelson
chore: report highest available tenure from downloader via NetworkResult
93fdd22
@jcnelson
chore: pass through highest available tenure
b90d5d5
@jcnelson
chore: API sync
2429f50
@jcnelson
feat: add way to set IBD
2b45248
@jcnelson
feat: infer IBD from burnchain IBD and stacks IBD
6979a64
@jcnelson
fix: load IBD from globals
9331e25
@jcnelson
chore: document pox_sync_wait() better
6d39033
@jcnelson
Merge branch 'develop' into fix/5642
c5f7af5
@jcnelson
fix: immediately compute highest-available tenure since available_ten…
b8011da 
…ures is mutated over the course of the state machine's lifetime
@jcnelson
chore: pass ongoing stacks tenure ID to NetworkResult
e1feabe
@jcnelson
chore: pass highest available tenure from downloader to NetworkResult
fb61989
@jcnelson
chore: API sync
4102306
@jcnelson
docs: get_headers_height() is 1-indexed
5049ee4
@jcnelson
fix: the highest available tenure may be lower than the ongoing stack…
4506b46 
…s tenure, so check this when inferring IBD
@jcnelson
chore: make method private again
6dabc04
@jcnelson
chore: expand follower_bootup_simple() to test is_fully_synced flag i…
4474f2d 
…n /v2/info
@jcnelson
Merge branch 'develop' into fix/5642
84658a4
@jcnelson jcnelson requested a review from a team as a code owner January 4, 2025 05:38
obycode
obycode previously approved these changes Jan 6, 2025
View reviewed changes
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!

jbencin
jbencin previously approved these changes Jan 6, 2025
View reviewed changes
Copy link
Contributor

@jbencin jbencin left a comment

Choose a reason for hiding this comment

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

Left some style comments but looks fine to me

@aldur aldur added this to the 3.1.0.0.3 milestone Jan 7, 2025
jcnelson added 10 commits January 7, 2025 17:16
@jcnelson
chore: more structured logging on why a block proposal is rejected
bd254b7
@jcnelson
chore: log ibd in debug mode
f632795
@jcnelson
feat: move test flags for stalling and skipping stacks block announce…
83aee92 
…s to net::relay since we need to be able to signal to the relayer to _not_ process Stacks blocks
@jcnelson
chore: remove unused ibd variable and don't announce stacks blocks if…
ed37e67 
… instructed not to by fault injection
@jcnelson
chore: use relayer fault injection logic
b53c4c6
@jcnelson
chore: don't announce a stacks block if fault injection is active
08d3028
@jcnelson
chore: log if we're still sync'ing
b5b6038
@jcnelson
chore: don't set ibd based on burnchain block download
0f42b62
@jcnelson
fix: don't allow tenure b to process until after we've mined the bloc…
315c4d8 
…k for tenure c
@jcnelson
Merge branch 'fix/forked-tenure-okay-ci-test' into fix/5642
bdc6b82
@jcnelson jcnelson dismissed stale reviews from jbencin and obycode via bdc6b82 January 7, 2025 22:23
@jcnelson jcnelson requested a review from a team as a code owner January 7, 2025 22:23
@kantai kantai mentioned this pull request Jan 9, 2025
Open
@jcnelson jcnelson modified the milestones: 3.1.0.0.4, 3.1.0.0.3 Jan 13, 2025
@aldur aldur modified the milestones: 3.1.0.0.3, 3.1.0.0.4 Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbencin jbencin jbencin left review comments

@obycode obycode obycode left review comments

@jferrant jferrant Awaiting requested review from jferrant

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
Stacks Core Eng
Status: Status: In Review
Milestone
3.1.0.0.4
Development

Successfully merging this pull request may close these issues.
4 participants
@jcnelson @obycode @jbencin @aldur