[AI Review] Verkle Trees group offset when indexing sortingKey

[mratsim]

From #609 (review)

Unconfirmed bug reported by AI on Ethereum Verkle Trees

constantine/commitments/eth_verkle_ipa.nim (2)

848-850: ⚠️ Potential issue | 🟠 Major

Fix the stale parameter name in the debug assertion.

opening_challenges is not a parameter of ipa_multi_verify; debug builds should assert against opening_challenges_in_domain.

🐛 Proposed fix

   debug:
-    doAssert commitments.len == opening_challenges.len, "Commitments and opening challenges inputs must be of the same length"
+    doAssert commitments.len == opening_challenges_in_domain.len, "Commitments and opening challenges inputs must be of the same length"
     doAssert commitments.len == evals_at_challenges.len, "Commitments and evaluations at challenges inputs must be of the same length"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@constantine/commitments/eth_verkle_ipa.nim` around lines 848 - 850, The debug
assertions use the stale parameter name opening_challenges; update the first
assertion inside debug to compare commitments.len against
opening_challenges_in_domain.len (the actual parameter passed into
ipa_multi_verify) so it reads that commitments and opening_challenges_in_domain
have the same length; leave the second assertion comparing commitments.len to
evals_at_challenges.len as-is.

---

554-568: ⚠️ Potential issue | 🔴 Critical

Use the current group offset when accumulating grouped entries.

These loops index sortingKeys[i] instead of sortingKeys[q+i], so every group after the first can pull entries from the beginning of the sorted array and corrupt the combined polynomial/evaluation/commitment.

🐛 Proposed fix

       for i in 2 ..< count:
-        fs[idx] += ungrouped_polys[sortingKeys[i].idx]
+        fs[idx] += ungrouped_polys[sortingKeys[q+i].idx]
@@
         for i in 2 ..< count:
-          evals_by_challenges[eidx] += ungrouped_evals[sortingKeys[i].idx]
+          evals_by_challenges[eidx] += ungrouped_evals[sortingKeys[q+i].idx]
@@
         tmp[tidx].fromAffine(ungrouped_comms[sortingKeys[q].idx])
         for i in 1 ..< count:
-          tmp[tidx] ~+= ungrouped_comms[sortingKeys[i].idx]
+          tmp[tidx] ~+= ungrouped_comms[sortingKeys[q+i].idx]

Also applies to: 599-643

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@constantine/commitments/eth_verkle_ipa.nim` around lines 554 - 568, The
grouped-entry accumulation uses the wrong base index into sortingKeys: replace
any use of sortingKeys[i] inside the grouped accumulation loop with
sortingKeys[q + i] so each group's entries are taken relative to the current
group offset q (affecting the fs accumulation that reads
ungrouped_polys[sortingKeys[...].idx] and similar uses); update the same pattern
in the corresponding block around the later range (the other occurrence
mentioned) so both grouped loops use sortingKeys[q + i] rather than
sortingKeys[i].

[coderabbitai]

🔗 Related PRs

#589 - [EC] Fix mutable assignment in batch affine - followup #579 [merged]
#598 - Workaround GMP regressions and breaking changes from subsetpark/nim-gmp#3 [merged]
#599 - Fix non-assembly reduction for secp256k1 [merged]
#602 - Fix task failures behind swallowed [merged]
#607 - Replace GT Pairing + isOne().bool() pattern with pairing_check [PeerDAS streamlining] [closed]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!