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

refactor: aztec-macros return values #4786

Closed
LHerskind opened this issue Feb 27, 2024 · 0 comments · Fixed by #4799
Closed

refactor: aztec-macros return values #4786

LHerskind opened this issue Feb 27, 2024 · 0 comments · Fixed by #4799
Assignees
Labels
T-bug Type: Bug. Something is broken.

Comments

@LHerskind
Copy link
Contributor

LHerskind commented Feb 27, 2024

When the macros for return values are applied they will current "copy" the node into the return_values of the context.

This means that for functions like:

#[aztec(private)]
fn is_valid(message_hash: Field) -> Field {
    let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl);
    actions.is_valid(message_hash)
}

We will really be executing something along the lines of:

#[aztec(private)]
fn is_valid(message_hash: Field) -> Field {
    let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl);
    actions.is_valid(message_hash);
    context.return_values.push(actions.is_valid(message_hash));
}

This executes the function is_valid twice. Which can be "fatal" in some cases where the function is emitting nullifiers as it would create an instant double-spend, (two nullifiers in the same tx).


Point of interest:

// Abstract return types such that they get added to the kernel's return_values
if let Some(return_values) = abstract_return_values(func) {
func.def.body.0.push(return_values);
}

@LHerskind LHerskind added the T-bug Type: Bug. Something is broken. label Feb 27, 2024
@github-project-automation github-project-automation bot moved this to Todo in A3 Feb 27, 2024
@LHerskind LHerskind self-assigned this Feb 27, 2024
@LHerskind LHerskind linked a pull request Feb 27, 2024 that will close this issue
@LHerskind LHerskind moved this from Todo to Done in A3 Feb 28, 2024
LHerskind added a commit that referenced this issue Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: Bug. Something is broken.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant