From b454b82fec0fa3b6a32833bc0a2822f04efa8f25 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 1 Nov 2024 13:53:00 +0200 Subject: [PATCH 01/19] Sketch deployed contract query --- core/lib/contract_verifier/src/lib.rs | 160 ++++++++++-------- ...87f3b5d4ba906ec9a38e10912e3c7832fc93e.json | 42 +++++ ...c84b0dd496700a61569929dcc7602ec678b09.json | 36 ---- core/lib/dal/src/contract_verification_dal.rs | 81 +++++---- .../types/src/contract_verification_api.rs | 6 - 5 files changed, 176 insertions(+), 149 deletions(-) create mode 100644 core/lib/dal/.sqlx/query-40a7a619ed490eb3848a45fdde787f3b5d4ba906ec9a38e10912e3c7832fc93e.json delete mode 100644 core/lib/dal/.sqlx/query-8ab1634beba74aaef952562a3bcc84b0dd496700a61569929dcc7602ec678b09.json diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index f5face9f8a56..c91543024128 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -11,14 +11,14 @@ use anyhow::Context as _; use chrono::Utc; use ethabi::{Contract, Token}; use tokio::time; -use zksync_dal::{ConnectionPool, Core, CoreDal}; +use zksync_dal::{contract_verification_dal::DeployedContractData, ConnectionPool, Core, CoreDal}; use zksync_queued_job_processor::{async_trait, JobProcessor}; use zksync_types::{ contract_verification_api::{ - CompilationArtifacts, CompilerType, DeployContractCalldata, SourceCodeData, - VerificationIncomingRequest, VerificationInfo, VerificationRequest, + CompilationArtifacts, CompilerType, SourceCodeData, VerificationIncomingRequest, + VerificationInfo, VerificationRequest, }, - Address, + Address, CONTRACT_DEPLOYER_ADDRESS, }; use crate::{ @@ -149,7 +149,7 @@ impl ContractVerifier { .connection_pool .connection_tagged("contract_verifier") .await?; - let (deployed_bytecode, creation_tx_calldata) = storage + let deployed_contract = storage .contract_verification_dal() .get_contract_info_for_verification(request.req.contract_address) .await? @@ -160,16 +160,17 @@ impl ContractVerifier { ) })?; drop(storage); + // FIXME: make compilation depend on bytecode type let constructor_args = - self.decode_constructor_args(creation_tx_calldata, request.req.contract_address)?; + self.decode_constructor_args(&deployed_contract, request.req.contract_address)?; - if artifacts.bytecode != deployed_bytecode { + if artifacts.bytecode != deployed_contract.bytecode { tracing::info!( "Bytecode mismatch req {}, deployed: 0x{}, compiled: 0x{}", request.id, - hex::encode(deployed_bytecode), - hex::encode(artifacts.bytecode) + hex::encode(&deployed_contract.bytecode), + hex::encode(&artifacts.bytecode) ); return Err(ContractVerifierError::BytecodeMismatch); } @@ -342,76 +343,87 @@ impl ContractVerifier { #[tracing::instrument(level = "trace", skip_all, ret, err)] fn decode_constructor_args( &self, - calldata: DeployContractCalldata, + contract: &DeployedContractData, contract_address_to_verify: Address, ) -> anyhow::Result { - match calldata { - DeployContractCalldata::Deploy(calldata) => { - anyhow::ensure!( - calldata.len() >= 4, - "calldata doesn't include Solidity function selector" - ); - - let contract_deployer = &self.contract_deployer; - let create = contract_deployer - .function("create") - .context("no `create` in contract deployer ABI")?; - let create2 = contract_deployer - .function("create2") - .context("no `create2` in contract deployer ABI")?; - let create_acc = contract_deployer - .function("createAccount") - .context("no `createAccount` in contract deployer ABI")?; - let create2_acc = contract_deployer - .function("create2Account") - .context("no `create2Account` in contract deployer ABI")?; - let force_deploy = contract_deployer - .function("forceDeployOnAddresses") - .context("no `forceDeployOnAddresses` in contract deployer ABI")?; - - let (selector, token_data) = calldata.split_at(4); - // It's assumed that `create` and `create2` methods have the same parameters - // and the same for `createAccount` and `create2Account`. - Ok(match selector { - selector - if selector == create.short_signature() - || selector == create2.short_signature() => - { - let tokens = create - .decode_input(token_data) - .context("failed to decode `create` / `create2` input")?; - // Constructor arguments are in the third parameter. - ConstructorArgs::Check(tokens[2].clone().into_bytes().context( - "third parameter of `create/create2` should be of type `bytes`", - )?) - } - selector - if selector == create_acc.short_signature() - || selector == create2_acc.short_signature() => - { - let tokens = create - .decode_input(token_data) - .context("failed to decode `createAccount` / `create2Account` input")?; - // Constructor arguments are in the third parameter. - ConstructorArgs::Check(tokens[2].clone().into_bytes().context( - "third parameter of `createAccount/create2Account` should be of type `bytes`", - )?) - } - selector if selector == force_deploy.short_signature() => { - Self::decode_force_deployment( - token_data, - force_deploy, - contract_address_to_verify, - ) - .context("failed decoding force deployment")? - } - _ => ConstructorArgs::Ignore, - }) - } - DeployContractCalldata::Ignore => Ok(ConstructorArgs::Ignore), + let Some(calldata) = &contract.calldata else { + return Ok(ConstructorArgs::Ignore); + }; + + if contract.contract_address == Some(CONTRACT_DEPLOYER_ADDRESS) { + self.decode_contract_deployer_call(calldata, contract_address_to_verify) + } else { + // FIXME: handle EVM contracts + Ok(ConstructorArgs::Ignore) } } + fn decode_contract_deployer_call( + &self, + calldata: &[u8], + contract_address_to_verify: Address, + ) -> anyhow::Result { + anyhow::ensure!( + calldata.len() >= 4, + "calldata doesn't include Solidity function selector" + ); + + let contract_deployer = &self.contract_deployer; + let create = contract_deployer + .function("create") + .context("no `create` in contract deployer ABI")?; + let create2 = contract_deployer + .function("create2") + .context("no `create2` in contract deployer ABI")?; + let create_acc = contract_deployer + .function("createAccount") + .context("no `createAccount` in contract deployer ABI")?; + let create2_acc = contract_deployer + .function("create2Account") + .context("no `create2Account` in contract deployer ABI")?; + let force_deploy = contract_deployer + .function("forceDeployOnAddresses") + .context("no `forceDeployOnAddresses` in contract deployer ABI")?; + + let (selector, token_data) = calldata.split_at(4); + // It's assumed that `create` and `create2` methods have the same parameters + // and the same for `createAccount` and `create2Account`. + Ok(match selector { + selector + if selector == create.short_signature() + || selector == create2.short_signature() => + { + let tokens = create + .decode_input(token_data) + .context("failed to decode `create` / `create2` input")?; + // Constructor arguments are in the third parameter. + ConstructorArgs::Check( + tokens[2] + .clone() + .into_bytes() + .context("third parameter of `create/create2` should be of type `bytes`")?, + ) + } + selector + if selector == create_acc.short_signature() + || selector == create2_acc.short_signature() => + { + let tokens = create + .decode_input(token_data) + .context("failed to decode `createAccount` / `create2Account` input")?; + // Constructor arguments are in the third parameter. + ConstructorArgs::Check(tokens[2].clone().into_bytes().context( + "third parameter of `createAccount/create2Account` should be of type `bytes`", + )?) + } + selector if selector == force_deploy.short_signature() => { + Self::decode_force_deployment(token_data, force_deploy, contract_address_to_verify) + .context("failed decoding force deployment")? + } + _ => ConstructorArgs::Ignore, + }) + } + fn decode_force_deployment( token_data: &[u8], force_deploy: ðabi::Function, diff --git a/core/lib/dal/.sqlx/query-40a7a619ed490eb3848a45fdde787f3b5d4ba906ec9a38e10912e3c7832fc93e.json b/core/lib/dal/.sqlx/query-40a7a619ed490eb3848a45fdde787f3b5d4ba906ec9a38e10912e3c7832fc93e.json new file mode 100644 index 000000000000..1b68326b8962 --- /dev/null +++ b/core/lib/dal/.sqlx/query-40a7a619ed490eb3848a45fdde787f3b5d4ba906ec9a38e10912e3c7832fc93e.json @@ -0,0 +1,42 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT\n factory_deps.bytecode_hash,\n factory_deps.bytecode,\n transactions.data -> 'calldata' AS \"calldata?\",\n transactions.contract_address AS \"contract_address?\"\n FROM\n (\n SELECT\n miniblock_number,\n tx_hash,\n topic3\n FROM\n events\n WHERE\n address = $1\n AND topic1 = $2\n AND topic4 = $3\n LIMIT\n 1\n ) deploy_event\n JOIN factory_deps ON factory_deps.bytecode_hash = deploy_event.topic3\n LEFT JOIN transactions ON transactions.hash = deploy_event.tx_hash\n WHERE\n deploy_event.miniblock_number <= (\n SELECT\n MAX(number)\n FROM\n miniblocks\n )\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "bytecode_hash", + "type_info": "Bytea" + }, + { + "ordinal": 1, + "name": "bytecode", + "type_info": "Bytea" + }, + { + "ordinal": 2, + "name": "calldata?", + "type_info": "Jsonb" + }, + { + "ordinal": 3, + "name": "contract_address?", + "type_info": "Bytea" + } + ], + "parameters": { + "Left": [ + "Bytea", + "Bytea", + "Bytea" + ] + }, + "nullable": [ + false, + false, + null, + true + ] + }, + "hash": "40a7a619ed490eb3848a45fdde787f3b5d4ba906ec9a38e10912e3c7832fc93e" +} diff --git a/core/lib/dal/.sqlx/query-8ab1634beba74aaef952562a3bcc84b0dd496700a61569929dcc7602ec678b09.json b/core/lib/dal/.sqlx/query-8ab1634beba74aaef952562a3bcc84b0dd496700a61569929dcc7602ec678b09.json deleted file mode 100644 index 5869c1d37a04..000000000000 --- a/core/lib/dal/.sqlx/query-8ab1634beba74aaef952562a3bcc84b0dd496700a61569929dcc7602ec678b09.json +++ /dev/null @@ -1,36 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n SELECT\n factory_deps.bytecode,\n transactions.data AS \"data?\",\n transactions.contract_address AS \"contract_address?\"\n FROM\n (\n SELECT\n miniblock_number,\n tx_hash,\n topic3\n FROM\n events\n WHERE\n address = $1\n AND topic1 = $2\n AND topic4 = $3\n LIMIT\n 1\n ) deploy_event\n JOIN factory_deps ON factory_deps.bytecode_hash = deploy_event.topic3\n LEFT JOIN transactions ON transactions.hash = deploy_event.tx_hash\n WHERE\n deploy_event.miniblock_number <= (\n SELECT\n MAX(number)\n FROM\n miniblocks\n )\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "bytecode", - "type_info": "Bytea" - }, - { - "ordinal": 1, - "name": "data?", - "type_info": "Jsonb" - }, - { - "ordinal": 2, - "name": "contract_address?", - "type_info": "Bytea" - } - ], - "parameters": { - "Left": [ - "Bytea", - "Bytea", - "Bytea" - ] - }, - "nullable": [ - false, - true, - true - ] - }, - "hash": "8ab1634beba74aaef952562a3bcc84b0dd496700a61569929dcc7602ec678b09" -} diff --git a/core/lib/dal/src/contract_verification_dal.rs b/core/lib/dal/src/contract_verification_dal.rs index 291e60a50d90..c168ffb36aa7 100644 --- a/core/lib/dal/src/contract_verification_dal.rs +++ b/core/lib/dal/src/contract_verification_dal.rs @@ -1,4 +1,5 @@ #![doc = include_str!("../doc/ContractVerificationDal.md")] + use std::{ fmt::{Display, Formatter}, time::Duration, @@ -6,23 +7,20 @@ use std::{ use anyhow::Context as _; use sqlx::postgres::types::PgInterval; -use zksync_db_connection::connection::Connection; +use zksync_db_connection::{error::SqlxContext, instrument::InstrumentExt}; use zksync_types::{ contract_verification_api::{ - DeployContractCalldata, VerificationIncomingRequest, VerificationInfo, VerificationRequest, + VerificationIncomingRequest, VerificationInfo, VerificationRequest, VerificationRequestStatus, }, - Address, CONTRACT_DEPLOYER_ADDRESS, + web3, Address, CONTRACT_DEPLOYER_ADDRESS, H256, }; use zksync_utils::address_to_h256; use zksync_vm_interface::VmEvent; -use crate::{models::storage_verification_request::StorageVerificationRequest, Core}; - -#[derive(Debug)] -pub struct ContractVerificationDal<'a, 'c> { - pub(crate) storage: &'a mut Connection<'c, Core>, -} +use crate::{ + models::storage_verification_request::StorageVerificationRequest, Connection, Core, DalResult, +}; #[derive(Debug)] enum Compiler { @@ -43,6 +41,23 @@ impl Display for Compiler { } } +#[derive(Debug)] +pub struct DeployedContractData { + pub bytecode_hash: H256, + /// Bytecode as persisted in Postgres (i.e., with additional padding for EVM bytecodes). + pub bytecode: Vec, + /// Recipient of the deployment transaction. + pub contract_address: Option
, + /// Call data for the deployment transaction. + pub calldata: Option>, +} + +#[derive(Debug)] +pub struct ContractVerificationDal<'a, 'c> { + pub(crate) storage: &'a mut Connection<'c, Core>, +} + +// FIXME: instrument other queries impl ContractVerificationDal<'_, '_> { pub async fn get_count_of_queued_verification_requests(&mut self) -> sqlx::Result { sqlx::query!( @@ -286,17 +301,18 @@ impl ContractVerificationDal<'_, '_> { } /// Returns bytecode and calldata from the contract and the transaction that created it. + // FIXME: add unit test pub async fn get_contract_info_for_verification( &mut self, address: Address, - ) -> anyhow::Result, DeployContractCalldata)>> { + ) -> DalResult> { let address_h256 = address_to_h256(&address); - - let Some(row) = sqlx::query!( + sqlx::query!( r#" SELECT + factory_deps.bytecode_hash, factory_deps.bytecode, - transactions.data AS "data?", + transactions.data -> 'calldata' AS "calldata?", transactions.contract_address AS "contract_address?" FROM ( @@ -327,26 +343,25 @@ impl ContractVerificationDal<'_, '_> { VmEvent::DEPLOY_EVENT_SIGNATURE.as_bytes(), address_h256.as_bytes(), ) - .fetch_optional(self.storage.conn()) - .await? - else { - return Ok(None); - }; - let calldata = match row.contract_address { - Some(contract_address) if contract_address == CONTRACT_DEPLOYER_ADDRESS.0.to_vec() => { - // `row.contract_address` and `row.data` are either both `None` or both `Some(_)`. - // In this arm it's checked that `row.contract_address` is `Some(_)`, so it's safe to unwrap `row.data`. - let data: serde_json::Value = row.data.context("data missing")?; - let calldata_str: String = serde_json::from_value( - data.get("calldata").context("calldata missing")?.clone(), - ) - .context("failed parsing calldata")?; - let calldata = hex::decode(&calldata_str[2..]).context("invalid calldata")?; - DeployContractCalldata::Deploy(calldata) - } - _ => DeployContractCalldata::Ignore, - }; - Ok(Some((row.bytecode, calldata))) + .try_map(|row| { + Ok(DeployedContractData { + bytecode_hash: H256::from_slice(&row.bytecode_hash), + bytecode: row.bytecode, + contract_address: row.contract_address.as_deref().map(Address::from_slice), + calldata: row + .calldata + .map(|calldata| { + serde_json::from_value::(calldata) + .decode_column("calldata") + .map(|bytes| bytes.0) + }) + .transpose()?, + }) + }) + .instrument("get_contract_info_for_verification") + .with_arg("address", &address) + .fetch_optional(self.storage) + .await } /// Returns true if the contract has a stored contracts_verification_info. diff --git a/core/lib/types/src/contract_verification_api.rs b/core/lib/types/src/contract_verification_api.rs index 8ee1d3ec6491..977202d0a8fc 100644 --- a/core/lib/types/src/contract_verification_api.rs +++ b/core/lib/types/src/contract_verification_api.rs @@ -235,12 +235,6 @@ pub struct VerificationRequestStatus { pub compilation_errors: Option>, } -#[derive(Debug)] -pub enum DeployContractCalldata { - Deploy(Vec), - Ignore, -} - #[cfg(test)] mod tests { use assert_matches::assert_matches; From 034f9c66229fbbf19fd607e16dbbb12b822aba92 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 1 Nov 2024 14:17:07 +0200 Subject: [PATCH 02/19] Use `DalResult`s in verification DAL --- core/lib/contract_verifier/src/lib.rs | 8 +- core/lib/dal/src/contract_verification_dal.rs | 184 ++++++++++-------- 2 files changed, 105 insertions(+), 87 deletions(-) diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index c91543024128..2afce61476bc 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -500,7 +500,7 @@ impl ContractVerifier { }; storage .contract_verification_dal() - .save_verification_error(request_id, error_message, compilation_errors, None) + .save_verification_error(request_id, &error_message, &compilation_errors, None) .await?; tracing::info!("Request with id = {request_id} was failed"); } @@ -547,9 +547,9 @@ impl JobProcessor for ContractVerifier { .contract_verification_dal() .save_verification_error( job_id, - "Internal error".to_string(), - serde_json::Value::Array(Vec::new()), - Some(error), + "Internal error", + &serde_json::Value::Array(Vec::new()), + Some(&error), ) .await .unwrap(); diff --git a/core/lib/dal/src/contract_verification_dal.rs b/core/lib/dal/src/contract_verification_dal.rs index c168ffb36aa7..602bc847f3de 100644 --- a/core/lib/dal/src/contract_verification_dal.rs +++ b/core/lib/dal/src/contract_verification_dal.rs @@ -5,7 +5,6 @@ use std::{ time::Duration, }; -use anyhow::Context as _; use sqlx::postgres::types::PgInterval; use zksync_db_connection::{error::SqlxContext, instrument::InstrumentExt}; use zksync_types::{ @@ -57,9 +56,8 @@ pub struct ContractVerificationDal<'a, 'c> { pub(crate) storage: &'a mut Connection<'c, Core>, } -// FIXME: instrument other queries impl ContractVerificationDal<'_, '_> { - pub async fn get_count_of_queued_verification_requests(&mut self) -> sqlx::Result { + pub async fn get_count_of_queued_verification_requests(&mut self) -> DalResult { sqlx::query!( r#" SELECT @@ -70,7 +68,8 @@ impl ContractVerificationDal<'_, '_> { status = 'queued' "# ) - .fetch_one(self.storage.conn()) + .instrument("get_count_of_queued_verification_requests") + .fetch_one(self.storage) .await .map(|row| row.count as usize) } @@ -78,7 +77,7 @@ impl ContractVerificationDal<'_, '_> { pub async fn add_contract_verification_request( &mut self, query: VerificationIncomingRequest, - ) -> sqlx::Result { + ) -> DalResult { sqlx::query!( r#" INSERT INTO @@ -114,7 +113,9 @@ impl ContractVerificationDal<'_, '_> { query.is_system, query.force_evmla, ) - .fetch_one(self.storage.conn()) + .instrument("add_contract_verification_request") + .with_arg("address", &query.contract_address) + .fetch_one(self.storage) .await .map(|row| row.id as usize) } @@ -126,7 +127,7 @@ impl ContractVerificationDal<'_, '_> { pub async fn get_next_queued_verification_request( &mut self, processing_timeout: Duration, - ) -> sqlx::Result> { + ) -> DalResult> { let processing_timeout = PgInterval { months: 0, days: 0, @@ -175,7 +176,9 @@ impl ContractVerificationDal<'_, '_> { "#, &processing_timeout ) - .fetch_optional(self.storage.conn()) + .instrument("get_next_queued_verification_request") + .with_arg("processing_timeout", &processing_timeout) + .fetch_optional(self.storage) .await? .map(Into::into); Ok(result) @@ -185,12 +188,10 @@ impl ContractVerificationDal<'_, '_> { pub async fn save_verification_info( &mut self, verification_info: VerificationInfo, - ) -> anyhow::Result<()> { - let mut transaction = self - .storage - .start_transaction() - .await - .context("start_transaction()")?; + ) -> DalResult<()> { + let mut transaction = self.storage.start_transaction().await?; + let id = verification_info.request.id; + let address = verification_info.request.req.contract_address; sqlx::query!( r#" @@ -203,10 +204,12 @@ impl ContractVerificationDal<'_, '_> { "#, verification_info.request.id as i64, ) - .execute(transaction.conn()) + .instrument("save_verification_info#set_status") + .with_arg("id", &id) + .with_arg("address", &address) + .execute(&mut transaction) .await?; - let address = verification_info.request.req.contract_address; // Serialization should always succeed. let verification_info_json = serde_json::to_value(verification_info) .expect("Failed to serialize verification info into serde_json"); @@ -224,20 +227,22 @@ impl ContractVerificationDal<'_, '_> { address.as_bytes(), &verification_info_json ) - .execute(transaction.conn()) + .instrument("save_verification_info#insert") + .with_arg("id", &id) + .with_arg("address", &address) + .execute(&mut transaction) .await?; - transaction.commit().await.context("commit()")?; - Ok(()) + transaction.commit().await } pub async fn save_verification_error( &mut self, id: usize, - error: String, - compilation_errors: serde_json::Value, - panic_message: Option, - ) -> sqlx::Result<()> { + error: &str, + compilation_errors: &serde_json::Value, + panic_message: Option<&str>, + ) -> DalResult<()> { sqlx::query!( r#" UPDATE contract_verification_requests @@ -251,11 +256,14 @@ impl ContractVerificationDal<'_, '_> { id = $1 "#, id as i64, - error.as_str(), - &compilation_errors, + error, + compilation_errors, panic_message ) - .execute(self.storage.conn()) + .instrument("save_verification_error") + .with_arg("id", &id) + .with_arg("error", &error) + .execute(self.storage) .await?; Ok(()) } @@ -263,8 +271,8 @@ impl ContractVerificationDal<'_, '_> { pub async fn get_verification_request_status( &mut self, id: usize, - ) -> anyhow::Result> { - let Some(row) = sqlx::query!( + ) -> DalResult> { + sqlx::query!( r#" SELECT status, @@ -277,31 +285,35 @@ impl ContractVerificationDal<'_, '_> { "#, id as i64, ) - .fetch_optional(self.storage.conn()) - .await? - else { - return Ok(None); - }; - - let mut compilation_errors = vec![]; - if let Some(errors) = row.compilation_errors { - for value in errors.as_array().context("expected an array")? { - compilation_errors.push(value.as_str().context("expected string")?.to_string()); + .try_map(|row| { + let mut compilation_errors = vec![]; + if let Some(errors) = row.compilation_errors { + let serde_json::Value::Array(errors) = errors else { + return Err(anyhow::anyhow!("errors are not an array")) + .decode_column("compilation_errors")?; + }; + for value in errors { + let serde_json::Value::String(err) = value else { + return Err(anyhow::anyhow!("error is not a string")) + .decode_column("compilation_errors")?; + }; + compilation_errors.push(err.to_owned()); + } } - } - Ok(Some(VerificationRequestStatus { - status: row.status, - error: row.error, - compilation_errors: if compilation_errors.is_empty() { - None - } else { - Some(compilation_errors) - }, - })) + + Ok(VerificationRequestStatus { + status: row.status, + error: row.error, + compilation_errors: (!compilation_errors.is_empty()).then_some(compilation_errors), + }) + }) + .instrument("get_verification_request_status") + .with_arg("id", &id) + .fetch_optional(self.storage) + .await } /// Returns bytecode and calldata from the contract and the transaction that created it. - // FIXME: add unit test pub async fn get_contract_info_for_verification( &mut self, address: Address, @@ -365,7 +377,7 @@ impl ContractVerificationDal<'_, '_> { } /// Returns true if the contract has a stored contracts_verification_info. - pub async fn is_contract_verified(&mut self, address: Address) -> sqlx::Result { + pub async fn is_contract_verified(&mut self, address: Address) -> DalResult { let count = sqlx::query!( r#" SELECT @@ -377,13 +389,15 @@ impl ContractVerificationDal<'_, '_> { "#, address.as_bytes() ) - .fetch_one(self.storage.conn()) + .instrument("is_contract_verified") + .with_arg("address", &address) + .fetch_one(self.storage) .await? .count; Ok(count > 0) } - async fn get_compiler_versions(&mut self, compiler: Compiler) -> sqlx::Result> { + async fn get_compiler_versions(&mut self, compiler: Compiler) -> DalResult> { let compiler = format!("{compiler}"); let versions: Vec<_> = sqlx::query!( r#" @@ -398,7 +412,9 @@ impl ContractVerificationDal<'_, '_> { "#, &compiler ) - .fetch_all(self.storage.conn()) + .instrument("get_compiler_versions") + .with_arg("compiler", &compiler) + .fetch_all(self.storage) .await? .into_iter() .map(|row| row.version) @@ -406,19 +422,19 @@ impl ContractVerificationDal<'_, '_> { Ok(versions) } - pub async fn get_zksolc_versions(&mut self) -> sqlx::Result> { + pub async fn get_zksolc_versions(&mut self) -> DalResult> { self.get_compiler_versions(Compiler::ZkSolc).await } - pub async fn get_solc_versions(&mut self) -> sqlx::Result> { + pub async fn get_solc_versions(&mut self) -> DalResult> { self.get_compiler_versions(Compiler::Solc).await } - pub async fn get_zkvyper_versions(&mut self) -> sqlx::Result> { + pub async fn get_zkvyper_versions(&mut self) -> DalResult> { self.get_compiler_versions(Compiler::ZkVyper).await } - pub async fn get_vyper_versions(&mut self) -> sqlx::Result> { + pub async fn get_vyper_versions(&mut self) -> DalResult> { self.get_compiler_versions(Compiler::Vyper).await } @@ -426,12 +442,8 @@ impl ContractVerificationDal<'_, '_> { &mut self, compiler: Compiler, versions: Vec, - ) -> anyhow::Result<()> { - let mut transaction = self - .storage - .start_transaction() - .await - .context("start_transaction")?; + ) -> DalResult<()> { + let mut transaction = self.storage.start_transaction().await?; let compiler = format!("{compiler}"); sqlx::query!( @@ -442,7 +454,9 @@ impl ContractVerificationDal<'_, '_> { "#, &compiler ) - .execute(transaction.conn()) + .instrument("set_compiler_versions#delete") + .with_arg("compiler", &compiler) + .execute(&mut transaction) .await?; sqlx::query!( @@ -461,31 +475,33 @@ impl ContractVerificationDal<'_, '_> { &versions, &compiler, ) - .execute(transaction.conn()) + .instrument("set_compiler_versions#insert") + .with_arg("compiler", &compiler) + .with_arg("versions.len", &versions.len()) + .execute(&mut transaction) .await?; - transaction.commit().await.context("commit()")?; - Ok(()) + transaction.commit().await } - pub async fn set_zksolc_versions(&mut self, versions: Vec) -> anyhow::Result<()> { + pub async fn set_zksolc_versions(&mut self, versions: Vec) -> DalResult<()> { self.set_compiler_versions(Compiler::ZkSolc, versions).await } - pub async fn set_solc_versions(&mut self, versions: Vec) -> anyhow::Result<()> { + pub async fn set_solc_versions(&mut self, versions: Vec) -> DalResult<()> { self.set_compiler_versions(Compiler::Solc, versions).await } - pub async fn set_zkvyper_versions(&mut self, versions: Vec) -> anyhow::Result<()> { + pub async fn set_zkvyper_versions(&mut self, versions: Vec) -> DalResult<()> { self.set_compiler_versions(Compiler::ZkVyper, versions) .await } - pub async fn set_vyper_versions(&mut self, versions: Vec) -> anyhow::Result<()> { + pub async fn set_vyper_versions(&mut self, versions: Vec) -> DalResult<()> { self.set_compiler_versions(Compiler::Vyper, versions).await } - pub async fn get_all_successful_requests(&mut self) -> sqlx::Result> { + pub async fn get_all_successful_requests(&mut self) -> DalResult> { let result = sqlx::query_as!( StorageVerificationRequest, r#" @@ -509,7 +525,8 @@ impl ContractVerificationDal<'_, '_> { id "#, ) - .fetch_all(self.storage.conn()) + .instrument("get_all_successful_requests") + .fetch_all(self.storage) .await? .into_iter() .map(Into::into) @@ -520,8 +537,8 @@ impl ContractVerificationDal<'_, '_> { pub async fn get_contract_verification_info( &mut self, address: Address, - ) -> anyhow::Result> { - let Some(row) = sqlx::query!( + ) -> DalResult> { + Ok(sqlx::query!( r#" SELECT verification_info @@ -532,14 +549,15 @@ impl ContractVerificationDal<'_, '_> { "#, address.as_bytes(), ) - .fetch_optional(self.storage.conn()) + .try_map(|row| { + row.verification_info + .map(|info| serde_json::from_value(info).decode_column("verification_info")) + .transpose() + }) + .instrument("get_contract_verification_info") + .with_arg("address", &address) + .fetch_optional(self.storage) .await? - else { - return Ok(None); - }; - let Some(info) = row.verification_info else { - return Ok(None); - }; - Ok(Some(serde_json::from_value(info).context("invalid info")?)) + .flatten()) } } From 901dd51c9ea9b72424b9d6dc707f48b28df065b0 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 1 Nov 2024 14:55:26 +0200 Subject: [PATCH 03/19] Test `get_contract_info_for_verification` --- core/lib/dal/src/contract_verification_dal.rs | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/core/lib/dal/src/contract_verification_dal.rs b/core/lib/dal/src/contract_verification_dal.rs index 602bc847f3de..cff5a61a7e38 100644 --- a/core/lib/dal/src/contract_verification_dal.rs +++ b/core/lib/dal/src/contract_verification_dal.rs @@ -561,3 +561,84 @@ impl ContractVerificationDal<'_, '_> { .flatten()) } } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use zksync_types::{ + tx::IncludedTxLocation, Execute, L1BatchNumber, L2BlockNumber, ProtocolVersion, + }; + use zksync_utils::bytecode::hash_bytecode; + use zksync_vm_interface::TransactionExecutionMetrics; + + use super::*; + use crate::{ + tests::{create_l2_block_header, mock_l2_transaction}, + ConnectionPool, CoreDal, + }; + + #[tokio::test] + async fn getting_contract_info_for_verification() { + let pool = ConnectionPool::::test_pool().await; + let mut conn = pool.connection().await.unwrap(); + + conn.protocol_versions_dal() + .save_protocol_version_with_tx(&ProtocolVersion::default()) + .await + .unwrap(); + conn.blocks_dal() + .insert_l2_block(&create_l2_block_header(0)) + .await + .unwrap(); + + // Add a transaction, its bytecode and the bytecode deployment event. + let deployed_address = Address::repeat_byte(12); + let mut tx = mock_l2_transaction(); + let bytecode = vec![1; 32]; + let bytecode_hash = hash_bytecode(&bytecode); + tx.execute = Execute::for_deploy(H256::zero(), bytecode.clone(), &[]); + conn.transactions_dal() + .insert_transaction_l2(&tx, TransactionExecutionMetrics::default()) + .await + .unwrap(); + conn.factory_deps_dal() + .insert_factory_deps( + L2BlockNumber(0), + &HashMap::from([(bytecode_hash, bytecode.clone())]), + ) + .await + .unwrap(); + let location = IncludedTxLocation { + tx_hash: tx.hash(), + tx_index_in_l2_block: 0, + tx_initiator_address: tx.initiator_account(), + }; + let deploy_event = VmEvent { + location: (L1BatchNumber(0), 0), + address: CONTRACT_DEPLOYER_ADDRESS, + indexed_topics: vec![ + VmEvent::DEPLOY_EVENT_SIGNATURE, + address_to_h256(&tx.initiator_account()), + bytecode_hash, + address_to_h256(&deployed_address), + ], + value: vec![], + }; + conn.events_dal() + .save_events(L2BlockNumber(0), &[(location, vec![&deploy_event])]) + .await + .unwrap(); + + let contract = conn + .contract_verification_dal() + .get_contract_info_for_verification(deployed_address) + .await + .unwrap() + .expect("no info"); + assert_eq!(contract.bytecode_hash, bytecode_hash); + assert_eq!(contract.bytecode, bytecode); + assert_eq!(contract.contract_address, Some(CONTRACT_DEPLOYER_ADDRESS)); + assert_eq!(contract.calldata.unwrap(), tx.execute.calldata); + } +} From d75c7968e902c894c5b240744227a8ea1571e9e8 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 1 Nov 2024 15:52:41 +0200 Subject: [PATCH 04/19] Refactor compiler inputs --- .../contract_verifier/src/compilers/mod.rs | 7 + .../{zksolc_utils.rs => compilers/zksolc.rs} | 96 ++++++++++--- .../zkvyper.rs} | 26 +++- core/lib/contract_verifier/src/lib.rs | 134 +++--------------- core/lib/contract_verifier/src/resolver.rs | 11 +- core/lib/contract_verifier/src/tests/mod.rs | 13 +- core/lib/contract_verifier/src/tests/real.rs | 8 +- 7 files changed, 148 insertions(+), 147 deletions(-) create mode 100644 core/lib/contract_verifier/src/compilers/mod.rs rename core/lib/contract_verifier/src/{zksolc_utils.rs => compilers/zksolc.rs} (77%) rename core/lib/contract_verifier/src/{zkvyper_utils.rs => compilers/zkvyper.rs} (80%) diff --git a/core/lib/contract_verifier/src/compilers/mod.rs b/core/lib/contract_verifier/src/compilers/mod.rs new file mode 100644 index 000000000000..5a01ed4824cb --- /dev/null +++ b/core/lib/contract_verifier/src/compilers/mod.rs @@ -0,0 +1,7 @@ +pub(crate) use self::{ + zksolc::{ZkSolc, ZkSolcInput}, + zkvyper::{ZkVyper, ZkVyperInput}, +}; + +mod zksolc; +mod zkvyper; diff --git a/core/lib/contract_verifier/src/zksolc_utils.rs b/core/lib/contract_verifier/src/compilers/zksolc.rs similarity index 77% rename from core/lib/contract_verifier/src/zksolc_utils.rs rename to core/lib/contract_verifier/src/compilers/zksolc.rs index 0a3d84ab555b..6786dfc1f5f5 100644 --- a/core/lib/contract_verifier/src/zksolc_utils.rs +++ b/core/lib/contract_verifier/src/compilers/zksolc.rs @@ -6,7 +6,9 @@ use semver::Version; use serde::{Deserialize, Serialize}; use tokio::io::AsyncWriteExt; use zksync_queued_job_processor::async_trait; -use zksync_types::contract_verification_api::CompilationArtifacts; +use zksync_types::contract_verification_api::{ + CompilationArtifacts, SourceCodeData, VerificationIncomingRequest, +}; use crate::{ error::ContractVerifierError, @@ -14,7 +16,7 @@ use crate::{ }; #[derive(Debug)] -pub enum ZkSolcInput { +pub(crate) enum ZkSolcInput { StandardJson { input: StandardJson, contract_name: String, @@ -28,7 +30,7 @@ pub enum ZkSolcInput { #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct StandardJson { +pub(crate) struct StandardJson { /// The input language. pub language: String, /// The input source code files hashmap. @@ -39,7 +41,7 @@ pub struct StandardJson { #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct Source { +pub(crate) struct Source { /// The source code file content. pub content: String, } @@ -49,7 +51,7 @@ pub struct Source { /// Other fields are accumulated in `other`, this way every field that was in the original request will be passed to a compiler. #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct Settings { +pub(crate) struct Settings { /// The output selection filters. pub output_selection: Option, /// Flag for system compilation mode. @@ -58,29 +60,18 @@ pub struct Settings { /// Flag to force `evmla` IR. #[serde(default)] pub force_evmla: bool, - /// Other fields. - #[serde(flatten)] - pub other: serde_json::Value, + pub optimizer: Optimizer, } #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct Optimizer { +pub(crate) struct Optimizer { /// Whether the optimizer is enabled. pub enabled: bool, /// The optimization mode string. pub mode: Option, } -impl Default for Optimizer { - fn default() -> Self { - Self { - enabled: true, - mode: None, - } - } -} - #[derive(Debug)] pub(crate) struct ZkSolc { paths: CompilerPaths, @@ -95,6 +86,73 @@ impl ZkSolc { } } + pub fn build_input( + req: VerificationIncomingRequest, + ) -> Result { + // Users may provide either just contract name or + // source file name and contract name joined with ":". + let (file_name, contract_name) = + if let Some((file_name, contract_name)) = req.contract_name.rsplit_once(':') { + (file_name.to_string(), contract_name.to_string()) + } else { + ( + format!("{}.sol", req.contract_name), + req.contract_name.clone(), + ) + }; + let default_output_selection = serde_json::json!({ + "*": { + "*": [ "abi" ], + "": [ "abi" ] + } + }); + + match req.source_code_data { + SourceCodeData::SolSingleFile(source_code) => { + let source = Source { + content: source_code, + }; + let sources = HashMap::from([(file_name.clone(), source)]); + let settings = Settings { + output_selection: Some(default_output_selection), + is_system: req.is_system, + force_evmla: req.force_evmla, + optimizer: Optimizer { + enabled: req.optimization_used, + mode: req.optimizer_mode.and_then(|s| s.chars().next()), + }, + }; + + Ok(ZkSolcInput::StandardJson { + input: StandardJson { + language: "Solidity".to_string(), + sources, + settings, + }, + contract_name, + file_name, + }) + } + SourceCodeData::StandardJsonInput(map) => { + let mut compiler_input: StandardJson = + serde_json::from_value(serde_json::Value::Object(map)) + .map_err(|_| ContractVerifierError::FailedToDeserializeInput)?; + // Set default output selection even if it is different in request. + compiler_input.settings.output_selection = Some(default_output_selection); + Ok(ZkSolcInput::StandardJson { + input: compiler_input, + contract_name, + file_name, + }) + } + SourceCodeData::YulSingleFile(source_code) => Ok(ZkSolcInput::YulSingleFile { + source_code, + is_system: req.is_system, + }), + other => unreachable!("Unexpected `SourceCodeData` variant: {other:?}"), + } + } + fn parse_standard_json_output( output: &serde_json::Value, contract_name: String, @@ -279,7 +337,7 @@ impl Compiler for ZkSolc { mod tests { use std::path::PathBuf; - use crate::{resolver::CompilerPaths, zksolc_utils::ZkSolc}; + use super::*; #[test] fn check_is_post_1_5_0() { diff --git a/core/lib/contract_verifier/src/zkvyper_utils.rs b/core/lib/contract_verifier/src/compilers/zkvyper.rs similarity index 80% rename from core/lib/contract_verifier/src/zkvyper_utils.rs rename to core/lib/contract_verifier/src/compilers/zkvyper.rs index bc2cd7e996c1..cc65b7d0c92e 100644 --- a/core/lib/contract_verifier/src/zkvyper_utils.rs +++ b/core/lib/contract_verifier/src/compilers/zkvyper.rs @@ -2,7 +2,9 @@ use std::{collections::HashMap, fs::File, io::Write, path::Path, process::Stdio} use anyhow::Context as _; use zksync_queued_job_processor::async_trait; -use zksync_types::contract_verification_api::CompilationArtifacts; +use zksync_types::contract_verification_api::{ + CompilationArtifacts, SourceCodeData, VerificationIncomingRequest, +}; use crate::{ error::ContractVerifierError, @@ -26,6 +28,28 @@ impl ZkVyper { Self { paths } } + pub fn build_input( + req: VerificationIncomingRequest, + ) -> Result { + // Users may provide either just contract name or + // source file name and contract name joined with ":". + let contract_name = if let Some((_, contract_name)) = req.contract_name.rsplit_once(':') { + contract_name.to_owned() + } else { + req.contract_name.clone() + }; + + let sources = match req.source_code_data { + SourceCodeData::VyperMultiFile(s) => s, + other => unreachable!("unexpected `SourceCodeData` variant: {other:?}"), + }; + Ok(ZkVyperInput { + contract_name, + sources, + optimizer_mode: req.optimizer_mode, + }) + } + fn parse_output( output: &serde_json::Value, contract_name: String, diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index 2afce61476bc..45a90962e1ce 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -1,7 +1,6 @@ //! Contract verifier able to verify contracts created with `zksolc` or `zkvyper` toolchains. use std::{ - collections::HashMap, fmt, sync::Arc, time::{Duration, Instant}, @@ -15,27 +14,26 @@ use zksync_dal::{contract_verification_dal::DeployedContractData, ConnectionPool use zksync_queued_job_processor::{async_trait, JobProcessor}; use zksync_types::{ contract_verification_api::{ - CompilationArtifacts, CompilerType, SourceCodeData, VerificationIncomingRequest, - VerificationInfo, VerificationRequest, + CompilationArtifacts, CompilerType, VerificationIncomingRequest, VerificationInfo, + VerificationRequest, }, Address, CONTRACT_DEPLOYER_ADDRESS, }; +use zksync_utils::bytecode::BytecodeMarker; use crate::{ + compilers::{ZkSolc, ZkVyper}, error::ContractVerifierError, metrics::API_CONTRACT_VERIFIER_METRICS, resolver::{CompilerResolver, EnvCompilerResolver}, - zksolc_utils::{Optimizer, Settings, Source, StandardJson, ZkSolcInput}, - zkvyper_utils::ZkVyperInput, }; +mod compilers; pub mod error; mod metrics; mod resolver; #[cfg(test)] mod tests; -mod zksolc_utils; -mod zkvyper_utils; enum ConstructorArgs { Check(Vec), @@ -142,8 +140,6 @@ impl ContractVerifier { &self, mut request: VerificationRequest, ) -> Result { - let artifacts = self.compile(request.req.clone()).await?; - // Bytecode should be present because it is checked when accepting request. let mut storage = self .connection_pool @@ -160,11 +156,15 @@ impl ContractVerifier { ) })?; drop(storage); - // FIXME: make compilation depend on bytecode type + + let bytecode_marker = BytecodeMarker::new(deployed_contract.bytecode_hash) + .context("unknown bytecode kind")?; + let artifacts = self.compile(request.req.clone(), bytecode_marker).await?; let constructor_args = self.decode_constructor_args(&deployed_contract, request.req.contract_address)?; + // FIXME: post-process EVM bytecode if artifacts.bytecode != deployed_contract.bytecode { tracing::info!( "Bytecode mismatch req {}, deployed: 0x{}, compiled: 0x{}", @@ -207,10 +207,10 @@ impl ContractVerifier { ) -> Result { let zksolc = self .compiler_resolver - .resolve_solc(&req.compiler_versions) + .resolve_zksolc(&req.compiler_versions) .await?; tracing::debug!(?zksolc, ?req.compiler_versions, "resolved compiler"); - let input = Self::build_zksolc_input(req)?; + let input = ZkSolc::build_input(req)?; time::timeout(self.compilation_timeout, zksolc.compile(input)) .await @@ -223,10 +223,10 @@ impl ContractVerifier { ) -> Result { let zkvyper = self .compiler_resolver - .resolve_vyper(&req.compiler_versions) + .resolve_zkvyper(&req.compiler_versions) .await?; tracing::debug!(?zkvyper, ?req.compiler_versions, "resolved compiler"); - let input = Self::build_zkvyper_input(req)?; + let input = ZkVyper::build_input(req)?; time::timeout(self.compilation_timeout, zkvyper.compile(input)) .await .map_err(|_| ContractVerifierError::CompilationTimeout)? @@ -236,109 +236,17 @@ impl ContractVerifier { async fn compile( &self, req: VerificationIncomingRequest, + bytecode_marker: BytecodeMarker, ) -> Result { - match req.source_code_data.compiler_type() { - CompilerType::Solc => self.compile_zksolc(req).await, - CompilerType::Vyper => self.compile_zkvyper(req).await, + let compiler_type = req.source_code_data.compiler_type(); + match (bytecode_marker, compiler_type) { + (BytecodeMarker::EraVm, CompilerType::Solc) => self.compile_zksolc(req).await, + (BytecodeMarker::EraVm, CompilerType::Vyper) => self.compile_zkvyper(req).await, + (BytecodeMarker::Evm, CompilerType::Solc) => todo!(), + (BytecodeMarker::Evm, CompilerType::Vyper) => todo!(), } } - fn build_zksolc_input( - req: VerificationIncomingRequest, - ) -> Result { - // Users may provide either just contract name or - // source file name and contract name joined with ":". - let (file_name, contract_name) = - if let Some((file_name, contract_name)) = req.contract_name.rsplit_once(':') { - (file_name.to_string(), contract_name.to_string()) - } else { - ( - format!("{}.sol", req.contract_name), - req.contract_name.clone(), - ) - }; - let default_output_selection = serde_json::json!({ - "*": { - "*": [ "abi" ], - "": [ "abi" ] - } - }); - - match req.source_code_data { - SourceCodeData::SolSingleFile(source_code) => { - let source = Source { - content: source_code, - }; - let sources = HashMap::from([(file_name.clone(), source)]); - let optimizer = Optimizer { - enabled: req.optimization_used, - mode: req.optimizer_mode.and_then(|s| s.chars().next()), - }; - let optimizer_value = serde_json::to_value(optimizer).unwrap(); - - let settings = Settings { - output_selection: Some(default_output_selection), - is_system: req.is_system, - force_evmla: req.force_evmla, - other: serde_json::Value::Object( - vec![("optimizer".to_string(), optimizer_value)] - .into_iter() - .collect(), - ), - }; - - Ok(ZkSolcInput::StandardJson { - input: StandardJson { - language: "Solidity".to_string(), - sources, - settings, - }, - contract_name, - file_name, - }) - } - SourceCodeData::StandardJsonInput(map) => { - let mut compiler_input: StandardJson = - serde_json::from_value(serde_json::Value::Object(map)) - .map_err(|_| ContractVerifierError::FailedToDeserializeInput)?; - // Set default output selection even if it is different in request. - compiler_input.settings.output_selection = Some(default_output_selection); - Ok(ZkSolcInput::StandardJson { - input: compiler_input, - contract_name, - file_name, - }) - } - SourceCodeData::YulSingleFile(source_code) => Ok(ZkSolcInput::YulSingleFile { - source_code, - is_system: req.is_system, - }), - other => unreachable!("Unexpected `SourceCodeData` variant: {other:?}"), - } - } - - fn build_zkvyper_input( - req: VerificationIncomingRequest, - ) -> Result { - // Users may provide either just contract name or - // source file name and contract name joined with ":". - let contract_name = if let Some((_, contract_name)) = req.contract_name.rsplit_once(':') { - contract_name.to_owned() - } else { - req.contract_name.clone() - }; - - let sources = match req.source_code_data { - SourceCodeData::VyperMultiFile(s) => s, - other => unreachable!("unexpected `SourceCodeData` variant: {other:?}"), - }; - Ok(ZkVyperInput { - contract_name, - sources, - optimizer_mode: req.optimizer_mode, - }) - } - /// All returned errors are internal. #[tracing::instrument(level = "trace", skip_all, ret, err)] fn decode_constructor_args( diff --git a/core/lib/contract_verifier/src/resolver.rs b/core/lib/contract_verifier/src/resolver.rs index 5e7729006698..4376180c3811 100644 --- a/core/lib/contract_verifier/src/resolver.rs +++ b/core/lib/contract_verifier/src/resolver.rs @@ -7,9 +7,8 @@ use zksync_types::contract_verification_api::{CompilationArtifacts, CompilerVers use zksync_utils::env::Workspace; use crate::{ + compilers::{ZkSolc, ZkSolcInput, ZkVyper, ZkVyperInput}, error::ContractVerifierError, - zksolc_utils::{ZkSolc, ZkSolcInput}, - zkvyper_utils::{ZkVyper, ZkVyperInput}, }; /// Compiler versions supported by a [`CompilerResolver`]. @@ -49,13 +48,13 @@ pub(crate) trait CompilerResolver: fmt::Debug + Send + Sync { async fn supported_versions(&self) -> anyhow::Result; /// Resolves a `zksolc` compiler. - async fn resolve_solc( + async fn resolve_zksolc( &self, versions: &CompilerVersions, ) -> Result>, ContractVerifierError>; /// Resolves a `zkvyper` compiler. - async fn resolve_vyper( + async fn resolve_zkvyper( &self, versions: &CompilerVersions, ) -> Result>, ContractVerifierError>; @@ -128,7 +127,7 @@ impl CompilerResolver for EnvCompilerResolver { }) } - async fn resolve_solc( + async fn resolve_zksolc( &self, versions: &CompilerVersions, ) -> Result>, ContractVerifierError> { @@ -173,7 +172,7 @@ impl CompilerResolver for EnvCompilerResolver { Ok(Box::new(ZkSolc::new(compiler_paths, zksolc_version))) } - async fn resolve_vyper( + async fn resolve_zkvyper( &self, versions: &CompilerVersions, ) -> Result>, ContractVerifierError> { diff --git a/core/lib/contract_verifier/src/tests/mod.rs b/core/lib/contract_verifier/src/tests/mod.rs index ae7b23ebab8a..5196afc7dc2d 100644 --- a/core/lib/contract_verifier/src/tests/mod.rs +++ b/core/lib/contract_verifier/src/tests/mod.rs @@ -1,10 +1,12 @@ //! Tests for the contract verifier. +use std::collections::HashMap; + use tokio::sync::watch; use zksync_dal::Connection; use zksync_node_test_utils::{create_l1_batch, create_l2_block}; use zksync_types::{ - contract_verification_api::{CompilerVersions, VerificationIncomingRequest}, + contract_verification_api::{CompilerVersions, SourceCodeData, VerificationIncomingRequest}, get_code_key, get_known_code_key, l2::L2Tx, tx::IncludedTxLocation, @@ -15,7 +17,10 @@ use zksync_utils::{address_to_h256, bytecode::hash_bytecode}; use zksync_vm_interface::{tracer::ValidationTraces, TransactionExecutionMetrics, VmEvent}; use super::*; -use crate::resolver::{Compiler, SupportedCompilerVersions}; +use crate::{ + compilers::{ZkSolcInput, ZkVyperInput}, + resolver::{Compiler, SupportedCompilerVersions}, +}; mod real; @@ -127,7 +132,7 @@ impl CompilerResolver for MockCompilerResolver { }) } - async fn resolve_solc( + async fn resolve_zksolc( &self, versions: &CompilerVersions, ) -> Result>, ContractVerifierError> { @@ -146,7 +151,7 @@ impl CompilerResolver for MockCompilerResolver { Ok(Box::new(self.clone())) } - async fn resolve_vyper( + async fn resolve_zkvyper( &self, _versions: &CompilerVersions, ) -> Result>, ContractVerifierError> { diff --git a/core/lib/contract_verifier/src/tests/real.rs b/core/lib/contract_verifier/src/tests/real.rs index b7acc753fd6d..064c39f26df0 100644 --- a/core/lib/contract_verifier/src/tests/real.rs +++ b/core/lib/contract_verifier/src/tests/real.rs @@ -29,12 +29,12 @@ async fn using_real_compiler() { compiler_zksolc_version: supported_compilers.zksolc[0].clone(), compiler_solc_version: supported_compilers.solc[0].clone(), }; - let compiler = compiler_resolver.resolve_solc(&versions).await.unwrap(); + let compiler = compiler_resolver.resolve_zksolc(&versions).await.unwrap(); let req = VerificationIncomingRequest { compiler_versions: versions, ..test_request(Address::repeat_byte(1)) }; - let input = ContractVerifier::build_zksolc_input(req).unwrap(); + let input = ZkSolc::build_input(req).unwrap(); let output = compiler.compile(input).await.unwrap(); validate_bytecode(&output.bytecode).unwrap(); @@ -53,12 +53,12 @@ async fn using_real_compiler_in_verifier() { compiler_solc_version: supported_compilers.solc[0].clone(), }; let address = Address::repeat_byte(1); - let compiler = compiler_resolver.resolve_solc(&versions).await.unwrap(); + let compiler = compiler_resolver.resolve_zksolc(&versions).await.unwrap(); let req = VerificationIncomingRequest { compiler_versions: versions, ..test_request(Address::repeat_byte(1)) }; - let input = ContractVerifier::build_zksolc_input(req.clone()).unwrap(); + let input = ZkSolc::build_input(req.clone()).unwrap(); let output = compiler.compile(input).await.unwrap(); let pool = ConnectionPool::test_pool().await; From 8d37ec77c5ed296a9e77ad7f5bbc8349227c17e7 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 1 Nov 2024 16:34:00 +0200 Subject: [PATCH 05/19] Sketch standalone `solc` compiler support --- .../contract_verifier/src/compilers/mod.rs | 60 +++++++ .../contract_verifier/src/compilers/solc.rs | 165 ++++++++++++++++++ .../contract_verifier/src/compilers/zksolc.rs | 67 ++----- core/lib/contract_verifier/src/lib.rs | 20 ++- core/lib/contract_verifier/src/resolver.rs | 56 ++++-- core/lib/contract_verifier/src/tests/mod.rs | 15 +- 6 files changed, 306 insertions(+), 77 deletions(-) create mode 100644 core/lib/contract_verifier/src/compilers/solc.rs diff --git a/core/lib/contract_verifier/src/compilers/mod.rs b/core/lib/contract_verifier/src/compilers/mod.rs index 5a01ed4824cb..e146f364dbaf 100644 --- a/core/lib/contract_verifier/src/compilers/mod.rs +++ b/core/lib/contract_verifier/src/compilers/mod.rs @@ -1,7 +1,67 @@ +use serde::{Deserialize, Serialize}; +use zksync_types::contract_verification_api::CompilationArtifacts; + pub(crate) use self::{ + solc::{Solc, SolcInput}, zksolc::{ZkSolc, ZkSolcInput}, zkvyper::{ZkVyper, ZkVyperInput}, }; +use crate::error::ContractVerifierError; +mod solc; mod zksolc; mod zkvyper; + +#[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct Source { + /// The source code file content. + pub content: String, +} + +/// Parsing logic shared between `solc` and `zksolc`. +// FIXME: deployed bytecode. +fn parse_standard_json_output( + output: &serde_json::Value, + contract_name: String, + file_name: String, +) -> Result { + if let Some(errors) = output.get("errors") { + let errors = errors.as_array().unwrap().clone(); + if errors + .iter() + .any(|err| err["severity"].as_str().unwrap() == "error") + { + let error_messages = errors + .into_iter() + .map(|err| err["formattedMessage"].clone()) + .collect(); + return Err(ContractVerifierError::CompilationError( + serde_json::Value::Array(error_messages), + )); + } + } + + let contracts = output["contracts"] + .get(&file_name) + .ok_or(ContractVerifierError::MissingSource(file_name))?; + let Some(contract) = contracts.get(&contract_name) else { + return Err(ContractVerifierError::MissingContract(contract_name)); + }; + + let bytecode_str = contract["evm"]["bytecode"]["object"] + .as_str() + .ok_or(ContractVerifierError::AbstractContract(contract_name))?; + let bytecode = hex::decode(bytecode_str).unwrap(); + + let abi = contract["abi"].clone(); + if !abi.is_array() { + let err = anyhow::anyhow!( + "unexpected value for ABI: {}", + serde_json::to_string_pretty(&abi).unwrap() + ); + return Err(err.into()); + } + + Ok(CompilationArtifacts { bytecode, abi }) +} diff --git a/core/lib/contract_verifier/src/compilers/solc.rs b/core/lib/contract_verifier/src/compilers/solc.rs new file mode 100644 index 000000000000..4a617a817cc7 --- /dev/null +++ b/core/lib/contract_verifier/src/compilers/solc.rs @@ -0,0 +1,165 @@ +use std::{collections::HashMap, path::PathBuf, process::Stdio}; + +use anyhow::Context; +use serde::{Deserialize, Serialize}; +use tokio::io::AsyncWriteExt; +use zksync_queued_job_processor::async_trait; +use zksync_types::contract_verification_api::{ + CompilationArtifacts, SourceCodeData, VerificationIncomingRequest, +}; + +use super::{parse_standard_json_output, Source}; +use crate::{error::ContractVerifierError, resolver::Compiler}; + +#[derive(Debug)] +pub(crate) struct SolcInput { + standard_json: StandardJson, + contract_name: String, + file_name: String, +} + +#[derive(Debug, Serialize, Deserialize)] +struct StandardJson { + language: String, + sources: HashMap, + settings: Settings, +} + +#[derive(Debug, Serialize, Deserialize)] +struct Settings { + /// The output selection filters. + output_selection: Option, + /// Other settings (only filled when parsing `StandardJson` input from the request). + #[serde(flatten)] + other: serde_json::Value, +} + +#[derive(Debug)] +pub(crate) struct Solc { + path: PathBuf, +} + +impl Solc { + pub fn new(path: PathBuf) -> Self { + Self { path } + } + + pub fn build_input( + req: VerificationIncomingRequest, + ) -> Result { + // Users may provide either just contract name or + // source file name and contract name joined with ":". + let (file_name, contract_name) = + if let Some((file_name, contract_name)) = req.contract_name.rsplit_once(':') { + (file_name.to_string(), contract_name.to_string()) + } else { + ( + format!("{}.sol", req.contract_name), + req.contract_name.clone(), + ) + }; + let default_output_selection = serde_json::json!({ + "*": { + "*": [ "abi" ], + "": [ "abi" ] + } + }); + + let standard_json = match req.source_code_data { + SourceCodeData::SolSingleFile(source_code) => { + let source = Source { + content: source_code, + }; + let sources = HashMap::from([(file_name.clone(), source)]); + let settings = Settings { + output_selection: Some(default_output_selection), + other: serde_json::json!({ + "optimizer": { + "enabled": req.optimization_used, + }, + }), + }; + + StandardJson { + language: "Solidity".to_owned(), + sources, + settings, + } + } + SourceCodeData::StandardJsonInput(map) => { + let mut compiler_input: StandardJson = + serde_json::from_value(serde_json::Value::Object(map)) + .map_err(|_| ContractVerifierError::FailedToDeserializeInput)?; + // Set default output selection even if it is different in request. + compiler_input.settings.output_selection = Some(default_output_selection); + compiler_input + } + SourceCodeData::YulSingleFile(source_code) => { + let source = Source { + content: source_code, + }; + let sources = HashMap::from([(file_name.clone(), source)]); + let settings = Settings { + output_selection: Some(default_output_selection), + other: serde_json::json!({ + "optimizer": { + "enabled": req.optimization_used, + }, + }), + }; + StandardJson { + language: "Yul".to_owned(), + sources, + settings, + } + } + other => unreachable!("Unexpected `SourceCodeData` variant: {other:?}"), + }; + + Ok(SolcInput { + standard_json, + contract_name, + file_name, + }) + } +} + +#[async_trait] +impl Compiler for Solc { + async fn compile( + self: Box, + input: SolcInput, + ) -> Result { + let mut command = tokio::process::Command::new(&self.path); + let mut child = command + .arg("--standard-json") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .context("failed spawning solc")?; + let stdin = child.stdin.as_mut().unwrap(); + let content = serde_json::to_vec(&input.standard_json) + .context("cannot encode standard JSON input for solc")?; + stdin + .write_all(&content) + .await + .context("failed writing standard JSON to solc stdin")?; + stdin + .flush() + .await + .context("failed flushing standard JSON to solc")?; + + let output = child.wait_with_output().await.context("solc failed")?; + if output.status.success() { + let output = serde_json::from_slice(&output.stdout) + .context("zksolc output is not valid JSON")?; + parse_standard_json_output(&output, input.contract_name, input.file_name) + } else { + Err(ContractVerifierError::CompilerError( + "zksolc".to_string(), + String::from_utf8_lossy(&output.stderr).to_string(), + )) + } + } +} diff --git a/core/lib/contract_verifier/src/compilers/zksolc.rs b/core/lib/contract_verifier/src/compilers/zksolc.rs index 6786dfc1f5f5..5ce05804114c 100644 --- a/core/lib/contract_verifier/src/compilers/zksolc.rs +++ b/core/lib/contract_verifier/src/compilers/zksolc.rs @@ -10,6 +10,7 @@ use zksync_types::contract_verification_api::{ CompilationArtifacts, SourceCodeData, VerificationIncomingRequest, }; +use super::{parse_standard_json_output, Source}; use crate::{ error::ContractVerifierError, resolver::{Compiler, CompilerPaths}, @@ -39,13 +40,6 @@ pub(crate) struct StandardJson { pub settings: Settings, } -#[derive(Debug, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub(crate) struct Source { - /// The source code file content. - pub content: String, -} - /// Compiler settings. /// There are fields like `output_selection`, `is_system`, `force_evmla` which are accessed by contract verifier explicitly. /// Other fields are accumulated in `other`, this way every field that was in the original request will be passed to a compiler. @@ -60,7 +54,9 @@ pub(crate) struct Settings { /// Flag to force `evmla` IR. #[serde(default)] pub force_evmla: bool, - pub optimizer: Optimizer, + /// Other settings (only filled when parsing `StandardJson` input from the request). + #[serde(flatten)] + pub other: serde_json::Value, } #[derive(Debug, Serialize, Deserialize)] @@ -117,10 +113,12 @@ impl ZkSolc { output_selection: Some(default_output_selection), is_system: req.is_system, force_evmla: req.force_evmla, - optimizer: Optimizer { - enabled: req.optimization_used, - mode: req.optimizer_mode.and_then(|s| s.chars().next()), - }, + other: serde_json::json!({ + "optimizer": Optimizer { + enabled: req.optimization_used, + mode: req.optimizer_mode.and_then(|s| s.chars().next()), + }, + }), }; Ok(ZkSolcInput::StandardJson { @@ -153,49 +151,6 @@ impl ZkSolc { } } - fn parse_standard_json_output( - output: &serde_json::Value, - contract_name: String, - file_name: String, - ) -> Result { - if let Some(errors) = output.get("errors") { - let errors = errors.as_array().unwrap().clone(); - if errors - .iter() - .any(|err| err["severity"].as_str().unwrap() == "error") - { - let error_messages = errors - .into_iter() - .map(|err| err["formattedMessage"].clone()) - .collect(); - return Err(ContractVerifierError::CompilationError( - serde_json::Value::Array(error_messages), - )); - } - } - - let contracts = output["contracts"] - .get(&file_name) - .ok_or(ContractVerifierError::MissingSource(file_name))?; - let Some(contract) = contracts.get(&contract_name) else { - return Err(ContractVerifierError::MissingContract(contract_name)); - }; - let bytecode_str = contract["evm"]["bytecode"]["object"] - .as_str() - .ok_or(ContractVerifierError::AbstractContract(contract_name))?; - let bytecode = hex::decode(bytecode_str).unwrap(); - let abi = contract["abi"].clone(); - if !abi.is_array() { - let err = anyhow::anyhow!( - "zksolc returned unexpected value for ABI: {}", - serde_json::to_string_pretty(&abi).unwrap() - ); - return Err(err.into()); - } - - Ok(CompilationArtifacts { bytecode, abi }) - } - fn parse_single_file_yul_output( output: &str, ) -> Result { @@ -292,7 +247,7 @@ impl Compiler for ZkSolc { if output.status.success() { let output = serde_json::from_slice(&output.stdout) .context("zksolc output is not valid JSON")?; - Self::parse_standard_json_output(&output, contract_name, file_name) + parse_standard_json_output(&output, contract_name, file_name) } else { Err(ContractVerifierError::CompilerError( "zksolc".to_string(), diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index 45a90962e1ce..da3dc7bc13ef 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -22,7 +22,7 @@ use zksync_types::{ use zksync_utils::bytecode::BytecodeMarker; use crate::{ - compilers::{ZkSolc, ZkVyper}, + compilers::{Solc, ZkSolc, ZkVyper}, error::ContractVerifierError, metrics::API_CONTRACT_VERIFIER_METRICS, resolver::{CompilerResolver, EnvCompilerResolver}, @@ -232,6 +232,22 @@ impl ContractVerifier { .map_err(|_| ContractVerifierError::CompilationTimeout)? } + async fn compile_solc( + &self, + req: VerificationIncomingRequest, + ) -> Result { + let solc = self + .compiler_resolver + .resolve_solc(&req.compiler_versions.compiler_version()) + .await?; + tracing::debug!(?solc, ?req.compiler_versions, "resolved compiler"); + let input = Solc::build_input(req)?; + + time::timeout(self.compilation_timeout, solc.compile(input)) + .await + .map_err(|_| ContractVerifierError::CompilationTimeout)? + } + #[tracing::instrument(level = "debug", skip_all)] async fn compile( &self, @@ -242,7 +258,7 @@ impl ContractVerifier { match (bytecode_marker, compiler_type) { (BytecodeMarker::EraVm, CompilerType::Solc) => self.compile_zksolc(req).await, (BytecodeMarker::EraVm, CompilerType::Vyper) => self.compile_zkvyper(req).await, - (BytecodeMarker::Evm, CompilerType::Solc) => todo!(), + (BytecodeMarker::Evm, CompilerType::Solc) => self.compile_solc(req).await, (BytecodeMarker::Evm, CompilerType::Vyper) => todo!(), } } diff --git a/core/lib/contract_verifier/src/resolver.rs b/core/lib/contract_verifier/src/resolver.rs index 4376180c3811..ff200628cf0d 100644 --- a/core/lib/contract_verifier/src/resolver.rs +++ b/core/lib/contract_verifier/src/resolver.rs @@ -7,7 +7,7 @@ use zksync_types::contract_verification_api::{CompilationArtifacts, CompilerVers use zksync_utils::env::Workspace; use crate::{ - compilers::{ZkSolc, ZkSolcInput, ZkVyper, ZkVyperInput}, + compilers::{Solc, SolcInput, ZkSolc, ZkSolcInput, ZkVyper, ZkVyperInput}, error::ContractVerifierError, }; @@ -47,6 +47,12 @@ pub(crate) trait CompilerResolver: fmt::Debug + Send + Sync { /// Returned errors are assumed to be fatal. async fn supported_versions(&self) -> anyhow::Result; + /// Resolves a `solc` compiler. + async fn resolve_solc( + &self, + version: &str, + ) -> Result>, ContractVerifierError>; + /// Resolves a `zksolc` compiler. async fn resolve_zksolc( &self, @@ -102,6 +108,28 @@ impl EnvCompilerResolver { } Ok(versions) } + + async fn resolve_solc_path( + &self, + solc_version: String, + ) -> Result { + let solc_path = self + .home_dir + .join("etc") + .join("solc-bin") + .join(&solc_version) + .join("solc"); + if !fs::try_exists(&solc_path) + .await + .context("failed accessing solc")? + { + return Err(ContractVerifierError::UnknownCompilerVersion( + "solc".to_owned(), + solc_version, + )); + } + Ok(solc_path) + } } #[async_trait] @@ -127,6 +155,14 @@ impl CompilerResolver for EnvCompilerResolver { }) } + async fn resolve_solc( + &self, + version: &str, + ) -> Result>, ContractVerifierError> { + let solc_path = self.resolve_solc_path(version.to_owned()).await?; + Ok(Box::new(Solc::new(solc_path))) + } + async fn resolve_zksolc( &self, versions: &CompilerVersions, @@ -148,23 +184,7 @@ impl CompilerResolver for EnvCompilerResolver { )); } - let solc_version = versions.compiler_version(); - let solc_path = self - .home_dir - .join("etc") - .join("solc-bin") - .join(&solc_version) - .join("solc"); - if !fs::try_exists(&solc_path) - .await - .context("failed accessing solc")? - { - return Err(ContractVerifierError::UnknownCompilerVersion( - "solc".to_owned(), - solc_version, - )); - } - + let solc_path = self.resolve_solc_path(versions.compiler_version()).await?; let compiler_paths = CompilerPaths { base: solc_path, zk: zksolc_path, diff --git a/core/lib/contract_verifier/src/tests/mod.rs b/core/lib/contract_verifier/src/tests/mod.rs index 5196afc7dc2d..e19735c85f0b 100644 --- a/core/lib/contract_verifier/src/tests/mod.rs +++ b/core/lib/contract_verifier/src/tests/mod.rs @@ -18,7 +18,7 @@ use zksync_vm_interface::{tracer::ValidationTraces, TransactionExecutionMetrics, use super::*; use crate::{ - compilers::{ZkSolcInput, ZkVyperInput}, + compilers::{SolcInput, ZkSolcInput, ZkVyperInput}, resolver::{Compiler, SupportedCompilerVersions}, }; @@ -132,6 +132,19 @@ impl CompilerResolver for MockCompilerResolver { }) } + async fn resolve_solc( + &self, + version: &str, + ) -> Result>, ContractVerifierError> { + if version != SOLC_VERSION { + return Err(ContractVerifierError::UnknownCompilerVersion( + "solc".to_owned(), + version.to_owned(), + )); + } + todo!() + } + async fn resolve_zksolc( &self, versions: &CompilerVersions, From 2643cef29bd5ae7bbebae82bd733b8952f7d9523 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 1 Nov 2024 17:16:05 +0200 Subject: [PATCH 06/19] Sketch deployed bytecodes comparison --- Cargo.lock | 2 +- .../contract_verifier/src/compilers/mod.rs | 30 +++++++-- .../contract_verifier/src/compilers/solc.rs | 2 +- .../contract_verifier/src/compilers/zksolc.rs | 3 +- .../src/compilers/zkvyper.rs | 1 + core/lib/contract_verifier/src/lib.rs | 16 +++-- core/lib/contract_verifier/src/tests/mod.rs | 2 + .../types/src/contract_verification_api.rs | 9 +++ core/lib/utils/Cargo.toml | 1 + core/lib/utils/src/bytecode.rs | 64 ++++++++++++++++++- core/node/api_server/Cargo.toml | 1 - core/node/api_server/src/testonly.rs | 25 -------- core/node/api_server/src/utils.rs | 36 ----------- .../api_server/src/web3/namespaces/eth.rs | 21 +++--- core/node/api_server/src/web3/tests/mod.rs | 11 ++-- 15 files changed, 133 insertions(+), 91 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 158ed796f8cc..ed77e866a57e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11451,7 +11451,6 @@ dependencies = [ "async-trait", "axum 0.7.7", "chrono", - "const-decoder", "futures 0.3.31", "governor", "hex", @@ -12181,6 +12180,7 @@ dependencies = [ "assert_matches", "bigdecimal", "bincode", + "const-decoder", "futures 0.3.31", "hex", "num", diff --git a/core/lib/contract_verifier/src/compilers/mod.rs b/core/lib/contract_verifier/src/compilers/mod.rs index e146f364dbaf..a56b4e32d1a1 100644 --- a/core/lib/contract_verifier/src/compilers/mod.rs +++ b/core/lib/contract_verifier/src/compilers/mod.rs @@ -1,3 +1,4 @@ +use anyhow::Context as _; use serde::{Deserialize, Serialize}; use zksync_types::contract_verification_api::CompilationArtifacts; @@ -20,11 +21,11 @@ pub(crate) struct Source { } /// Parsing logic shared between `solc` and `zksolc`. -// FIXME: deployed bytecode. fn parse_standard_json_output( output: &serde_json::Value, contract_name: String, file_name: String, + get_deployed_bytecode: bool, ) -> Result { if let Some(errors) = output.get("errors") { let errors = errors.as_array().unwrap().clone(); @@ -49,10 +50,25 @@ fn parse_standard_json_output( return Err(ContractVerifierError::MissingContract(contract_name)); }; - let bytecode_str = contract["evm"]["bytecode"]["object"] + let Some(bytecode_str) = contract + .pointer("/evm/bytecode/object") + .context("missing bytecode in solc / zksolc output")? .as_str() - .ok_or(ContractVerifierError::AbstractContract(contract_name))?; - let bytecode = hex::decode(bytecode_str).unwrap(); + else { + return Err(ContractVerifierError::AbstractContract(contract_name)); + }; + let bytecode = hex::decode(bytecode_str).context("invalid bytecode")?; + + let deployed_bytecode = if get_deployed_bytecode { + let bytecode_str = contract + .pointer("/evm/deployedBytecode/object") + .context("missing deployed bytecode in solc output")? + .as_str() + .ok_or(ContractVerifierError::AbstractContract(contract_name))?; + Some(hex::decode(bytecode_str).context("invalid deployed bytecode")?) + } else { + None + }; let abi = contract["abi"].clone(); if !abi.is_array() { @@ -63,5 +79,9 @@ fn parse_standard_json_output( return Err(err.into()); } - Ok(CompilationArtifacts { bytecode, abi }) + Ok(CompilationArtifacts { + bytecode, + deployed_bytecode, + abi, + }) } diff --git a/core/lib/contract_verifier/src/compilers/solc.rs b/core/lib/contract_verifier/src/compilers/solc.rs index 4a617a817cc7..223aa820d1e5 100644 --- a/core/lib/contract_verifier/src/compilers/solc.rs +++ b/core/lib/contract_verifier/src/compilers/solc.rs @@ -154,7 +154,7 @@ impl Compiler for Solc { if output.status.success() { let output = serde_json::from_slice(&output.stdout) .context("zksolc output is not valid JSON")?; - parse_standard_json_output(&output, input.contract_name, input.file_name) + parse_standard_json_output(&output, input.contract_name, input.file_name, true) } else { Err(ContractVerifierError::CompilerError( "zksolc".to_string(), diff --git a/core/lib/contract_verifier/src/compilers/zksolc.rs b/core/lib/contract_verifier/src/compilers/zksolc.rs index 5ce05804114c..7fa2344ad3c2 100644 --- a/core/lib/contract_verifier/src/compilers/zksolc.rs +++ b/core/lib/contract_verifier/src/compilers/zksolc.rs @@ -162,6 +162,7 @@ impl ZkSolc { let bytecode = hex::decode(bytecode_str).context("invalid Yul output bytecode")?; Ok(CompilationArtifacts { bytecode, + deployed_bytecode: None, abi: serde_json::Value::Array(Vec::new()), }) } @@ -247,7 +248,7 @@ impl Compiler for ZkSolc { if output.status.success() { let output = serde_json::from_slice(&output.stdout) .context("zksolc output is not valid JSON")?; - parse_standard_json_output(&output, contract_name, file_name) + parse_standard_json_output(&output, contract_name, file_name, false) } else { Err(ContractVerifierError::CompilerError( "zksolc".to_string(), diff --git a/core/lib/contract_verifier/src/compilers/zkvyper.rs b/core/lib/contract_verifier/src/compilers/zkvyper.rs index cc65b7d0c92e..5602584fb14e 100644 --- a/core/lib/contract_verifier/src/compilers/zkvyper.rs +++ b/core/lib/contract_verifier/src/compilers/zkvyper.rs @@ -71,6 +71,7 @@ impl ZkVyper { return Ok(CompilationArtifacts { abi: artifact["abi"].clone(), bytecode, + deployed_bytecode: None, }); } } diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index da3dc7bc13ef..a7921c9557da 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -19,7 +19,7 @@ use zksync_types::{ }, Address, CONTRACT_DEPLOYER_ADDRESS, }; -use zksync_utils::bytecode::BytecodeMarker; +use zksync_utils::bytecode::{prepare_evm_bytecode, BytecodeMarker}; use crate::{ compilers::{Solc, ZkSolc, ZkVyper}, @@ -161,16 +161,22 @@ impl ContractVerifier { .context("unknown bytecode kind")?; let artifacts = self.compile(request.req.clone(), bytecode_marker).await?; + // FIXME: extract constructor args / check bytecode for EVM contracts let constructor_args = self.decode_constructor_args(&deployed_contract, request.req.contract_address)?; - // FIXME: post-process EVM bytecode - if artifacts.bytecode != deployed_contract.bytecode { + let deployed_bytecode = match bytecode_marker { + BytecodeMarker::EraVm => deployed_contract.bytecode.as_slice(), + BytecodeMarker::Evm => prepare_evm_bytecode(&deployed_contract.bytecode) + .context("invalid stored EVM bytecode")?, + }; + + if artifacts.deployed_bytecode() != deployed_bytecode { tracing::info!( "Bytecode mismatch req {}, deployed: 0x{}, compiled: 0x{}", request.id, - hex::encode(&deployed_contract.bytecode), - hex::encode(&artifacts.bytecode) + hex::encode(deployed_bytecode), + hex::encode(artifacts.deployed_bytecode()) ); return Err(ContractVerifierError::BytecodeMismatch); } diff --git a/core/lib/contract_verifier/src/tests/mod.rs b/core/lib/contract_verifier/src/tests/mod.rs index e19735c85f0b..ee4dd590cf56 100644 --- a/core/lib/contract_verifier/src/tests/mod.rs +++ b/core/lib/contract_verifier/src/tests/mod.rs @@ -258,6 +258,7 @@ async fn contract_verifier_basics() { CompilationArtifacts { bytecode: vec![0; 32], + deployed_bytecode: None, abi: counter_contract_abi(), } }); @@ -341,6 +342,7 @@ async fn bytecode_mismatch_error() { let mock_resolver = MockCompilerResolver::new(|_| CompilationArtifacts { bytecode: vec![0; 32], + deployed_bytecode: None, abi: counter_contract_abi(), }); let verifier = ContractVerifier::with_resolver( diff --git a/core/lib/types/src/contract_verification_api.rs b/core/lib/types/src/contract_verification_api.rs index 977202d0a8fc..4c8c31a66d6e 100644 --- a/core/lib/types/src/contract_verification_api.rs +++ b/core/lib/types/src/contract_verification_api.rs @@ -213,10 +213,19 @@ pub struct VerificationRequest { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CompilationArtifacts { + /// In case of EVM contracts, this is the original bytecode (`bytecode` in `solc` output). pub bytecode: Vec, + /// Only set for EVM contracts. + pub deployed_bytecode: Option>, pub abi: serde_json::Value, } +impl CompilationArtifacts { + pub fn deployed_bytecode(&self) -> &[u8] { + self.deployed_bytecode.as_deref().unwrap_or(&self.bytecode) + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct VerificationInfo { diff --git a/core/lib/utils/Cargo.toml b/core/lib/utils/Cargo.toml index b87b2ad98964..9b65ccdd29cb 100644 --- a/core/lib/utils/Cargo.toml +++ b/core/lib/utils/Cargo.toml @@ -16,6 +16,7 @@ zk_evm.workspace = true zksync_vlog.workspace = true bigdecimal.workspace = true +const-decoder.workspace = true num = { workspace = true, features = ["serde"] } serde = { workspace = true, features = ["derive"] } tokio = { workspace = true, features = ["time"] } diff --git a/core/lib/utils/src/bytecode.rs b/core/lib/utils/src/bytecode.rs index 01cce5bc34d0..4fda5e9d48a0 100644 --- a/core/lib/utils/src/bytecode.rs +++ b/core/lib/utils/src/bytecode.rs @@ -1,7 +1,8 @@ // FIXME: move to basic_types? +use anyhow::Context as _; use zk_evm::k256::sha2::{Digest, Sha256}; -use zksync_basic_types::H256; +use zksync_basic_types::{H256, U256}; use crate::bytes_to_chunks; @@ -98,9 +99,62 @@ pub fn hash_evm_bytecode(bytecode: &[u8]) -> H256 { H256(output) } +pub fn prepare_evm_bytecode(raw: &[u8]) -> anyhow::Result<&[u8]> { + // EVM bytecodes are prefixed with a big-endian `U256` bytecode length. + let bytecode_len_bytes = raw.get(..32).context("length < 32")?; + let bytecode_len = U256::from_big_endian(bytecode_len_bytes); + let bytecode_len: usize = bytecode_len + .try_into() + .map_err(|_| anyhow::anyhow!("length ({bytecode_len}) overflow"))?; + let bytecode = raw.get(32..(32 + bytecode_len)).with_context(|| { + format!( + "prefixed length ({bytecode_len}) exceeds real length ({})", + raw.len() - 32 + ) + })?; + // Since slicing above succeeded, this one is safe. + let padding = &raw[(32 + bytecode_len)..]; + anyhow::ensure!( + padding.iter().all(|&b| b == 0), + "bytecode padding contains non-zero bytes" + ); + Ok(bytecode) +} + +pub mod testonly { + use const_decoder::Decoder; + + pub const RAW_EVM_BYTECODE: &[u8] = &const_decoder::decode!( + Decoder::Hex, + b"00000000000000000000000000000000000000000000000000000000000001266080604052348015\ + 600e575f80fd5b50600436106030575f3560e01c8063816898ff146034578063fb5343f314604c57\ + 5b5f80fd5b604a60048036038101906046919060a6565b6066565b005b6052606f565b604051605d\ + 919060d9565b60405180910390f35b805f8190555050565b5f5481565b5f80fd5b5f819050919050\ + 565b6088816078565b81146091575f80fd5b50565b5f8135905060a0816081565b92915050565b5f\ + 6020828403121560b85760b76074565b5b5f60c3848285016094565b91505092915050565b60d381\ + 6078565b82525050565b5f60208201905060ea5f83018460cc565b9291505056fea2646970667358\ + 221220caca1247066da378f2ec77c310f2ae51576272367b4fa11cc4350af4e9ce4d0964736f6c63\ + 4300081a00330000000000000000000000000000000000000000000000000000" + ); + pub const PROCESSED_EVM_BYTECODE: &[u8] = &const_decoder::decode!( + Decoder::Hex, + b"6080604052348015600e575f80fd5b50600436106030575f3560e01c8063816898ff146034578063\ + fb5343f314604c575b5f80fd5b604a60048036038101906046919060a6565b6066565b005b605260\ + 6f565b604051605d919060d9565b60405180910390f35b805f8190555050565b5f5481565b5f80fd\ + 5b5f819050919050565b6088816078565b81146091575f80fd5b50565b5f8135905060a081608156\ + 5b92915050565b5f6020828403121560b85760b76074565b5b5f60c3848285016094565b91505092\ + 915050565b60d3816078565b82525050565b5f60208201905060ea5f83018460cc565b9291505056\ + fea2646970667358221220caca1247066da378f2ec77c310f2ae51576272367b4fa11cc4350af4e9\ + ce4d0964736f6c634300081a0033" + ); +} + #[cfg(test)] mod tests { - use super::*; + use super::{ + testonly::{PROCESSED_EVM_BYTECODE, RAW_EVM_BYTECODE}, + *, + }; #[test] fn bytecode_markers_are_valid() { @@ -115,4 +169,10 @@ mod tests { Some(BytecodeMarker::Evm) ); } + + #[test] + fn preparing_evm_bytecode() { + let prepared = prepare_evm_bytecode(RAW_EVM_BYTECODE).unwrap(); + assert_eq!(prepared, PROCESSED_EVM_BYTECODE); + } } diff --git a/core/node/api_server/Cargo.toml b/core/node/api_server/Cargo.toml index 067b9b3e3722..d0723a9d23e7 100644 --- a/core/node/api_server/Cargo.toml +++ b/core/node/api_server/Cargo.toml @@ -61,5 +61,4 @@ zksync_node_genesis.workspace = true zksync_node_test_utils.workspace = true assert_matches.workspace = true -const-decoder.workspace = true test-casing.workspace = true diff --git a/core/node/api_server/src/testonly.rs b/core/node/api_server/src/testonly.rs index 3add9c2f165c..de6501716125 100644 --- a/core/node/api_server/src/testonly.rs +++ b/core/node/api_server/src/testonly.rs @@ -2,7 +2,6 @@ use std::{collections::HashMap, iter}; -use const_decoder::Decoder; use zk_evm_1_5_0::zkevm_opcode_defs::decoding::{EncodingModeProduction, VmEncodingMode}; use zksync_contracts::{ eth_contract, get_loadnext_contract, load_contract, read_bytecode, @@ -27,30 +26,6 @@ use zksync_types::{ }; use zksync_utils::{address_to_u256, u256_to_h256}; -pub(crate) const RAW_EVM_BYTECODE: &[u8] = &const_decoder::decode!( - Decoder::Hex, - b"00000000000000000000000000000000000000000000000000000000000001266080604052348015\ - 600e575f80fd5b50600436106030575f3560e01c8063816898ff146034578063fb5343f314604c57\ - 5b5f80fd5b604a60048036038101906046919060a6565b6066565b005b6052606f565b604051605d\ - 919060d9565b60405180910390f35b805f8190555050565b5f5481565b5f80fd5b5f819050919050\ - 565b6088816078565b81146091575f80fd5b50565b5f8135905060a0816081565b92915050565b5f\ - 6020828403121560b85760b76074565b5b5f60c3848285016094565b91505092915050565b60d381\ - 6078565b82525050565b5f60208201905060ea5f83018460cc565b9291505056fea2646970667358\ - 221220caca1247066da378f2ec77c310f2ae51576272367b4fa11cc4350af4e9ce4d0964736f6c63\ - 4300081a00330000000000000000000000000000000000000000000000000000" -); -pub(crate) const PROCESSED_EVM_BYTECODE: &[u8] = &const_decoder::decode!( - Decoder::Hex, - b"6080604052348015600e575f80fd5b50600436106030575f3560e01c8063816898ff146034578063\ - fb5343f314604c575b5f80fd5b604a60048036038101906046919060a6565b6066565b005b605260\ - 6f565b604051605d919060d9565b60405180910390f35b805f8190555050565b5f5481565b5f80fd\ - 5b5f819050919050565b6088816078565b81146091575f80fd5b50565b5f8135905060a081608156\ - 5b92915050565b5f6020828403121560b85760b76074565b5b5f60c3848285016094565b91505092\ - 915050565b60d3816078565b82525050565b5f60208201905060ea5f83018460cc565b9291505056\ - fea2646970667358221220caca1247066da378f2ec77c310f2ae51576272367b4fa11cc4350af4e9\ - ce4d0964736f6c634300081a0033" -); - const EXPENSIVE_CONTRACT_PATH: &str = "etc/contracts-test-data/artifacts-zk/contracts/expensive/expensive.sol/Expensive.json"; const PRECOMPILES_CONTRACT_PATH: &str = diff --git a/core/node/api_server/src/utils.rs b/core/node/api_server/src/utils.rs index c7a1134682bf..6769e773dc77 100644 --- a/core/node/api_server/src/utils.rs +++ b/core/node/api_server/src/utils.rs @@ -6,33 +6,9 @@ use std::{ time::{Duration, Instant}, }; -use anyhow::Context; use zksync_dal::{Connection, Core, DalError}; -use zksync_multivm::circuit_sequencer_api_latest::boojum::ethereum_types::U256; use zksync_web3_decl::error::Web3Error; -pub(crate) fn prepare_evm_bytecode(raw: &[u8]) -> anyhow::Result> { - // EVM bytecodes are prefixed with a big-endian `U256` bytecode length. - let bytecode_len_bytes = raw.get(..32).context("length < 32")?; - let bytecode_len = U256::from_big_endian(bytecode_len_bytes); - let bytecode_len: usize = bytecode_len - .try_into() - .map_err(|_| anyhow::anyhow!("length ({bytecode_len}) overflow"))?; - let bytecode = raw.get(32..(32 + bytecode_len)).with_context(|| { - format!( - "prefixed length ({bytecode_len}) exceeds real length ({})", - raw.len() - 32 - ) - })?; - // Since slicing above succeeded, this one is safe. - let padding = &raw[(32 + bytecode_len)..]; - anyhow::ensure!( - padding.iter().all(|&b| b == 0), - "bytecode padding contains non-zero bytes" - ); - Ok(bytecode.to_vec()) -} - /// Opens a readonly transaction over the specified connection. pub(crate) async fn open_readonly_transaction<'r>( conn: &'r mut Connection<'_, Core>, @@ -90,15 +66,3 @@ macro_rules! report_filter { ReportFilter::new($interval, &LAST_TIMESTAMP) }}; } - -#[cfg(test)] -mod tests { - use super::*; - use crate::testonly::{PROCESSED_EVM_BYTECODE, RAW_EVM_BYTECODE}; - - #[test] - fn preparing_evm_bytecode() { - let prepared = prepare_evm_bytecode(RAW_EVM_BYTECODE).unwrap(); - assert_eq!(prepared, PROCESSED_EVM_BYTECODE); - } -} diff --git a/core/node/api_server/src/web3/namespaces/eth.rs b/core/node/api_server/src/web3/namespaces/eth.rs index ee37cb989f1b..e594af20d183 100644 --- a/core/node/api_server/src/web3/namespaces/eth.rs +++ b/core/node/api_server/src/web3/namespaces/eth.rs @@ -12,7 +12,10 @@ use zksync_types::{ web3::{self, Bytes, SyncInfo, SyncState}, AccountTreeId, L2BlockNumber, StorageKey, H256, L2_BASE_TOKEN_ADDRESS, U256, }; -use zksync_utils::{bytecode::BytecodeMarker, u256_to_h256}; +use zksync_utils::{ + bytecode::{prepare_evm_bytecode, BytecodeMarker}, + u256_to_h256, +}; use zksync_web3_decl::{ error::Web3Error, types::{Address, Block, Filter, FilterChanges, Log, U64}, @@ -21,7 +24,7 @@ use zksync_web3_decl::{ use crate::{ execution_sandbox::BlockArgs, tx_sender::BinarySearchKind, - utils::{open_readonly_transaction, prepare_evm_bytecode}, + utils::open_readonly_transaction, web3::{backend_jsonrpsee::MethodTracer, metrics::API_METRICS, state::RpcState, TypedFilter}, }; @@ -403,12 +406,14 @@ impl EthNamespace { // Check if the bytecode is an EVM bytecode, and if so, pre-process it correspondingly. let marker = BytecodeMarker::new(contract_code.bytecode_hash); let prepared_bytecode = if marker == Some(BytecodeMarker::Evm) { - prepare_evm_bytecode(&contract_code.bytecode).with_context(|| { - format!( - "malformed EVM bytecode at address {address:?}, hash = {:?}", - contract_code.bytecode_hash - ) - })? + prepare_evm_bytecode(&contract_code.bytecode) + .with_context(|| { + format!( + "malformed EVM bytecode at address {address:?}, hash = {:?}", + contract_code.bytecode_hash + ) + })? + .to_vec() } else { contract_code.bytecode }; diff --git a/core/node/api_server/src/web3/tests/mod.rs b/core/node/api_server/src/web3/tests/mod.rs index 17e92200d661..b35bb9f5fad7 100644 --- a/core/node/api_server/src/web3/tests/mod.rs +++ b/core/node/api_server/src/web3/tests/mod.rs @@ -45,7 +45,10 @@ use zksync_types::{ U256, U64, }; use zksync_utils::{ - bytecode::{hash_bytecode, hash_evm_bytecode}, + bytecode::{ + hash_bytecode, hash_evm_bytecode, + testonly::{PROCESSED_EVM_BYTECODE, RAW_EVM_BYTECODE}, + }, u256_to_h256, }; use zksync_vm_executor::oneshot::MockOneshotExecutor; @@ -64,11 +67,7 @@ use zksync_web3_decl::{ }; use super::*; -use crate::{ - testonly::{PROCESSED_EVM_BYTECODE, RAW_EVM_BYTECODE}, - tx_sender::SandboxExecutorOptions, - web3::testonly::TestServerBuilder, -}; +use crate::{tx_sender::SandboxExecutorOptions, web3::testonly::TestServerBuilder}; mod debug; mod filters; From 51df909c85e9225eef33f47916bc89277b17b317 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 1 Nov 2024 17:37:07 +0200 Subject: [PATCH 07/19] Test standalone `solc` compilation --- .../contract_verifier/src/compilers/solc.rs | 6 +- core/lib/contract_verifier/src/tests/mod.rs | 9 --- core/lib/contract_verifier/src/tests/real.rs | 73 ++++++++++++++++--- 3 files changed, 65 insertions(+), 23 deletions(-) diff --git a/core/lib/contract_verifier/src/compilers/solc.rs b/core/lib/contract_verifier/src/compilers/solc.rs index 223aa820d1e5..879fb9b3c876 100644 --- a/core/lib/contract_verifier/src/compilers/solc.rs +++ b/core/lib/contract_verifier/src/compilers/solc.rs @@ -19,6 +19,7 @@ pub(crate) struct SolcInput { } #[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] struct StandardJson { language: String, sources: HashMap, @@ -26,6 +27,7 @@ struct StandardJson { } #[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] struct Settings { /// The output selection filters. output_selection: Option, @@ -60,8 +62,8 @@ impl Solc { }; let default_output_selection = serde_json::json!({ "*": { - "*": [ "abi" ], - "": [ "abi" ] + "*": [ "abi", "evm.bytecode", "evm.deployedBytecode" ], + "": [ "abi", "evm.bytecode", "evm.deployedBytecode" ], } }); diff --git a/core/lib/contract_verifier/src/tests/mod.rs b/core/lib/contract_verifier/src/tests/mod.rs index ee4dd590cf56..014a3b308d8a 100644 --- a/core/lib/contract_verifier/src/tests/mod.rs +++ b/core/lib/contract_verifier/src/tests/mod.rs @@ -316,15 +316,6 @@ async fn assert_request_success( assert_eq!(verification_info.artifacts.abi, counter_contract_abi()); } -async fn checked_env_resolver() -> Option<(EnvCompilerResolver, SupportedCompilerVersions)> { - let compiler_resolver = EnvCompilerResolver::default(); - let supported_compilers = compiler_resolver.supported_versions().await.ok()?; - if supported_compilers.zksolc.is_empty() || supported_compilers.solc.is_empty() { - return None; - } - Some((compiler_resolver, supported_compilers)) -} - #[tokio::test] async fn bytecode_mismatch_error() { let pool = ConnectionPool::test_pool().await; diff --git a/core/lib/contract_verifier/src/tests/real.rs b/core/lib/contract_verifier/src/tests/real.rs index 064c39f26df0..55af72a28afe 100644 --- a/core/lib/contract_verifier/src/tests/real.rs +++ b/core/lib/contract_verifier/src/tests/real.rs @@ -8,6 +8,41 @@ use zksync_utils::bytecode::validate_bytecode; use super::*; +#[derive(Debug)] +struct TestCompilerVersions { + solc: String, + zksolc: String, +} + +impl TestCompilerVersions { + fn new(mut versions: SupportedCompilerVersions) -> Option { + let solc = versions + .solc + .into_iter() + .find(|ver| !ver.starts_with("zkVM"))?; + Some(Self { + solc, + zksolc: versions.zksolc.pop()?, + }) + } + + fn for_zksolc(self) -> CompilerVersions { + CompilerVersions::Solc { + compiler_solc_version: self.solc, + compiler_zksolc_version: self.zksolc, + } + } +} + +async fn checked_env_resolver() -> Option<(EnvCompilerResolver, TestCompilerVersions)> { + let compiler_resolver = EnvCompilerResolver::default(); + let supported_compilers = compiler_resolver.supported_versions().await.ok()?; + Some(( + compiler_resolver, + TestCompilerVersions::new(supported_compilers)?, + )) +} + fn assert_no_compilers_expected() { assert_ne!( env::var("RUN_CONTRACT_VERIFICATION_TEST").ok().as_deref(), @@ -25,10 +60,7 @@ async fn using_real_compiler() { return; }; - let versions = CompilerVersions::Solc { - compiler_zksolc_version: supported_compilers.zksolc[0].clone(), - compiler_solc_version: supported_compilers.solc[0].clone(), - }; + let versions = supported_compilers.for_zksolc(); let compiler = compiler_resolver.resolve_zksolc(&versions).await.unwrap(); let req = VerificationIncomingRequest { compiler_versions: versions, @@ -42,16 +74,36 @@ async fn using_real_compiler() { } #[tokio::test] -async fn using_real_compiler_in_verifier() { +async fn using_standalone_solc() { let Some((compiler_resolver, supported_compilers)) = checked_env_resolver().await else { assert_no_compilers_expected(); return; }; - let versions = CompilerVersions::Solc { - compiler_zksolc_version: supported_compilers.zksolc[0].clone(), - compiler_solc_version: supported_compilers.solc[0].clone(), + let version = &supported_compilers.solc; + let compiler = compiler_resolver.resolve_solc(version).await.unwrap(); + let req = VerificationIncomingRequest { + compiler_versions: CompilerVersions::Solc { + compiler_solc_version: version.clone(), + compiler_zksolc_version: "1000.0.0".to_owned(), // doesn't matter + }, + ..test_request(Address::repeat_byte(1)) }; + let input = Solc::build_input(req).unwrap(); + let output = compiler.compile(input).await.unwrap(); + + assert!(output.deployed_bytecode.is_some()); + assert_eq!(output.abi, counter_contract_abi()); +} + +#[tokio::test] +async fn using_real_compiler_in_verifier() { + let Some((compiler_resolver, supported_compilers)) = checked_env_resolver().await else { + assert_no_compilers_expected(); + return; + }; + + let versions = supported_compilers.for_zksolc(); let address = Address::repeat_byte(1); let compiler = compiler_resolver.resolve_zksolc(&versions).await.unwrap(); let req = VerificationIncomingRequest { @@ -92,10 +144,7 @@ async fn compilation_errors() { return; }; - let versions = CompilerVersions::Solc { - compiler_zksolc_version: supported_compilers.zksolc[0].clone(), - compiler_solc_version: supported_compilers.solc[0].clone(), - }; + let versions = supported_compilers.for_zksolc(); let address = Address::repeat_byte(1); let req = VerificationIncomingRequest { compiler_versions: versions, From af1ca4186605e468dec08aa32196794611d47b0f Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 10:10:10 +0200 Subject: [PATCH 08/19] Test EVM bytecode verification --- contracts | 2 +- .../contract_verifier/src/compilers/solc.rs | 13 +- core/lib/contract_verifier/src/tests/mod.rs | 139 +++++++++++++++++- core/lib/dal/src/contract_verification_dal.rs | 8 +- .../types/src/contract_verification_api.rs | 6 +- 5 files changed, 149 insertions(+), 19 deletions(-) diff --git a/contracts b/contracts index 9fb1264fce8c..84d5e3716f64 160000 --- a/contracts +++ b/contracts @@ -1 +1 @@ -Subproject commit 9fb1264fce8c0ebeefe8bf1846e89876027161d2 +Subproject commit 84d5e3716f645909e8144c7d50af9dd6dd9ded62 diff --git a/core/lib/contract_verifier/src/compilers/solc.rs b/core/lib/contract_verifier/src/compilers/solc.rs index 879fb9b3c876..47f777dd564e 100644 --- a/core/lib/contract_verifier/src/compilers/solc.rs +++ b/core/lib/contract_verifier/src/compilers/solc.rs @@ -11,18 +11,19 @@ use zksync_types::contract_verification_api::{ use super::{parse_standard_json_output, Source}; use crate::{error::ContractVerifierError, resolver::Compiler}; +// Here and below, fields are public for testing purposes. #[derive(Debug)] pub(crate) struct SolcInput { - standard_json: StandardJson, - contract_name: String, - file_name: String, + pub standard_json: StandardJson, + pub contract_name: String, + pub file_name: String, } #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -struct StandardJson { - language: String, - sources: HashMap, +pub(crate) struct StandardJson { + pub language: String, + pub sources: HashMap, settings: Settings, } diff --git a/core/lib/contract_verifier/src/tests/mod.rs b/core/lib/contract_verifier/src/tests/mod.rs index 014a3b308d8a..38ff095bbd49 100644 --- a/core/lib/contract_verifier/src/tests/mod.rs +++ b/core/lib/contract_verifier/src/tests/mod.rs @@ -1,6 +1,6 @@ //! Tests for the contract verifier. -use std::collections::HashMap; +use std::{collections::HashMap, iter}; use tokio::sync::watch; use zksync_dal::Connection; @@ -11,9 +11,12 @@ use zksync_types::{ l2::L2Tx, tx::IncludedTxLocation, Execute, L1BatchNumber, L2BlockNumber, ProtocolVersion, StorageLog, CONTRACT_DEPLOYER_ADDRESS, - H256, + H256, U256, +}; +use zksync_utils::{ + address_to_h256, + bytecode::{hash_bytecode, hash_evm_bytecode}, }; -use zksync_utils::{address_to_h256, bytecode::hash_bytecode}; use zksync_vm_interface::{tracer::ValidationTraces, TransactionExecutionMetrics, VmEvent}; use super::*; @@ -27,8 +30,58 @@ mod real; const SOLC_VERSION: &str = "0.8.27"; const ZKSOLC_VERSION: &str = "1.5.4"; +/// Pads an EVM bytecode in the same ways it's done by system contracts. +fn pad_evm_bytecode(deployed_bytecode: &[u8]) -> Vec { + let mut padded = Vec::with_capacity(deployed_bytecode.len() + 32); + let len = U256::from(deployed_bytecode.len()); + padded.extend_from_slice(&[0; 32]); + len.to_big_endian(&mut padded); + padded.extend_from_slice(deployed_bytecode); + + // Pad to the 32-byte word boundary. + if padded.len() % 32 != 0 { + padded.extend(iter::repeat(0).take(32 - padded.len() % 32)); + } + assert_eq!(padded.len() % 32, 0); + + // Pad to contain the odd number of words. + if (padded.len() / 32) % 2 != 1 { + padded.extend_from_slice(&[0; 32]); + } + assert_eq!((padded.len() / 32) % 2, 1); + padded +} + async fn mock_deployment(storage: &mut Connection<'_, Core>, address: Address, bytecode: Vec) { let bytecode_hash = hash_bytecode(&bytecode); + let deployment = Execute::for_deploy(H256::zero(), bytecode.clone(), &[]); + mock_deployment_inner(storage, address, bytecode_hash, bytecode, deployment).await; +} + +async fn mock_evm_deployment( + storage: &mut Connection<'_, Core>, + address: Address, + original_bytecode: Vec, + deployed_bytecode: &[u8], +) { + let deployment = Execute { + contract_address: None, + calldata: original_bytecode, // FIXME: check + value: 0.into(), + factory_deps: vec![], + }; + let bytecode = pad_evm_bytecode(deployed_bytecode); + let bytecode_hash = hash_evm_bytecode(&bytecode); + mock_deployment_inner(storage, address, bytecode_hash, bytecode, deployment).await; +} + +async fn mock_deployment_inner( + storage: &mut Connection<'_, Core>, + address: Address, + bytecode_hash: H256, + bytecode: Vec, + execute: Execute, +) { let logs = [ StorageLog::new_write_log(get_code_key(&address), bytecode_hash), StorageLog::new_write_log(get_known_code_key(&bytecode_hash), H256::from_low_u64_be(1)), @@ -48,7 +101,7 @@ async fn mock_deployment(storage: &mut Connection<'_, Core>, address: Address, b .unwrap(); let mut deploy_tx = L2Tx { - execute: Execute::for_deploy(H256::zero(), bytecode, &[]), + execute, common_data: Default::default(), received_timestamp_ms: 0, raw_bytes: Some(vec![0; 128].into()), @@ -88,11 +141,13 @@ async fn mock_deployment(storage: &mut Connection<'_, Core>, address: Address, b .unwrap(); } +type SharedMockFn = + Arc Result + Send + Sync>; + #[derive(Clone)] struct MockCompilerResolver { - zksolc: Arc< - dyn Fn(ZkSolcInput) -> Result + Send + Sync, - >, + zksolc: SharedMockFn, + solc: SharedMockFn, } impl fmt::Debug for MockCompilerResolver { @@ -107,6 +162,14 @@ impl MockCompilerResolver { fn new(zksolc: impl Fn(ZkSolcInput) -> CompilationArtifacts + 'static + Send + Sync) -> Self { Self { zksolc: Arc::new(move |input| Ok(zksolc(input))), + solc: Arc::new(|input| panic!("unexpected solc call: {input:?}")), + } + } + + fn solc(solc: impl Fn(SolcInput) -> CompilationArtifacts + 'static + Send + Sync) -> Self { + Self { + solc: Arc::new(move |input| Ok(solc(input))), + zksolc: Arc::new(|input| panic!("unexpected zksolc call: {input:?}")), } } } @@ -121,6 +184,16 @@ impl Compiler for MockCompilerResolver { } } +#[async_trait] +impl Compiler for MockCompilerResolver { + async fn compile( + self: Box, + input: SolcInput, + ) -> Result { + (self.solc)(input) + } +} + #[async_trait] impl CompilerResolver for MockCompilerResolver { async fn supported_versions(&self) -> anyhow::Result { @@ -142,7 +215,7 @@ impl CompilerResolver for MockCompilerResolver { version.to_owned(), )); } - todo!() + Ok(Box::new(self.clone())) } async fn resolve_zksolc( @@ -316,6 +389,56 @@ async fn assert_request_success( assert_eq!(verification_info.artifacts.abi, counter_contract_abi()); } +#[tokio::test] +async fn verifying_evm_bytecode() { + let pool = ConnectionPool::test_pool().await; + let mut storage = pool.connection().await.unwrap(); + let creation_bytecode = vec![3_u8; 20]; + let deployed_bytecode = vec![5_u8; 10]; + + prepare_storage(&mut storage).await; + let address = Address::repeat_byte(1); + mock_evm_deployment( + &mut storage, + address, + creation_bytecode.clone(), + &deployed_bytecode, + ) + .await; + let req = test_request(address); + let request_id = storage + .contract_verification_dal() + .add_contract_verification_request(req) + .await + .unwrap(); + + let artifacts = CompilationArtifacts { + bytecode: creation_bytecode.clone(), + deployed_bytecode: Some(deployed_bytecode), + abi: counter_contract_abi(), + }; + let mock_resolver = MockCompilerResolver::solc(move |input| { + assert_eq!(input.standard_json.language, "Solidity"); + assert_eq!(input.standard_json.sources.len(), 1); + let source = input.standard_json.sources.values().next().unwrap(); + assert!(source.content.contains("contract Counter"), "{source:?}"); + + artifacts.clone() + }); + let verifier = ContractVerifier::with_resolver( + Duration::from_secs(60), + pool.clone(), + Arc::new(mock_resolver), + ) + .await + .unwrap(); + + let (_stop_sender, stop_receiver) = watch::channel(false); + verifier.run(stop_receiver, Some(1)).await.unwrap(); + + assert_request_success(&mut storage, request_id, address, &creation_bytecode).await; +} + #[tokio::test] async fn bytecode_mismatch_error() { let pool = ConnectionPool::test_pool().await; diff --git a/core/lib/dal/src/contract_verification_dal.rs b/core/lib/dal/src/contract_verification_dal.rs index cff5a61a7e38..1a827545ca13 100644 --- a/core/lib/dal/src/contract_verification_dal.rs +++ b/core/lib/dal/src/contract_verification_dal.rs @@ -570,7 +570,7 @@ mod tests { tx::IncludedTxLocation, Execute, L1BatchNumber, L2BlockNumber, ProtocolVersion, }; use zksync_utils::bytecode::hash_bytecode; - use zksync_vm_interface::TransactionExecutionMetrics; + use zksync_vm_interface::{tracer::ValidationTraces, TransactionExecutionMetrics}; use super::*; use crate::{ @@ -599,7 +599,11 @@ mod tests { let bytecode_hash = hash_bytecode(&bytecode); tx.execute = Execute::for_deploy(H256::zero(), bytecode.clone(), &[]); conn.transactions_dal() - .insert_transaction_l2(&tx, TransactionExecutionMetrics::default()) + .insert_transaction_l2( + &tx, + TransactionExecutionMetrics::default(), + ValidationTraces::default(), + ) .await .unwrap(); conn.factory_deps_dal() diff --git a/core/lib/types/src/contract_verification_api.rs b/core/lib/types/src/contract_verification_api.rs index 4c8c31a66d6e..44f5efcdcac8 100644 --- a/core/lib/types/src/contract_verification_api.rs +++ b/core/lib/types/src/contract_verification_api.rs @@ -213,9 +213,11 @@ pub struct VerificationRequest { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CompilationArtifacts { - /// In case of EVM contracts, this is the original bytecode (`bytecode` in `solc` output). + /// In case of EVM contracts, this is the creation bytecode (`bytecode` in `solc` output). pub bytecode: Vec, - /// Only set for EVM contracts. + /// Deployed bytecode (`deployedBytecode` in `solc` output). Only set for EVM contracts; for EraVM contracts, the deployed bytecode + /// is always `bytecode` (i.e., there's no distinction between creation and deployed bytecodes). + #[serde(default, skip_serializing_if = "Option::is_none")] pub deployed_bytecode: Option>, pub abi: serde_json::Value, } From faffe0365b6a8044977ca15effa24285c81fdfff Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 11:22:33 +0200 Subject: [PATCH 09/19] Verify creation code / args for EVM bytecodes --- core/lib/contract_verifier/src/error.rs | 2 ++ core/lib/contract_verifier/src/lib.rs | 33 ++++++++++++++++++++----- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/core/lib/contract_verifier/src/error.rs b/core/lib/contract_verifier/src/error.rs index e55d2499c93b..4ba9880fd704 100644 --- a/core/lib/contract_verifier/src/error.rs +++ b/core/lib/contract_verifier/src/error.rs @@ -6,6 +6,8 @@ pub enum ContractVerifierError { Internal(#[from] anyhow::Error), #[error("Deployed bytecode is not equal to generated one from given source")] BytecodeMismatch, + #[error("Creation bytecode is not equal to generated one from given source")] + CreationBytecodeMismatch, #[error("Constructor arguments are not correct")] IncorrectConstructorArguments, #[error("Compilation takes too much time")] diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index a7921c9557da..7aa5e1b658fa 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -160,10 +160,13 @@ impl ContractVerifier { let bytecode_marker = BytecodeMarker::new(deployed_contract.bytecode_hash) .context("unknown bytecode kind")?; let artifacts = self.compile(request.req.clone(), bytecode_marker).await?; - - // FIXME: extract constructor args / check bytecode for EVM contracts - let constructor_args = - self.decode_constructor_args(&deployed_contract, request.req.contract_address)?; + let constructor_args = match bytecode_marker { + BytecodeMarker::EraVm => self + .decode_era_vm_constructor_args(&deployed_contract, request.req.contract_address)?, + BytecodeMarker::Evm => { + Self::decode_evm_constructor_args(&deployed_contract, &artifacts.bytecode)? + } + }; let deployed_bytecode = match bytecode_marker { BytecodeMarker::EraVm => deployed_contract.bytecode.as_slice(), @@ -271,7 +274,7 @@ impl ContractVerifier { /// All returned errors are internal. #[tracing::instrument(level = "trace", skip_all, ret, err)] - fn decode_constructor_args( + fn decode_era_vm_constructor_args( &self, contract: &DeployedContractData, contract_address_to_verify: Address, @@ -283,7 +286,6 @@ impl ContractVerifier { if contract.contract_address == Some(CONTRACT_DEPLOYER_ADDRESS) { self.decode_contract_deployer_call(calldata, contract_address_to_verify) } else { - // FIXME: handle EVM contracts Ok(ConstructorArgs::Ignore) } } @@ -395,6 +397,25 @@ impl ContractVerifier { anyhow::bail!("couldn't find force deployment for address {contract_address_to_verify:?}"); } + fn decode_evm_constructor_args( + contract: &DeployedContractData, + creation_bytecode: &[u8], + ) -> Result { + let Some(calldata) = &contract.calldata else { + return Ok(ConstructorArgs::Ignore); + }; + if contract.contract_address.is_some() { + // Not a deployment transaction + return Ok(ConstructorArgs::Ignore); + } + + dbg!(&calldata, creation_bytecode); + let args = calldata + .strip_prefix(creation_bytecode) + .ok_or(ContractVerifierError::CreationBytecodeMismatch)?; + Ok(ConstructorArgs::Check(args.to_vec())) + } + #[tracing::instrument(level = "debug", skip_all, err, fields(id = request_id))] async fn process_result( &self, From ad3725fc154451a721892142f68cfe88e2dc4910 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 11:23:26 +0200 Subject: [PATCH 10/19] Test more verification errors --- core/lib/contract_verifier/Cargo.toml | 1 + core/lib/contract_verifier/src/tests/mod.rs | 204 ++++++++++++++++--- core/lib/contract_verifier/src/tests/real.rs | 62 ++++-- 3 files changed, 225 insertions(+), 42 deletions(-) diff --git a/core/lib/contract_verifier/Cargo.toml b/core/lib/contract_verifier/Cargo.toml index 3b2ab33294e3..bdbfa90bf76a 100644 --- a/core/lib/contract_verifier/Cargo.toml +++ b/core/lib/contract_verifier/Cargo.toml @@ -34,3 +34,4 @@ semver.workspace = true [dev-dependencies] zksync_node_test_utils.workspace = true zksync_vm_interface.workspace = true +test-casing.workspace = true diff --git a/core/lib/contract_verifier/src/tests/mod.rs b/core/lib/contract_verifier/src/tests/mod.rs index 38ff095bbd49..4ba5480e7a17 100644 --- a/core/lib/contract_verifier/src/tests/mod.rs +++ b/core/lib/contract_verifier/src/tests/mod.rs @@ -2,6 +2,7 @@ use std::{collections::HashMap, iter}; +use test_casing::{test_casing, Product}; use tokio::sync::watch; use zksync_dal::Connection; use zksync_node_test_utils::{create_l1_batch, create_l2_block}; @@ -30,6 +31,55 @@ mod real; const SOLC_VERSION: &str = "0.8.27"; const ZKSOLC_VERSION: &str = "1.5.4"; +const BYTECODE_KINDS: [BytecodeMarker; 2] = [BytecodeMarker::EraVm, BytecodeMarker::Evm]; + +const COUNTER_CONTRACT: &str = r#" + contract Counter { + uint256 value; + + function increment(uint256 x) external { + value += x; + } + } +"#; +const COUNTER_CONTRACT_WITH_CONSTRUCTOR: &str = r#" + contract Counter { + uint256 value; + + constructor(uint256 _value) { + value = _value; + } + + function increment(uint256 x) external { + value += x; + } + } +"#; + +#[derive(Debug, Clone, Copy)] +enum TestContract { + Counter, + CounterWithConstructor, +} + +impl TestContract { + const ALL: [Self; 2] = [Self::Counter, Self::CounterWithConstructor]; + + fn source(self) -> &'static str { + match self { + Self::Counter => COUNTER_CONTRACT, + Self::CounterWithConstructor => COUNTER_CONTRACT_WITH_CONSTRUCTOR, + } + } + + fn constructor_args(self) -> &'static [Token] { + match self { + Self::Counter => &[], + Self::CounterWithConstructor => &[Token::Uint(U256([42, 0, 0, 0]))], + } + } +} + /// Pads an EVM bytecode in the same ways it's done by system contracts. fn pad_evm_bytecode(deployed_bytecode: &[u8]) -> Vec { let mut padded = Vec::with_capacity(deployed_bytecode.len() + 32); @@ -52,21 +102,29 @@ fn pad_evm_bytecode(deployed_bytecode: &[u8]) -> Vec { padded } -async fn mock_deployment(storage: &mut Connection<'_, Core>, address: Address, bytecode: Vec) { +async fn mock_deployment( + storage: &mut Connection<'_, Core>, + address: Address, + bytecode: Vec, + constructor_args: &[Token], +) { let bytecode_hash = hash_bytecode(&bytecode); - let deployment = Execute::for_deploy(H256::zero(), bytecode.clone(), &[]); + let deployment = Execute::for_deploy(H256::zero(), bytecode.clone(), constructor_args); mock_deployment_inner(storage, address, bytecode_hash, bytecode, deployment).await; } async fn mock_evm_deployment( storage: &mut Connection<'_, Core>, address: Address, - original_bytecode: Vec, + creation_bytecode: Vec, deployed_bytecode: &[u8], + constructor_args: &[Token], ) { + let mut calldata = creation_bytecode; + calldata.extend_from_slice(ðabi::encode(constructor_args)); let deployment = Execute { contract_address: None, - calldata: original_bytecode, // FIXME: check + calldata, // FIXME: check value: 0.into(), factory_deps: vec![], }; @@ -159,7 +217,9 @@ impl fmt::Debug for MockCompilerResolver { } impl MockCompilerResolver { - fn new(zksolc: impl Fn(ZkSolcInput) -> CompilationArtifacts + 'static + Send + Sync) -> Self { + fn zksolc( + zksolc: impl Fn(ZkSolcInput) -> CompilationArtifacts + 'static + Send + Sync, + ) -> Self { Self { zksolc: Arc::new(move |input| Ok(zksolc(input))), solc: Arc::new(|input| panic!("unexpected solc call: {input:?}")), @@ -245,19 +305,10 @@ impl CompilerResolver for MockCompilerResolver { } } -fn test_request(address: Address) -> VerificationIncomingRequest { - let contract_source = r#" - contract Counter { - uint256 value; - - function increment(uint256 x) external { - value += x; - } - } - "#; +fn test_request(address: Address, source: &str) -> VerificationIncomingRequest { VerificationIncomingRequest { contract_address: address, - source_code_data: SourceCodeData::SolSingleFile(contract_source.into()), + source_code_data: SourceCodeData::SolSingleFile(source.into()), contract_name: "Counter".to_owned(), compiler_versions: CompilerVersions::Solc { compiler_zksolc_version: ZKSOLC_VERSION.to_owned(), @@ -304,23 +355,31 @@ async fn prepare_storage(storage: &mut Connection<'_, Core>) { .unwrap(); } +#[test_casing(2, TestContract::ALL)] #[tokio::test] -async fn contract_verifier_basics() { +async fn contract_verifier_basics(contract: TestContract) { let pool = ConnectionPool::test_pool().await; let mut storage = pool.connection().await.unwrap(); let expected_bytecode = vec![0_u8; 32]; prepare_storage(&mut storage).await; let address = Address::repeat_byte(1); - mock_deployment(&mut storage, address, expected_bytecode.clone()).await; - let req = test_request(address); + mock_deployment( + &mut storage, + address, + expected_bytecode.clone(), + contract.constructor_args(), + ) + .await; + let mut req = test_request(address, contract.source()); + req.constructor_arguments = ethabi::encode(contract.constructor_args()).into(); let request_id = storage .contract_verification_dal() .add_contract_verification_request(req) .await .unwrap(); - let mock_resolver = MockCompilerResolver::new(|input| { + let mock_resolver = MockCompilerResolver::zksolc(|input| { let ZkSolcInput::StandardJson { input, .. } = &input else { panic!("unexpected input"); }; @@ -368,7 +427,7 @@ async fn assert_request_success( request_id: usize, address: Address, expected_bytecode: &[u8], -) { +) -> VerificationInfo { let status = storage .contract_verification_dal() .get_verification_request_status(request_id) @@ -387,10 +446,12 @@ async fn assert_request_success( .expect("no verification info"); assert_eq!(verification_info.artifacts.bytecode, *expected_bytecode); assert_eq!(verification_info.artifacts.abi, counter_contract_abi()); + verification_info } +#[test_casing(2, TestContract::ALL)] #[tokio::test] -async fn verifying_evm_bytecode() { +async fn verifying_evm_bytecode(contract: TestContract) { let pool = ConnectionPool::test_pool().await; let mut storage = pool.connection().await.unwrap(); let creation_bytecode = vec![3_u8; 20]; @@ -403,9 +464,11 @@ async fn verifying_evm_bytecode() { address, creation_bytecode.clone(), &deployed_bytecode, + contract.constructor_args(), ) .await; - let req = test_request(address); + let mut req = test_request(address, contract.source()); + req.constructor_arguments = ethabi::encode(contract.constructor_args()).into(); let request_id = storage .contract_verification_dal() .add_contract_verification_request(req) @@ -446,15 +509,15 @@ async fn bytecode_mismatch_error() { prepare_storage(&mut storage).await; let address = Address::repeat_byte(1); - mock_deployment(&mut storage, address, vec![0xff; 32]).await; - let req = test_request(address); + mock_deployment(&mut storage, address, vec![0xff; 32], &[]).await; + let req = test_request(address, COUNTER_CONTRACT); let request_id = storage .contract_verification_dal() .add_contract_verification_request(req) .await .unwrap(); - let mock_resolver = MockCompilerResolver::new(|_| CompilationArtifacts { + let mock_resolver = MockCompilerResolver::zksolc(|_| CompilationArtifacts { bytecode: vec![0; 32], deployed_bytecode: None, abi: counter_contract_abi(), @@ -482,6 +545,89 @@ async fn bytecode_mismatch_error() { assert!(error.contains("bytecode"), "{error}"); } +#[test_casing(4, Product((TestContract::ALL, BYTECODE_KINDS)))] +#[tokio::test] +async fn args_mismatch_error(contract: TestContract, bytecode_kind: BytecodeMarker) { + let pool = ConnectionPool::test_pool().await; + let mut storage = pool.connection().await.unwrap(); + + prepare_storage(&mut storage).await; + let address = Address::repeat_byte(1); + let bytecode = vec![0_u8; 32]; + match bytecode_kind { + BytecodeMarker::EraVm => { + mock_deployment( + &mut storage, + address, + bytecode.clone(), + contract.constructor_args(), + ) + .await; + } + BytecodeMarker::Evm => { + let creation_bytecode = vec![3_u8; 48]; + mock_evm_deployment( + &mut storage, + address, + creation_bytecode, + &bytecode, + contract.constructor_args(), + ) + .await; + } + } + + let mut req = test_request(address, contract.source()); + // Intentionally encode incorrect constructor args + req.constructor_arguments = match contract { + TestContract::Counter => ethabi::encode(&[Token::Bool(true)]).into(), + TestContract::CounterWithConstructor => ethabi::encode(&[]).into(), + }; + let request_id = storage + .contract_verification_dal() + .add_contract_verification_request(req) + .await + .unwrap(); + + let mock_resolver = match bytecode_kind { + BytecodeMarker::EraVm => MockCompilerResolver::zksolc(move |_| CompilationArtifacts { + bytecode: bytecode.clone(), + deployed_bytecode: None, + abi: counter_contract_abi(), + }), + BytecodeMarker::Evm => MockCompilerResolver::solc(move |_| CompilationArtifacts { + bytecode: vec![3_u8; 48], + deployed_bytecode: Some(bytecode.clone()), + abi: counter_contract_abi(), + }), + }; + let verifier = ContractVerifier::with_resolver( + Duration::from_secs(60), + pool.clone(), + Arc::new(mock_resolver), + ) + .await + .unwrap(); + + let (_stop_sender, stop_receiver) = watch::channel(false); + verifier.run(stop_receiver, Some(1)).await.unwrap(); + + assert_constructor_args_mismatch(&mut storage, request_id).await; +} + +async fn assert_constructor_args_mismatch(storage: &mut Connection<'_, Core>, request_id: usize) { + let status = storage + .contract_verification_dal() + .get_verification_request_status(request_id) + .await + .unwrap() + .expect("no status"); + assert_eq!(status.status, "failed"); + assert_eq!(status.compilation_errors, None); + let err = status.error.unwrap().to_lowercase(); + assert!(err.contains("constructor arguments"), "{err}"); +} + #[tokio::test] async fn no_compiler_version() { let pool = ConnectionPool::test_pool().await; @@ -489,13 +635,13 @@ async fn no_compiler_version() { prepare_storage(&mut storage).await; let address = Address::repeat_byte(1); - mock_deployment(&mut storage, address, vec![0xff; 32]).await; + mock_deployment(&mut storage, address, vec![0xff; 32], &[]).await; let req = VerificationIncomingRequest { compiler_versions: CompilerVersions::Solc { compiler_zksolc_version: ZKSOLC_VERSION.to_owned(), compiler_solc_version: "1.0.0".to_owned(), // a man can dream }, - ..test_request(address) + ..test_request(address, COUNTER_CONTRACT) }; let request_id = storage .contract_verification_dal() @@ -504,7 +650,7 @@ async fn no_compiler_version() { .unwrap(); let mock_resolver = - MockCompilerResolver::new(|_| unreachable!("should reject unknown solc version")); + MockCompilerResolver::zksolc(|_| unreachable!("should reject unknown solc version")); let verifier = ContractVerifier::with_resolver( Duration::from_secs(60), pool.clone(), diff --git a/core/lib/contract_verifier/src/tests/real.rs b/core/lib/contract_verifier/src/tests/real.rs index 55af72a28afe..58e95e7714c4 100644 --- a/core/lib/contract_verifier/src/tests/real.rs +++ b/core/lib/contract_verifier/src/tests/real.rs @@ -64,7 +64,7 @@ async fn using_real_compiler() { let compiler = compiler_resolver.resolve_zksolc(&versions).await.unwrap(); let req = VerificationIncomingRequest { compiler_versions: versions, - ..test_request(Address::repeat_byte(1)) + ..test_request(Address::repeat_byte(1), COUNTER_CONTRACT) }; let input = ZkSolc::build_input(req).unwrap(); let output = compiler.compile(input).await.unwrap(); @@ -85,9 +85,9 @@ async fn using_standalone_solc() { let req = VerificationIncomingRequest { compiler_versions: CompilerVersions::Solc { compiler_solc_version: version.clone(), - compiler_zksolc_version: "1000.0.0".to_owned(), // doesn't matter + compiler_zksolc_version: "1000.0.0".to_owned(), // not used }, - ..test_request(Address::repeat_byte(1)) + ..test_request(Address::repeat_byte(1), COUNTER_CONTRACT) }; let input = Solc::build_input(req).unwrap(); let output = compiler.compile(input).await.unwrap(); @@ -96,27 +96,55 @@ async fn using_standalone_solc() { assert_eq!(output.abi, counter_contract_abi()); } +#[test_casing(2, BYTECODE_KINDS)] #[tokio::test] -async fn using_real_compiler_in_verifier() { +async fn using_real_compiler_in_verifier(bytecode_kind: BytecodeMarker) { let Some((compiler_resolver, supported_compilers)) = checked_env_resolver().await else { assert_no_compilers_expected(); return; }; let versions = supported_compilers.for_zksolc(); - let address = Address::repeat_byte(1); - let compiler = compiler_resolver.resolve_zksolc(&versions).await.unwrap(); let req = VerificationIncomingRequest { compiler_versions: versions, - ..test_request(Address::repeat_byte(1)) + ..test_request(Address::repeat_byte(1), COUNTER_CONTRACT) + }; + let address = Address::repeat_byte(1); + let output = match bytecode_kind { + BytecodeMarker::EraVm => { + let compiler = compiler_resolver + .resolve_zksolc(&req.compiler_versions) + .await + .unwrap(); + let input = ZkSolc::build_input(req.clone()).unwrap(); + compiler.compile(input).await.unwrap() + } + BytecodeMarker::Evm => { + let solc_version = req.compiler_versions.compiler_version(); + let compiler = compiler_resolver.resolve_solc(&solc_version).await.unwrap(); + let input = Solc::build_input(req.clone()).unwrap(); + compiler.compile(input).await.unwrap() + } }; - let input = ZkSolc::build_input(req.clone()).unwrap(); - let output = compiler.compile(input).await.unwrap(); let pool = ConnectionPool::test_pool().await; let mut storage = pool.connection().await.unwrap(); prepare_storage(&mut storage).await; - mock_deployment(&mut storage, address, output.bytecode.clone()).await; + match bytecode_kind { + BytecodeMarker::EraVm => { + mock_deployment(&mut storage, address, output.bytecode.clone(), &[]).await; + } + BytecodeMarker::Evm => { + mock_evm_deployment( + &mut storage, + address, + output.bytecode.clone(), + output.deployed_bytecode(), + &[], + ) + .await; + } + } let request_id = storage .contract_verification_dal() .add_contract_verification_request(req) @@ -137,8 +165,9 @@ async fn using_real_compiler_in_verifier() { assert_request_success(&mut storage, request_id, address, &output.bytecode).await; } +#[test_casing(2, BYTECODE_KINDS)] #[tokio::test] -async fn compilation_errors() { +async fn compilation_errors(bytecode_kind: BytecodeMarker) { let Some((compiler_resolver, supported_compilers)) = checked_env_resolver().await else { assert_no_compilers_expected(); return; @@ -149,13 +178,20 @@ async fn compilation_errors() { let req = VerificationIncomingRequest { compiler_versions: versions, source_code_data: SourceCodeData::SolSingleFile("contract ???".to_owned()), - ..test_request(Address::repeat_byte(1)) + ..test_request(Address::repeat_byte(1), COUNTER_CONTRACT) }; let pool = ConnectionPool::test_pool().await; let mut storage = pool.connection().await.unwrap(); prepare_storage(&mut storage).await; - mock_deployment(&mut storage, address, vec![0; 32]).await; + match bytecode_kind { + BytecodeMarker::EraVm => { + mock_deployment(&mut storage, address, vec![0; 32], &[]).await; + } + BytecodeMarker::Evm => { + mock_evm_deployment(&mut storage, address, vec![3; 20], &[5; 10], &[]).await; + } + } let request_id = storage .contract_verification_dal() From 1fac11711f864834d41f31d6829e9ed1097a6ccd Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 12:44:50 +0200 Subject: [PATCH 11/19] Update lockfiles --- Cargo.lock | 1 + prover/Cargo.lock | 12 +++++++++++- zkstack_cli/Cargo.lock | 10 ++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index ed77e866a57e..92bc745fe9ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10833,6 +10833,7 @@ dependencies = [ "serde", "serde_json", "tempfile", + "test-casing", "thiserror", "tokio", "tracing", diff --git a/prover/Cargo.lock b/prover/Cargo.lock index d13af11fde9f..b5536f85eea2 100644 --- a/prover/Cargo.lock +++ b/prover/Cargo.lock @@ -1071,6 +1071,15 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5241cd7938b1b415942e943ea96f615953d500b50347b505b0b507080bad5a6f" +[[package]] +name = "const-decoder" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b381abde2cdc1bc3817e394b24e05667a2dc89f37570cbd34d9c397d99e56e3f" +dependencies = [ + "compile-fmt", +] + [[package]] name = "const-oid" version = "0.9.6" @@ -8622,6 +8631,7 @@ version = "0.1.0" dependencies = [ "anyhow", "bigdecimal", + "const-decoder 0.4.0", "futures 0.3.30", "hex", "num", @@ -8747,7 +8757,7 @@ dependencies = [ "async-trait", "bincode", "circuit_definitions", - "const-decoder", + "const-decoder 0.3.0", "ctrlc", "futures 0.3.30", "jemallocator", diff --git a/zkstack_cli/Cargo.lock b/zkstack_cli/Cargo.lock index 7770d06a1977..0f0657c60278 100644 --- a/zkstack_cli/Cargo.lock +++ b/zkstack_cli/Cargo.lock @@ -795,6 +795,15 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "const-decoder" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b381abde2cdc1bc3817e394b24e05667a2dc89f37570cbd34d9c397d99e56e3f" +dependencies = [ + "compile-fmt", +] + [[package]] name = "const-hex" version = "1.13.1" @@ -7009,6 +7018,7 @@ version = "0.1.0" dependencies = [ "anyhow", "bigdecimal", + "const-decoder", "futures", "hex", "num", From 2e2f0081585f51fdb42b190708434ff0e5b8b68f Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 12:55:09 +0200 Subject: [PATCH 12/19] Test creation bytecode mismatch --- core/lib/contract_verifier/src/tests/mod.rs | 66 +++++++++++++++++++-- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/core/lib/contract_verifier/src/tests/mod.rs b/core/lib/contract_verifier/src/tests/mod.rs index 4ba5480e7a17..90350a927f7d 100644 --- a/core/lib/contract_verifier/src/tests/mod.rs +++ b/core/lib/contract_verifier/src/tests/mod.rs @@ -541,8 +541,8 @@ async fn bytecode_mismatch_error() { .expect("no status"); assert_eq!(status.status, "failed"); assert!(status.compilation_errors.is_none(), "{status:?}"); - let error = status.error.unwrap(); - assert!(error.contains("bytecode"), "{error}"); + let err = status.error.unwrap(); + assert_eq!(err, ContractVerifierError::BytecodeMismatch.to_string()); } #[test_casing(4, Product((TestContract::ALL, BYTECODE_KINDS)))] @@ -624,8 +624,66 @@ async fn assert_constructor_args_mismatch(storage: &mut Connection<'_, Core>, re .expect("no status"); assert_eq!(status.status, "failed"); assert_eq!(status.compilation_errors, None); - let err = status.error.unwrap().to_lowercase(); - assert!(err.contains("constructor arguments"), "{err}"); + let err = status.error.unwrap(); + assert_eq!( + err, + ContractVerifierError::IncorrectConstructorArguments.to_string() + ); +} + +#[tokio::test] +async fn creation_bytecode_mismatch() { + let pool = ConnectionPool::test_pool().await; + let mut storage = pool.connection().await.unwrap(); + prepare_storage(&mut storage).await; + + let address = Address::repeat_byte(1); + let creation_bytecode = vec![3; 20]; + let deployed_bytecode = vec![5; 10]; + mock_evm_deployment( + &mut storage, + address, + creation_bytecode, + &deployed_bytecode, + &[], + ) + .await; + let req = test_request(address, COUNTER_CONTRACT); + let request_id = storage + .contract_verification_dal() + .add_contract_verification_request(req) + .await + .unwrap(); + + let mock_resolver = MockCompilerResolver::solc(move |_| CompilationArtifacts { + bytecode: vec![4; 20], // differs from `creation_bytecode` + deployed_bytecode: Some(deployed_bytecode.clone()), + abi: counter_contract_abi(), + }); + let verifier = ContractVerifier::with_resolver( + Duration::from_secs(60), + pool.clone(), + Arc::new(mock_resolver), + ) + .await + .unwrap(); + + let (_stop_sender, stop_receiver) = watch::channel(false); + verifier.run(stop_receiver, Some(1)).await.unwrap(); + + let status = storage + .contract_verification_dal() + .get_verification_request_status(request_id) + .await + .unwrap() + .expect("no status"); + assert_eq!(status.status, "failed"); + assert!(status.compilation_errors.is_none(), "{status:?}"); + let err = status.error.unwrap(); + assert_eq!( + err, + ContractVerifierError::CreationBytecodeMismatch.to_string() + ); } #[tokio::test] From 2e75e362534a6c2d2f83278d829f2b839629e711 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 13:03:20 +0200 Subject: [PATCH 13/19] Improve string ownership for compiler versions --- core/lib/contract_verifier/src/error.rs | 2 +- core/lib/contract_verifier/src/lib.rs | 2 +- core/lib/contract_verifier/src/resolver.rs | 28 +++++++++---------- core/lib/contract_verifier/src/tests/mod.rs | 10 +++---- core/lib/contract_verifier/src/tests/real.rs | 2 +- .../types/src/contract_verification_api.rs | 22 +++++++-------- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/core/lib/contract_verifier/src/error.rs b/core/lib/contract_verifier/src/error.rs index 4ba9880fd704..4e6273765a75 100644 --- a/core/lib/contract_verifier/src/error.rs +++ b/core/lib/contract_verifier/src/error.rs @@ -17,7 +17,7 @@ pub enum ContractVerifierError { #[error("Compilation error")] CompilationError(serde_json::Value), #[error("Unknown {0} version: {1}")] - UnknownCompilerVersion(String, String), + UnknownCompilerVersion(&'static str, String), #[error("Contract with {0} name is missing in sources")] MissingContract(String), #[error("There is no {0} source file")] diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index 7aa5e1b658fa..0513e0aa7239 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -247,7 +247,7 @@ impl ContractVerifier { ) -> Result { let solc = self .compiler_resolver - .resolve_solc(&req.compiler_versions.compiler_version()) + .resolve_solc(req.compiler_versions.compiler_version()) .await?; tracing::debug!(?solc, ?req.compiler_versions, "resolved compiler"); let input = Solc::build_input(req)?; diff --git a/core/lib/contract_verifier/src/resolver.rs b/core/lib/contract_verifier/src/resolver.rs index ff200628cf0d..09845ac2b90b 100644 --- a/core/lib/contract_verifier/src/resolver.rs +++ b/core/lib/contract_verifier/src/resolver.rs @@ -111,21 +111,21 @@ impl EnvCompilerResolver { async fn resolve_solc_path( &self, - solc_version: String, + solc_version: &str, ) -> Result { let solc_path = self .home_dir .join("etc") .join("solc-bin") - .join(&solc_version) + .join(solc_version) .join("solc"); if !fs::try_exists(&solc_path) .await .context("failed accessing solc")? { return Err(ContractVerifierError::UnknownCompilerVersion( - "solc".to_owned(), - solc_version, + "solc", + solc_version.to_owned(), )); } Ok(solc_path) @@ -159,7 +159,7 @@ impl CompilerResolver for EnvCompilerResolver { &self, version: &str, ) -> Result>, ContractVerifierError> { - let solc_path = self.resolve_solc_path(version.to_owned()).await?; + let solc_path = self.resolve_solc_path(version).await?; Ok(Box::new(Solc::new(solc_path))) } @@ -167,7 +167,7 @@ impl CompilerResolver for EnvCompilerResolver { &self, versions: &CompilerVersions, ) -> Result>, ContractVerifierError> { - let zksolc_version = versions.zk_compiler_version(); + let zksolc_version = versions.zk_compiler_version().to_owned(); let zksolc_path = self .home_dir .join("etc") @@ -179,8 +179,8 @@ impl CompilerResolver for EnvCompilerResolver { .context("failed accessing zksolc")? { return Err(ContractVerifierError::UnknownCompilerVersion( - "zksolc".to_owned(), - zksolc_version, + "zksolc", + zksolc_version.to_owned(), )); } @@ -201,15 +201,15 @@ impl CompilerResolver for EnvCompilerResolver { .home_dir .join("etc") .join("zkvyper-bin") - .join(&zkvyper_version) + .join(zkvyper_version) .join("zkvyper"); if !fs::try_exists(&zkvyper_path) .await .context("failed accessing zkvyper")? { return Err(ContractVerifierError::UnknownCompilerVersion( - "zkvyper".to_owned(), - zkvyper_version, + "zkvyper", + zkvyper_version.to_owned(), )); } @@ -218,15 +218,15 @@ impl CompilerResolver for EnvCompilerResolver { .home_dir .join("etc") .join("vyper-bin") - .join(&vyper_version) + .join(vyper_version) .join("vyper"); if !fs::try_exists(&vyper_path) .await .context("failed accessing vyper")? { return Err(ContractVerifierError::UnknownCompilerVersion( - "vyper".to_owned(), - vyper_version, + "vyper", + vyper_version.to_owned(), )); } diff --git a/core/lib/contract_verifier/src/tests/mod.rs b/core/lib/contract_verifier/src/tests/mod.rs index 90350a927f7d..f05d3155a6d4 100644 --- a/core/lib/contract_verifier/src/tests/mod.rs +++ b/core/lib/contract_verifier/src/tests/mod.rs @@ -271,7 +271,7 @@ impl CompilerResolver for MockCompilerResolver { ) -> Result>, ContractVerifierError> { if version != SOLC_VERSION { return Err(ContractVerifierError::UnknownCompilerVersion( - "solc".to_owned(), + "solc", version.to_owned(), )); } @@ -284,14 +284,14 @@ impl CompilerResolver for MockCompilerResolver { ) -> Result>, ContractVerifierError> { if versions.compiler_version() != SOLC_VERSION { return Err(ContractVerifierError::UnknownCompilerVersion( - "solc".to_owned(), - versions.compiler_version(), + "solc", + versions.compiler_version().to_owned(), )); } if versions.zk_compiler_version() != ZKSOLC_VERSION { return Err(ContractVerifierError::UnknownCompilerVersion( - "zksolc".to_owned(), - versions.zk_compiler_version(), + "zksolc", + versions.zk_compiler_version().to_owned(), )); } Ok(Box::new(self.clone())) diff --git a/core/lib/contract_verifier/src/tests/real.rs b/core/lib/contract_verifier/src/tests/real.rs index 58e95e7714c4..01593bbe602c 100644 --- a/core/lib/contract_verifier/src/tests/real.rs +++ b/core/lib/contract_verifier/src/tests/real.rs @@ -121,7 +121,7 @@ async fn using_real_compiler_in_verifier(bytecode_kind: BytecodeMarker) { } BytecodeMarker::Evm => { let solc_version = req.compiler_versions.compiler_version(); - let compiler = compiler_resolver.resolve_solc(&solc_version).await.unwrap(); + let compiler = compiler_resolver.resolve_solc(solc_version).await.unwrap(); let input = Solc::build_input(req.clone()).unwrap(); compiler.compile(input).await.unwrap() } diff --git a/core/lib/types/src/contract_verification_api.rs b/core/lib/types/src/contract_verification_api.rs index 44f5efcdcac8..fcaa1aa9a535 100644 --- a/core/lib/types/src/contract_verification_api.rs +++ b/core/lib/types/src/contract_verification_api.rs @@ -157,7 +157,7 @@ pub enum CompilerType { pub enum CompilerVersions { #[serde(rename_all = "camelCase")] Solc { - compiler_zksolc_version: String, + compiler_zksolc_version: String, // FIXME: optional? compiler_solc_version: String, }, #[serde(rename_all = "camelCase")] @@ -175,29 +175,29 @@ impl CompilerVersions { } } - pub fn zk_compiler_version(&self) -> String { + pub fn zk_compiler_version(&self) -> &str { match self { - CompilerVersions::Solc { + Self::Solc { compiler_zksolc_version, .. - } => compiler_zksolc_version.clone(), - CompilerVersions::Vyper { + } => compiler_zksolc_version, + Self::Vyper { compiler_zkvyper_version, .. - } => compiler_zkvyper_version.clone(), + } => compiler_zkvyper_version, } } - pub fn compiler_version(&self) -> String { + pub fn compiler_version(&self) -> &str { match self { - CompilerVersions::Solc { + Self::Solc { compiler_solc_version, .. - } => compiler_solc_version.clone(), - CompilerVersions::Vyper { + } => compiler_solc_version, + Self::Vyper { compiler_vyper_version, .. - } => compiler_vyper_version.clone(), + } => compiler_vyper_version, } } } From c17db75de750d25828f6153b74fc217874ff8405 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 13:25:13 +0200 Subject: [PATCH 14/19] Improve compiler path resolution --- core/lib/contract_verifier/src/resolver.rs | 151 ++++++++++----------- 1 file changed, 74 insertions(+), 77 deletions(-) diff --git a/core/lib/contract_verifier/src/resolver.rs b/core/lib/contract_verifier/src/resolver.rs index 09845ac2b90b..347db8fff094 100644 --- a/core/lib/contract_verifier/src/resolver.rs +++ b/core/lib/contract_verifier/src/resolver.rs @@ -1,4 +1,7 @@ -use std::{fmt, path::PathBuf}; +use std::{ + fmt, + path::{Path, PathBuf}, +}; use anyhow::Context as _; use tokio::fs; @@ -11,6 +14,58 @@ use crate::{ error::ContractVerifierError, }; +#[derive(Debug, Clone, Copy)] +enum CompilerType { + Solc, + ZkSolc, + Vyper, + ZkVyper, +} + +impl CompilerType { + fn as_str(self) -> &'static str { + match self { + Self::Solc => "solc", + Self::ZkSolc => "zksolc", + Self::Vyper => "vyper", + Self::ZkVyper => "zkvyper", + } + } + + /// Returns the absolute path to the compiler binary. + fn bin_path_unchecked(self, home_dir: &Path, version: &str) -> PathBuf { + let compiler_dir = match self { + Self::Solc => "solc-bin", + Self::ZkSolc => "zksolc-bin", + Self::Vyper => "vyper-bin", + Self::ZkVyper => "zkvyper-bin", + }; + home_dir + .join("etc") + .join(compiler_dir) + .join(version) + .join(self.as_str()) + } + + async fn bin_path( + self, + home_dir: &Path, + version: &str, + ) -> Result { + let path = self.bin_path_unchecked(home_dir, version); + if !fs::try_exists(&path) + .await + .with_context(|| format!("failed accessing `{}`", self.as_str()))? + { + return Err(ContractVerifierError::UnknownCompilerVersion( + self.as_str(), + version.to_owned(), + )); + } + Ok(path) + } +} + /// Compiler versions supported by a [`CompilerResolver`]. #[derive(Debug)] pub(crate) struct SupportedCompilerVersions { @@ -108,28 +163,6 @@ impl EnvCompilerResolver { } Ok(versions) } - - async fn resolve_solc_path( - &self, - solc_version: &str, - ) -> Result { - let solc_path = self - .home_dir - .join("etc") - .join("solc-bin") - .join(solc_version) - .join("solc"); - if !fs::try_exists(&solc_path) - .await - .context("failed accessing solc")? - { - return Err(ContractVerifierError::UnknownCompilerVersion( - "solc", - solc_version.to_owned(), - )); - } - Ok(solc_path) - } } #[async_trait] @@ -159,7 +192,7 @@ impl CompilerResolver for EnvCompilerResolver { &self, version: &str, ) -> Result>, ContractVerifierError> { - let solc_path = self.resolve_solc_path(version).await?; + let solc_path = CompilerType::Solc.bin_path(&self.home_dir, version).await?; Ok(Box::new(Solc::new(solc_path))) } @@ -167,69 +200,33 @@ impl CompilerResolver for EnvCompilerResolver { &self, versions: &CompilerVersions, ) -> Result>, ContractVerifierError> { - let zksolc_version = versions.zk_compiler_version().to_owned(); - let zksolc_path = self - .home_dir - .join("etc") - .join("zksolc-bin") - .join(&zksolc_version) - .join("zksolc"); - if !fs::try_exists(&zksolc_path) - .await - .context("failed accessing zksolc")? - { - return Err(ContractVerifierError::UnknownCompilerVersion( - "zksolc", - zksolc_version.to_owned(), - )); - } - - let solc_path = self.resolve_solc_path(versions.compiler_version()).await?; + let zksolc_version = versions.zk_compiler_version(); + let zksolc_path = CompilerType::ZkSolc + .bin_path(&self.home_dir, zksolc_version) + .await?; + let solc_path = CompilerType::Solc + .bin_path(&self.home_dir, versions.compiler_version()) + .await?; let compiler_paths = CompilerPaths { base: solc_path, zk: zksolc_path, }; - Ok(Box::new(ZkSolc::new(compiler_paths, zksolc_version))) + Ok(Box::new(ZkSolc::new( + compiler_paths, + zksolc_version.to_owned(), + ))) } async fn resolve_zkvyper( &self, versions: &CompilerVersions, ) -> Result>, ContractVerifierError> { - let zkvyper_version = versions.zk_compiler_version(); - let zkvyper_path = self - .home_dir - .join("etc") - .join("zkvyper-bin") - .join(zkvyper_version) - .join("zkvyper"); - if !fs::try_exists(&zkvyper_path) - .await - .context("failed accessing zkvyper")? - { - return Err(ContractVerifierError::UnknownCompilerVersion( - "zkvyper", - zkvyper_version.to_owned(), - )); - } - - let vyper_version = versions.compiler_version(); - let vyper_path = self - .home_dir - .join("etc") - .join("vyper-bin") - .join(vyper_version) - .join("vyper"); - if !fs::try_exists(&vyper_path) - .await - .context("failed accessing vyper")? - { - return Err(ContractVerifierError::UnknownCompilerVersion( - "vyper", - vyper_version.to_owned(), - )); - } - + let zkvyper_path = CompilerType::ZkVyper + .bin_path(&self.home_dir, versions.zk_compiler_version()) + .await?; + let vyper_path = CompilerType::Vyper + .bin_path(&self.home_dir, versions.compiler_version()) + .await?; let compiler_paths = CompilerPaths { base: vyper_path, zk: zkvyper_path, From 70915fb3e39c5390fd8f8687e0e252f334b30ee8 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 13:33:15 +0200 Subject: [PATCH 15/19] Simplify resolver initialization in tests --- core/lib/contract_verifier/src/tests/real.rs | 33 ++++++++++---------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/core/lib/contract_verifier/src/tests/real.rs b/core/lib/contract_verifier/src/tests/real.rs index 01593bbe602c..5f550a5feea8 100644 --- a/core/lib/contract_verifier/src/tests/real.rs +++ b/core/lib/contract_verifier/src/tests/real.rs @@ -53,12 +53,22 @@ fn assert_no_compilers_expected() { println!("No compilers found, skipping the test"); } +/// Simplifies initializing real compiler resolver in tests. +macro_rules! real_resolver { + () => { + match checked_env_resolver().await { + Some(resolver_and_versions) => resolver_and_versions, + None => { + assert_no_compilers_expected(); + return; + } + } + }; +} + #[tokio::test] async fn using_real_compiler() { - let Some((compiler_resolver, supported_compilers)) = checked_env_resolver().await else { - assert_no_compilers_expected(); - return; - }; + let (compiler_resolver, supported_compilers) = real_resolver!(); let versions = supported_compilers.for_zksolc(); let compiler = compiler_resolver.resolve_zksolc(&versions).await.unwrap(); @@ -75,10 +85,7 @@ async fn using_real_compiler() { #[tokio::test] async fn using_standalone_solc() { - let Some((compiler_resolver, supported_compilers)) = checked_env_resolver().await else { - assert_no_compilers_expected(); - return; - }; + let (compiler_resolver, supported_compilers) = real_resolver!(); let version = &supported_compilers.solc; let compiler = compiler_resolver.resolve_solc(version).await.unwrap(); @@ -99,10 +106,7 @@ async fn using_standalone_solc() { #[test_casing(2, BYTECODE_KINDS)] #[tokio::test] async fn using_real_compiler_in_verifier(bytecode_kind: BytecodeMarker) { - let Some((compiler_resolver, supported_compilers)) = checked_env_resolver().await else { - assert_no_compilers_expected(); - return; - }; + let (compiler_resolver, supported_compilers) = real_resolver!(); let versions = supported_compilers.for_zksolc(); let req = VerificationIncomingRequest { @@ -168,10 +172,7 @@ async fn using_real_compiler_in_verifier(bytecode_kind: BytecodeMarker) { #[test_casing(2, BYTECODE_KINDS)] #[tokio::test] async fn compilation_errors(bytecode_kind: BytecodeMarker) { - let Some((compiler_resolver, supported_compilers)) = checked_env_resolver().await else { - assert_no_compilers_expected(); - return; - }; + let (compiler_resolver, supported_compilers) = real_resolver!(); let versions = supported_compilers.for_zksolc(); let address = Address::repeat_byte(1); From 163ba0a5b1dd959df9ddd4f12615e61ed8426a21 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 13:51:51 +0200 Subject: [PATCH 16/19] Brush up code --- core/lib/contract_verifier/src/lib.rs | 38 ++++++++++++++++++--------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index 0513e0aa7239..6b4a677f7c63 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -163,9 +163,11 @@ impl ContractVerifier { let constructor_args = match bytecode_marker { BytecodeMarker::EraVm => self .decode_era_vm_constructor_args(&deployed_contract, request.req.contract_address)?, - BytecodeMarker::Evm => { - Self::decode_evm_constructor_args(&deployed_contract, &artifacts.bytecode)? - } + BytecodeMarker::Evm => Self::decode_evm_constructor_args( + request.id, + &deployed_contract, + &artifacts.bytecode, + )?, }; let deployed_bytecode = match bytecode_marker { @@ -176,10 +178,10 @@ impl ContractVerifier { if artifacts.deployed_bytecode() != deployed_bytecode { tracing::info!( - "Bytecode mismatch req {}, deployed: 0x{}, compiled: 0x{}", - request.id, - hex::encode(deployed_bytecode), - hex::encode(artifacts.deployed_bytecode()) + request_id = request.id, + deployed = hex::encode(deployed_bytecode), + compiled = hex::encode(artifacts.deployed_bytecode()), + "Deployed (runtime) bytecode mismatch", ); return Err(ContractVerifierError::BytecodeMismatch); } @@ -268,7 +270,11 @@ impl ContractVerifier { (BytecodeMarker::EraVm, CompilerType::Solc) => self.compile_zksolc(req).await, (BytecodeMarker::EraVm, CompilerType::Vyper) => self.compile_zkvyper(req).await, (BytecodeMarker::Evm, CompilerType::Solc) => self.compile_solc(req).await, - (BytecodeMarker::Evm, CompilerType::Vyper) => todo!(), + (BytecodeMarker::Evm, CompilerType::Vyper) => { + // TODO: add vyper support + let err = anyhow::anyhow!("vyper toolchain is not yet supported for EVM contracts"); + return Err(err.into()); + } } } @@ -398,6 +404,7 @@ impl ContractVerifier { } fn decode_evm_constructor_args( + request_id: usize, contract: &DeployedContractData, creation_bytecode: &[u8], ) -> Result { @@ -405,14 +412,19 @@ impl ContractVerifier { return Ok(ConstructorArgs::Ignore); }; if contract.contract_address.is_some() { - // Not a deployment transaction + // Not an EVM deployment transaction return Ok(ConstructorArgs::Ignore); } - dbg!(&calldata, creation_bytecode); - let args = calldata - .strip_prefix(creation_bytecode) - .ok_or(ContractVerifierError::CreationBytecodeMismatch)?; + let args = calldata.strip_prefix(creation_bytecode).ok_or_else(|| { + tracing::info!( + request_id, + calldata = hex::encode(calldata), + compiled = hex::encode(creation_bytecode), + "Creation bytecode mismatch" + ); + ContractVerifierError::CreationBytecodeMismatch + })?; Ok(ConstructorArgs::Check(args.to_vec())) } From 9318e1258e8905938151d46dc345bd1fb34c2c36 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 14:05:11 +0200 Subject: [PATCH 17/19] Revert `contracts` submodule changes --- contracts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts b/contracts index 84d5e3716f64..9fb1264fce8c 160000 --- a/contracts +++ b/contracts @@ -1 +1 @@ -Subproject commit 84d5e3716f645909e8144c7d50af9dd6dd9ded62 +Subproject commit 9fb1264fce8c0ebeefe8bf1846e89876027161d2 From 84c3838ba0f5baea4100f7d4e506d4bc389cf2ea Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 15:10:26 +0200 Subject: [PATCH 18/19] Use `&'static str` for another error variant --- core/lib/contract_verifier/src/compilers/solc.rs | 2 +- core/lib/contract_verifier/src/compilers/zksolc.rs | 4 ++-- core/lib/contract_verifier/src/compilers/zkvyper.rs | 2 +- core/lib/contract_verifier/src/error.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/lib/contract_verifier/src/compilers/solc.rs b/core/lib/contract_verifier/src/compilers/solc.rs index 47f777dd564e..bb453cb729c2 100644 --- a/core/lib/contract_verifier/src/compilers/solc.rs +++ b/core/lib/contract_verifier/src/compilers/solc.rs @@ -160,7 +160,7 @@ impl Compiler for Solc { parse_standard_json_output(&output, input.contract_name, input.file_name, true) } else { Err(ContractVerifierError::CompilerError( - "zksolc".to_string(), + "solc", String::from_utf8_lossy(&output.stderr).to_string(), )) } diff --git a/core/lib/contract_verifier/src/compilers/zksolc.rs b/core/lib/contract_verifier/src/compilers/zksolc.rs index 7fa2344ad3c2..0d6b5828e31c 100644 --- a/core/lib/contract_verifier/src/compilers/zksolc.rs +++ b/core/lib/contract_verifier/src/compilers/zksolc.rs @@ -251,7 +251,7 @@ impl Compiler for ZkSolc { parse_standard_json_output(&output, contract_name, file_name, false) } else { Err(ContractVerifierError::CompilerError( - "zksolc".to_string(), + "zksolc", String::from_utf8_lossy(&output.stderr).to_string(), )) } @@ -280,7 +280,7 @@ impl Compiler for ZkSolc { Self::parse_single_file_yul_output(&output) } else { Err(ContractVerifierError::CompilerError( - "zksolc".to_string(), + "zksolc", String::from_utf8_lossy(&output.stderr).to_string(), )) } diff --git a/core/lib/contract_verifier/src/compilers/zkvyper.rs b/core/lib/contract_verifier/src/compilers/zkvyper.rs index 5602584fb14e..b3dacce64e77 100644 --- a/core/lib/contract_verifier/src/compilers/zkvyper.rs +++ b/core/lib/contract_verifier/src/compilers/zkvyper.rs @@ -122,7 +122,7 @@ impl Compiler for ZkVyper { Self::parse_output(&output, input.contract_name) } else { Err(ContractVerifierError::CompilerError( - "zkvyper".to_string(), + "zkvyper", String::from_utf8_lossy(&output.stderr).to_string(), )) } diff --git a/core/lib/contract_verifier/src/error.rs b/core/lib/contract_verifier/src/error.rs index 4e6273765a75..c777df24e226 100644 --- a/core/lib/contract_verifier/src/error.rs +++ b/core/lib/contract_verifier/src/error.rs @@ -13,7 +13,7 @@ pub enum ContractVerifierError { #[error("Compilation takes too much time")] CompilationTimeout, #[error("{0} error: {1}")] - CompilerError(String, String), + CompilerError(&'static str, String), #[error("Compilation error")] CompilationError(serde_json::Value), #[error("Unknown {0} version: {1}")] From 31b8b307286a026194d9ff429d17ae3b70fa4bb8 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 4 Nov 2024 15:15:21 +0200 Subject: [PATCH 19/19] Sanity-check compiler type --- core/lib/contract_verifier/src/lib.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index 6b4a677f7c63..425440fa2eb6 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -266,6 +266,16 @@ impl ContractVerifier { bytecode_marker: BytecodeMarker, ) -> Result { let compiler_type = req.source_code_data.compiler_type(); + let compiler_type_by_versions = req.compiler_versions.compiler_type(); + if compiler_type != compiler_type_by_versions { + // Should be checked when receiving a request, so here it's more of a sanity check + let err = anyhow::anyhow!( + "specified compiler versions {:?} belong to a differing toolchain than source code ({compiler_type:?})", + req.compiler_versions + ); + return Err(err.into()); + } + match (bytecode_marker, compiler_type) { (BytecodeMarker::EraVm, CompilerType::Solc) => self.compile_zksolc(req).await, (BytecodeMarker::EraVm, CompilerType::Vyper) => self.compile_zkvyper(req).await,