Skip to content

fix(test): row-pair leaf layout in merkle_root_parity GPU parity tests#745

Open
MauroToscano wants to merge 1 commit into
mainfrom
fix/merkle-parity-row-pair
Open

fix(test): row-pair leaf layout in merkle_root_parity GPU parity tests#745
MauroToscano wants to merge 1 commit into
mainfrom
fix/merkle-parity-row-pair

Conversation

@MauroToscano

Copy link
Copy Markdown
Contributor

What

Two GPU↔CPU root-parity cases in crypto/math-cuda/tests/merkle_root_parity.rs compare against the wrong Merkle leaf layout on main:

  • gpu_and_cpu_row_major_merkle_roots_match
  • gpu_and_cpu_ext3_merkle_roots_match

Their GPU helpers (gpu_merkle_root / gpu_ext3_merkle_root) call keccak_leaves_base / keccak_leaves_ext3 with rows_per_leaf = 1 (per-row → num_rows leaves), but the CPU reference commit_rows_bit_reversed uses the row-pair layout (ROWS_PER_LEAF = 2num_rows / 2 leaves, each hashing a bit-reversed row pair). Different leaf count and different leaf bytes ⇒ different root, so these two cases can never match main's row-pair commitment scheme.

Fix

Pass rows_per_leaf = 2 at both call sites so the generic GPU keccak-leaves + Merkle path uses the same bit-reversed row-pair layout as the CPU commit. (Plus updated the now-inaccurate "per-row" comments.)

Why this is a test fix, not a production bug

  • Test-only surface. keccak_leaves_base / keccak_leaves_ext3 have no production callers — the proving path uses the fused coset_lde_*_row_major_with_merkle_tree_keep, which is already row-pair. GPU/CPU proof parity was never affected, which is exactly why proofs verify with and without the GPU.
  • The row-pair primitive is already proven. tests/keccak_leaves.rs::keccak_leaves_base_row_pair_matches_cpu (and the ext3 equivalent) already verify that keccak_leaves_base(.., 2) matches the CPU row-pair prover helper.
  • The production-path cases in this same file already pass. new_row_major_pipeline_base_root_matches_cpu / …_ext3_… exercise the real fused pipeline (row-pair on both sides) and are green.

Background

These two cases were introduced in #715 (per-row GPU leaves). #735 ("unify & clean up the commitment layer") moved the scheme to row-pair — adding ROWS_PER_LEAF = 2, the row-pair leaf kernels, and a row-pair commit_rows_bit_reversed — but left these two test calls at the old rows_per_leaf = 1. This realigns them. The mismatch went unnoticed because GPU tests don't run in CI (no nvcc).

Testing

Not run locally — no GPU/CUDA on the dev box, and math-cuda only builds under the cuda feature. To be confirmed on a GPU machine: all four cases in merkle_root_parity.rs should pass. rustfmt clean.

gpu_merkle_root / gpu_ext3_merkle_root called keccak_leaves_base/ext3 with
rows_per_leaf=1 (per-row, num_rows leaves) but compare the resulting root
against the CPU commit_rows_bit_reversed, which uses the row-pair layout
(ROWS_PER_LEAF=2, num_rows/2 leaves, each hashing a bit-reversed row pair).
Different leaf count and bytes => different root, so the two cases never
matched main's row-pair commitment scheme.

Pass rows_per_leaf=2 so the generic GPU keccak-leaves + Merkle path uses the
same bit-reversed row-pair layout as the CPU reference. keccak_leaves.rs
already proves keccak_leaves_base(.., 2) matches the CPU row-pair prover, and
the production-pipeline parity cases (new_row_major_pipeline_*) already pass,
so this only realigns the generic-helper cases with main.

Test-only change; the proving path uses the fused row-pair pipeline and is
unaffected. GPU tests don't run in CI (no nvcc) — to be confirmed on a GPU.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant