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

Merkle tree #1

Closed
wants to merge 23 commits into from
Closed

Merkle tree #1

wants to merge 23 commits into from

Conversation

vqhuy
Copy link
Member

@vqhuy vqhuy commented Jun 5, 2016

Implement the Merkle prefix tree #2 and STR #3

@vqhuy vqhuy force-pushed the merkle-tree branch 2 times, most recently from 11eb024 to f0f7333 Compare June 5, 2016 16:37
interiorNode
key string
value []byte
index []byte
Copy link
Member

Choose a reason for hiding this comment

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

The field for the cryptographic commitment commit(key || value) should still be included since it needs to be returned as part of the auth path.

@vqhuy vqhuy closed this Jun 7, 2016
@vqhuy vqhuy reopened this Jun 7, 2016
return nil
}

func (m *MerkleTree) RecomputeHash() {
Copy link
Member

Choose a reason for hiding this comment

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

Is this wrapper function really needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it will be removed in the refactoring phase.

@vqhuy vqhuy force-pushed the merkle-tree branch 3 times, most recently from 01a1ac9 to ec01cde Compare June 9, 2016 05:12

// Private Index calculation function
// would be replaced with Ismail's VRF implementation
func (m *MerkleTree) computePrivateIndex(key string) []byte {
Copy link
Member

@masomel masomel Jun 9, 2016

Choose a reason for hiding this comment

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

Same as above, I would put computePrivateIndex in a separate module for all things crypto.

@vqhuy
Copy link
Member Author

vqhuy commented Jun 12, 2016

I decided to remove db module for the sake of simplicity. Thus, tree/STR storage will be implemented outside this library. All look-up operations now are in-memory only.

return nil
}

currentNodeKey := computePrivateIndex(currentNodeUL.key)
Copy link
Member

@arlolra arlolra Jun 21, 2016

Choose a reason for hiding this comment

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

Is this expected to be different from currentNodeUL.index already?

Copy link
Member

Choose a reason for hiding this comment

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

I think at this point it is since the check for equality happens in the if statement above. But as you said in your previous note, the check in that if statement should compare indices, not the keys.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this expected to be different from currentNodeUL.index already?

No, it should be the same. I removed this redundant code.
cc @masomel

@arlolra
Copy link
Member

arlolra commented Jun 22, 2016

Ok, I'm going to merge some of this now.

@arlolra
Copy link
Member

arlolra commented Jun 22, 2016

Let's see if 14956a9 works first though :)

@vqhuy
Copy link
Member Author

vqhuy commented Jun 22, 2016

This is the travis log of the previous check: https://s3.amazonaws.com/archive.travis-ci.org/jobs/139557242/log.txt (before 14956a9)

Actually Travis did run all subpackage tests

=== RUN TestHistoryHashChain-2
--- PASS: TestHistoryHashChain-2 (0.01s)
=== RUN TestOneEntry-2
--- PASS: TestOneEntry-2 (0.00s)
=== RUN TestTwoEntries-2
--- PASS: TestTwoEntries-2 (0.00s)
=== RUN TestThreeEntries-2
--- PASS: TestThreeEntries-2 (0.00s)
=== RUN TestInsertExistedKey-2
--- PASS: TestInsertExistedKey-2 (0.00s)
=== RUN TestTreeClone-2
--- PASS: TestTreeClone-2 (0.00s)
PASS
ok      github.com/coniks-sys/libmerkleprefixtree-go    0.022s
=== RUN TestSignVerify-2
--- PASS: TestSignVerify-2 (0.00s)
PASS
ok      github.com/coniks-sys/libmerkleprefixtree-go/crypto 0.017s
=== RUN TestBitsBytesConvert-2
--- PASS: TestBitsBytesConvert-2 (0.00s)
PASS
ok      github.com/coniks-sys/libmerkleprefixtree-go/internal   0.002s

So I guess you want to run subpackage tests first?

@arlolra
Copy link
Member

arlolra commented Jun 22, 2016

Oh, no, I just didn't realize the default for go was go test -v ./...

https://docs.travis-ci.com/user/languages/go#Default-Test-Script

My bad. Sorry for the noise.

@arlolra
Copy link
Member

arlolra commented Jun 22, 2016

Ok, most of this is merged in 57c9831.

I'll open up a pull with the rest for review.

@arlolra arlolra mentioned this pull request Jun 22, 2016
@arlolra
Copy link
Member

arlolra commented Jun 22, 2016

Ok, #8 is open for business. Let's talk about the salt there in #9.

@arlolra arlolra closed this Jun 22, 2016
@arlolra arlolra deleted the merkle-tree branch June 22, 2016 20:13
@masomel masomel mentioned this pull request Jul 15, 2016
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.

4 participants