bug: `Commit` panics with "index out of range" when re-committing a variable that is already part of an earlier commitment

[shramee]

Description

TLDR; privateCommittedSeeker.Seek returns index from original commitment indexes, but code tries to find it in the sliced-in-loop commitment indexes.

builder.Commit (Groth16 R1CS builder) panics with a slice-index-out-of-range error when a circuit commits to a variable, and then later commits again to a variable that is already covered by an earlier commitment (i.e. the "re-commit-to-a-commitment" path).

The bug is in frontend/cs/r1cs/api.go, in (*builder[E]).Commit. The function computes existingCommitmentIndexes := commitments.CommitmentIndexes() and then, over the course of the sort/merge loop, progressively slices this same variable down (existingCommitmentIndexes = existingCommitmentIndexes[1:]) as it consumes committed wires in VID order:

https://github.com/Consensys/gnark/blob/master/frontend/cs/r1cs/api.go#L881

commitments := builder.cs.GetCommitments().(constraint.Groth16Commitments)
existingCommitmentIndexes := commitments.CommitmentIndexes()
privateCommittedSeeker := utils.MultiListSeeker(commitments.GetPrivateCommitted())
...
for len(existingCommitmentIndexes) > 0 && existingCommitmentIndexes[0] < t.VID {
    existingCommitmentIndexes = existingCommitmentIndexes[1:]
}
...
if committer := privateCommittedSeeker.Seek(t.VID); committer != -1 {
    committerWireIndex := existingCommitmentIndexes[committer] // BUG
    ...
}

privateCommittedSeeker.Seek(t.VID) returns committer, an index into the original, unsliced list of commitments (it indexes commitments.GetPrivateCommitted(), which has one entry per commitment, in original commitment order). But by the time the loop reaches a given t.VID, existingCommitmentIndexes has already been sliced down (its front elements dropped) by the existingCommitmentIndexes[0] < t.VID loop above. Indexing the sliced slice with an index meant for the original slice reads the wrong element, or — as soon as the slice has been shortened past committer's original position — panics with an out-of-range index.

Expected Behavior

Compiling a circuit that commits to a variable, commits to a second variable, and then commits again to a variable already part of a prior commitment should succeed (gnark redirects the re-commitment to commit to the existing commitment's wire instead), matching the documented behavior of Commit.

Actual Behavior

frontend.Compile panics:

panic: runtime error: index out of range [1] with length 1

goroutine ...:
github.com/consensys/gnark/frontend/cs/r1cs.(*builder[...]).Commit(...)
	frontend/cs/r1cs/api.go:882
github.com/consensys/gnark/std/multicommit.(*multicommitter).commitAndCall(...)
	std/multicommit/multicommit.go:116
github.com/consensys/gnark/frontend.(*parser[...]).parseCircuit.func2(...)
	frontend/compile.go:151

Possible Fix

Keep two separate variables: an immutable copy of the full commitment-index list (to index with committer, which is always relative to the original, unsliced list), and the mutable/sliced view (only used for the existingCommitmentIndexes[0] < t.VID / == t.VID comparisons against the current wire):

allCommitmentIndexes := commitments.CommitmentIndexes()
existingCommitmentIndexes := allCommitmentIndexes
...
if committer := privateCommittedSeeker.Seek(t.VID); committer != -1 {
    committerWireIndex := allCommitmentIndexes[committer] // index into the ORIGINAL list
    ...
}

Steps to Reproduce

1. Define a circuit that issues three Commit calls, where the third
   re-commits a variable already included in the second commitment. And run the test.

   package repro
   
   import (
       "testing"
   
       "github.com/consensys/gnark-crypto/ecc"
       "github.com/consensys/gnark/frontend"
       "github.com/consensys/gnark/frontend/cs/r1cs"
   )
   
   type C struct{ A, B frontend.Variable }
   
   func (c *C) Define(api frontend.API) error {
       cm := api.(frontend.Committer)
       v0 := api.Mul(c.A, c.A)
       if _, err := cm.Commit(v0); err != nil { // 1st commitment
           return err
       }
       v1 := api.Mul(c.B, c.B)
       if _, err := cm.Commit(v1); err != nil { // 2nd commitment, privateCommitted[1] = [v1]
           return err
       }
       _, err := cm.Commit(v1) // re-commit v1 -> triggers the bug
       return err
   }
   
   func TestRepro(t *testing.T) {
       _, err := frontend.Compile(ecc.BN254.ScalarField(), r1cs.NewBuilder, &C{})
       if err != nil {
           t.Fatal(err)
       }
   }

2. Observe panic: runtime error: index out of range [1] with length 1 at
   frontend/cs/r1cs/api.go:881.

3. Applying the fix described above (splitting allCommitmentIndexes from
   existingCommitmentIndexes) makes the same test pass.

Context

This was originally observed while compiling a larger circuit for a multi-round protocol for polynomial ring operation verifications in emulated field arithmetic, via a deferred multicommit.WithCommitment call from std/multicommit. We have provided a minimal test circuit highlighting the issue with the builder logic itself.

Your Environment

• gnark version used (e.g. v0.8.1, HEAD@develop): HEAD@master
• gnark-crypto version used: v0.21.0
• go version (e.g. 1.20.6): go1.26.2 linux/arm64
• Operating System and version: Linux 6.12.76 (container) and MacOS 27 Golden gate