Skip to content

Commit

Permalink
fix(contract-verifier): Correctly process partial verification for EV…
Browse files Browse the repository at this point in the history
…M contracts
  • Loading branch information
popzxc committed Mar 7, 2025
1 parent 7a2ccac commit 912bcc3
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 28 deletions.
50 changes: 40 additions & 10 deletions core/lib/contract_verifier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ impl ContractVerifier {
request.id,
&deployed_contract,
&artifacts.bytecode,
&identifier,
)?,
};

Expand Down Expand Up @@ -556,7 +557,33 @@ impl ContractVerifier {
request_id: usize,
contract: &DeployedContractData,
creation_bytecode: &[u8],
contract_identifier: &ContractIdentifier,
) -> Result<ConstructorArgs, ContractVerifierError> {
fn extract_arguments<'a>(
calldata: &'a [u8],
creation_bytecode: &'a [u8],
metadata_len: usize,
) -> Result<&'a [u8], &'static str> {
if creation_bytecode.len() < metadata_len {
// 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];

// 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.
let constructor_args_with_metadata = calldata
.strip_prefix(creation_bytecode_without_metadata)
.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");
}
Ok(&constructor_args_with_metadata[metadata_len..])
}

let Some(calldata) = &contract.calldata else {
return Ok(ConstructorArgs::Ignore);
};
Expand All @@ -565,16 +592,19 @@ impl ContractVerifier {
return Ok(ConstructorArgs::Ignore);
}

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()))
let metadata_len = contract_identifier.metadata_length();
match extract_arguments(calldata, creation_bytecode, metadata_len) {
Ok(args) => Ok(ConstructorArgs::Check(args.to_vec())),
Err(err) => {
tracing::info!(
request_id,
calldata = hex::encode(calldata),
compiled = hex::encode(creation_bytecode),
"Creation bytecode mismatch: {err}"
);
Err(ContractVerifierError::CreationBytecodeMismatch)
}
}
}

#[tracing::instrument(level = "debug", skip_all, err, fields(id = request_id))]
Expand Down
6 changes: 3 additions & 3 deletions core/lib/contract_verifier/src/tests/real.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ async fn using_real_compiler_in_verifier(bytecode_kind: BytecodeMarker, toolchai
(BytecodeMarker::Evm, Toolchain::Solidity) => {
assert_matches!(
identifier.detected_metadata,
Some(DetectedMetadata::Cbor),
Some(DetectedMetadata::Cbor { .. }),
"Cbor metadata for EVM Solidity by default"
);
}
Expand Down Expand Up @@ -760,11 +760,11 @@ async fn using_zksolc_partial_match(use_cbor: bool) {
if use_cbor {
assert_matches!(
identifier_for_request.detected_metadata,
Some(DetectedMetadata::Cbor)
Some(DetectedMetadata::Cbor { .. })
);
assert_matches!(
identifier_for_storage.detected_metadata,
Some(DetectedMetadata::Cbor)
Some(DetectedMetadata::Cbor { .. })
);
} else {
assert_matches!(
Expand Down
57 changes: 42 additions & 15 deletions core/lib/types/src/contract_verification/contract_identifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,20 @@ pub enum DetectedMetadata {
/// keccak256 metadata (only for EraVM)
Keccak256,
/// CBOR metadata
Cbor,
Cbor {
/// Length of metadata in the bytecode, including encoded length of CBOR and padding.
full_length: usize,
},
}

impl DetectedMetadata {
/// Returns full length (in bytes) of metadata in the bytecode.
pub fn length(self) -> usize {
match self {
DetectedMetadata::Keccak256 => 32,
DetectedMetadata::Cbor { full_length } => full_length,
}
}
}

/// Possible values for the metadata hashes structure.
Expand Down Expand Up @@ -76,15 +89,19 @@ impl ContractIdentifier {
// Try to detect metadata.
// CBOR takes precedence (since keccak doesn't have direct markers, so it's partially a
// fallback).
let (detected_metadata, bytecode_without_metadata_keccak256) =
if let Some(hash) = Self::detect_cbor_metadata(bytecode_marker, bytecode) {
(Some(DetectedMetadata::Cbor), hash)
} else if let Some(hash) = Self::detect_keccak_metadata(bytecode_marker, bytecode) {
(Some(DetectedMetadata::Keccak256), hash)
} else {
// Fallback
(None, bytecode_keccak256)
};
let (detected_metadata, bytecode_without_metadata_keccak256) = if let Some((
full_length,
hash,
)) =
Self::detect_cbor_metadata(bytecode_marker, bytecode)
{
(Some(DetectedMetadata::Cbor { full_length }), hash)
} else if let Some(hash) = Self::detect_keccak_metadata(bytecode_marker, bytecode) {
(Some(DetectedMetadata::Keccak256), hash)
} else {
// Fallback
(None, bytecode_keccak256)
};

Self {
bytecode_marker,
Expand All @@ -110,7 +127,10 @@ impl ContractIdentifier {
}

/// Will try to detect CBOR metadata.
fn detect_cbor_metadata(bytecode_marker: BytecodeMarker, bytecode: &[u8]) -> Option<H256> {
fn detect_cbor_metadata(
bytecode_marker: BytecodeMarker,
bytecode: &[u8],
) -> Option<(usize, H256)> {
let length = bytecode.len();

// Last two bytes is the length of the metadata in big endian.
Expand Down Expand Up @@ -148,7 +168,7 @@ impl ContractIdentifier {
}
};
let hash = H256(keccak256(bytecode_without_metadata));
Some(hash)
Some((full_metadata_length, hash))
}

/// Adds one word to the metadata length and check if it's a padding word.
Expand Down Expand Up @@ -193,6 +213,11 @@ impl ContractIdentifier {

Match::None
}

/// Returns the length of the metadata in the bytecode.
pub fn metadata_length(&self) -> usize {
self.detected_metadata.map_or(0, DetectedMetadata::length)
}
}

#[cfg(test)]
Expand All @@ -217,7 +242,7 @@ mod tests {
);
assert_eq!(
identifier.detected_metadata,
Some(DetectedMetadata::Cbor),
Some(DetectedMetadata::Cbor { full_length: 44 }),
"Incorrect detected metadata"
);
assert_eq!(
Expand All @@ -242,7 +267,7 @@ mod tests {
);
assert_eq!(
identifier.detected_metadata,
Some(DetectedMetadata::Cbor),
Some(DetectedMetadata::Cbor { full_length: 44 }),
"Incorrect detected metadata"
);
assert_eq!(
Expand Down Expand Up @@ -384,7 +409,9 @@ mod tests {
);
assert_eq!(
identifier.detected_metadata,
Some(DetectedMetadata::Cbor),
Some(DetectedMetadata::Cbor {
full_length: full_metadata_len
}),
"{label}: Incorrect detected metadata"
);
assert_eq!(
Expand Down

0 comments on commit 912bcc3

Please sign in to comment.