Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Keys_Viewing): Update encrypted log format #5901

Closed
Tracked by #5606
LHerskind opened this issue Apr 22, 2024 · 3 comments · Fixed by #6411
Closed
Tracked by #5606

feat(Keys_Viewing): Update encrypted log format #5901

LHerskind opened this issue Apr 22, 2024 · 3 comments · Fixed by #6411
Assignees

Comments

@LHerskind
Copy link
Contributor

LHerskind commented Apr 22, 2024

The format of encrypted logs should be altered to better match our requirements.

From the diagram we currently have a description as
image

However, this can be horribly inefficient for transactions that emit notes but don't wish to emit an outgoing note.

Instead, we can alter it such that we have a general incoming_encrypted_log as follows:

incoming_encrypted_log = {
  tag,
  ephemeral_public_key,
  ciphertext_header,
  ciphertext_note
}

And then have a separate outgoing_encrypted_log that have the rest.

outgoing_encrypted_log = {
  tag,
  ciphertext_header,
  ciphertext_note
}

Note, that there is not direct link between the two, but we know they are part of the same tx, so for decryption of the outgoing a user would match it against all the incoming_encrypted_logs in the transaction.

@github-project-automation github-project-automation bot moved this to Todo in A3 Apr 22, 2024
@LHerskind LHerskind changed the title feat(Keys_Viewing): Update encrypted log format feat(Keys_Incoming): Update encrypted log format May 1, 2024
@LHerskind LHerskind changed the title feat(Keys_Incoming): Update encrypted log format feat(Keys_Viewing): Update encrypted log format May 1, 2024
@LHerskind
Copy link
Contributor Author

We should consider the exact format we will be using. Namely, when creating the final object, we have that using raw bytes that are only made into fields at the end will likely be using less data. A particularly annoying thing is that encrypting the address for the ciphertext_header when encrypted can be a full 32 bytes, and no longer fits into one field.

I think we should collect the bytes, as we can then use the other 30 bytes that we have available from this 1 byte extra to some other values, for example, we could likely have both the tag and the header sharing a bit.

@LHerskind LHerskind assigned LHerskind and unassigned nventuro May 3, 2024
@LHerskind
Copy link
Contributor Author

LHerskind commented May 8, 2024

As part of this work, we should be altering the compute_encrypted_log function in the private_context such that will be using the constrained ciphertext.

Currently the function is using the emit_encrypted_log oracle, to compute the encrypted message, which it is then blindly passing along for the log_hash.

I suggest we alter this to draw a eph_sh, query needed keys and then construct the header and the body as done in #5867 and #5899. Potentially alter these directly to slices. Then, we can change compute_encrypted_log_hash to take these encrypted bytes that we now have, and spit out the hash. For now, we can just set the tag to some placeholder that is ignored. Overall, we should sit with a slice of bytes that contains [tag, ephemeral_public_key, ciphertext_header, ciphertext_note]

The remainder of the function would be mainly the same, but we would change the emit_encrypted_log oracle to just be telling the node about the pre-image to the hash, such that the client_execution_context can still add it to its encryptedLogs and logsCache.

@LHerskind
Copy link
Contributor Author

LHerskind commented May 10, 2024

Speaking with Mike, and we will go back to the "one big log" for a note instead of splitting it into multiple sections. From #6348 there is a typescript implementation showing the encryption and decryption (both incoming and outgoing).

It should be "fairly" straightforward to implement the encrypt function in noir, alter the oracle that is currently emitting to just take raw bytes instead, and then alter the note_processor.

In particular, we should be able to modify the TaggedNote and EncryptedPayload such that the combined take the roled of the TaggedNote with the additional tag.

const taggedNote = TaggedNote.fromEncryptedBuffer(log.data, secretKey);

A interesting note that should make it slightly easier to implement. It is possible to set it up such that the plumbing can be handled before the noir implementation is even written 👀 But likely more convenient to match it first with just the payload similar to the other log tests (as #6334 and #6325)

LHerskind added a commit that referenced this issue May 18, 2024
Fixes #5901. 

Uses the new format to match the keys and logs spec. The current
implementation here is still using a typescript implementation to
encrypt the data, mainly using the new flow.

The intention is that #6408 and #1139 can be addresses separately and
than we can just replace the import to use the constrained version
instead of the oracle at that point.

Note, that the outgoing logs are currently less than meaningful, as some
of the infrastructure is not yet in place to handle those nicely, see
#6410 for more on that.

What was called the encrypted_log_payload in #6348 have been moved into
the l1_payload, to better integrate with the rest of the setup.
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 May 18, 2024
signorecello pushed a commit that referenced this issue May 20, 2024
Fixes #5901.

Uses the new format to match the keys and logs spec. The current
implementation here is still using a typescript implementation to
encrypt the data, mainly using the new flow.

The intention is that #6408 and #1139 can be addresses separately and
than we can just replace the import to use the constrained version
instead of the oracle at that point.

Note, that the outgoing logs are currently less than meaningful, as some
of the infrastructure is not yet in place to handle those nicely, see

What was called the encrypted_log_payload in #6348 have been moved into
the l1_payload, to better integrate with the rest of the setup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants