From d2239149fd6e7f99da187ec3009549aed4368e18 Mon Sep 17 00:00:00 2001 From: Igor Aleksanov Date: Mon, 10 Mar 2025 12:21:55 +0400 Subject: [PATCH] Use separate metadata for compiled/deployed --- core/lib/contract_verifier/src/lib.rs | 58 +++++++++++-------- core/lib/contract_verifier/src/tests/real.rs | 10 +++- .../contract_identifier.rs | 10 +--- 3 files changed, 46 insertions(+), 32 deletions(-) diff --git a/core/lib/contract_verifier/src/lib.rs b/core/lib/contract_verifier/src/lib.rs index 5cab66ec8d7c..63b073af86b8 100644 --- a/core/lib/contract_verifier/src/lib.rs +++ b/core/lib/contract_verifier/src/lib.rs @@ -259,18 +259,8 @@ 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?; - let identifier = + let compiled_identifier = ContractIdentifier::from_bytecode(bytecode_marker, artifacts.deployed_bytecode()); - 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( - request.id, - &deployed_contract, - &artifacts.bytecode, - &identifier, - )?, - }; let deployed_bytecode = match bytecode_marker { BytecodeMarker::EraVm => deployed_contract.bytecode.as_slice(), @@ -281,10 +271,24 @@ impl ContractVerifier { ) .context("invalid stored EVM bytecode")?, }; + let deployed_identifier = + ContractIdentifier::from_bytecode(bytecode_marker, deployed_bytecode); + + 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( + request.id, + &deployed_contract, + &artifacts.bytecode, + &compiled_identifier, + &deployed_identifier, + )?, + }; let mut verification_problems = Vec::new(); - match identifier.matches(deployed_bytecode) { + match compiled_identifier.matches(&deployed_identifier) { Match::Full => {} Match::Partial => { tracing::trace!( @@ -336,7 +340,7 @@ impl ContractVerifier { verified_at, verification_problems, }; - Ok((info, identifier)) + Ok((info, compiled_identifier)) } async fn compile_zksolc( @@ -557,19 +561,21 @@ impl ContractVerifier { request_id: usize, contract: &DeployedContractData, creation_bytecode: &[u8], - contract_identifier: &ContractIdentifier, + compiled_identifier: &ContractIdentifier, + deployed_identifier: &ContractIdentifier, ) -> Result { fn extract_arguments<'a>( calldata: &'a [u8], creation_bytecode: &'a [u8], - metadata_len: usize, + compiled_identifier: &ContractIdentifier, + deployed_identifier: &ContractIdentifier, ) -> Result<&'a [u8], &'static str> { - if creation_bytecode.len() < metadata_len { + if creation_bytecode.len() < compiled_identifier.metadata_length() { // This shouldn't normally happen, since we calculated contract identifier based on this code. return Err("Creation bytecode doesn't fit metadata"); } - let creation_bytecode_without_metadata = - &creation_bytecode[..creation_bytecode.len() - metadata_len]; + let creation_bytecode_without_metadata = &creation_bytecode + [..creation_bytecode.len() - compiled_identifier.metadata_length()]; // Ensure equivalence of the creation bytecode (which can be different from the deployed bytecode). // Note that metadata hash may still be different; this is checked by other part of the code. @@ -578,10 +584,12 @@ impl ContractVerifier { .ok_or("Creation bytecode is different")?; // Skip metadata to get to the constructor arguments. - if constructor_args_with_metadata.len() < metadata_len { - return Err("Provided bytecode has no metadata"); + // Note that deployed contract may have different metadata, so we use another + // identifier here. + if constructor_args_with_metadata.len() < deployed_identifier.metadata_length() { + return Err("Calldata doesn't fit metadata"); } - Ok(&constructor_args_with_metadata[metadata_len..]) + Ok(&constructor_args_with_metadata[deployed_identifier.metadata_length()..]) } let Some(calldata) = &contract.calldata else { @@ -592,8 +600,12 @@ impl ContractVerifier { return Ok(ConstructorArgs::Ignore); } - let metadata_len = contract_identifier.metadata_length(); - match extract_arguments(calldata, creation_bytecode, metadata_len) { + match extract_arguments( + calldata, + creation_bytecode, + compiled_identifier, + deployed_identifier, + ) { Ok(args) => Ok(ConstructorArgs::Check(args.to_vec())), Err(err) => { tracing::info!( diff --git a/core/lib/contract_verifier/src/tests/real.rs b/core/lib/contract_verifier/src/tests/real.rs index e621fcdccf9f..0057962257f5 100644 --- a/core/lib/contract_verifier/src/tests/real.rs +++ b/core/lib/contract_verifier/src/tests/real.rs @@ -748,12 +748,18 @@ async fn using_zksolc_partial_match(use_cbor: bool) { ); assert_eq!( - identifier_for_request.matches(output_for_storage.deployed_bytecode()), + identifier_for_request.matches(&ContractIdentifier::from_bytecode( + BytecodeMarker::EraVm, + output_for_storage.deployed_bytecode() + )), Match::Partial, "must be a partial match (1)" ); assert_eq!( - identifier_for_storage.matches(output_for_request.deployed_bytecode()), + identifier_for_storage.matches(&ContractIdentifier::from_bytecode( + BytecodeMarker::EraVm, + output_for_request.deployed_bytecode() + )), Match::Partial, "must be a partial match (2)" ); diff --git a/core/lib/types/src/contract_verification/contract_identifier.rs b/core/lib/types/src/contract_verification/contract_identifier.rs index 4596645edb67..43d8f4bfbf8f 100644 --- a/core/lib/types/src/contract_verification/contract_identifier.rs +++ b/core/lib/types/src/contract_verification/contract_identifier.rs @@ -193,10 +193,8 @@ impl ContractIdentifier { } /// Checks the kind of match between identifier and other bytecode. - pub fn matches(&self, other: &[u8]) -> Match { - let other_identifier = Self::from_bytecode(self.bytecode_marker, other); - - if self.bytecode_keccak256 == other_identifier.bytecode_keccak256 { + pub fn matches(&self, other: &Self) -> Match { + if self.bytecode_keccak256 == other.bytecode_keccak256 { return Match::Full; } @@ -205,9 +203,7 @@ impl ContractIdentifier { // and presence in another, or different kinds of metadata. This is OK: partial // match is needed mostly when you cannot reproduce the original metadata, but one always // can submit the contract with the same metadata kind. - if self.bytecode_without_metadata_keccak256 - == other_identifier.bytecode_without_metadata_keccak256 - { + if self.bytecode_without_metadata_keccak256 == other.bytecode_without_metadata_keccak256 { return Match::Partial; }