Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

Backend::black_box_function_supported does not satisfy type IsOpcodeSupported #183

Closed
1 task done
TomAFrench opened this issue Apr 6, 2023 · 3 comments · Fixed by #273
Closed
1 task done

Backend::black_box_function_supported does not satisfy type IsOpcodeSupported #183

TomAFrench opened this issue Apr 6, 2023 · 3 comments · Fixed by #273
Labels
bug Something isn't working

Comments

@TomAFrench
Copy link
Member

Aim

The method black_box_function_supported was added to the backends so that ACVM can query which opcodes it supports and fallback as necessary.

I'm wanting to pass the method black_box_function_supported which is defined on my backend into acvm::compiler::compile so that I can use the most accurate spec of what my backend supports.

Expected behavior

I should be able to pass in the method on the backend so that the ACVM uses the set of black box functions which I have marked as available (as opposed to using the function acvm::default_is_opcode_supported).

Bug

If I were to try to pass in Backend::black_box_function_supported however I run into two issues

  1. IsOpcodeSupported accepts an Opcode as an argument but black_box_function_supported expects a BlackBoxFunc.
  2. IsOpcodeSupported specifies a function pointer however this prevents us from passing a closure.

Due to 2., trying to use Backend::black_box_function_supported anyway results in the error below

error[E0308]: mismatched types
   --> crates/nargo_cli/src/cli/compile_cmd.rs:143:65
    |
143 |         acvm::compiler::compile(circuit, backend.np_language(), |opcode |{backend.black_box_function_supported(opcode)}).unwrap();
    |         -----------------------                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found closure
    |         |
    |         arguments to this function are incorrect
    |
    = note: expected fn pointer `for<'a> fn(&'a acvm::acir::circuit::Opcode) -> bool`
                  found closure `[closure@crates/nargo_cli/src/cli/compile_cmd.rs:143:65: 143:74]`

This then means that we have to define a function near where we're calling acvm::compiler::compile similar to

fn is_opcode_supported(opcode: &Opcode) -> bool {
    let backend = crate::backends::ConcreteBackend;
    match opcode {
       Opcode::Arithmetic(_)
            | Opcode::Directive(_)
            | Opcode::Block(_)
            | Opcode::ROM(_)
            | Opcode::RAM(_)
            | Opcode::Oracle(_) => true,
        Opcode::BlackBoxFuncCall(black_box_func) => {
            backend.black_box_function_supported(&black_box_func.name)
        }
    }
}

This in turn means that we need to hardcode the backend which we are using to call this function which isn't workable.

To reproduce

Versions

No response

Additional context

No response

Submission Checklist

  • Once I hit submit, I will assign this issue to the Project Board with the appropriate tags.
@kevaundray
Copy link
Contributor

The problem here is that a closure can only be coerced into a function pointer if it does not close over its environment. I had assumed this would be the case for backend functions, but its fine to make it accept both by using the Fn trait.

type trait impls are unstable, so this would need to go on each function that needs it instead

@TomAFrench
Copy link
Member Author

@guipublic and I spoke about this yesterday and agreed that it would be cleaner to pass in a &impl Backend to acvm::compile instead.

@kevaundray
Copy link
Contributor

kevaundray commented May 9, 2023

@guipublic and I spoke about this yesterday and agreed that it would be cleaner to pass in a &impl Backend to acvm::compile instead.

Its cleaner, but its not clearer since you don't need the proving and verification. I'll open up a PR to show the diff with Fn trait

@TomAFrench TomAFrench linked a pull request May 14, 2023 that will close this issue
5 tasks
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir May 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants