WEB3-144: Fix incorrect encoding used by prove cheatcode (#254)

With the change from `ethers` to `alloy`, we made an error in assuming
they'd handle ABI encoding of `Vec<_>` the same. This broke the `prove`
cheatcode that is used in the Foundry template tests. We did not catch
this error prior to releasing, because we had moved all our tests in
this repo over to using `RiscZeroMockVerifier.mockProve` instead and
forgot to add test coverage specifically for this cheatcode.

This PR fixes the encoding issue and additionally adds a simple test for
using `risc0-forge-ffi` in dev mode.

Additionally, this PR solves a problem where the FFI cheatcode would
fail if `RUST_LOG` is set.

---------

Co-authored-by: Wolfgang Welz <[email protected]>

Cargo.toml

@@ -1,6 +1,6 @@
 [workspace]
 resolver = "2"
-members = ["build", "contracts", "ffi", "steel"]
+members = ["build", "contracts", "ffi", "ffi/guests", "steel"]
 
 [workspace.package]
 version = "1.1.1"

contracts/src/test/RiscZeroCheats.sol

@@ -37,7 +37,7 @@ abstract contract RiscZeroCheats is CommonBase {
         return vm.envOr("RISC0_DEV_MODE", false);
     }
 
-    /// @notice Returns the journal, and Groth16 seal, resulting from running the
+    /// @notice Returns the journal, and seal, resulting from running the
     ///     guest with elf_path using input on the RISC Zero zkVM.
     /// @dev Based on whether `devMode()` is `true`, will take one of two actions:
     ///     * When `devMode()` is `true`
@@ -47,11 +47,15 @@ abstract contract RiscZeroCheats is CommonBase {
     ///       Uses the local prover or the Bonsai proving service to run the guest and produce an on-chain verifiable
     ///       SNARK attesting to the correctness of the journal output. URL and API key for Bonsai
     ///       should be specified using the BONSAI_API_URL and BONSAI_API_KEY environment variables.
-    function prove(string memory elf_path, bytes memory input) internal returns (bytes memory, bytes memory) {
-        string[] memory imageRunnerInput = new string[](10);
+    function prove(string memory elf_path, bytes memory input)
+        internal
+        returns (bytes memory journal, bytes memory seal)
+    {
+        string[] memory imageRunnerInput = new string[](11);
         uint256 i = 0;
         imageRunnerInput[i++] = "cargo";
         imageRunnerInput[i++] = "run";
+        imageRunnerInput[i++] = "--locked";
         imageRunnerInput[i++] = "--manifest-path";
         imageRunnerInput[i++] = "lib/risc0-ethereum/ffi/Cargo.toml";
         imageRunnerInput[i++] = "--bin";

ffi/Cargo.toml

@@ -12,5 +12,9 @@ alloy = { workspace = true }
 anyhow = { workspace = true }
 clap = { version = "4.5", features = ["derive", "env"] }
 hex = { version = "0.4" }
+libc = "0.2.159"
 risc0-ethereum-contracts = { workspace = true }
 risc0-zkvm = { workspace = true, features = ["client"] }
+
+[dev-dependencies]
+ffi-guests = { path = "./guests" }

ffi/guests/Cargo.toml

@@ -0,0 +1,10 @@
+[package]
+name = "ffi-guests"
+version = "0.1.0"
+edition = "2021"
+
+[build-dependencies]
+risc0-build = { workspace = true }
+
+[package.metadata.risc0]
+methods = ["echo"]

ffi/guests/build.rs

@@ -0,0 +1,17 @@
+// Copyright 2024 RISC Zero, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+fn main() {
+    risc0_build::embed_methods();
+}

ffi/guests/echo/Cargo.toml

@@ -0,0 +1,13 @@
+[package]
+name = "echo"
+version = "0.1.0"
+edition = "2021"
+
+[workspace]
+
+[dependencies]
+risc0-zkvm = { git = "https://github.com/risc0/risc0", branch = "main", default-features = false, features = ["std"] }
+
+[profile.release]
+debug = 1
+lto = "thin"

ffi/guests/echo/src/main.rs

@@ -0,0 +1,26 @@
+// Copyright 2024 RISC Zero, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use std::io::Read;
+
+use risc0_zkvm::guest::env;
+
+pub fn main() {
+    // Read the entire input stream as raw bytes.
+    let mut message = Vec::<u8>::new();
+    env::stdin().read_to_end(&mut message).unwrap();
+
+    // Commit exactly what the host provided to the journal.
+    env::commit_slice(message.as_slice());
+}

ffi/guests/src/lib.rs

@@ -0,0 +1,15 @@
+// Copyright 2024 RISC Zero, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+include!(concat!(env!("OUT_DIR"), "/methods.rs"));

ffi/src/main.rs

@@ -12,10 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::io::Write;
+use std::{
+    fs::{File, OpenOptions},
+    io,
+    io::Write,
+    os::unix::io::{AsRawFd, FromRawFd},
+};
 
 use alloy::{primitives::Bytes, sol_types::SolValue};
-use anyhow::{Context, Result};
+use anyhow::{ensure, Context, Result};
 use clap::Parser;
 use risc0_ethereum_contracts::encode_seal;
 use risc0_zkvm::{default_prover, ExecutorEnv, ProverOpts, VerifierContext};
@@ -35,7 +40,10 @@ enum Command {
 
 /// Run the CLI.
 pub fn main() -> Result<()> {
-    match Command::parse() {
+    // Take stdout is ensure no extra data is written to it.
+    let mut stdout = take_stdout()?;
+
+    let output = match Command::parse() {
         Command::Prove {
             guest_binary_path,
             input,
@@ -45,22 +53,19 @@ pub fn main() -> Result<()> {
         )?,
     };
 
+    // Forge test FFI calls expect hex encoded bytes sent to stdout
+    write!(&mut stdout, "{}", hex::encode(output)).context("failed to write to stdout")?;
+    stdout.flush().context("failed to flush stdout")?;
+
     Ok(())
 }
 
 /// Prints on stdio the Ethereum ABI and hex encoded proof.
-fn prove_ffi(elf_path: String, input: Vec<u8>) -> Result<()> {
-    let elf = std::fs::read(elf_path).unwrap();
+fn prove_ffi(elf_path: String, input: Vec<u8>) -> Result<Vec<u8>> {
+    let elf = std::fs::read(elf_path).expect("failed to read guest ELF");
     let (journal, seal) = prove(&elf, &input)?;
-    let calldata = vec![Bytes(journal.into()), Bytes(seal.into())];
-    let output = hex::encode(calldata.abi_encode());
-
-    // Forge test FFI calls expect hex encoded bytes sent to stdout
-    print!("{output}");
-    std::io::stdout()
-        .flush()
-        .context("failed to flush stdout buffer")?;
-    Ok(())
+    let calldata = (Bytes(journal.into()), Bytes(seal.into()));
+    Ok(calldata.abi_encode())
 }
 
 /// Generates journal and snark seal as a pair (`Vec<u8>`, `Vec<u8>)
@@ -78,3 +83,29 @@ fn prove(elf: &[u8], input: &[u8]) -> Result<(Vec<u8>, Vec<u8>)> {
     let seal = encode_seal(&receipt)?;
     Ok((journal, seal))
 }
+
+/// "Takes" stdout, returning a handle and ensuring no other code in this process can write to it.
+/// This is used to ensure that no additional data (e.g. log lines) is written to stdout, as any
+/// extra will cause a decoding failure in the Forge FFI cheatcode.
+fn take_stdout() -> Result<File> {
+    let stdout = io::stdout();
+    let mut handle = stdout.lock();
+    // Ensure all buffered data is written before redirection
+    handle.flush()?;
+
+    let devnull = OpenOptions::new().write(true).open("/dev/null")?;
+
+    unsafe {
+        // Create a copy of stdout to use for our output.
+        let dup_fd = libc::dup(handle.as_raw_fd());
+        ensure!(dup_fd >= 0, "call to libc::dup failed: {}", dup_fd);
+        // Redirect stdout to the fd we opened for /dev/null
+        let dup2_result = libc::dup2(devnull.as_raw_fd(), libc::STDOUT_FILENO);
+        ensure!(
+            dup2_result >= 0,
+            "call to libc::dup2 failed: {}",
+            dup2_result
+        );
+        Ok(File::from_raw_fd(dup_fd))
+    }
+}

ffi/tests/basic.rs

@@ -0,0 +1,85 @@
+// Copyright 2024 RISC Zero, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use std::process::Command;
+
+use alloy::{primitives::Bytes, sol_types::SolValue};
+use ffi_guests::{ECHO_ID, ECHO_PATH};
+use risc0_ethereum_contracts::encode_seal;
+use risc0_zkvm::{FakeReceipt, InnerReceipt, Receipt, ReceiptClaim};
+
+#[test]
+fn basic_usage() {
+    let exe_path = env!("CARGO_BIN_EXE_risc0-forge-ffi");
+    let args = ["prove", ECHO_PATH, "0xdeadbeef"];
+    println!("{} {:?}", exe_path, args);
+    let output = Command::new(exe_path)
+        .env_clear()
+        // PATH is required so r0vm can be found.
+        .env("PATH", std::env::var("PATH").unwrap())
+        .env("RISC0_DEV_MODE", "1")
+        .args(args)
+        .output()
+        .unwrap();
+
+    println!("{:#?}", &output);
+
+    let output_bytes = hex::decode(output.stdout).unwrap();
+    let (journal, seal) = <(Bytes, Bytes)>::abi_decode(&output_bytes, true).unwrap();
+
+    assert_eq!(journal, hex::decode("deadbeef").unwrap());
+    let expected_receipt = Receipt::new(
+        InnerReceipt::Fake(FakeReceipt::new(ReceiptClaim::ok(
+            ECHO_ID,
+            journal.to_vec(),
+        ))),
+        journal.into(),
+    );
+    let expect_seal = encode_seal(&expected_receipt).unwrap();
+    assert_eq!(expect_seal, seal.to_vec());
+}
+
+// It's important that `risc0-forge-ffi` only send to stdout the output to be consumed by forge
+// with the FFI cheatcode. If any extra output is sent, ABI decoding will fail.
+#[test]
+fn basic_usage_with_rust_log() {
+    let exe_path = env!("CARGO_BIN_EXE_risc0-forge-ffi");
+    let args = ["prove", ECHO_PATH, "0xdeadbeef"];
+    println!("{} {:?}", exe_path, args);
+    let output = Command::new(exe_path)
+        .env_clear()
+        // PATH is required so r0vm can be found.
+        .env("PATH", std::env::var("PATH").unwrap())
+        .env("RISC0_DEV_MODE", "1")
+        .env("RUST_LOG", "debug")
+        .args(args)
+        .output()
+        .unwrap();
+
+    println!("{:#?}", &output);
+
+    let output_bytes = hex::decode(output.stdout).unwrap();
+    let (journal, seal) = <(Bytes, Bytes)>::abi_decode(&output_bytes, true).unwrap();
+
+    assert_eq!(journal, hex::decode("deadbeef").unwrap());
+    let expected_receipt = Receipt::new(
+        InnerReceipt::Fake(FakeReceipt::new(ReceiptClaim::ok(
+            ECHO_ID,
+            journal.to_vec(),
+        ))),
+        journal.into(),
+    );
+    let expect_seal = encode_seal(&expected_receipt).unwrap();
+    assert_eq!(expect_seal, seal.to_vec());
+}