Open LHerskind opened 1 week ago
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.
In
PrivateContext
we gotemit_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 aemit_raw_event_log_with_masked_address
where you are masking usingrandomness = 0
. The main diff really "what" list you expect it to be in 🤷♂.