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

WIP generalized merkle proofs #85

Closed
wants to merge 10 commits into from
Closed

WIP generalized merkle proofs #85

wants to merge 10 commits into from

Conversation

jaekwon
Copy link
Contributor

@jaekwon jaekwon commented Jul 10, 2018

Depends on tendermint/tendermint#1912
Replaces #84

import (
"fmt"

"github.com/tendermint/tendermint/crypto/merkle"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want tendermint as a dependency here? If so, wouldn't it make sense to do the merge (into tm) first? Or, do we want to move the interface ProofOperator to this repo instead? I like the latter as Tendermint will import iavl anyways.

@@ -1,5 +1,30 @@
# Changelog

## 0.10.0 (July 11, 2018)
Copy link
Contributor

Choose a reason for hiding this comment

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

The version here does not match the number in version.go.

}
// XXX What is the encoding for keys?
// We should decode the key depending on whether it's a string or hex,
// maybe based on quotes and 0x prefix?
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not clear if this is a todo that will be dealt later or if this should be clarified in this PR. @mossid?

@liamsi
Copy link
Contributor

liamsi commented Sep 11, 2018

This is superseded by #105 now. Can be closed.

@liamsi liamsi closed this Sep 11, 2018
@AdityaSripal AdityaSripal deleted the jae/generalmerkle branch July 26, 2019 16:56
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