-
Notifications
You must be signed in to change notification settings - Fork 323
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!: Unify ABIs between nargo and yarn-project #3906
Changes from 4 commits
0fb3262
672bded
2ab38fa
583cd45
95c035e
165610c
d7a3e39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ use gloo_utils::format::JsValueSerdeExt; | |
use js_sys::{JsString, Object}; | ||
use nargo::artifacts::{ | ||
contract::{ContractArtifact, ContractFunctionArtifact}, | ||
debug::DebugArtifact, | ||
program::ProgramArtifact, | ||
}; | ||
use noirc_driver::{ | ||
|
@@ -33,28 +32,25 @@ export type CompiledContract = { | |
name: string; | ||
functions: Array<any>; | ||
events: Array<any>; | ||
file_map: Record<number, any>; | ||
warnings: Array<any>; | ||
}; | ||
|
||
export type CompiledProgram = { | ||
noir_version: string; | ||
abi: any; | ||
bytecode: string; | ||
} | ||
|
||
export type DebugArtifact = { | ||
debug_symbols: Array<any>; | ||
debug_symbols: any; | ||
file_map: Record<number, any>; | ||
warnings: Array<any>; | ||
}; | ||
} | ||
|
||
export type CompileResult = ( | ||
| { | ||
contract: CompiledContract; | ||
debug: DebugArtifact; | ||
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. As we're pulling the warnings out of the artifact, we'll need to include them on the |
||
} | ||
| { | ||
program: CompiledProgram; | ||
debug: DebugArtifact; | ||
} | ||
); | ||
"#; | ||
|
@@ -76,40 +72,25 @@ extern "C" { | |
impl JsCompileResult { | ||
const CONTRACT_PROP: &'static str = "contract"; | ||
const PROGRAM_PROP: &'static str = "program"; | ||
const DEBUG_PROP: &'static str = "debug"; | ||
|
||
pub fn new(resp: CompileResult) -> JsCompileResult { | ||
let obj = JsCompileResult::constructor(); | ||
match resp { | ||
CompileResult::Contract { contract, debug } => { | ||
CompileResult::Contract { contract } => { | ||
js_sys::Reflect::set( | ||
&obj, | ||
&JsString::from(JsCompileResult::CONTRACT_PROP), | ||
&<JsValue as JsValueSerdeExt>::from_serde(&contract).unwrap(), | ||
) | ||
.unwrap(); | ||
|
||
js_sys::Reflect::set( | ||
&obj, | ||
&JsString::from(JsCompileResult::DEBUG_PROP), | ||
&<JsValue as JsValueSerdeExt>::from_serde(&debug).unwrap(), | ||
) | ||
.unwrap(); | ||
} | ||
CompileResult::Program { program, debug } => { | ||
CompileResult::Program { program } => { | ||
js_sys::Reflect::set( | ||
&obj, | ||
&JsString::from(JsCompileResult::PROGRAM_PROP), | ||
&<JsValue as JsValueSerdeExt>::from_serde(&program).unwrap(), | ||
) | ||
.unwrap(); | ||
|
||
js_sys::Reflect::set( | ||
&obj, | ||
&JsString::from(JsCompileResult::DEBUG_PROP), | ||
&<JsValue as JsValueSerdeExt>::from_serde(&debug).unwrap(), | ||
) | ||
.unwrap(); | ||
} | ||
}; | ||
|
||
|
@@ -148,8 +129,8 @@ impl PathToFileSourceMap { | |
} | ||
|
||
pub enum CompileResult { | ||
Contract { contract: ContractArtifact, debug: DebugArtifact }, | ||
Program { program: ProgramArtifact, debug: DebugArtifact }, | ||
Contract { contract: ContractArtifact }, | ||
Program { program: ProgramArtifact }, | ||
} | ||
|
||
#[wasm_bindgen] | ||
|
@@ -273,31 +254,22 @@ fn add_noir_lib(context: &mut Context, library_name: &CrateName) -> CrateId { | |
} | ||
|
||
pub(crate) fn generate_program_artifact(program: CompiledProgram) -> CompileResult { | ||
let debug_artifact = DebugArtifact { | ||
debug_symbols: vec![program.debug.clone()], | ||
file_map: program.file_map.clone(), | ||
warnings: program.warnings.clone(), | ||
}; | ||
|
||
CompileResult::Program { program: program.into(), debug: debug_artifact } | ||
CompileResult::Program { program: program.into() } | ||
} | ||
|
||
pub(crate) fn generate_contract_artifact(contract: CompiledContract) -> CompileResult { | ||
let debug_artifact = DebugArtifact { | ||
debug_symbols: contract.functions.iter().map(|function| function.debug.clone()).collect(), | ||
file_map: contract.file_map, | ||
warnings: contract.warnings, | ||
}; | ||
let functions = contract.functions.into_iter().map(ContractFunctionArtifact::from).collect(); | ||
|
||
let contract_artifact = ContractArtifact { | ||
noir_version: String::from(NOIR_ARTIFACT_VERSION_STRING), | ||
name: contract.name, | ||
functions, | ||
events: contract.events, | ||
file_map: contract.file_map, | ||
warnings: contract.warnings, | ||
}; | ||
|
||
CompileResult::Contract { contract: contract_artifact, debug: debug_artifact } | ||
CompileResult::Contract { contract: contract_artifact } | ||
} | ||
|
||
#[cfg(test)] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,15 @@ | ||
use acvm::acir::circuit::Circuit; | ||
use noirc_abi::{Abi, ContractEvent}; | ||
use noirc_driver::{ContractFunction, ContractFunctionType}; | ||
use noirc_driver::{ContractFunction, ContractFunctionType, CompiledContract}; | ||
use serde::{Deserialize, Serialize}; | ||
use noirc_evaluator::errors::SsaReport; | ||
|
||
use noirc_driver::DebugFile; | ||
use noirc_errors::debug_info::DebugInfo; | ||
use std::collections::BTreeMap; | ||
|
||
use fm::FileId; | ||
|
||
|
||
#[derive(Serialize, Deserialize)] | ||
pub struct ContractArtifact { | ||
|
@@ -13,8 +21,26 @@ pub struct ContractArtifact { | |
pub functions: Vec<ContractFunctionArtifact>, | ||
/// All the events defined inside the contract scope. | ||
pub events: Vec<ContractEvent>, | ||
/// Map of file Id to the source code so locations in debug info can be mapped to source code they point to. | ||
pub file_map: BTreeMap<FileId, DebugFile>, | ||
/// Compilation warnings. | ||
pub warnings: Vec<SsaReport>, | ||
Comment on lines
+25
to
+26
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 don't need to store these on the artifact as they're not really relevant after compilation in my mind. |
||
} | ||
|
||
impl From<CompiledContract> for ContractArtifact { | ||
fn from(contract: CompiledContract) -> Self { | ||
ContractArtifact { | ||
noir_version: contract.noir_version, | ||
name: contract.name, | ||
functions: contract.functions.into_iter().map(ContractFunctionArtifact::from).collect(), | ||
events: contract.events, | ||
file_map: contract.file_map, | ||
warnings: contract.warnings, | ||
} | ||
} | ||
} | ||
|
||
|
||
/// Each function in the contract will be compiled as a separate noir program. | ||
/// | ||
/// A contract function unlike a regular Noir program however can have additional properties. | ||
|
@@ -34,6 +60,12 @@ pub struct ContractFunctionArtifact { | |
deserialize_with = "Circuit::deserialize_circuit_base64" | ||
)] | ||
pub bytecode: Circuit, | ||
|
||
#[serde( | ||
serialize_with = "DebugInfo::serialize_compressed_base64_json", | ||
deserialize_with = "DebugInfo::deserialize_compressed_base64_json" | ||
)] | ||
pub debug_symbols: DebugInfo, | ||
} | ||
|
||
impl From<ContractFunction> for ContractFunctionArtifact { | ||
|
@@ -44,6 +76,7 @@ impl From<ContractFunction> for ContractFunctionArtifact { | |
is_internal: func.is_internal, | ||
abi: func.abi, | ||
bytecode: func.bytecode, | ||
debug_symbols: func.debug, | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,12 @@ | ||
use std::collections::BTreeMap; | ||
|
||
use acvm::acir::circuit::Circuit; | ||
use fm::FileId; | ||
use noirc_abi::Abi; | ||
use noirc_driver::CompiledProgram; | ||
use noirc_driver::DebugFile; | ||
use noirc_errors::debug_info::DebugInfo; | ||
use noirc_evaluator::errors::SsaReport; | ||
use serde::{Deserialize, Serialize}; | ||
|
||
#[derive(Serialize, Deserialize, Debug)] | ||
|
@@ -20,6 +26,18 @@ pub struct ProgramArtifact { | |
deserialize_with = "Circuit::deserialize_circuit_base64" | ||
)] | ||
pub bytecode: Circuit, | ||
|
||
#[serde( | ||
serialize_with = "DebugInfo::serialize_compressed_base64_json", | ||
deserialize_with = "DebugInfo::deserialize_compressed_base64_json" | ||
)] | ||
pub debug_symbols: DebugInfo, | ||
|
||
/// Map of file Id to the source code so locations in debug info can be mapped to source code they point to. | ||
pub file_map: BTreeMap<FileId, DebugFile>, | ||
|
||
/// Compilation warnings. | ||
pub warnings: Vec<SsaReport>, | ||
Comment on lines
+39
to
+40
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. Similarly here. |
||
} | ||
|
||
impl From<CompiledProgram> for ProgramArtifact { | ||
|
@@ -29,6 +47,23 @@ impl From<CompiledProgram> for ProgramArtifact { | |
abi: program.abi, | ||
noir_version: program.noir_version, | ||
bytecode: program.circuit, | ||
debug_symbols: program.debug, | ||
file_map: program.file_map, | ||
warnings: program.warnings, | ||
} | ||
} | ||
} | ||
|
||
impl Into<CompiledProgram> for ProgramArtifact { | ||
fn into(self) -> CompiledProgram { | ||
CompiledProgram { | ||
hash: self.hash, | ||
abi: self.abi, | ||
noir_version: self.noir_version, | ||
circuit: self.bytecode, | ||
debug: self.debug_symbols, | ||
file_map: self.file_map, | ||
warnings: self.warnings, | ||
} | ||
} | ||
} |
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.
Can you rename these types to align with
ProgramArtifact
andContractArtifact
? I think ideally we should have these be consistent across JS and Rust.This is a preexisting issue but I think this PR is a good time to make the change tbh.