Implement ABI fix in the compiler trait impl

crates/compiler/src/lib.rs

@@ -47,6 +47,7 @@ pub struct CompilerInput<T: PartialEq + Eq + Hash> {
 }
 
 /// The generic compilation output configuration.
+#[derive(Debug)]
 pub struct CompilerOutput<T: PartialEq + Eq + Hash> {
     /// The solc standard JSON input.
     pub input: CompilerInput<T>,

crates/compiler/src/revive_resolc.rs

@@ -11,6 +11,7 @@ use revive_dt_config::Arguments;
 use revive_solc_json_interface::SolcStandardJsonOutput;
 
 /// A wrapper around the `resolc` binary, emitting PVM-compatible bytecode.
+#[derive(Debug)]
 pub struct Resolc {
     /// Path to the `resolc` executable
     resolc_path: PathBuf,
@@ -19,6 +20,7 @@ pub struct Resolc {
 impl SolidityCompiler for Resolc {
     type Options = Vec<String>;
 
+    #[tracing::instrument(level = "debug", ret)]
     fn build(
         &self,
         input: CompilerInput<Self::Options>,
@@ -55,12 +57,55 @@ impl SolidityCompiler for Resolc {
             });
         }
 
-        let parsed: SolcStandardJsonOutput = serde_json::from_slice(&stdout).map_err(|e| {
+        let mut parsed =
-            anyhow::anyhow!(
+            serde_json::from_slice::<SolcStandardJsonOutput>(&stdout).map_err(|e| {
-                "failed to parse resolc JSON output: {e}\nstderr: {}",
+                anyhow::anyhow!(
-                String::from_utf8_lossy(&stderr)
+                    "failed to parse resolc JSON output: {e}\nstderr: {}",
-            )
+                    String::from_utf8_lossy(&stderr)
-        })?;
+                )
+            })?;
+
+        // We need to do some post processing on the output to make it in the same format that solc
+        // outputs. More specifically, for each contract, the `.metadata` field should be replaced
+        // with the `.metadata.solc_metadata` field which contains the ABI and other information
+        // about the compiled contracts. We do this because we do not want any downstream logic to
+        // need to differentiate between which compiler is being used when extracting the ABI of the
+        // contracts.
+        if let Some(ref mut contracts) = parsed.contracts {
+            for (contract_path, contracts_map) in contracts.iter_mut() {
+                for (contract_name, contract_info) in contracts_map.iter_mut() {
+                    let Some(metadata) = contract_info.metadata.take() else {
+                        continue;
+                    };
+
+                    // Get the `solc_metadata` in the metadata of the contract.
+                    let Some(solc_metadata) = metadata
+                        .get("solc_metadata")
+                        .and_then(|metadata| metadata.as_str())
+                    else {
+                        tracing::error!(
+                            contract_path,
+                            contract_name,
+                            metadata = serde_json::to_string(&metadata).unwrap(),
+                            "Encountered a contract compiled with resolc that has no solc_metadata"
+                        );
+                        anyhow::bail!(
+                            "Contract {} compiled with resolc that has no solc_metadata",
+                            contract_name
+                        );
+                    };
+
+                    // Replace the original metadata with the new solc_metadata.
+                    contract_info.metadata =
+                        Some(serde_json::Value::String(solc_metadata.to_string()));
+                }
+            }
+        }
+
+        tracing::debug!(
+            output = %serde_json::to_string(&parsed).unwrap(),
+            "Compiled successfully"
+        );
 
         Ok(CompilerOutput {
             input,

crates/compiler/src/solc.rs

@@ -10,6 +10,7 @@ use crate::{CompilerInput, CompilerOutput, SolidityCompiler};
 use revive_dt_config::Arguments;
 use revive_dt_solc_binaries::download_solc;
 
+#[derive(Debug)]
 pub struct Solc {
     solc_path: PathBuf,
 }
@@ -17,6 +18,7 @@ pub struct Solc {
 impl SolidityCompiler for Solc {
     type Options = ();
 
+    #[tracing::instrument(level = "debug", ret)]
     fn build(
         &self,
         input: CompilerInput<Self::Options>,
@@ -42,6 +44,11 @@ impl SolidityCompiler for Solc {
             });
         }
 
+        tracing::debug!(
+            output = %String::from_utf8_lossy(&output.stdout).to_string(),
+            "Compiled successfully"
+        );
+
         Ok(CompilerOutput {
             input,
             output: serde_json::from_slice(&output.stdout)?,

crates/core/src/driver/mod.rs

@@ -283,90 +283,36 @@ where
                         std::any::type_name::<T>()
                     );
 
-                    // We need to find the ABI of the contract that we published. Depending on the
+                    let Some(Value::String(metadata)) = &contract.metadata else {
-                    // compiler that we used, this can be in one of two possible paths:
+                        tracing::error!(?contract, "Contract does not have a metadata field");
-                    // - In the case of `solc` it's at `.metadata.output.abi` where `.metadata` is a
+                        anyhow::bail!("Contract does not have a metadata field: {contract:?}");
-                    //   JSON string that needs to be deserialized.
-                    // - In the case of `resolc` it's at `.metadata.solc_metadata.output.abi` where
-                    //   the `.metadata.solc_metadata` is a JSON string that needs to be
-                    //   deserialized.
-                    //
-                    // 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
-                    // term fix for the problem.
-                    //
-                    // Short term fix: make this piece of code aware that the metadata could be at
-                    // these two different JSON paths and that it should look for both of them in
-                    // 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
-                    // more difficult and is not the best solution from a maintenance point of view.
-                    //
-                    // Long term fix: Make the `Compiler` implementation output something that is
-                    // more structured and easier to consume by the program. For example, it could
-                    // produce an `Artifacts` object that stores the byte code and the ABI and also
-                    // contains the actual compiler output as well. This is better as it doesn't
-                    // break the abstractions that we have around the compilers and doesn't require
-                    // 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
-                    // metadata exists at and should be the one handling it.
-                    let solc_metadata = match &contract.metadata {
-                        // If the metadata field is defined and it's a string value then we know
-                        // 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
-                        // for the ABIs.
-                        Some(Value::Object(resolc_metadata)) => {
-                            let Some(Value::String(solc_metadata)) =
-                                resolc_metadata.get("solc_metadata")
-                            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
-                        }
-                        // The metadata field is either undefined or it's of a type that we don't
-                        // 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 {
+                    let Ok(metadata) = serde_json::from_str::<Value>(metadata) else {
-                        tracing::error!(%solc_metadata, "Failed to parse solc metadata into a structured value");
+                        tracing::error!(%metadata, "Failed to parse solc metadata into a structured value");
                         anyhow::bail!(
-                            "Failed to parse solc metadata into a structured value {solc_metadata}"
+                            "Failed to parse solc metadata into a structured value {metadata}"
                         );
                     };
 
                     // Accessing the ABI on the solc metadata and erroring if the accessing failed
-                    let Some(abi) = solc_metadata
+                    let Some(abi) = metadata.get("output").and_then(|value| value.get("abi"))
-                        .get("output")
-                        .and_then(|value| value.get("abi"))
                     else {
-                        tracing::error!(%solc_metadata, "Failed to access the .output.abi field of the solc metadata");
+                        tracing::error!(%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}"
+                            "Failed to access the .output.abi field of the solc metadata {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");
+                        tracing::error!(%metadata, "Failed to deserialize ABI into a structured format");
                         anyhow::bail!(
-                            "Failed to deserialize ABI into a structured format {solc_metadata}"
+                            "Failed to deserialize ABI into a structured format {metadata}"
                         );
                     };
 

crates/format/src/input.rs

@@ -340,7 +340,7 @@ mod tests {
 
         type T = (bool,);
         let decoded: T = T::abi_decode(&encoded.0[4..]).unwrap();
-        assert_eq!(decoded.0, true);
+        assert!(decoded.0);
     }
 
     #[test]