Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 22 additions & 11 deletions prover/cmd/prover/cmd/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,23 @@ var AllCircuits = []circuits.CircuitID{
}

// PayloadCircuits defines the ordered list of payload circuits that can be aggregated.
// This order corresponds to circuit IDs 0-5 in GlobalCircuitIDMapping.
// This order corresponds to circuit IDs 0-13 in GlobalCircuitIDMapping.
// Infrastructure circuits (emulation, aggregation, pi-interconnection, emulation-dummy) are NOT included here.
var PayloadCircuits = []string{
"execution-dummy", // ID 0
"data-availability-dummy", // ID 1
"execution", // ID 2
"execution-large", // ID 3
"execution-limitless", // ID 4
"data-availability-v2", // ID 5
"execution-dummy", // ID 0
"data-availability-dummy", // ID 1
"execution", // ID 2
"execution-large", // ID 3
"execution-limitless", // ID 4
"data-availability-v2", // ID 5
"invalidity-nonce-balance-dummy", // ID 6
"invalidity-precompile-logs-dummy", // ID 7
"invalidity-filtered-address-dummy", // ID 8
"invalidity-nonce-balance", // ID 9
"invalidity-precompile-logs", // ID 10
"invalidity-filtered-address", // ID 11
"invalidity-precompile-logs-limitless", // ID 12
"invalidity-precompile-logs-large", // ID 13
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swapped order of large and limitless invalidity circuits

High Severity

PayloadCircuits lists "invalidity-precompile-logs-limitless" at index 12 and "invalidity-precompile-logs-large" at index 13, but AllCircuits has the opposite order — InvalidityPrecompileLogsLargeCircuitID at position 12 and InvalidityPrecompileLogsLimitlessCircuitID at position 13. If GlobalCircuitIDMapping assigns IDs consistent with AllCircuits, this swap causes verifying keys to be collected at wrong indices, leading to VK digest mismatches during aggregation proof generation.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 433a014. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

False positive. PayloadCircuits order matches GlobalCircuitIDMapping (the canonical ID source), not AllCircuits. AllCircuits has no positional ID semantics — it's just a set for the --circuits flag. This is verified by TestPayloadCircuitsMatchGlobalMapping

}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already addressed. We added TestPayloadCircuitsMatchGlobalMapping which does exactly what it asks.

// Setup orchestrates the setup process for specified circuits, ensuring assets are generated or updated as needed.
Expand Down Expand Up @@ -146,7 +154,7 @@ func Setup(ctx context.Context, args SetupArgs) error {
return fmt.Errorf("%s failed to load public input interconnection setup: %w", cmdName, err)
}

// Collect verifying keys for payload circuits only (IDs 0-5)
// Collect verifying keys for payload circuits only (IDs 0-13)
// The IsAllowedCircuitID bitmask in the config determines which ones are actually allowed at runtime
payloadVks, err := collectPayloadVerifyingKeys(ctx, cfg, srsProvider)
if err != nil {
Expand Down Expand Up @@ -398,10 +406,10 @@ func createCircuitBuilder(c circuits.CircuitID, cfg *config.Config, args SetupAr
}
}

// collectPayloadVerifyingKeys gathers verifying keys for payload circuits only (IDs 0-5).
// collectPayloadVerifyingKeys gathers verifying keys for payload circuits only (IDs 0-13).
// These are the circuits that can be aggregated. Infrastructure circuits (emulation,
// aggregation, pi-interconnection, emulation-dummy) are excluded.
// The returned slice is indexed by payload circuit ID (0-5).
// The returned slice is indexed by payload circuit ID (0-13).
func collectPayloadVerifyingKeys(ctx context.Context, cfg *config.Config, srsProvider circuits.SRSProvider) ([]plonk.VerifyingKey, error) {
payloadVks := make([]plonk.VerifyingKey, len(PayloadCircuits))

Expand Down Expand Up @@ -439,7 +447,10 @@ func collectPayloadVerifyingKeys(ctx context.Context, cfg *config.Config, srsPro
// Note: emulation-dummy is NOT a payload dummy - it's an infrastructure circuit dummy.
func isPayloadDummyCircuit(cID string) bool {
switch circuits.CircuitID(cID) {
case circuits.ExecutionDummyCircuitID, circuits.DataAvailabilityDummyCircuitID:
case circuits.ExecutionDummyCircuitID, circuits.DataAvailabilityDummyCircuitID,
circuits.InvalidityNonceBalanceDummyCircuitID,
circuits.InvalidityPrecompileLogsDummyCircuitID,
circuits.InvalidityFilteredAddressDummyCircuitID:
return true
}
return false
Expand Down
39 changes: 39 additions & 0 deletions prover/cmd/prover/cmd/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,45 @@ func pow5(s []frBls.Element) []frBls.Element {
return res
}

// TestPayloadCircuitsMatchGlobalMapping validates that PayloadCircuits is consistent
// with GlobalCircuitIDMapping. This prevents the bug where new circuits are added to
// GlobalCircuitIDMapping but not to PayloadCircuits, causing VK digest mismatches at
// aggregation proof time.
func TestPayloadCircuitsMatchGlobalMapping(t *testing.T) {
// Determine expected payload circuit count: all circuits with ID < 14 (infrastructure threshold)
const infrastructureThreshold = 14
expectedPayload := make(map[string]uint)
for name, id := range circuits.GlobalCircuitIDMapping {
if id < infrastructureThreshold {
expectedPayload[name] = id
}
}

// PayloadCircuits must contain exactly the non-infrastructure circuits
assert.Equal(t, len(expectedPayload), len(PayloadCircuits),
"PayloadCircuits has %d entries but GlobalCircuitIDMapping has %d circuits with ID < %d",
len(PayloadCircuits), len(expectedPayload), infrastructureThreshold)

// Each entry in PayloadCircuits must exist in GlobalCircuitIDMapping with matching index
for i, name := range PayloadCircuits {
id, exists := circuits.GlobalCircuitIDMapping[name]
require.True(t, exists,
"PayloadCircuits[%d] = %q is not in GlobalCircuitIDMapping", i, name)
assert.Equal(t, uint(i), id,
"PayloadCircuits[%d] = %q has GlobalCircuitIDMapping ID %d (expected %d)", i, name, id, i)
}

// Every non-infrastructure circuit in GlobalCircuitIDMapping must be in PayloadCircuits
for name, id := range circuits.GlobalCircuitIDMapping {
if id >= infrastructureThreshold {
continue
}
if int(id) >= len(PayloadCircuits) || PayloadCircuits[id] != name {
t.Errorf("GlobalCircuitIDMapping[%q] = %d is not at PayloadCircuits[%d]", name, id, id)
}
}
}

// circuitNameToIdx returns the index of a circuit name in the allowedInputs slice
func circuitNameToIdx(allowedInputs []string, name string) int {
for i, n := range allowedInputs {
Expand Down
Loading