Hash logs inside Noir #1165
Labels
A-security
Area: Relates to security. Something is insecure.
C-aztec.nr
Component: Aztec smart contract framework
Comments
6 tasks
iAmMichaelConnor
modified the milestones:
📣 Emit Events,
🚀 Initial Aztec Sandbox Release
|
Not blocked anymore :) |
iAmMichaelConnor
added
C-aztec.nr
Cool, cool. Anyway, it makes sense to wait for this issue to be resolved before tackling this. |
MirandaWood
added a commit
that referenced
this issue
Apr 17, 2024
Closes #5017 This PR adds side effect counters to logs, tracks them throughout the kernels, sorts them (only in private kernel tail, public kernel tail is todo), and hashes them to one value in the tail. ## Private path Encrypted and unencrypted logs flow via a private call: - A private fn calling `emit_(un)encrypted_log` now actually pushes the hash of the log to the `PrivateContext` as a `SideEffect` - These logs hashes are maintained through `PrivateCircuitPublicInputs` - They are collected in `init` and `inner` kernels - Like note hashes, we now provide index hints in the `tail` and sort the logs hashes - In the final step of `tail`, we either run `to_public`, keeping individual logs hashes, or `finish_tail` (converting `.end` to `CombinedAccumulatedData`), where logs are hashed to a single value - As before, the single value is rolled up and recalculated in L1 ## Public path Unencrypted logs flow via a public call: - A private fn calling `emit_unencrypted_log` now actually pushes the hash of the log to the `PublicContext` as a `SideEffect` - These logs hashes are maintained through `PublicCircuitPublicInputs` - They are collected in `init` and `inner` kernels - Like note hashes, we currently don't sort in the public tail (I've added a ts hack like the note hash one) - In the final step of `tail`, we hash logs to a single value - As before, the single value is rolled up and recalculated in L1 ## TODOs - Since individual logs are not hashed by the circuits (this is part of #1165 which should be done once #1641 is finished), instead a hash is passed via the oracles, the context circuits can't calculate the length of each log. So the length is still passed in. - In the kernels, logs currently sit as `SideEffects` but should be some special counted logs struct. It makes sense to me to first separate logs which are linked to note preimages (no issue, but part of kernel epic), then assign each type of logs its own struct. - I added some hacks/qs (see comments below). --------- Co-authored-by: Leila Wang <leizciw@gmail.com>
MirandaWood
added a commit
that referenced
this issue
May 2, 2024
## A follow-up to #5718: - [x] - Hash logs inside the circuit (closing #1165) Complete, with some caveats. In most cases, whatever log is being broadcast can be converted to bytes with one of the traits (in `log_traits`) and sha hashed. Note we now need these traits because `sha256_slice` has (understandably) been removed, so we must define a fixed length input for generic types. The one exception to this is broadcasting a contract class which ends up emitting up to 518,400 bytes. This is still hashed in ts via a new oracle method `emit_contract_class_unencrypted_log`. I believe it's fine to hash this outside of the circuit, because correctness will be guaranteed by the bytecode commitment (as opposed to a generic log, where we need the hash to come from the circuit to trust it). - [x] - Accumulate logs length inside the circuit Complete, perhaps we should track lengths of each log to more easily split them into revertible/non-revertible later on? - [x] - Flat hash the logs in `tail` + update documentation Now that `sha256_slice` is removed, we would have to flat hash all the empty space up to max logs - unsure whether this would give us any benefit? EDIT: After some testing, it is more efficient for larger numbers of logs (~9+) in both the circuit and L1, so I have implemented flat hashing. - [x] - Add a `logsCache`, like `noteCache`, to track logs + ordering in ts in nested executions Note that the `logsCache` is only implemented (and required) in the private context for now. Public calls don't require squashing and removal of logs representing nullified notes, so an array (`allUnencryptedLogs`) will do. It currently just keeps track of ordering when we have nested calls, but will be useful for removing transient logs (#1641). - [x] - Investigate + solve issue with `tx.ts` error not throwing I'm not sure why this check exists - the comment: ``` // This check is present because each private function invocation creates encrypted FunctionL2Logs object and // both public and private function invocations create unencrypted FunctionL2Logs object. Hence "num unencrypted" // >= "num encrypted". ``` implies that functions must emit both types of logs? A tx with one private call, which emits one encrypted log, should fail here, and I don't see why. EDIT: Have removed the check as it seems redundant. --- Note that in nested calls that have more than one side effect, logs will have duplicate side effect counters and so cannot be sorted correctly (#6052). Currently the logs collected in `allUnencryptedLogs` are the only place that have correctly ordered logs in this case.
|
Closing as it's tackled |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently we inject the logs hashes and lengths in TS here. This was a hack to get the logs working when Noir was too buggy.
Blocked by noir-lang/noir#1889.
The text was updated successfully, but these errors were encountered: