Skip to content

Add hash to ping/pong#1827

Closed
shargon wants to merge 5 commits intoneo-project:masterfrom
shargon:add-hash-to-ping
Closed

Add hash to ping/pong#1827
shargon wants to merge 5 commits intoneo-project:masterfrom
shargon:add-hash-to-ping

Conversation

@shargon
Copy link
Copy Markdown
Member

@shargon shargon commented Aug 6, 2020

Close #1825
Compatible with #1826

@shargon shargon mentioned this pull request Aug 6, 2020
@ShawnYun
Copy link
Copy Markdown
Contributor

ShawnYun commented Aug 6, 2020

Adding hash to ping/pong is a good way to solve the problem of different chains.

// We have a different chain

var header = Blockchain.Singleton.GetHeader(payload.LastBlockIndex);
if (header != null && header.Hash != payload.LastBlockHash)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure it'll help. Suppose I'm a fresh node starting at block 0, there is a right chain with a height of 1000 and a wrong chain of height 300000 on the network. I'm synchronizing headers (and blocks) from the right chain up to 1000, then I'm receiving a pong message with [300000, SOME_HASH], now I don't (and won't for quite some time) have a header for block 300000 to verify SOME_HASH, so I'll try to get blocks/headers from this node anyway because it says it's at the height of 300000.

I don't think we can reliably do anything at this stage, the only thing we can do is react on wrong headers/blocks received from other nodes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we can reliably do anything at this stage

Well, maybe if we're to respond with a hash of the block from the ping (so that the protocol would be like [ping, HEIGHT_A, HASH_A, NONCE] -> [pong, HEIGHT_B, HASH_A, NONCE]) that would change something, still I don't think it's worth the effort as in general this data can't be trusted, it's just whatever some random node on the network wants to tell us. Reacting to header/block verification problems is way more reliable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This data can't be trusted, but you will remove a big percentage of the unwanted data, the bad nodes will send you the blocks, but when you will be able to validate them you will discard them.

@erikzhang
Copy link
Copy Markdown
Member

I don't understand. If we receive a wrong block, we can find it and disconnect from the node. Why should we check it in Ping messages?

@erikzhang
Copy link
Copy Markdown
Member

Maybe #1826 is enough.

@shargon shargon closed this Aug 7, 2020
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

Successfully merging this pull request may close these issues.

Incompatible block isn't throwed out but make sync stop (NEO3)

4 participants