Fix general merkle keypath to start w/ last op's key (#2733)

* Fix general merkle keypath to start w/ last op's key

* Update CHANGELOG_PENDING.md

CHANGELOG_PENDING.md

@@ -109,6 +109,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 - [consensus] [\#1637](https://github.com/tendermint/tendermint/issues/1637) Limit the amount of evidence that can be included in a
   block
 - [consensus] [\#2646](https://github.com/tendermint/tendermint/issues/2646) Simplify Proposal message (align with spec)
+- [crypto] [\#2733](https://github.com/tendermint/tendermint/pull/2733) Fix general merkle keypath to start w/ last op's key
 - [evidence] [\#2515](https://github.com/tendermint/tendermint/issues/2515) Fix db iter leak (@goolAdapter)
 - [libs/event] [\#2518](https://github.com/tendermint/tendermint/issues/2518) Fix event concurrency flaw (@goolAdapter)
 - [node] [\#2434](https://github.com/tendermint/tendermint/issues/2434) Make node respond to signal interrupts while sleeping for genesis time

crypto/merkle/proof.go

@@ -43,10 +43,11 @@ func (poz ProofOperators) Verify(root []byte, keypath string, args [][]byte) (er
 	for i, op := range poz {
 		key := op.GetKey()
 		if len(key) != 0 {
-			if !bytes.Equal(keys[0], key) {
-				return cmn.NewError("Key mismatch on operation #%d: expected %+v but %+v", i, []byte(keys[0]), []byte(key))
+			lastKey := keys[len(keys)-1]
+			if !bytes.Equal(lastKey, key) {
+				return cmn.NewError("Key mismatch on operation #%d: expected %+v but got %+v", i, string(lastKey), string(key))
 			}
-			keys = keys[1:]
+			keys = keys[:len(keys)-1]
 		}
 		args, err = op.Run(args)
 		if err != nil {
@@ -54,7 +55,7 @@ func (poz ProofOperators) Verify(root []byte, keypath string, args [][]byte) (er
 		}
 	}
 	if !bytes.Equal(root, args[0]) {
-		return cmn.NewError("Calculated root hash is invalid: expected %+v but %+v", root, args[0])
+		return cmn.NewError("Calculated root hash is invalid: expected %+v but got %+v", root, args[0])
 	}
 	if len(keys) != 0 {
 		return cmn.NewError("Keypath not consumed all")

crypto/merkle/proof_test.go

@@ -0,0 +1,136 @@
+package merkle
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/tendermint/go-amino"
+	cmn "github.com/tendermint/tendermint/libs/common"
+)
+
+const ProofOpDomino = "test:domino"
+
+// Expects given input, produces given output.
+// Like the game dominos.
+type DominoOp struct {
+	key    string // unexported, may be empty
+	Input  string
+	Output string
+}
+
+func NewDominoOp(key, input, output string) DominoOp {
+	return DominoOp{
+		key:    key,
+		Input:  input,
+		Output: output,
+	}
+}
+
+func DominoOpDecoder(pop ProofOp) (ProofOperator, error) {
+	if pop.Type != ProofOpDomino {
+		panic("unexpected proof op type")
+	}
+	var op DominoOp // a bit strange as we'll discard this, but it works.
+	err := amino.UnmarshalBinaryLengthPrefixed(pop.Data, &op)
+	if err != nil {
+		return nil, cmn.ErrorWrap(err, "decoding ProofOp.Data into SimpleValueOp")
+	}
+	return NewDominoOp(string(pop.Key), op.Input, op.Output), nil
+}
+
+func (dop DominoOp) ProofOp() ProofOp {
+	bz := amino.MustMarshalBinaryLengthPrefixed(dop)
+	return ProofOp{
+		Type: ProofOpDomino,
+		Key:  []byte(dop.key),
+		Data: bz,
+	}
+}
+
+func (dop DominoOp) Run(input [][]byte) (output [][]byte, err error) {
+	if len(input) != 1 {
+		return nil, cmn.NewError("Expected input of length 1")
+	}
+	if string(input[0]) != dop.Input {
+		return nil, cmn.NewError("Expected input %v, got %v",
+			dop.Input, string(input[0]))
+	}
+	return [][]byte{[]byte(dop.Output)}, nil
+}
+
+func (dop DominoOp) GetKey() []byte {
+	return []byte(dop.key)
+}
+
+//----------------------------------------
+
+func TestProofOperators(t *testing.T) {
+	var err error
+
+	// ProofRuntime setup
+	// TODO test this somehow.
+	// prt := NewProofRuntime()
+	// prt.RegisterOpDecoder(ProofOpDomino, DominoOpDecoder)
+
+	// ProofOperators setup
+	op1 := NewDominoOp("KEY1", "INPUT1", "INPUT2")
+	op2 := NewDominoOp("KEY2", "INPUT2", "INPUT3")
+	op3 := NewDominoOp("", "INPUT3", "INPUT4")
+	op4 := NewDominoOp("KEY4", "INPUT4", "OUTPUT4")
+
+	// Good
+	popz := ProofOperators([]ProofOperator{op1, op2, op3, op4})
+	err = popz.Verify(bz("OUTPUT4"), "/KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.Nil(t, err)
+	err = popz.VerifyValue(bz("OUTPUT4"), "/KEY4/KEY2/KEY1", bz("INPUT1"))
+	assert.Nil(t, err)
+
+	// BAD INPUT
+	err = popz.Verify(bz("OUTPUT4"), "/KEY4/KEY2/KEY1", [][]byte{bz("INPUT1_WRONG")})
+	assert.NotNil(t, err)
+	err = popz.VerifyValue(bz("OUTPUT4"), "/KEY4/KEY2/KEY1", bz("INPUT1_WRONG"))
+	assert.NotNil(t, err)
+
+	// BAD KEY 1
+	err = popz.Verify(bz("OUTPUT4"), "/KEY3/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+
+	// BAD KEY 2
+	err = popz.Verify(bz("OUTPUT4"), "KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+
+	// BAD KEY 3
+	err = popz.Verify(bz("OUTPUT4"), "/KEY4/KEY2/KEY1/", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+
+	// BAD KEY 4
+	err = popz.Verify(bz("OUTPUT4"), "//KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+
+	// BAD OUTPUT 1
+	err = popz.Verify(bz("OUTPUT4_WRONG"), "/KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+
+	// BAD OUTPUT 2
+	err = popz.Verify(bz(""), "/KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+
+	// BAD POPZ 1
+	popz = []ProofOperator{op1, op2, op4}
+	err = popz.Verify(bz("OUTPUT4"), "/KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+
+	// BAD POPZ 2
+	popz = []ProofOperator{op4, op3, op2, op1}
+	err = popz.Verify(bz("OUTPUT4"), "/KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+
+	// BAD POPZ 3
+	popz = []ProofOperator{}
+	err = popz.Verify(bz("OUTPUT4"), "/KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")})
+	assert.NotNil(t, err)
+}
+
+func bz(s string) []byte {
+	return []byte(s)
+}