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: updated emit_unencrypted_log with contract address and topic #2582

Closed
wants to merge 2 commits into from

Conversation

benesjan
Copy link
Contributor

@benesjan benesjan commented Sep 28, 2023

Fixes #2580
Fixes #2581

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

  • If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
  • I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
  • Every change is related to the PR description.
  • I have linked this pull request to relevant issues (if any exist).

// --> might be a better approach to force devs to make a public function call that emits the log if needed then
// it would be less easy to accidentally leak information.
// If we decide to keep this function around would make sense to wait for traits and then merge it with emit_unencrypted_log.
fn emit_unencrypted_log_from_private<T>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you create this function so that it is easy to identify what to remove if we choose to do so?

Copy link
Contributor Author

@benesjan benesjan Sep 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was one of the reasons.

But the main one was that if I wanted to have 1 function for use in both private and public contexts I would have to use the Context wrapper and that seemed like too much of a boilerplate.

I would have to use:
emit_unencrypted_log(Context::public(&mut context), "Coins transferred");
Instead of:
emit_unencrypted_log(&mut context, "Coins transferred");

@benesjan
Copy link
Contributor Author

Replaced with #2595

@benesjan benesjan closed this Sep 29, 2023
@benesjan benesjan deleted the janb/emitting_address_and_topic_from_noir branch October 4, 2023 11:29
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 this pull request may close these issues.

Emit contract address from Noir Emit log topic/event selector from Noir
2 participants