diff --git a/CHANGELOG.md b/CHANGELOG.md index e33653a9..ed15f30e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +# Unreleased + +- fix(go): interpret max_depth in proof specs as 128 if left to 0 [#352](https://github.com/cosmos/ics23/pull/352). + # 0.12.0 ## Rust diff --git a/go/fuzz_test.go b/go/fuzz_test.go index 80dcf7e7..f972fde5 100644 --- a/go/fuzz_test.go +++ b/go/fuzz_test.go @@ -48,7 +48,7 @@ func FuzzExistenceProofCheckAgainstSpec(f *testing.F) { seedDataMap := CheckAgainstSpecTestData(f) for _, seed := range seedDataMap { - if seed.IsErr { + if seed.Err != "" { // Erroneous data, skip it. continue } diff --git a/go/proof.go b/go/proof.go index 26fc369e..91fa232e 100644 --- a/go/proof.go +++ b/go/proof.go @@ -190,7 +190,13 @@ func (p *ExistenceProof) CheckAgainstSpec(spec *ProofSpec) error { if spec.MinDepth > 0 && len(p.Path) < int(spec.MinDepth) { return fmt.Errorf("innerOps depth too short: %d", len(p.Path)) } - if spec.MaxDepth > 0 && len(p.Path) > int(spec.MaxDepth) { + + maxDepth := spec.MaxDepth + if maxDepth == 0 { + maxDepth = 128 + } + + if len(p.Path) > int(maxDepth) { return fmt.Errorf("innerOps depth too long: %d", len(p.Path)) } diff --git a/go/proof_data_test.go b/go/proof_data_test.go index fceb3837..e9b90c14 100644 --- a/go/proof_data_test.go +++ b/go/proof_data_test.go @@ -54,7 +54,7 @@ func CheckLeafTestData(tb testing.TB) map[string]CheckLeafTestStruct { type CheckAgainstSpecTestStruct struct { Proof *ExistenceProof Spec *ProofSpec - IsErr bool + Err string } func CheckAgainstSpecTestData(tb testing.TB) map[string]CheckAgainstSpecTestStruct { @@ -70,6 +70,7 @@ func CheckAgainstSpecTestData(tb testing.TB) map[string]CheckAgainstSpecTestStru if err != nil { tb.Fatal(err) } + return cases } diff --git a/go/proof_test.go b/go/proof_test.go index de5ab4e6..fd55e019 100644 --- a/go/proof_test.go +++ b/go/proof_test.go @@ -44,16 +44,15 @@ func TestCheckLeaf(t *testing.T) { } func TestCheckAgainstSpec(t *testing.T) { - t.Skip() cases := CheckAgainstSpecTestData(t) for name, tc := range cases { t.Run(name, func(t *testing.T) { err := tc.Proof.CheckAgainstSpec(tc.Spec) - if tc.IsErr && err == nil { - t.Fatal("Expected error, but got nil") - } else if !tc.IsErr && err != nil { + if tc.Err == "" && err != nil { t.Fatalf("Unexpected error: %v", err) + } else if tc.Err != "" && tc.Err != err.Error() { + t.Fatalf("Expected error: %s, got %s", tc.Err, err.Error()) } }) } diff --git a/go/proofs.pb.go b/go/proofs.pb.go index 49ad173a..1f64cb00 100644 --- a/go/proofs.pb.go +++ b/go/proofs.pb.go @@ -577,6 +577,7 @@ type ProofSpec struct { LeafSpec *LeafOp `protobuf:"bytes,1,opt,name=leaf_spec,json=leafSpec,proto3" json:"leaf_spec,omitempty"` InnerSpec *InnerSpec `protobuf:"bytes,2,opt,name=inner_spec,json=innerSpec,proto3" json:"inner_spec,omitempty"` // max_depth (if > 0) is the maximum number of InnerOps allowed (mainly for fixed-depth tries) + // the max_depth is interpreted as 128 if set to 0 MaxDepth int32 `protobuf:"varint,3,opt,name=max_depth,json=maxDepth,proto3" json:"max_depth,omitempty"` // min_depth (if > 0) is the minimum number of InnerOps allowed (mainly for fixed-depth tries) MinDepth int32 `protobuf:"varint,4,opt,name=min_depth,json=minDepth,proto3" json:"min_depth,omitempty"` diff --git a/proto/cosmos/ics23/v1/proofs.proto b/proto/cosmos/ics23/v1/proofs.proto index 1c8b6ad7..7605aa1a 100644 --- a/proto/cosmos/ics23/v1/proofs.proto +++ b/proto/cosmos/ics23/v1/proofs.proto @@ -163,6 +163,7 @@ message ProofSpec { LeafOp leaf_spec = 1; InnerSpec inner_spec = 2; // max_depth (if > 0) is the maximum number of InnerOps allowed (mainly for fixed-depth tries) + // the max_depth is interpreted as 128 if set to 0 int32 max_depth = 3; // min_depth (if > 0) is the minimum number of InnerOps allowed (mainly for fixed-depth tries) int32 min_depth = 4; diff --git a/rust/src/cosmos.ics23.v1.rs b/rust/src/cosmos.ics23.v1.rs index eeef5534..43755969 100644 --- a/rust/src/cosmos.ics23.v1.rs +++ b/rust/src/cosmos.ics23.v1.rs @@ -1,3 +1,4 @@ +// This file is @generated by prost-build. /// * /// ExistenceProof takes a key and a value and a set of steps to perform on it. /// The result of peforming all these steps will provide a "root hash", which can @@ -146,6 +147,7 @@ pub struct ProofSpec { #[prost(message, optional, tag = "2")] pub inner_spec: ::core::option::Option, /// max_depth (if > 0) is the maximum number of InnerOps allowed (mainly for fixed-depth tries) + /// the max_depth is interpreted as 128 if set to 0 #[prost(int32, tag = "3")] pub max_depth: i32, /// min_depth (if > 0) is the minimum number of InnerOps allowed (mainly for fixed-depth tries) diff --git a/rust/src/proto_descriptor.bin b/rust/src/proto_descriptor.bin index 709b343d..34139d2d 100644 Binary files a/rust/src/proto_descriptor.bin and b/rust/src/proto_descriptor.bin differ diff --git a/testdata/TestCheckAgainstSpecData.json b/testdata/TestCheckAgainstSpecData.json index f15a8ada..94a00943 100644 --- a/testdata/TestCheckAgainstSpecData.json +++ b/testdata/TestCheckAgainstSpecData.json @@ -7,16 +7,16 @@ "hash": 1, "prehash_value": 1, "length": 1, - "prefix": "AA==" + "prefix": "AAKKAQ==" }, "path": [ { "hash": 1, - "prefix": "qrvM3Q==" + "prefix": "AgQIIGe3bHuC1g6+5/Qd0RoCU0waFu+nDCFzEDViMN/VrQwgIA==" }, { "hash": 1, - "prefix": "qrvM3Q==" + "prefix": "BAYIIA==" } ] }, @@ -38,7 +38,7 @@ "hash": 1 } }, - "IsErr": false + "Err": "" }, "accepts one proper leaf": { "Proof": { @@ -48,7 +48,7 @@ "hash": 1, "prehash_value": 1, "length": 1, - "prefix": "AA==" + "prefix": "AAKKAQ==" } }, "Spec": { @@ -69,7 +69,7 @@ "hash": 1 } }, - "IsErr": false + "Err": "" }, "allows depth limited in proper range": { "Proof": { @@ -79,20 +79,20 @@ "hash": 1, "prehash_value": 1, "length": 1, - "prefix": "AA==" + "prefix": "AAKKAQ==" }, "path": [ { "hash": 1, - "prefix": "qrvM3Q==" + "prefix": "AgQIIGe3bHuC1g6+5/Qd0RoCU0waFu+nDCFzEDViMN/VrQwgIA==" }, { "hash": 1, - "prefix": "qrvM3Q==" + "prefix": "BAYIIA==" }, { "hash": 1, - "prefix": "qrvM3Q==" + "prefix": "BgwKIA==" } ] }, @@ -116,7 +116,7 @@ "max_depth": 4, "min_depth": 2 }, - "IsErr": false + "Err": "" }, "empty proof fails": { "Proof": { @@ -141,7 +141,7 @@ "hash": 1 } }, - "IsErr": true + "Err": "existence Proof needs defined LeafOp" }, "reject depth limited with too few inner nodes": { "Proof": { @@ -151,7 +151,7 @@ "hash": 1, "prehash_value": 1, "length": 1, - "prefix": "AA==" + "prefix": "AAKKAQ==" }, "path": [ { @@ -180,7 +180,7 @@ "max_depth": 4, "min_depth": 2 }, - "IsErr": true + "Err": "innerOps depth too short: 1" }, "reject depth limited with too many inner nodes": { "Proof": { @@ -190,7 +190,7 @@ "hash": 1, "prehash_value": 1, "length": 1, - "prefix": "AA==" + "prefix": "AAKKAQ==" }, "path": [ { @@ -235,9 +235,9 @@ "max_depth": 4, "min_depth": 2 }, - "IsErr": true + "Err": "innerOps depth too long: 5" }, - "rejects invalid inner proof (hash mismatch)": { + "reject too many inner nodes (129) using default limited depth (128)": { "Proof": { "key": "Zm9vZA==", "value": "YmFy", @@ -245,11 +245,11 @@ "hash": 1, "prehash_value": 1, "length": 1, - "prefix": "AA==" + "prefix": "AAKKAQ==" }, "path": [ { - "hash": 2, + "hash": 1, "prefix": "qrvM3Q==" }, { @@ -259,113 +259,670 @@ { "hash": 1, "prefix": "qrvM3Q==" - } - ] - }, - "Spec": { - "leaf_spec": { - "hash": 1, - "prehash_value": 1, - "length": 1, - "prefix": "AA==" - }, - "inner_spec": { - "child_order": [ - 0, - 1 - ], - "child_size": 33, - "min_prefix_length": 4, - "max_prefix_length": 12, - "hash": 1 - } - }, - "IsErr": true - }, - "rejects invalid leaf": { - "Proof": { - "key": "Zm9vZA==", - "value": "YmFy", - "leaf": { - "hash": 1, - "length": 1, - "prefix": "AA==" - } - }, - "Spec": { - "leaf_spec": { - "hash": 1, - "prehash_value": 1, - "length": 1, - "prefix": "AA==" - }, - "inner_spec": { - "child_order": [ - 0, - 1 - ], - "child_size": 33, - "min_prefix_length": 4, - "max_prefix_length": 12, - "hash": 1 - } - }, - "IsErr": true - }, - "rejects leaf with invalid inner proofs": { - "Proof": { - "key": "Zm9vZA==", - "value": "YmFy", - "leaf": { - "hash": 1, - "prehash_value": 1, - "length": 1, - "prefix": "AA==" - }, - "path": [ + }, { "hash": 1, "prefix": "qrvM3Q==" }, { "hash": 1, - "prefix": "AKq7zN0=", - "suffix": "uw==" + "prefix": "qrvM3Q==" }, { "hash": 1, "prefix": "qrvM3Q==" - } - ] - }, - "Spec": { - "leaf_spec": { - "hash": 1, - "prehash_value": 1, - "length": 1, - "prefix": "AA==" - }, - "inner_spec": { - "child_order": [ - 0, - 1 - ], - "child_size": 33, - "min_prefix_length": 4, - "max_prefix_length": 12, - "hash": 1 - } - }, - "IsErr": true - }, - "rejects only inner proof": { - "Proof": { - "key": "Zm9vZA==", - "value": "YmFy", - "path": [ + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, { "hash": 1, "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + } + ] + }, + "Spec": { + "leaf_spec": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AA==" + }, + "inner_spec": { + "child_order": [ + 0, + 1 + ], + "child_size": 33, + "min_prefix_length": 4, + "max_prefix_length": 12, + "hash": 1 + }, + "max_depth": 0, + "min_depth": 2 + }, + "Err": "innerOps depth too long: 129" + }, + "rejects invalid inner proof (hash mismatch)": { + "Proof": { + "key": "Zm9vZA==", + "value": "YmFy", + "leaf": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AAKKAQ==" + }, + "path": [ + { + "hash": 2, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + }, + { + "hash": 1, + "prefix": "qrvM3Q==" + } + ] + }, + "Spec": { + "leaf_spec": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AA==" + }, + "inner_spec": { + "child_order": [ + 0, + 1 + ], + "child_size": 33, + "min_prefix_length": 4, + "max_prefix_length": 12, + "hash": 1 + } + }, + "Err": "inner, unexpected HashOp: 2" + }, + "rejects invalid leaf": { + "Proof": { + "key": "Zm9vZA==", + "value": "YmFy", + "leaf": { + "hash": 1, + "length": 1, + "prefix": "AAKKAQ==" + } + }, + "Spec": { + "leaf_spec": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AA==" + }, + "inner_spec": { + "child_order": [ + 0, + 1 + ], + "child_size": 33, + "min_prefix_length": 4, + "max_prefix_length": 12, + "hash": 1 + } + }, + "Err": "leaf, unexpected PrehashValue: 0" + }, + "rejects leaf with invalid inner proofs": { + "Proof": { + "key": "Zm9vZA==", + "value": "YmFy", + "leaf": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AAKKAQ==" + }, + "path": [ + { + "hash": 1, + "prefix": "AgQIIGe3bHuC1g6+5/Qd0RoCU0waFu+nDCFzEDViMN/VrQwgIA==" + }, + { + "hash": 1, + "prefix": "BAYIIA==" + }, + { + "hash": 1, + "prefix": "AKq7zN0=", + "suffix": "uw==" + } + ] + }, + "Spec": { + "leaf_spec": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AA==" + }, + "inner_spec": { + "child_order": [ + 0, + 1 + ], + "child_size": 33, + "min_prefix_length": 4, + "max_prefix_length": 12, + "hash": 1 + } + }, + "Err": "inner, unexpected EOF" + }, + "rejects only inner proof (hash mismatch)": { + "Proof": { + "key": "Zm9vZA==", + "value": "YmFy", + "leaf": { + "hash": 1, + "prehash_value": 1, + "length": 1, + "prefix": "AAKKAQ==" + }, + "path": [ + { + "hash": 0, + "prefix": "qrvM3Q==" } ] }, @@ -387,6 +944,6 @@ "hash": 1 } }, - "IsErr": true + "Err": "inner, unexpected HashOp: 0" } }