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

Add merkleblock and proof of work check #153

Closed
wants to merge 3 commits into from
Closed

Add merkleblock and proof of work check #153

wants to merge 3 commits into from

Conversation

pebble8888
Copy link
Contributor

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.

Description of the Change

Add merkleblock message validity check.
You should look out that the validity check here is not enough to insert to the local database.
Moreover you need timestamp check and etc.

  1. Build merkle root from merkleblock message.
  2. Check if calculated root match root hash in the message.
  3. Check if block hash has enough proof of work.

UInt256 is for PoW difficulty check.
This implementation is large for it.

I have not enough time to amend this PR unfortunately.
If the implementation doesn't match your design, please reject it or pick up what you need.

Alternate Designs

This check is implemented in BreadWallet.
I don't know we should check which message received.

Benefits

Prevent interpolation and attack.

Possible Drawbacks

Code maintainavility.
If a valid message is dropped, comment out the message check logic.
I tested some message is passed in test net.

Applicable Issues

None

@akifuji
Copy link
Contributor

akifuji commented Sep 4, 2018

@pebble8888
Please modify copyright for each file, according to the existing files.

// invalid bits
return false
}
if target == UInt256.zero {
Copy link
Contributor

Choose a reason for hiding this comment

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

guard is better than if.
This adapts to the following validations.

@@ -103,6 +103,24 @@ public class PeerGroup: PeerDelegate {
}

public func peer(_ peer: Peer, didReceiveMerkleBlockMessage message: MerkleBlockMessage, hash: Data) {
if !ProofOfWork.isValidProofOfWork(blockHash: hash, bits: message.bits) {
Copy link
Contributor

@akifuji akifuji Sep 5, 2018

Choose a reason for hiding this comment

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

Please use guard

@pebble8888
Copy link
Contributor Author

I fixed it for the code review comment.

Copy link
Contributor

@akifuji akifuji left a comment

Choose a reason for hiding this comment

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

LGTM

@akifuji
Copy link
Contributor

akifuji commented Sep 19, 2018

Thanks!

@akifuji akifuji mentioned this pull request Sep 26, 2018
@pebble8888 pebble8888 closed this Mar 2, 2019
@pebble8888 pebble8888 deleted the develop branch March 2, 2019 14:07
usatie added a commit that referenced this pull request Sep 12, 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

Successfully merging this pull request may close these issues.

2 participants