Fix the ABI finding logic

This commit is contained in:
Omar Abdulla
2025-07-13 15:59:23 +03:00
parent 0513a4befb
commit 4d4398f83e
+86 -43
View File
@@ -283,51 +283,94 @@ where
std::any::type_name::<T>() std::any::type_name::<T>()
); );
if let Some(Value::String(metadata_json_str)) = &contract.metadata { // We need to find the ABI of the contract that we published. Depending on the
tracing::trace!( // compiler that we used, this can be in one of two possible paths:
"metadata found for contract {contract_name}, {metadata_json_str}" // - In the case of `solc` it's at `.metadata.output.abi` where `.metadata` is a
); // JSON string that needs to be deserialized.
// - In the case of `resolc` it's at `.metadata.solc_metadata.output.abi` where
match serde_json::from_str::<serde_json::Value>(metadata_json_str) { // the `.metadata.solc_metadata` is a JSON string that needs to be
Ok(metadata_json) => { // deserialized.
if let Some(abi_value) = //
metadata_json.get("output").and_then(|o| o.get("abi")) // Therefore, in here we implement a solution to handle both cases. But, this
{ // isn't really the most ideal solution to this problem, but more of a short
match serde_json::from_value::<JsonAbi>(abi_value.clone()) { // term fix for the problem.
Ok(parsed_abi) => { //
tracing::trace!( // Short term fix: make this piece of code aware that the metadata could be at
"ABI found in metadata for contract {}", // these two different JSON paths and that it should look for both of them in
&contract_name // order to find the ABI. The biggest downside of this short-term fix is that it
); // breaks the abstractions that we have and therefore it makes extensibility
self.deployed_abis // more difficult and is not the best solution from a maintenance point of view.
.insert(contract_name.clone(), parsed_abi); //
} // Long term fix: Make the `Compiler` implementation output something that is
Err(err) => { // more structured and easier to consume by the program. For example, it could
anyhow::bail!( // produce an `Artifacts` object that stores the byte code and the ABI and also
"Failed to parse ABI from metadata for contract {}: {}", // contains the actual compiler output as well. This is better as it doesn't
contract_name, // break the abstractions that we have around the compilers and doesn't require
err // other parts of the code to know about the differences that exist between them
); // and therefore it keeps the compiler a black box from the point of view of the
} // rest of the codebase. Essentially, the compiler implementation itself should
} // be the piece of code aware of the differences around the paths that the
} else { // metadata exists at and should be the one handling it.
anyhow::bail!( let solc_metadata = match &contract.metadata {
"No ABI found in metadata for contract {}", // If the metadata field is defined and it's a string value then we know
contract_name // that this came from `solc`.
); Some(Value::String(solc_metadata)) => solc_metadata,
} // If the metadata field is defined and it's an object then we know that it
} // came from resolc and we know where to find the solc metadata that we need
Err(err) => { // for the ABIs.
anyhow::bail!( Some(Value::Object(resolc_metadata)) => {
"Failed to parse metadata JSON string for contract {}: {}", let Some(Value::String(solc_metadata)) =
contract_name, resolc_metadata.get("solc_metadata")
err else {
tracing::error!(
?contract,
"Contract does not have a solc_metadata field"
); );
} anyhow::bail!(
"Contract does not have a solc_metadata field: {contract:?}"
);
};
solc_metadata
} }
} else { // The metadata field is either undefined or it's of a type that we don't
anyhow::bail!("No metadata found for contract {}", contract_name); // know how to handle and therefore we return an error here.
} Some(_) | None => {
tracing::error!(?contract, "Contract does not have a metadata field");
anyhow::bail!("Contract does not have a metadata field: {contract:?}");
}
};
// Deserialize the solc metadata into a JSON object so we can get the ABI of the
// contracts. If we fail to perform the deserialization then we return an error
// as there's no other way to handle this.
let Ok(solc_metadata) = serde_json::from_str::<Value>(solc_metadata) else {
tracing::error!(%solc_metadata, "Failed to parse solc metadata into a structured value");
anyhow::bail!(
"Failed to parse solc metadata into a structured value {solc_metadata}"
);
};
// Accessing the ABI on the solc metadata and erroring if the accessing failed
let Some(abi) = solc_metadata
.get("output")
.and_then(|value| value.get("abi"))
else {
tracing::error!(%solc_metadata, "Failed to access the .output.abi field of the solc metadata");
anyhow::bail!(
"Failed to access the .output.abi field of the solc metadata {solc_metadata}"
);
};
// Deserialize the ABI object that we got from the unstructured JSON into a
// structured ABI object and error out if we fail.
let Ok(abi) = serde_json::from_value::<JsonAbi>(abi.clone()) else {
tracing::error!(%solc_metadata, "Failed to deserialize ABI into a structured format");
anyhow::bail!(
"Failed to deserialize ABI into a structured format {solc_metadata}"
);
};
self.deployed_abis.insert(contract_name.clone(), abi);
} }
} }
} }