Commit 4be7c4c
authored
chore: add tests around MSM invalid scalar handling (#23176)
## Summary
Adds focused DSL tests for the `MultiScalarMul` ACIR opcode covering
Grumpkin scalars that lie **outside the Grumpkin scalar field** (i.e.
`>= bb::fq::modulus`), and writes up the resulting safety analysis. No
production code changes — this is test coverage plus a small refactor to
share the existing single-term MSM circuit helpers.
### What changed
`barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.test.cpp`:
- Extracted the single-term MSM circuit helpers (`push_point`,
`push_scalar`, `make_msm`, `run_circuit`, CRS setup) out of
`MultiScalarMulInfinityTests` into a shared base fixture
`MsmSingleTermFixture<Builder>`. `MultiScalarMulInfinityTests` now
derives from it; behaviour and test names are unchanged.
- New suite `MultiScalarMulScalarBoundsTests` (run for both
`UltraCircuitBuilder` and `MegaCircuitBuilder`), driving the opcode
through the normal ACIR → circuit path with `predicate = 1`:
| Test | Scalar value `v` | Expectation |
|---|---|---|
| `ScalarEqualToModulusFails` | `r` | circuit unsatisfiable |
| `ScalarModulusPlusOneFails` | `r + 1` | circuit unsatisfiable |
| `ScalarModulusMinusOneProves` | `r - 1` (largest in-field scalar) |
proves (baseline) |
| `MaxRepresentableScalarFails` | `2^254 - 1` (limbs in range, value `>
r`) | circuit unsatisfiable |
| `ScalarWithOversizedHiLimbFails` | `2^254` (`hi` limb `= 2^126`, 127
bits) | circuit unsatisfiable |
| `AddingGrumpkinModulusDoesNotReproveSameOutput` | `5` vs `5 + r` | `5`
proves the output; `5 + r` does not, even though `(5+r)·P = 5·P` |
Here `r == bb::fq::modulus` is the Grumpkin scalar field modulus, which
is also the order of the Grumpkin group (Grumpkin's scalar field is
BN254's base field). For the rejected cases the test claims the
mathematically-correct reduced result `(v mod r)·P`, so the rejection is
purely from the scalar-validity constraints, not a mismatched output.
## Test commands and results
```
cd barretenberg/cpp
cmake --preset default
cd build && ninja dsl_tests
./bin/dsl_tests --gtest_filter='*MultiScalarMulScalarBoundsTests*:*MultiScalarMulInfinityTests*'
```
Result: **18/18 passed** (6 new tests × 2 builders, plus the 6
pre-existing infinity tests confirming the refactor is
behaviour-preserving). For the rejected-scalar cases the circuit checker
reports `circuit contains invalid witnesses: field_t::range_constraint`
— the borrow-comparison range checks added by `cycle_scalar`'s
`validate_scalar_is_in_field`.
Running the full `*MultiScalarMul*` filter also surfaces 8 failures in
`MultiScalarMul{None,Points,Scalars,Both}Constant.GenerateVKFromConstraints`
— these are **pre-existing and unrelated**: they throw `bn254 g1 data
not found at /aztec-packages-private/.bb-crs` because the BN254 G1 CRS
is not downloaded in this environment (VK generation needs it;
`CircuitChecker`-based tests don't). They fail identically without this
change.
## Safety report: out-of-field Grumpkin scalars in the MSM opcode
### Background
The `MultiScalarMul` opcode (`dsl/acir_format/multi_scalar_mul.cpp`)
computes `sum(scalars[i] · points[i])` on Grumpkin. Each scalar is
supplied as two field-element limbs — `scalar_lo` (low 128 bits) and
`scalar_hi` (next 126 bits) — and reconstructed as `v = lo + hi ·
2^128`. The limb encoding can represent any value in `[0, 2^254)`, which
strictly contains `[0, r)` (`r ≈ 2^253.2`), so non-canonical scalars are
expressible.
`to_grumpkin_scalar` (`dsl/acir_format/witness_constant.cpp`) builds a
`cycle_scalar` from the two limbs via the public `cycle_scalar(lo, hi)`
constructor, which calls `validate_scalar_is_in_field()`. That adds
in-circuit constraints (via `validate_split_in_field_unsafe`, a
borrow-subtraction comparison) enforcing `v < r`. Separately,
`cycle_group::batch_mul` range-constrains the limbs to `lo < 2^128` and
`hi < 2^126` as part of the MSM algorithm — and
`validate_split_in_field_unsafe` relies on exactly those range
constraints being applied.
### Answers
1. **Is it impossible to prove a circuit which is passed an invalid
Grumpkin scalar?** Yes. Any scalar `v >= r` makes the circuit
unsatisfiable: `validate_scalar_is_in_field` is violated (its hi/lo
borrow range checks cannot all hold), so no satisfying witness exists —
neither an honest nor a dishonest prover can produce a valid proof. A
value `>= 2^254` additionally violates the `hi < 2^126` limb range
constraint applied by `batch_mul`. The check is sound because the limb
range constraints it depends on are applied unconditionally inside the
MSM. (When `predicate` is witness-false the scalar is first replaced by
the constant `1`, and during VK generation the limbs are overwritten
with a dummy in-field value — so neither path can smuggle an
out-of-field scalar past the check.)
2. **Can adding the Grumpkin modulus to a scalar prove the same
output?** No. Although `s·P = (s + r)·P` in the group, the circuit
enforces a canonical scalar `< r`, so `s + r` (which is `>= r`) is
rejected. There is no representable witness that both equals `s + r` and
passes `validate_scalar_is_in_field`. Caller-side malleability of the
scalar therefore cannot produce an alternate proof of an existing MSM
output. `AddingGrumpkinModulusDoesNotReproveSameOutput` demonstrates
both halves of this.
3. **Other edge cases / boundary values.** `r - 1` is the largest
accepted scalar and proves correctly; `r`, `r + 1`, and `2^254 - 1` are
all rejected — the last shows the limb range constraints alone aren't
enough and the explicit `< r` check does the work; `2^254` is rejected
by both the limb range constraint and the field check. `0` is accepted
and yields the point at infinity (already covered by the existing
`ResultIsInfinity` test).
### Conclusion
Allowing invalid Grumpkin scalars through to barretenberg is **safe** in
the soundness sense. If a caller (Noir, hand-written ACIR, or a
buggy/malicious frontend) hands the MSM opcode a scalar outside `[0,
r)`, the worst that happens is the circuit becomes unsatisfiable and
proving fails — fail-closed. There is no soundness gap: an out-of-field
scalar is not silently reduced into a different valid statement, and `s
+ r` cannot stand in for `s`. So canonicalising scalars does not have to
be done entirely on the caller side; barretenberg's in-circuit
`validate_scalar_is_in_field` is a real backstop. The only caveat is a
liveness one — a frontend that emits a non-canonical scalar will produce
circuits that cannot be proven, which is the desired behaviour.
---
*Created by
[claudebox](https://claudebox.work/v2/sessions/bebfc4a2ecc4b733) ·
group: `cli-startup-research`*1 parent 26f56c8 commit 4be7c4c
1 file changed
Lines changed: 137 additions & 1 deletion
Lines changed: 137 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
341 | 341 | | |
342 | 342 | | |
343 | 343 | | |
344 | | - | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
345 | 347 | | |
346 | 348 | | |
347 | 349 | | |
| |||
402 | 404 | | |
403 | 405 | | |
404 | 406 | | |
| 407 | + | |
| 408 | + | |
405 | 409 | | |
406 | 410 | | |
407 | 411 | | |
| |||
449 | 453 | | |
450 | 454 | | |
451 | 455 | | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
0 commit comments