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

TUI stay on "Downloading headers: 6%, step 1/4" #2233

Closed
garyyu opened this issue Dec 26, 2018 · 10 comments
Closed

TUI stay on "Downloading headers: 6%, step 1/4" #2233

garyyu opened this issue Dec 26, 2018 · 10 comments

Comments

@garyyu
Copy link
Contributor

garyyu commented Dec 26, 2018

Already saw several reports on similar issue such as "4%", "6%", etc, and stuck here.

So I look into an old log and find this:

20181223 20:56:39.533 DEBUG grin_servers::common::types - sync_state: sync_status: HeaderSync { current_height: 3698, highest_height: 3698 } -> BodySync { current_height: 3698, highest_height: 3698 }
...
20181223 20:56:49.317 DEBUG grin_servers::common::types - sync_state: sync_status: BodySync { current_height: 3698, highest_height: 3698 } -> HeaderSync { current_height: 3698, highest_height: 94148 }
...
20181223 20:56:53.744 DEBUG grin_servers::common::types - sync_state: sync_status: HeaderSync { current_height: 3698, highest_height: 94148 } -> HeaderSync { current_height: 3699, highest_height: 94148 }

That means the Floonet network see a highest height at 94,148! that's why this 3699 show 4%. It definitely is a problem for us, since the current highest height is 7,566!

I check those old Testnets:

  • Testnet4, the highest height stay in 90,900 since 5 days ago
  • Testnet3 stay in 142,700 since 69 days ago
  • Testnet2 134,400 at 170 days ago
  • Testnet1 130,000 at 274 days ago

So, it could not be caused by an un-upgraded node which stay on an old testnet, but since #2208 gave different magic code for the new Floonet and Mainnet, it will not be problem in the future to get a peer connected from TestnetN.

But let's give a further think about a possibility of a fraud peer, if this node is broadcasting a highest height far bigger than any other nodes (and a far bigger total_difficulty), in current implementation, it will draw all other nodes into syncing state from NoSync state, and because current grin-miner depends on the node state to start mining, that will cause all other grin-miner paused, and only that fraud node can continue mining.

IIRC, this fraud peer issue had been discussed before, but I can't remember what we did to get the immunity on this. Could somebody remind me?

Anyway, since the issue is still there, looks like we still need a fix to resist such kind of fraud peers.

@ignopeverell
Copy link
Contributor

#1139 has a lot of background about this.

And I think the check on the miner's block submission has been removed. Or are you talking about startup?

@gary-yu
Copy link

gary-yu commented Dec 27, 2018

Thanks for remind.

And I would do a real test today, for 3 cases, but sorry to those peers which connected to my node 😸

  • impact to mining node
  • impact to new node (syncing)
  • impact to running node

@ignopeverell
Copy link
Contributor

Excellent idea!

@garyyu
Copy link
Contributor Author

garyyu commented Dec 27, 2018

We could kill a batch of bugs here 😄
Prefer to split them, and give fix and verification test one by one.

Here is No.1 - If a new node is syncing but with this fraud peer connected, it will skip the state sync, and directly enter body sync, which is very slow and could be impossible to achieve since we don't have much archive nodes. Fixing now.

[Updated]:

This is not a problem! It's the designed behavior.

pub const CUT_THROUGH_HORIZON: u32 = WEEK_HEIGHT as u32;

Because we set horizon as one week, before the chain height rise to one week's height, no state syncing is allowed. That means, we're making sure every node has at least 10080 blocks stored locally before any one start state syncing.

@garyyu
Copy link
Contributor Author

garyyu commented Dec 27, 2018

Here is No.2 - If a new node is syncing but with this fraud peer connected, it will stuck at Header Syncing stage for ever.

Note: it should be 1st issue, but I missed the Ping fraud message, that cause above No.1 issue come to me firstly.

@garyyu
Copy link
Contributor Author

garyyu commented Dec 27, 2018

For test case: a fraud peer connects to an existing running node, it will trigger the running node to start a new sync, then the problem will be almost same as No.2, it will stuck at Header Syncing stage. So the same fix in #2237 will apply for this case.

INFO grin_servers::grin::sync::syncer - sync: total_difficulty 632483442, peer_difficulty 1623147966, threshold 166419 (last 5 blocks), enabling sync

@garyyu
Copy link
Contributor Author

garyyu commented Dec 27, 2018

Here is No.3 - If a fraud peer connects to a running mining node, it will trigger that node into a new sync state, then it will stuck at syncing state (without #2237 fix), all connected miners will not get any job updating when received new block(s), i.e. all associated miners will always work on same job, before the node jump out of syncing state.

A fix will be given in #2239

@ignopeverell
Copy link
Contributor

Thought some more about this. #2337 definitely helps. But I think we can improve on #2240 by introducing a new sync state or flag for a restart. This would only be enabled when the server just starts up and disabled once we're caught up. And we can check on that to stop mined block relay, instead of any sync. What do you think?

@garyyu
Copy link
Contributor Author

garyyu commented Dec 29, 2018

Good idea! will implement it on #2240

@garyyu
Copy link
Contributor Author

garyyu commented Jan 2, 2019

I closed #2240, since we already fixed the fraud peer (with fake height and total difficulty) banning issue, then we should never come to a case that need broadcasting new mined block when in syncing state.

Then all found problems in this issue have been addressed or fixed. Let's close this one.

@garyyu garyyu closed this as completed Jan 2, 2019
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

No branches or pull requests

3 participants