Skip to content

Commit 7000936

Browse files
crodriguezvegacolin-axner
crodriguezvega
and
colin-axner
authored
fix: guarantee that max prefix length is < min prefix length + child size (#369)
* fix: guarantee that max prefix length is < min prefix length + child size * same fix for rust implementation * add test for rust * lint * Update rust/src/verify.rs * chore: rust formatting * proto: add inner op max prefix length restriction comment * test: add test for forging non existence proof * fix: test * lint * test: add unit test for new restriction * chore: update changelog --------- Co-authored-by: colin axnér <[email protected]>
1 parent 22a0173 commit 7000936

9 files changed

+175
-5
lines changed

‎go/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# Unreleased
44

55
- deps: bump golang to v1.22 ([#363](https://github.com/cosmos/ics23/pull/363)).
6+
- fix: guarantee that `spec.InnerSpec.MaxPrefixLength` < `spec.InnerSpec.MinPrefixLength` + `spec.InnerSpec.ChildSize` ([#369](https://github.com/cosmos/ics23/pull/369))
67

78
# v0.11.0
89

‎go/ops.go

+4
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ func (op *InnerOp) CheckAgainstSpec(spec *ProofSpec, b int) error {
166166
return errors.New("spec.InnerSpec.ChildSize must be >= 1")
167167
}
168168

169+
if spec.InnerSpec.MaxPrefixLength >= spec.InnerSpec.MinPrefixLength+spec.InnerSpec.ChildSize {
170+
return errors.New("spec.InnerSpec.MaxPrefixLength must be < spec.InnerSpec.MinPrefixLength + spec.InnerSpec.ChildSize")
171+
}
172+
169173
// ensures soundness, with suffix having to be of correct length
170174
if len(op.Suffix)%int(spec.InnerSpec.ChildSize) != 0 {
171175
return fmt.Errorf("InnerOp suffix malformed")

‎go/ops_test.go

+134
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,13 @@ func TestInnerOpCheckAgainstSpec(t *testing.T) {
150150
},
151151
errors.New("spec.InnerSpec.ChildSize must be >= 1"),
152152
},
153+
{
154+
"failure: MaxPrefixLength >= MinPrefixLength + ChildSize",
155+
func() {
156+
spec.InnerSpec.MaxPrefixLength = spec.InnerSpec.MinPrefixLength + spec.InnerSpec.ChildSize
157+
},
158+
errors.New("spec.InnerSpec.MaxPrefixLength must be < spec.InnerSpec.MinPrefixLength + spec.InnerSpec.ChildSize"),
159+
},
153160
{
154161
"failure: inner op suffix malformed",
155162
func() {
@@ -201,6 +208,133 @@ func TestInnerOpCheckAgainstSpec(t *testing.T) {
201208
}
202209
}
203210

211+
func TestForgeNonExistenceProofWithIncorrectMaxPrefixLength(t *testing.T) {
212+
spec := &ProofSpec{ // TendermintSpec
213+
LeafSpec: &LeafOp{
214+
Prefix: []byte{0},
215+
PrehashKey: HashOp_NO_HASH,
216+
Hash: HashOp_SHA256,
217+
PrehashValue: HashOp_SHA256,
218+
Length: LengthOp_VAR_PROTO,
219+
},
220+
InnerSpec: &InnerSpec{
221+
ChildOrder: []int32{0, 1},
222+
MinPrefixLength: 1,
223+
MaxPrefixLength: 1,
224+
ChildSize: 32, // (no length byte)
225+
Hash: HashOp_SHA256,
226+
},
227+
}
228+
229+
spec.InnerSpec.MaxPrefixLength = 33
230+
leafOp := spec.LeafSpec
231+
aLeaf, _ := leafOp.Apply([]byte("a"), []byte("a"))
232+
bLeaf, _ := leafOp.Apply([]byte("b"), []byte("b"))
233+
b2Leaf, _ := leafOp.Apply([]byte("b2"), []byte("b2"))
234+
235+
cLeaf, _ := leafOp.Apply([]byte("c"), []byte("c"))
236+
aExist := ExistenceProof{
237+
Key: []byte("a"),
238+
Value: []byte("a"),
239+
Leaf: leafOp,
240+
Path: []*InnerOp{
241+
{
242+
Hash: spec.InnerSpec.Hash,
243+
Prefix: []byte{1},
244+
Suffix: append(bLeaf, b2Leaf...),
245+
},
246+
{
247+
Hash: spec.InnerSpec.Hash,
248+
Prefix: []byte{1},
249+
Suffix: cLeaf,
250+
},
251+
},
252+
}
253+
bExist := ExistenceProof{
254+
Key: []byte("b"),
255+
Value: []byte("b"),
256+
Leaf: leafOp,
257+
Path: []*InnerOp{
258+
{
259+
Hash: spec.InnerSpec.Hash,
260+
Prefix: append([]byte{1}, aLeaf...),
261+
Suffix: b2Leaf,
262+
},
263+
{
264+
Hash: spec.InnerSpec.Hash,
265+
Prefix: []byte{1},
266+
Suffix: cLeaf,
267+
},
268+
},
269+
}
270+
b2Exist := ExistenceProof{
271+
Key: []byte("b2"),
272+
Value: []byte("b2"),
273+
Leaf: leafOp,
274+
Path: []*InnerOp{
275+
{
276+
Hash: spec.InnerSpec.Hash,
277+
Prefix: append(append([]byte{1}, aLeaf...), bLeaf...),
278+
Suffix: []byte{},
279+
},
280+
{
281+
Hash: spec.InnerSpec.Hash,
282+
Prefix: []byte{1},
283+
Suffix: cLeaf,
284+
},
285+
},
286+
}
287+
yHash, _ := aExist.Path[0].Apply(aLeaf)
288+
cExist := ExistenceProof{
289+
Key: []byte("c"),
290+
Value: []byte("c"),
291+
Leaf: leafOp,
292+
Path: []*InnerOp{
293+
{
294+
Hash: spec.InnerSpec.Hash,
295+
Prefix: append([]byte{1}, yHash...),
296+
Suffix: []byte{},
297+
},
298+
},
299+
}
300+
aNotExist := NonExistenceProof{
301+
Key: []byte("a"),
302+
Left: nil,
303+
Right: &bExist,
304+
}
305+
root, err := aExist.Calculate()
306+
if err != nil {
307+
t.Fatal("failed to calculate existence proof of leaf a")
308+
}
309+
310+
expError := fmt.Errorf("inner, %w", errors.New("spec.InnerSpec.MaxPrefixLength must be < spec.InnerSpec.MinPrefixLength + spec.InnerSpec.ChildSize"))
311+
err = aExist.Verify(spec, root, []byte("a"), []byte("a"))
312+
if err.Error() != expError.Error() {
313+
t.Fatal("attempting to prove existence of leaf a returned incorrect error")
314+
}
315+
316+
err = bExist.Verify(spec, root, []byte("b"), []byte("b"))
317+
if err.Error() != expError.Error() {
318+
t.Fatal("attempting to prove existence of leaf b returned incorrect error")
319+
}
320+
321+
err = b2Exist.Verify(spec, root, []byte("b2"), []byte("b2"))
322+
if err.Error() != expError.Error() {
323+
t.Fatal("attempting to prove existence of third leaf returned incorrect error")
324+
}
325+
326+
err = cExist.Verify(spec, root, []byte("c"), []byte("c"))
327+
if err.Error() != expError.Error() {
328+
t.Fatal("attempting to prove existence of leaf c returned incorrect error")
329+
}
330+
331+
err = aNotExist.Verify(spec, root, []byte("a"))
332+
expError = fmt.Errorf("right proof, %w", expError)
333+
if err.Error() != expError.Error() {
334+
t.Fatal("attempting to prove non-existence of leaf a returned incorrect error")
335+
}
336+
}
337+
204338
// generatePrefix generates a valid iavl prefix for an inner op.
205339
func generateInnerOpPrefix() []byte {
206340
var (

‎go/proofs.pb.go

+2-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎proto/cosmos/ics23/v1/proofs.proto

+2-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,8 @@ message InnerSpec {
190190
repeated int32 child_order = 1;
191191
int32 child_size = 2;
192192
int32 min_prefix_length = 3;
193-
int32 max_prefix_length = 4;
193+
// the max prefix length must be less than the minimum prefix length + child size
194+
int32 max_prefix_length = 4;
194195
// empty child is the prehash image that is used when one child is nil (eg. 20 bytes of 0)
195196
bytes empty_child = 5;
196197
// hash is the algorithm that must be used for each InnerOp

‎rust/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
# Unreleased
4+
5+
- fix: guarantee that `spec.InnerSpec.MaxPrefixLength` < `spec.InnerSpec.MinPrefixLength` + `spec.InnerSpec.ChildSize` ([#369](https://github.com/cosmos/ics23/pull/369))
6+
37
# v0.12.0
48

59
- chore(rust): Update `prost` to v0.13 ([#335](https://github.com/cosmos/ics23/pull/335), [#336](https://github.com/cosmos/ics23/pull/336))

‎rust/src/cosmos.ics23.v1.rs

+1
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ pub struct InnerSpec {
180180
pub child_size: i32,
181181
#[prost(int32, tag = "3")]
182182
pub min_prefix_length: i32,
183+
/// the max prefix length must be less than the minimum prefix length + child size
183184
#[prost(int32, tag = "4")]
184185
pub max_prefix_length: i32,
185186
/// empty child is the prehash image that is used when one child is nil (eg. 20 bytes of 0)

‎rust/src/proto_descriptor.bin

82 Bytes
Binary file not shown.

‎rust/src/verify.rs

+27-3
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,11 @@ fn ensure_inner(inner: &ics23::InnerOp, spec: &ics23::ProofSpec) -> Result<()> {
203203
);
204204
ensure!(
205205
inner_spec.child_size > 0,
206-
"spec.InnerSpec.ChildSize must be >= 1"
206+
"spec.inner_spec.child_size must be >= 1"
207+
);
208+
ensure!(
209+
inner_spec.max_prefix_length < inner_spec.min_prefix_length + inner_spec.child_size,
210+
"spec.inner_spec.max_prefix_length must be < spec.inner_spec.min_prefix_length + spec.inner_spec.child_size"
207211
);
208212
ensure!(
209213
inner.suffix.len() % (inner_spec.child_size as usize) == 0,
@@ -484,6 +488,13 @@ mod tests {
484488
depth_limited_spec.min_depth = 2;
485489
depth_limited_spec.max_depth = 4;
486490

491+
let mut max_prefix_length_too_large_spec = api::iavl_spec();
492+
let inner_spec = max_prefix_length_too_large_spec
493+
.inner_spec
494+
.as_mut()
495+
.unwrap();
496+
inner_spec.max_prefix_length = 100;
497+
487498
let cases: HashMap<&'static str, ExistenceCase> = [
488499
(
489500
"empty proof fails",
@@ -612,19 +623,32 @@ mod tests {
612623
proof: ExistenceProof {
613624
key: b"foo".to_vec(),
614625
value: b"bar".to_vec(),
615-
leaf: Some(leaf),
626+
leaf: Some(leaf.clone()),
616627
path: vec![
617628
valid_inner.clone(),
618629
valid_inner.clone(),
619630
valid_inner.clone(),
620631
valid_inner.clone(),
621-
valid_inner,
632+
valid_inner.clone(),
622633
],
623634
},
624635
spec: depth_limited_spec,
625636
valid: false,
626637
},
627638
),
639+
(
640+
"rejects inner spec with max prefix length >= min prefix length + child size",
641+
ExistenceCase {
642+
proof: ExistenceProof {
643+
key: b"foo".to_vec(),
644+
value: b"bar".to_vec(),
645+
leaf: Some(leaf),
646+
path: vec![valid_inner],
647+
},
648+
spec: max_prefix_length_too_large_spec,
649+
valid: false,
650+
},
651+
),
628652
]
629653
.into_iter()
630654
.collect();

0 commit comments

Comments
 (0)