-
Notifications
You must be signed in to change notification settings - Fork 63
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
Added the ability to create and verify merkle proofs #61
Conversation
retest this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I'll merge it once these few minor things are cleared up.
src/kernel/merkletree.cpp
Outdated
@@ -1,14 +1,21 @@ | |||
#include <queue> | |||
|
|||
#include <iostream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think iostream
is needed here.
src/kernel/merkletree.h
Outdated
class MerkleProof { | ||
public: | ||
MerkleProof(); | ||
MerkleProof(Json::Value& json); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The json
param should be marked const
src/kernel/merkletree.h
Outdated
MerkleProof(Json::Value& json); | ||
int positionInTotalSet; | ||
std::vector<BigNum> leaves; | ||
Json::Value toJson(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toJson
should be marked const
.
src/kernel/merkletree.h
Outdated
|
||
std::shared_ptr<MerkleNode> getLeftNode(); | ||
std::shared_ptr<MerkleNode> getRightNode(); | ||
MerkleNode* getAncestor(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getAncestor
should be marked const
src/kernel/merkletree.h
Outdated
BigNum root; | ||
|
||
|
||
CryptoKernel::MerkleNode* findDescendant(BigNum needle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needle
should be both const
and passed by reference. I think the function itself can be marked const
too.
tests/MerkletreeTests.cpp
Outdated
@@ -1,4 +1,5 @@ | |||
#include "MerkletreeTests.h" | |||
#include <iostream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iostream
probably not needed here
Cleaned up the code based on your suggestions (thanks!). Let me know if it requires any additional changes. |
In order to support the pay-to-merkletree effort, i needed the merkletree class to support building and verifying merkle proofs.
I added this to the merkletree class, introduced two new classes:
MerkleProof holds the proof data, and can serialize/deserialize them to and from Json
MerkleRootNode can be initialized with a precomputed hash in case of reading a merkletree back from a proof - the nodes where the hash was calculated from are not availble, so the root value is just set from the constructor. LeftVal() and RightVal() will return empty.