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

Add conditional function override #1402

Closed
Savio-Sou opened this issue May 25, 2023 · 1 comment
Closed

Add conditional function override #1402

Savio-Sou opened this issue May 25, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@Savio-Sou
Copy link
Collaborator

Savio-Sou commented May 25, 2023

Problem

When a user is importing a library (e.g. greet_person) that uses a foreign function (e.g. wave_hand), but the function:

  • Is not supported by the proving backend used
  • Does not have a fallback implementation in ACVM

The Noir program would not be compilable as expected.

If an alternative implementation of wave_hand written in Noir is available in another library (e.g. noir_gestures), however, the current best way for the user to make use of it would be to:

  1. Fork greet_person
  2. Change all wave_hand within to use noir_gestures's implementation instead
  3. Consume the edited forked version of greet_person

Which is burdensome to both execute and maintain down the road.

Happy Case

The user should be able to override all instances of wave_hand with a simple config edit:

Nargo.toml before override:

[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
greet_person = { path = "./path/to/greet_person" }

Nargo.toml after override:

[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
greet_person = { path = "./path/to/greet_person" }

[override.wave_hand]
greet_person = { path = "./path/to/noir_gestures" }

The overriding function should have the same function signature as the original function.

This might also be useful if e.g. the original library uses a slow Noir implementation, where the user can override it with a fast blackbox implementation.

Alternatives Considered

Conditional compilation

We can opt for some form of conditional compilation instead of config changes, but I didn't manage to picture a decent approach.

Strictly require ACVM fallback

We can require a fallback implementation in ACVM must be added for each foreign function addition to the Noir stdlib.

This might be undesirable in terms of engineering efforts and speed on delivering foreign functions already usable with certain proving backends.

Additional Context

The new override keyword was chosen instead of:

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@Savio-Sou Savio-Sou added the enhancement New feature or request label May 25, 2023
@Savio-Sou Savio-Sou added this to Noir May 25, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir May 25, 2023
@TomAFrench
Copy link
Member

See discussion in #1258

@Savio-Sou Savio-Sou changed the title Add conditional foreign function override Add conditional function override May 25, 2023
@TomAFrench TomAFrench closed this as not planned Won't fix, can't repro, duplicate, stale Apr 21, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

3 participants