Simple range proof verification by rkuris · Pull Request #1566 · ava-labs/firewood

rkuris · 2025-12-17T20:05:44Z

Why this should be merged

Partial verification of range proofs, without generating tries and checking hashes.

How this works

The current implementation provides a foundation for range proof verification by:

Checking structural validity (key ordering, non-empty proof)
Validating that the requested range matches the provided proof
Cryptographically verifying the start and end boundary proofs

What's NOT included (tracked in #738):

Full trie reconstruction from the proof
Verification of intermediate nodes between boundaries
Complete validation that no keys are missing within the proven range

How this was tested

The test suite in range.rs, which includes tests for:

Valid range proofs with various configurations
Invalid proofs (modified keys, values, gaps, etc.)
Edge cases (single element, empty ranges, full trie)

Note: Most tests are currently #ignore because they are not checked yet and will be fixed in followup PRs.

Copilot

Pull request overview

This PR implements partial range proof verification for merkle tries, focusing on structural validation and boundary proof verification without full trie reconstruction.

Key Changes:

Implements basic range proof verification checking key ordering, range boundaries, and cryptographic proofs
Enables previously ignored tests for range proof validation
Adjusts test logic to skip invalid test cases that would fail with current partial implementation

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
firewood/src/merkle/mod.rs	Implements verify_range_proof with key ordering checks, boundary validation, and start/end proof verification
firewood/src/merkle/tests/range.rs	Removes #[ignore] attributes and adds continue statements to skip test cases that require full verification
firewood/src/lib.rs	Changes default logging level from "trace" to "info"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-17T22:26:46Z

firewood/src/merkle/tests/range.rs

    let merkle = init_merkle(items.clone());

-    for _ in 0..10 {
+    'skip_test: for _ in 0..10 {


The label 'skip_test' is misleading since it doesn't skip the test itself, but rather continues to the next iteration. Consider renaming to 'next_iteration' or 'retry' to better reflect its purpose.

Suggested change

'skip_test: for _ in 0..10 {

'next_iteration: for _ in 0..10 {

Copilot · 2025-12-17T22:26:46Z

firewood/src/merkle/tests/range.rs

            0 => {
                // Modified key
                keys[index] = rng.random::<[u8; 32]>(); // In theory it can't be same
+                continue 'skip_test;


These continue statements skip validation of intentionally corrupted proofs. The test cases for modified keys, modified values, gapped entries, empty keys, and nil values are no longer being verified as invalid, which defeats the purpose of test_bad_range_proof. These should either be properly validated or the test should remain ignored until full verification is implemented.

The whole point of this PR is to enable the parts of the test that do work, and out of order is an integral part of this test. The others will get skipped.

firewood/src/merkle/tests/range.rs

firewood/src/merkle/mod.rs

Copilot · 2025-12-17T22:26:48Z

firewood/src/merkle/mod.rs

+                start_key: right.0.as_ref().to_vec().into(),
+                end_key: requested_last.as_ref().to_vec().into(),


Similar to the first_key validation, this uses InvalidRange incorrectly. The start_key is set to right.0 and end_key to requested_last, which reverses the logical ordering and misrepresents the nature of the error (proof boundary validation vs request validation).

Suggested change

start_key: right.0.as_ref().to_vec().into(),

end_key: requested_last.as_ref().to_vec().into(),

start_key: requested_last.as_ref().to_vec().into(),

end_key: right.0.as_ref().to_vec().into(),

rkuris · 2025-12-17T22:26:50Z

firewood/src/lib.rs

 /// looking elsewhere.
 fn init_logger() {
-    env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("trace"))
+    env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("info"))


trace logging for all tests is very expensive, noticed a big improvement here.

Sorry to sneak this in to this PR, really it belongs as a separate PR. Complain if you agree and I'll move it.

demosdemon · 2025-12-18T16:14:53Z

firewood/src/merkle/mod.rs

+        proof
+            .start_proof()
+            .verify(&left.0, Some(&left.1), root_hash)?;


This is incorrect. When we generate proofs, we always generate a proof for the lower bound. The proof for the lower bound may or may not prove the first key provided in the range. If the lower bound does not exist, the provided proof may not have all the information required to prove the inclusion of the first key.

Seems like we should perhaps construct the key provided and make sure its <= to the first key.

demosdemon · 2025-12-18T16:15:57Z

firewood/src/merkle/mod.rs

+        proof
+            .end_proof()
+            .verify(&right.0, Some(&right.1), root_hash)?;


Similarly here, when we generate the upper bound proof, we generate a proof for the final key in the yielded range only if we truncated the range. Otherwise, we generate a proof for the requested upper bound. This proof may or may not be sufficient to verify the final yielded key.

demosdemon · 2025-12-18T16:20:08Z

firewood/src/merkle/mod.rs

+        // check that the start and end proofs are valid
+        let left = key_values
+            .first()
+            .ok_or(api::Error::ProofError(ProofError::Empty))?;


This is incorrect. An empty set of key-values is only invalid if both key bounds are empty. An empty set of key-values with a bounded range correctly represents an empty set of key-values between the two keys.

RodrigoVillar · 2025-12-18T19:28:45Z

firewood/src/merkle/tests/range.rs

 // Tests a few cases which the proof is wrong.
 // The prover is expected to detect the error.
-#[ignore = "https://github.com/ava-labs/firewood/issues/738"]
 fn test_bad_range_proof() {


(q / feel-free-to-ignore): I wonder if it's cleaner to split this test into two - one test that tests with verification and one test that doesn't. Having the 'skip_test loop label is hard to follow considering that we also have continue statements which should be kept around.

bernard-avalabs · 2026-02-13T18:00:56Z

firewood/src/merkle/mod.rs

-        todo!()
+        // check that the keys are in ascending order
+        let key_values = proof.key_values();
+        if !key_values


nit: Using is_sorted_by is slightly simpler:

if !key_values .iter() .is_sorted_by(|a, b| a.0.as_ref() < b.0.as_ref())

github-actions · 2026-02-16T13:55:56Z

Metrics Change Detection ⚠️

This PR contains changes related to metrics:

-            ::metrics::gauge!($name).set($value);
+            ::metrics::gauge!($name).set($value as f64);
-        ::metrics::gauge!($name).set($value)
+        ::metrics::gauge!($name).set($value as f64)
-        ::metrics::gauge!($name, $($labels)+).set($value)
+        ::metrics::gauge!($name, $($labels)+).set($value as f64)
+    describe_gauge!(
+    describe_gauge!(
+    describe_gauge!(

However, the dashboard was not modified.

You may need to update benchmark/Grafana-dashboard.json accordingly.

This check is automated to help maintain the dashboard.

rkuris self-assigned this Dec 17, 2025

rkuris requested a review from demosdemon as a code owner December 17, 2025 20:05

rkuris force-pushed the rkuris/simple-range-proof-verification branch from 9737ecc to e7f2062 Compare December 17, 2025 21:41

rkuris requested a review from Copilot December 17, 2025 22:25

Copilot AI reviewed Dec 17, 2025

View reviewed changes

rkuris commented Dec 17, 2025

View reviewed changes

rkuris added the enhancement New feature or request label Dec 17, 2025

rkuris requested a review from RodrigoVillar as a code owner December 18, 2025 15:26

demosdemon requested changes Dec 18, 2025

View reviewed changes

RodrigoVillar reviewed Dec 18, 2025

View reviewed changes

rkuris force-pushed the rkuris/simple-range-proof-verification branch from ca332e5 to 9292db6 Compare January 28, 2026 02:29

WIP

90fb040

rkuris force-pushed the rkuris/simple-range-proof-verification branch from f5c42dc to 90fb040 Compare February 3, 2026 18:45

rkuris requested a review from bernard-avalabs as a code owner February 3, 2026 18:45

Merge branch 'main' into rkuris/simple-range-proof-verification

7760bf7

bernard-avalabs reviewed Feb 13, 2026

View reviewed changes

Merge branch 'main' into rkuris/simple-range-proof-verification

db1a32b

	'skip_test: for _ in 0..10 {
	'next_iteration: for _ in 0..10 {

		start_key: right.0.as_ref().to_vec().into(),
		end_key: requested_last.as_ref().to_vec().into(),

Conversation

rkuris commented Dec 17, 2025

Why this should be merged

How this works

How this was tested

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

rkuris Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

rkuris Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

demosdemon Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

rkuris Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

demosdemon Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

demosdemon Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RodrigoVillar Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bernard-avalabs Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 16, 2026

Metrics Change Detection ⚠️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

demosdemon Dec 18, 2025 •

edited

Loading

RodrigoVillar Dec 18, 2025 •

edited

Loading