feat(Event_Logs): Remove unencrypted emit in private

In PrivateContext we got emit_unencrypted_log. To reduce the option for error, I think it might be the best idea (as it is also written in the comment above the code) to kill it. It lures people in, and can wreck them. Also, if you "REALLY" want it, you can get very similar functionality by doing a emit_raw_event_log_with_masked_address where you are masking using randomness = 0. The main diff really "what" list you expect it to be in 🤷‍♂.

@LHerskind

After discussing with @LHerskind, we will not be removing this yet as it's used in the ContractInstanceDeployer and there is currently no good / clean way to replace this usage.

So it is here for prosperity. It is possible to use the raw masked data emission with a mask of 0, but this would end up broadcasting additional data, and since we are mainly interested in using it to broadcast code to be used in public, it makes sense to emit it using something that we would usually do only in public.

While slightly painful, just got thinking, could be actually have the function implemented inside the deployer itself? e.g., have emit_contract_class_unencrypted_log in there, but still purge it and emit_unencrypted_log from the context? While we need the logic, we might not need it be as easily available from there.

Separately, are we even able to constrain these without making an absolute explosion in the circuit size? Since it needs to know sizes at compile time don't we end up having absolutely crazy expensive deployments? 👀

From the code.

    // This fn exists separately from emit_unencrypted_log because sha hashing the preimage
    // is too large to compile (16,200 fields, 518,400 bytes) => the oracle hashes it
    // It is ONLY used with contract_class_registerer_contract since we already assert correctness:
    // - Contract class -> we will commit to the packed bytecode (currently a TODO)
    // - Private function -> we provide a membership proof
    // - Unconstrained function -> we provide a membership proof
    // Ordinary logs are not protected by the above so this fn shouldn't be called by anything else
    pub fn emit_contract_class_unencrypted_log<N>(&mut self, log: [Field; N]) {
        let event_selector = 5; // TODO: compute actual event selector.
        let contract_address = self.this_address();
        let counter = self.next_counter();
        let log_hash = emit_contract_class_unencrypted_log_private_internal(contract_address, event_selector, log, counter);
        // 44 = addr (32) + selector (4) + raw log len (4) + processed log len (4)
        let len = 44 + N * 32;
        let side_effect = LogHash { value: log_hash, counter, length: len };
        self.unencrypted_logs_hashes.push(side_effect);
    }

While the bytecode commits would allow you to "later" verify it, nothing here really enforces it. I guess if the hashing is not done correctly it would not be possible to include, so there might be a "kinda nasty" optimisation/assumption here that they will match, but if we have that assumption here, then we should also be able to hold similar assumptions for all the other logs (in which case we can save ~40K for the note encryption because of the hashing etc). The L1 contracts are enforcing that the data published will match the hash, so that might actually be a fair assumption. And since the kernels don't even know about those logs, it don't seem completely unfair to do, since the sequencer already need to be given the data directly, so it seems like it don't actually alter the assumptions?

Idea: Use oracle for the log_hash, the tx will only be included if the data is included AND matches the hash due to the data availability oracle, so it is essentially constrained on L1.

While #7232 helped make the user flows more aligned, there are now a bit of duplication of data, e.g., the event_selector in emit_unencrypted_log is for example populated and used, even though it is already inside the log argument, so we can cut off a few things here.

refactor: gets rid of unencrypted emit in private_context (#7236)

Closing in favor of #8978. We've already removed the unencrypted logs from the private context (though the oracles remain as the instance deployer uses them) - logs will likely be refactored in the kernel soon anyway.

Navigation Menu

Repository navigation

feat(Event_Logs): Remove unencrypted emit in private #7161

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

feat(Event_Logs): Remove unencrypted emit in private #7161

Description

Activity

sklppy88 commented on Jun 27, 2024

LHerskind commented on Jun 27, 2024

LHerskind commented on Jun 28, 2024

nventuro commented on Oct 2, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions