-
Notifications
You must be signed in to change notification settings - Fork 248
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(brillig): foreign call/oracle compilation #1600
Changes from all commits
6184f52
823bd84
545d333
f6810af
c5e34c5
06543b5
3c9f106
9a9c461
043c3c4
e51d3c6
1ed956b
2a797a1
cd39144
ba8ae00
cf393ff
4644da9
d50c69f
398ddf9
c5b9579
aa2185c
3847f0a
828c116
1d52f5e
2d3ab61
801a739
2bcfc24
49a151d
5247a48
5995b30
6d83b22
1a9d33c
2787cc9
8713a89
70f8fe2
91defbc
4269ac2
48405c6
f49d69c
f309bac
efd84b8
f5bbb70
d6c7496
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,3 +18,4 @@ result | |
*.pk | ||
*.vk | ||
**/Verifier.toml | ||
**/target |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
[package] | ||
authors = [""] | ||
compiler_version = "0.1" | ||
|
||
[dependencies] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
x = "10" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// Tests oracle usage in brillig/unconstrained functions | ||
fn main(x: Field) { | ||
// call through a brillig wrapper | ||
oracle_print_wrapper(x); | ||
} | ||
|
||
|
||
#[oracle(oracle_print_impl)] | ||
unconstrained fn oracle_print(_x : Field) {} | ||
|
||
unconstrained fn oracle_print_wrapper(x: Field) { | ||
oracle_print(x); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,6 +92,10 @@ pub(crate) enum Instruction { | |
/// Performs a function call with a list of its arguments. | ||
Call { func: ValueId, arguments: Vec<ValueId> }, | ||
|
||
/// Executes an "oracle" call | ||
/// These are unconstrained functions that may access external state. | ||
ForeignCall { func: String, arguments: Vec<ValueId> }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't need a separate instruction type for this. Instead, we can follow the example used by intrinsic functions and have a separate Value type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made an issue here: #1643. Good call out, I will refactor this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like there is already a separate Value type for oracles as well that we can use |
||
|
||
/// Allocates a region of memory. Note that this is not concerned with | ||
/// the type of memory, the type of element is determined when loading this memory. | ||
/// This is used for representing mutable variables and references. | ||
|
@@ -128,9 +132,10 @@ impl Instruction { | |
} | ||
Instruction::ArraySet { array, .. } => InstructionResultType::Operand(*array), | ||
Instruction::Constrain(_) | Instruction::Store { .. } => InstructionResultType::None, | ||
Instruction::Load { .. } | Instruction::ArrayGet { .. } | Instruction::Call { .. } => { | ||
InstructionResultType::Unknown | ||
} | ||
Instruction::Load { .. } | ||
| Instruction::ArrayGet { .. } | ||
| Instruction::Call { .. } | ||
| Instruction::ForeignCall { .. } => InstructionResultType::Unknown, | ||
} | ||
} | ||
|
||
|
@@ -158,6 +163,10 @@ impl Instruction { | |
max_bit_size: *max_bit_size, | ||
}, | ||
Instruction::Constrain(value) => Instruction::Constrain(f(*value)), | ||
Instruction::ForeignCall { func, arguments } => Instruction::ForeignCall { | ||
func: func.to_owned(), | ||
arguments: vecmap(arguments.iter().copied(), f), | ||
}, | ||
Instruction::Call { func, arguments } => Instruction::Call { | ||
func: f(*func), | ||
arguments: vecmap(arguments.iter().copied(), f), | ||
|
@@ -245,6 +254,7 @@ impl Instruction { | |
} | ||
Instruction::Truncate { .. } => None, | ||
Instruction::Call { .. } => None, | ||
Instruction::ForeignCall { .. } => None, | ||
Instruction::Allocate { .. } => None, | ||
Instruction::Load { .. } => None, | ||
Instruction::Store { .. } => None, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment needs to be updated.
We should also have an enumeration for Oracles. It is unclear without it which values may be valid for this string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update comment as part of #1643 refactor. An enumeration would be good for
nargo
oracle builtins such as println, however, we want to let developers specify their own oracles if they so chose. Need to think on the best way to include these two flowsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, in that case we should likely give them a separate BrilligID like we do for FunctionIds