-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Use simple merkle proof for commit info #6323
Changes from all commits
4abd2bb
9d9d87a
a5b2979
d7f50e0
e37e82b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ import ( | |
|
||
"github.com/pkg/errors" | ||
abci "github.com/tendermint/tendermint/abci/types" | ||
"github.com/tendermint/tendermint/crypto/tmhash" | ||
"github.com/tendermint/tendermint/crypto/merkle" | ||
dbm "github.com/tendermint/tm-db" | ||
|
||
"github.com/cosmos/cosmos-sdk/codec" | ||
|
@@ -457,13 +457,8 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery { | |
} | ||
|
||
// Restore origin path and append proof op. | ||
res.Proof.Ops = append(res.Proof.Ops, NewMultiStoreProofOp( | ||
[]byte(storeName), | ||
NewMultiStoreProof(commitInfo.StoreInfos), | ||
).ProofOp()) | ||
res.Proof.Ops = append(res.Proof.Ops, commitInfo.ProofOp(storeName)) | ||
|
||
// TODO: handle in another TM v0.26 update PR | ||
// res.Proof = buildMultiStoreProof(res.Proof, storeName, commitInfo.StoreInfos) | ||
return res | ||
} | ||
|
||
|
@@ -561,15 +556,31 @@ type commitInfo struct { | |
StoreInfos []storeInfo | ||
} | ||
|
||
// Hash returns the simple merkle root hash of the stores sorted by name. | ||
func (ci commitInfo) Hash() []byte { | ||
// TODO: cache to ci.hash []byte | ||
func (ci commitInfo) toMap() map[string][]byte { | ||
m := make(map[string][]byte, len(ci.StoreInfos)) | ||
for _, storeInfo := range ci.StoreInfos { | ||
m[storeInfo.Name] = storeInfo.Hash() | ||
m[storeInfo.Name] = storeInfo.GetHash() | ||
} | ||
return m | ||
} | ||
|
||
// Hash returns the simple merkle root hash of the stores sorted by name. | ||
func (ci commitInfo) Hash() []byte { | ||
// we need a special case for empty set, as SimpleProofsFromMap requires at least one entry | ||
if len(ci.StoreInfos) == 0 { | ||
return nil | ||
} | ||
rootHash, _, _ := merkle.SimpleProofsFromMap(ci.toMap()) | ||
return rootHash | ||
} | ||
|
||
return SimpleHashFromMap(m) | ||
func (ci commitInfo) ProofOp(storeName string) merkle.ProofOp { | ||
_, proofs, _ := merkle.SimpleProofsFromMap(ci.toMap()) | ||
proof := proofs[storeName] | ||
if proof == nil { | ||
panic(fmt.Sprintf("ProofOp for %s but not registered store name", storeName)) | ||
} | ||
return merkle.NewSimpleValueOp([]byte(storeName), proof).ProofOp() | ||
} | ||
|
||
func (ci commitInfo) CommitID() types.CommitID { | ||
|
@@ -596,20 +607,15 @@ type storeCore struct { | |
// ... maybe add more state | ||
} | ||
|
||
// Implements merkle.Hasher. | ||
func (si storeInfo) Hash() []byte { | ||
// Doesn't write Name, since SimpleHashFromMap() will | ||
// include them via the keys. | ||
bz := si.Core.CommitID.Hash | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The In effect, to make a chained proof op work, you would have to take the root hash from the iavlstore and then hash it before feeding it as a value to the merkle simple proof, which makes no sense. We want the simple proof to commit directly to the root hash of the sub stores (not a hash of a hash of a hash... - we are not bitcoin mining). If anyone intends to use this as a real hashing op (which it never was used as), it must contain the name as well before feeding it to the hasher. In such a case we could rename this to eg. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, believe this was why the hashing in TM had to be removed. This approach works better. Could you please put the cruz of this comment in the godoc. Namely that this does not actually hash the storeInfo, rather it returns the embedded store hash There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I can definitely add it to the GoDoc. I can also rename the function to make it more explicit that we are not trying to implement the Actually I can do both (rename an explain the purpose of the function). Any preferences for names? |
||
hasher := tmhash.New() | ||
|
||
_, err := hasher.Write(bz) | ||
if err != nil { | ||
// TODO: Handle with #870 | ||
panic(err) | ||
} | ||
|
||
return hasher.Sum(nil) | ||
// GetHash returns the GetHash from the CommitID. | ||
// This is used in CommitInfo.Hash() | ||
// | ||
// When we commit to this in a merkle proof, we create a map of storeInfo.Name -> storeInfo.GetHash() | ||
// and build a merkle proof from that. | ||
// This is then chained with the substore proof, so we prove the root hash from the substore before this | ||
// and need to pass that (unmodified) as the leaf value of the multistore proof. | ||
func (si storeInfo) GetHash() []byte { | ||
return si.Core.CommitID.Hash | ||
} | ||
|
||
//---------------------------------------- | ||
|
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.
Note to review: This was for handling the serialized
StoreInfos[]
as a proof, which is now removed in favor of a merkle commitment (which scales much better for dozens or more of substores)