feat: recording circuit inputs + oracles

[benesjan]

Fixes #11855

This is an example of a (shortened) recoding:

{
  "circuitName": "AMM",
  "functionName": "add_liquidity",
  "bytecodeMd5Hash": "b46c640ed38f20eac5f61a5e41d8dd1e",
  "timestamp": 1740691464360,
  "inputs": {
    "0": "0x1e89de1f0ad5204263733b7ddf65bec45b8f44714a4da85a46474dad677679ef",
    "1": "0x00f4d59c0ff773427bb0fed5b422557ca4dc5655abe53d31fa9408cb3c5a672f",
    "5": "0x000000000000000000000000000000000000000000000000000000000000000f"
  },
  "oracleCalls": [
    {
      "name": "loadCapsule",
      "inputs": [
        [
          "0x102422483bad6abd385948435667e144ac4c272576e325e7563608876cd446fd"
        ],
        [
          "0x000000000000000000000000000000000000000000000000000000000000004d"
        ],
        [
          "0x0000000000000000000000000000000000000000000000000000000000000001"
        ]
      ],
      "outputs": [
        "0x0000000000000000000000000000000000000000000000000000000000000000",
        [
          "0x0000000000000000000000000000000000000000000000000000000000000000"
        ]
      ]
    },
    {
      "name": "syncNotes",
      "inputs": []
    }
  ]
}

[benesjan]

• feat: recording circuit inputs + oracles #12148 👈 (View in Graphite)
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[benesjan]

I introduced NoirCompiledCircuitWithName which as the name implies is just NoirCompiledCircuit type with the name. This was necessary as I needed the name in the circuit recorder.

[benesjan]

This was incorrect as it doesn't seem to ever be used with contracts. There is a lot of tech debt around.

[benesjan]

Not sure why this change got triggered here - I just bootstrapped and it happened

[benesjan]

This param was never used so I nuked it.

[Thunkar]

This is very bad, can we try not to propagate the foreignCallHandler to all the call sites? It's actually the reason why we need a package split (the /server one pulls from the blobs package) and besides not being very clean (why have different providers if you're feeding the only difference they have externally?), I fear we are going to end up with a /server autoimport somewhere we shouldn't

[benesjan]

The server foreign_call_handler is injected only in the tets_circuit_prover.ts so I don't see how is this a problem.

The SimulationProvider already accepted the callback for user circuits before my changes:

(why have different providers if you're feeding the only difference they have externally?),

It's not the only difference. WASMSimulator.executeProtocolCircuit runs the ACVM in WASM, NativeACVMSimulator runs it in NativeACVM.

But I see that WASMSimulatorWithBlobs became pointless. I would argue that this means we can just nuke it and not that my change is bad.

WDYT?

[Thunkar]

It's just that previously all the functionality was contained inside the simulation provider, as externally no one should have to be concerned with the foreign calls...every provider used the appropriate one and that was the argument that you gave PXE/the provers. Now that abstraction is leaking and you could end up in a situation in which you have a wasm browser provider being fed a server foreign call.

[benesjan]

I see. If we nuke the WASMSimulatorWithBlobs then essentially the dev should not make a mistake in the choice of the foreign call handler but he could no longer make a mistake in the choice of the simulator provider as there would only be 1 (WASMSimulator).

Given that I don't really see how better to approach this (because I need to hijack both the circuit inputs and the foreign call handler and for that the SimulationProvider recording wrapper seems to be the best tradeoff to me) I would just nuke the WASMSimulatorWithBlobs and rename the 2 functions called foreignCallHandler to be more descriptive.

WDYT?

[Thunkar]

Not super stoked, but go for it

[benesjan]

Will wait for Nico's review as well before doing that

[nventuro]

I'm afraid I don't understand enough about the simulators to comment on this. We can talk about it.