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

foreign call executor times out #4772

Closed
simon-something opened this issue Apr 10, 2024 · 3 comments · Fixed by #4780
Closed

foreign call executor times out #4772

simon-something opened this issue Apr 10, 2024 · 3 comments · Fixed by #4780
Labels
enhancement New feature or request oracle Related to the use of the `#[oracle(<name>)]` flag

Comments

@simon-something
Copy link

Aim

Using an external RPC oracle resolver, while running multiple tests

Expected Behavior

not to time-out

Bug

default minreq timeout (noir/noir-repo/tooling/nargo/src/ops/foreign_calls.rs ln165) is 12 seconds. This is fine for most of the single foreign calls to resolve but, since tests have been parallelised in #4484 , if there is a lot of calls to make with potential lock on the resolver side (awaiting offchain data for instance), it is too short

To Reproduce

  1. Follow the guide to use an external express json rpc server as oracle resolver
  2. create an endpoint which sleeps 15 seconds
  3. call this oracle from within a noir test

Project Impact

None

Impact Context

I've increased it myself in a project conducted on Aztec (integration test ran from within Noir) as our tests were timing-out.

Workaround

Yes

Workaround Description

manually increase the timeout when creating a new minreq Builder, as in
https://github.com/defi-wonderland/aztec-packages/blob/b3b7fb3b50498b1b2f45e1dc473109e37f087256/noir/noir-repo/tooling/nargo/src/ops/foreign_calls.rs#L167

This is hacky tho, and should rather rely on an environment variable (minreq MINREQ_TIMEOUT isn't working for Builders, if I'm correct)

Additional Context

No response

Installation Method

Compiled from source

Nargo Version

0.26.0 (aztec)

NoirJS Version

No response

Would you like to submit a PR for this Issue?

Maybe

Support Needs

Validation of the fix suggested, ie no unforeseen side-effect?

@simon-something simon-something added the bug Something isn't working label Apr 10, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Apr 10, 2024
@Savio-Sou Savio-Sou added the oracle Related to the use of the `#[oracle(<name>)]` flag label Apr 10, 2024
@Savio-Sou
Copy link
Collaborator

Approach aside, do you have a recommended timeout duration / what's the minimal timeout duration you're seeing with your project?

@simon-something
Copy link
Author

simon-something commented Apr 11, 2024

I don't know how representative to "classic" use-case it is @Savio-Sou , right now, I'm timing out if I set it under 40sec (I'm having some Aztec sandbox interactions in these foreign calls, so lots of async calls to resolve from within my rpc server before getting back to the minreq request).

On the other hand, wouldn't it be more flexible with an env variable (thinking of linear homomorphic encryption as it was recently mentioned on Aztec forum by Rahul - baby step giant step or pollard's rho could be run within an oracle call no? So pretty slow then)?

@TomAFrench
Copy link
Member

Agreed on having an environment variable is the best solution for this for the time being.

@Savio-Sou Savio-Sou added enhancement New feature or request and removed bug Something isn't working labels Apr 11, 2024
@Savio-Sou Savio-Sou moved this from 📋 Backlog to 🏗 In progress in Noir Apr 12, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 15, 2024
# Description

## Problem\*

Resolves #4772

## Summary\*

This PR adds the `NARGO_FOREIGN_CALL_TIMEOUT` environment variable which
allows setting the number of milliseconds which external foreign call
resolvers can take to return a result.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Noir Apr 15, 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 oracle Related to the use of the `#[oracle(<name>)]` flag
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants