TLDR: the op queue and the interactions around it are a mess.
As part of the "avoid zero commitments" issue, we add "mock" (arbitrary) ecc ops to Mega circuits in two places, both in the Decider PK constructor: 1) Via call to method op_queue->append_nonzero_ops() (see issue and comments here) and 2) via some ops added in MegaCircuitBuilder::add_gates_to_ensure_all_polys_are_non_zero(). Ok, fine. Now, in ClientIvc::accumulate(circuit) we call goblin.merge(circuit) prior to constructing an instance. So if no genuine ecc ops were added to the circuit during its construction, the merge protocol will try to 'merge' nothing, and this will lead to the commitment C_t_shift being the point at infinity.
One option would be to simply not run the merge prover if there are no ops to merge, but this leads to some icky issues because we ARE adding ecc ops to EVERY circuit, just sometimes not until after we've run the merge prover.
Possibly the right thing involves two updates: 1) split up the functionality in goblin.merge() into two: recursive merge verification of the previous merge proof, and construction of a new merge proof. Run the former prior to constructing an instance (since it adds gates) and run the latter AFTER. 2) Only run the merge prover if the circuit has ops to merge. (Right now they all have ops to avoid zero commitments but eventually some won't).
TLDR: the op queue and the interactions around it are a mess.
As part of the "avoid zero commitments" issue, we add "mock" (arbitrary) ecc ops to Mega circuits in two places, both in the Decider PK constructor: 1) Via call to method op_queue->append_nonzero_ops() (see issue and comments here) and 2) via some ops added in
MegaCircuitBuilder::add_gates_to_ensure_all_polys_are_non_zero(). Ok, fine. Now, inClientIvc::accumulate(circuit)we callgoblin.merge(circuit)prior to constructing an instance. So if no genuine ecc ops were added to the circuit during its construction, the merge protocol will try to 'merge' nothing, and this will lead to the commitmentC_t_shiftbeing the point at infinity.One option would be to simply not run the merge prover if there are no ops to merge, but this leads to some icky issues because we ARE adding ecc ops to EVERY circuit, just sometimes not until after we've run the merge prover.
Possibly the right thing involves two updates: 1) split up the functionality in goblin.merge() into two: recursive merge verification of the previous merge proof, and construction of a new merge proof. Run the former prior to constructing an instance (since it adds gates) and run the latter AFTER. 2) Only run the merge prover if the circuit has ops to merge. (Right now they all have ops to avoid zero commitments but eventually some won't).