Skip to content

Conversation

@ivokub
Copy link
Collaborator

@ivokub ivokub commented Oct 22, 2025

Description

We have implemented the MulAcc method for the circuit builder API. It is useful in the context of R1CS where we can reuse existing variables when they have sufficient capacity (recall variable in R1CS circuit is slice of terms, where term is cofficient/variable pair).

But for fixed operations (Sub(x,x), IsZero(1)) we return the stored local linear expressions ([]LinearExpression{Coefficient:0, Wire:0}. Now MulAcc also overwrite this, but this changes the hardcoded constant LE.

This PR avoid overwrite when the accumulator in MulAcc is not 0 or 1 constant.

Thanks @wuestholz for reporting.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested?

  • TestOverrideZeroConstant

How has this been benchmarked?

Simple circuit:

type manyMulaccsCircuit struct {
	MulAccL [1000000]frontend.Variable
	MulAccR [1000000]frontend.Variable
	Seed    frontend.Variable `gnark:",public"`
	Res     frontend.Variable `gnark:",public"`
}

func (c *manyMulaccsCircuit) Define(api frontend.API) error {
	acc := c.Seed
	for i := 0; i < 1000000; i++ {
		acc = api.MulAcc(acc, c.MulAccL[i], c.MulAccR[i])
	}
	api.AssertIsEqual(c.Res, acc)
	return nil
}

Compile time:

              │   old.txt   │           new.txt            │
              │   sec/op    │   sec/op     vs base         │
ManyMulAcc-16   1.818 ± 13%   1.811 ± 13%  ~ (p=0.699 n=6)

              │   old.txt    │            new.txt            │
              │     B/op     │     B/op      vs base         │
ManyMulAcc-16   1.174Gi ± 0%   1.174Gi ± 0%  ~ (p=0.589 n=6)

              │   old.txt   │           new.txt            │
              │  allocs/op  │  allocs/op   vs base         │
ManyMulAcc-16   20.02M ± 0%   20.02M ± 0%  ~ (p=0.515 n=6)

solving time:

                   │ old-solve.txt │        new-solve.txt         │
                   │    sec/op     │   sec/op     vs base         │
ManyMulAccSolve-16    65.96m ± 12%   65.59m ± 6%  ~ (p=0.937 n=6)

                   │ old-solve.txt │         new-solve.txt         │
                   │     B/op      │     B/op      vs base         │
ManyMulAccSolve-16    190.5Mi ± 0%   190.5Mi ± 0%  ~ (p=0.937 n=6)

                   │ old-solve.txt │        new-solve.txt         │
                   │   allocs/op   │  allocs/op   vs base         │
ManyMulAccSolve-16     110.5 ± 12%   110.0 ± 14%  ~ (p=0.868 n=6)

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I did not modify files generated from templates
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Note

Cursor Bugbot is generating a summary for commit 16107cf. Configure here.

@ivokub ivokub added this to the v0.15.0 milestone Oct 22, 2025
@ivokub ivokub self-assigned this Oct 22, 2025
@ivokub ivokub added type: bug Something isn't working src: fuzzing Issue found using a fuzzing tool labels Oct 22, 2025
@ivokub ivokub marked this pull request as ready for review October 23, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

src: fuzzing Issue found using a fuzzing tool type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant