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

Expose whether the node is synced in /v2/info #4847

Merged
merged 7 commits into from
Jun 25, 2024

Conversation

ASuciuX
Copy link
Contributor

@ASuciuX ASuciuX commented Jun 4, 2024

Description

Displays on the API v2/info whether the Stacks node is fully synced or not. I added a check based on the initial download that would specify is_fully_synced as true/false.

Unexpected Resulted Flows

On an M1 Mac, I synced a Nakamoto node from 0. During this process, the block heights fluctuated unpredictably: burn_block_height jumped from 1000 to 4000, then to 5000, back to 2000, and so on. Eventually, it fully synced at burn_block_height, but stacks_tip_height remained at 0. The node intermittently displayed as synced and then not synced. After reaching full sync at burn_block_height, with stacks_tip_height still at 0, it continuously reported is_fully_synced as true.

On Linux, I synced a Nakamoto node on top of what I already had synced from a previous test. The burn_block_height and stacks_tip_height were both in progress at around 9000. The is_fully_synced was always true when running on this.

Steps to reproduce

  1. build from this branch
    cargo build --release --bin stacks-node

  2. nakamoto config - copy config file and uncomment working-dir https://docs.stacks.co/nakamoto-upgrade/signing-and-stacking/sample-configuration-files#nakamoto-testnet-config

  3. run node
    ./stacks-node start --config ./node.toml

  4. create and run script to check

#!/bin/bash

while true; do
  fully=$(curl -s localhost:20443/v2/info | jq .is_fully_synced)
  btc_block=$(curl -s localhost:20443/v2/info | jq .burn_block_height)
  stx_block=$(curl -s localhost:20443/v2/info | jq .stacks_tip_height)

  if [[ "$fully" == "true" ]]; then
    echo "Fully synced at BTC" $btc_block "and STX" $stx_block
  fi

  sleep 1
done

Applicable issues

@ASuciuX ASuciuX self-assigned this Jun 4, 2024
@ASuciuX ASuciuX requested review from a team as code owners June 4, 2024 14:09
@ASuciuX ASuciuX changed the title Expose whether the node is synced in /v2/info Expose whether the node is synced in /v2/info Jun 4, 2024
@wileyj
Copy link
Collaborator

wileyj commented Jun 4, 2024

this reminds of what pavitthra was attempting here: #2768
the solution was a little more challenging that just looking at the data you're checking for - i recall she was looking at the heights of the neighbors she was syncing with.

@hstove
Copy link
Contributor

hstove commented Jun 4, 2024

One kinda cheaty way to do this could be to utilize the STACKS_TIP_HEIGHT_GUAGE prometheus gauge. This is only ever set if the node is fully synced.

The naive approach would be to only support this feature with prometheus_mod enabled, but we could also add a static boolean that is updated in update_stacks_tip_height

@wileyj
Copy link
Collaborator

wileyj commented Jun 4, 2024

One kinda cheaty way to do this could be to utilize the STACKS_TIP_HEIGHT_GUAGE prometheus gauge. This is only ever set if the node is fully synced.

The naive approach would be to only support this feature with prometheus_mod enabled, but we could also add a static boolean that is updated in update_stacks_tip_height

maybe - the challenge that i think pavitthra was trying to solve was that you need to keep track of remote peers alongside your current tip height.
obviously the remote tip height will change as you sync, so it became a little complicated. i only know the basics of what she was trying to accomplish, but it wasn't as simple as you'd like it to be.

it's been a long-running ask of mine for this feature too, i'd love to see it but the reality is that it's a difficult problem to solve correctly (same way progress bars in theory are simple, but for accuracy they become quite complex).

@ASuciuX ASuciuX linked an issue Jun 5, 2024 that may be closed by this pull request
@ASuciuX ASuciuX closed this Jun 6, 2024
@ASuciuX ASuciuX force-pushed the feat/expose-node-is-synced branch from 7ba6917 to aede203 Compare June 6, 2024 19:22
@ASuciuX
Copy link
Contributor Author

ASuciuX commented Jun 6, 2024

I removed my previous commits that weren't on the right track for the feature, I will open the PR with the new commit.

@ASuciuX ASuciuX reopened this Jun 7, 2024
@smcclellan smcclellan requested a review from jbencin June 7, 2024 14:00
@ASuciuX ASuciuX closed this Jun 10, 2024
@ASuciuX ASuciuX force-pushed the feat/expose-node-is-synced branch from 44d9aed to 5645949 Compare June 10, 2024 14:06
@ASuciuX ASuciuX reopened this Jun 11, 2024
@ASuciuX
Copy link
Contributor Author

ASuciuX commented Jun 11, 2024

Results

Once Bitcoin blocks are synchronised, the IBD state is set to false, causing the is_fully_synced variable to become true, even though the Stacks (STX) blocks are not yet synced.


Initial State

Initially, is_fully_synced was false after the bitcoin blocks were synchronised. However, after stopping the node (due to the STX blocks not syncing), is_fully_synced changed to true, despite the STX blocks still being unsynchronised:

{
  "peer_version": 4207599114,
  "pox_consensus": "74276adcd7b47e0a4450be98646ce22b5037b95f",
  "burn_block_height": 12868,
  "stable_pox_consensus": "1973494adb7951084506f567b327816ef8a23be1",
  "stable_burn_block_height": 12867,
  "server_version": "stacks-node 0.0.1 (:, debug build, macos [aarch64])",
  "network_id": 2147483648,
  "parent_network_id": 3669344250,
  "stacks_tip_height": 0,
  "stacks_tip": "0000000000000000000000000000000000000000000000000000000000000000",
  "stacks_tip_consensus_hash": "0000000000000000000000000000000000000000",
  "genesis_chainstate_hash": "74237aa39aa50a83de11a4f53e9d3bb7d43461d1de9873f402e5453ae60bc59b",
  "unanchored_tip": null,
  "unanchored_seq": null,
  "exit_at_block_height": null,
  "is_fully_synced": true,
  "node_public_key": "027e9f66abcf6f1b49dd1fdbed860565f50047f2828b751aabaedf80254893e036",
  "node_public_key_hash": "09d51263985862fd40f384ab6c1c519228f89d27",
  "affirmations": {
    "heaviest": "nnnnnnnnnpppp",
    "stacks_tip": "",
    "sortition_tip": "nnnnnnnnnaaaaa",
    "tentative_best": "nnnnnnnnnppppa"
  },
  "last_pox_anchor": {
    "anchor_block_hash": "686f07297258943d6eaa117060ac9db1b4fdec56710c0ec7075d0940c7406461",
    "anchor_block_txid": "fdf6d6b620dc44c721419ad66de2938ad215149794bfb4853c2ef96d1cd9c4a0"
  },
  "stackerdbs": []
}

Output on a New Block

  1. Syncing from Genesis:
Fully synced at BTC 12868 and STX 0
Fully synced at BTC 12868 and STX 0
Fully synced at BTC 12868 and STX 0
Fully synced at BTC 12868 and STX 0
Fully synced at BTC 9432 and STX 0
Fully synced at BTC 10949 and STX 0
Fully synced at BTC 12529 and STX 0
Fully synced at BTC 12869 and STX 0
Fully synced at BTC 12869 and STX 0
Fully synced at BTC 12869 and STX 0
Fully synced at BTC 12869 and STX 0
  1. Syncing from Archive:
Fully synced at BTC 12871 and STX 4337
Fully synced at BTC 12871 and STX 4357
Fully synced at BTC 12871 and STX 4385
Fully synced at BTC 12871 and STX 4385
Fully synced at BTC 12871 and STX 4385
Fully synced at BTC 12871 and STX 4385
Fully synced at BTC 12871 and STX 4385
Fully synced at BTC 12871 and STX 4385

@ASuciuX
Copy link
Contributor Author

ASuciuX commented Jun 12, 2024

Hello @jcnelson,

Could you please review the changes I made regarding the retrieval of the neighbors' maximum stacks tip height? You can see the diff here.

I added an iteration through the list of neighbors to get the maximum stacks tip height. This required several modifications, as I had to adjust a trait. I think there should be an alternative way to achieve this, but I couldn't find it.

I've tested the implementation on the current testnet, but the list always returns null. I suspect the issue is with my implementation since I should at least receive data from the neighbor node that I'm syncing from, but wanted to ask if in case there could be something with the node's configurations that could create this problem. Thank you.

@smcclellan smcclellan requested a review from jcnelson June 12, 2024 14:48
Co-authored-by: Jeff Bencin <[email protected]>
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.

Make sure to update docs/rpc/api/core-node/get-info.schema.json also

jbencin
jbencin previously approved these changes Jun 17, 2024
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.

LGTM

@ASuciuX
Copy link
Contributor Author

ASuciuX commented Jun 17, 2024

Hello @jcnelson,

Could you please review the changes I made regarding the retrieval of the neighbors' maximum stacks tip height? You can see the diff here.

I added an iteration through the list of neighbors to get the maximum stacks tip height. This required several modifications, as I had to adjust a trait. I think there should be an alternative way to achieve this, but I couldn't find it.

I've tested the implementation on the current testnet, but the list always returns null. I suspect the issue is with my implementation since I should at least receive data from the neighbor node that I'm syncing from, but wanted to ask if in case there could be something with the node's configurations that could create this problem. Thank you.

Hey @jbencin , I would also need an opinion on this. I have to integrate also this part ( otherwise it will only say if the bitcoin node is synced ). I made it as different PR as I'm pretty sure I'm not parsing something the best way possible but couldn't find the right spot to change. Thank you

@saralab saralab requested a review from kantai June 20, 2024 13:59
kantai
kantai previously approved these changes Jun 20, 2024
@smcclellan smcclellan enabled auto-merge June 21, 2024 14:14
jcnelson
jcnelson previously approved these changes Jun 21, 2024
@ASuciuX ASuciuX dismissed stale reviews from jbencin, kantai, and jcnelson via 4634479 June 25, 2024 17:48
@smcclellan smcclellan added this pull request to the merge queue Jun 25, 2024
Merged via the queue into develop with commit 17d917c Jun 25, 2024
1 check passed
@smcclellan smcclellan deleted the feat/expose-node-is-synced branch June 25, 2024 18:47
@blockstack-devops
Copy link
Contributor

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.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Oct 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[Feature request] - expose whether the node is synced in /v2/info
9 participants