fix: unsafe downcasts and clarification on transceiver message formats

crates/common/src/lib.rs

@@ -12,9 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use alloy_primitives::{Address, B256, Bytes};
+use alloy_primitives::{Address, Bytes, B256};
 use alloy_sol_types::sol;
-use risc0_steel::{Commitment, ethereum::EthEvmInput};
+use risc0_steel::{ethereum::EthEvmInput, Commitment};
 
 #[derive(serde::Serialize, serde::Deserialize)]
 pub struct GuestInput {
@@ -60,12 +60,18 @@ sol! {
 }
 
 /// Converts a Wormhole format B256 address to an Ethereum Address.
-pub fn from_wormhole_address(wormhole_addr: B256) -> Address {
+pub fn from_wormhole_address(wormhole_addr: B256) -> Result<Address, String> {
     // Extract the last 20 bytes from the 32-byte B256
     // This reverses the Solidity conversion: bytes32(uint256(uint160(address)))
     let bytes = wormhole_addr.as_slice();
-    let addr_bytes = &bytes[12..]; // Skip first 12 bytes, take last 20
-    Address::from_slice(addr_bytes)
+    let (garbage_bytes, addr_bytes) = bytes.split_at(12); // Skip first 12 bytes, take last 20
+
+    // Verify that the first 12 bytes are zero
+    if garbage_bytes.iter().any(|&b| b != 0) {
+        return Err(String::from("Malformed wormhole address"));
+    }
+
+    Ok(Address::from_slice(addr_bytes))
 }
 
 /// Converts a Ethereum Address to a Wormhole format address
@@ -74,3 +80,100 @@ pub fn to_wormhole_address(address: Address) -> B256 {
     bytes[12..].copy_from_slice(address.as_slice());
     B256::from(bytes)
 }
+
+/// Converts a Wormhole format B256 address to an Ethereum Address.
+fn from_wormhole_address_align(wormhole_addr: B256) -> Result<Address, String> {
+    // Extract the last 20 bytes from the 32-byte B256
+    // This reverses the Solidity conversion: bytes32(uint256(uint160(address)))
+    let bytes = wormhole_addr.as_slice();
+    let (garbage_bytes, addr_bytes) = bytes.split_at(12);
+
+    // Align 12 bytes into 4 byte chunks to create 3 u32 `ints`
+    let (prefix, ints, suffix) = unsafe { garbage_bytes.align_to::<u32>() };
+    if prefix.iter().any(|&b| b != 0)
+        || ints.iter().any(|&i| i != 0)
+        || suffix.iter().any(|&b| b != 0)
+    {
+        return Err(String::from("Malformed wormhole address"));
+    }
+
+    Ok(Address::from_slice(addr_bytes))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use alloy_primitives::{address, b256};
+
+    #[test]
+    fn test_from_wormhole_address_valid() {
+        // Test with a valid wormhole address
+        let wormhole_addr =
+            b256!("000000000000000000000000aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+        let expected_addr = address!("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+
+        let result = from_wormhole_address(wormhole_addr).unwrap();
+        assert_eq!(result, expected_addr);
+    }
+
+    #[test]
+    fn test_from_wormhole_address_align_valid() {
+        // Test with a valid wormhole address
+        let wormhole_addr =
+            b256!("000000000000000000000000aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+        let expected_addr = address!("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+
+        let result = from_wormhole_address_align(wormhole_addr).unwrap();
+        assert_eq!(result, expected_addr);
+    }
+
+    #[test]
+    fn test_both_functions_identical_result() {
+        // Test that both functions produce identical results for valid inputs
+        let wormhole_addr =
+            b256!("000000000000000000000000bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
+        let expected_addr = address!("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
+
+        let result1 = from_wormhole_address(wormhole_addr).unwrap();
+        let result2 = from_wormhole_address_align(wormhole_addr).unwrap();
+
+        assert_eq!(result1, result2);
+        assert_eq!(result1, expected_addr);
+    }
+
+    #[test]
+    fn test_from_wormhole_address_invalid() {
+        // Test with an invalid wormhole address (non-zero prefix)
+        let wormhole_addr =
+            b256!("000000000000000000000001aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+
+        let result = from_wormhole_address(wormhole_addr);
+        assert!(result.is_err());
+        assert_eq!(result.unwrap_err(), "Malformed wormhole address");
+    }
+
+    #[test]
+    fn test_from_wormhole_address_align_invalid() {
+        // Test with an invalid wormhole address (non-zero prefix)
+        let wormhole_addr =
+            b256!("000000000000000000000001aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+
+        let result = from_wormhole_address_align(wormhole_addr);
+        assert!(result.is_err());
+        assert_eq!(result.unwrap_err(), "Malformed wormhole address");
+    }
+
+    #[test]
+    fn test_zero_address() {
+        // Test with zero address
+        let wormhole_addr =
+            b256!("0000000000000000000000000000000000000000000000000000000000000000");
+        let expected_addr = address!("0000000000000000000000000000000000000000");
+
+        let result1 = from_wormhole_address(wormhole_addr).unwrap();
+        let result2 = from_wormhole_address_align(wormhole_addr).unwrap();
+
+        assert_eq!(result1, result2);
+        assert_eq!(result1, expected_addr);
+    }
+}

crates/zkvm/guest/src/main.rs

@@ -14,8 +14,8 @@
 #![no_main]
 
 use alloy_sol_types::SolValue;
-use common::{from_wormhole_address, GuestInput, IBoundlessTransceiver, Journal};
-use risc0_steel::{ethereum::ETH_MAINNET_CHAIN_SPEC, Event};
+use common::{GuestInput, IBoundlessTransceiver, Journal, from_wormhole_address};
+use risc0_steel::{Event, ethereum::ETH_MAINNET_CHAIN_SPEC};
 use risc0_zkvm::guest::env;
 
 risc0_zkvm::guest::entry!(main);
@@ -30,7 +30,9 @@ fn main() {
     // Query the `SendTransceiverMessage` events of the contract and ensure it contains the expected message digest
     let event = Event::new::<IBoundlessTransceiver::SendTransceiverMessage>(&env);
     let logs = &event
-        .address(from_wormhole_address(input.contract_addr))
+        .address(
+            from_wormhole_address(input.contract_addr).expect("Failed to parse wormhole address"),
+        )
         .query();
     assert!(
         logs.iter()

crates/zkvm/src/lib.rs

@@ -18,29 +18,32 @@ include!(concat!(env!("OUT_DIR"), "/methods.rs"));
 mod tests {
     use super::*;
     use alloy::{
-        dyn_abi::SolType, network::EthereumWallet, node_bindings::Anvil, primitives::Bytes,
-        providers::ProviderBuilder, signers::local::PrivateKeySigner, sol,
+        dyn_abi::SolType, network::EthereumWallet, node_bindings::Anvil, primitives::Address,
+        primitives::Bytes, providers::ProviderBuilder, signers::local::PrivateKeySigner, sol,
     };
-    use common::{GuestInput, Journal, from_wormhole_address, to_wormhole_address};
+    use common::{from_wormhole_address, to_wormhole_address, GuestInput, Journal};
     use risc0_steel::{
+        ethereum::{EthEvmEnv, ETH_MAINNET_CHAIN_SPEC},
         Event,
-        ethereum::{ETH_MAINNET_CHAIN_SPEC, EthEvmEnv},
     };
-    use risc0_zkvm::{ExecutorEnv, default_executor};
+    use risc0_zkvm::{default_executor, ExecutorEnv};
     use std::sync::LazyLock;
 
     // A minimal contract that emits a `SendTransceiverMessage` events when `emitEvent` is called.
+    // Compiler version: 0.8.30
+    // Reproduction can be done with the following command:
+    // `echo 'pragma solidity ^0.8.30; contract SendTransceiverMessageEmitter {event SendTransceiverMessage(uint16 recipientChain, bytes encodedMessage); function emitEvent(uint16 recipientChain, bytes calldata encodedMessage) external {emit SendTransceiverMessage(recipientChain, encodedMessage);}}' | solc --bin -` Reproduction can be done with:
     sol! {
     #[sol(rpc, bytecode="6080604052348015600e575f5ffd5b5061016c8061001c5f395ff3fe608060405234801561000f575f5ffd5b5060043610610029575f3560e01c80631e08b77e1461002d575b5f5ffd5b61004061003b366004610082565b610042565b005b7f0d4a24add37c1972207e3dcfa8359764948caf868db363ee8fa1cb7f55f0a74c83838360405161007593929190610108565b60405180910390a1505050565b5f5f5f60408486031215610094575f5ffd5b833561ffff811681146100a5575f5ffd5b9250602084013567ffffffffffffffff8111156100c0575f5ffd5b8401601f810186136100d0575f5ffd5b803567ffffffffffffffff8111156100e6575f5ffd5b8660208284010111156100f7575f5ffd5b939660209190910195509293505050565b61ffff8416815260406020820152816040820152818360608301375f818301606090810191909152601f9092017fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe01601019291505056fea164736f6c634300081e000a")]
-    contract SendTransceiverMessageEmitter {
-          event SendTransceiverMessage(
-              uint16 recipientChain, bytes encodedMessage
-          );
-
-          function emitEvent(uint16 recipientChain, bytes calldata encodedMessage) external {
-              emit SendTransceiverMessage(recipientChain, encodedMessage);
-          }
+      contract SendTransceiverMessageEmitter {
+        event SendTransceiverMessage(
+            uint16 recipientChain, bytes encodedMessage
+        );
+
+        function emitEvent(uint16 recipientChain, bytes calldata encodedMessage) external {
+            emit SendTransceiverMessage(recipientChain, encodedMessage);
         }
+      }
     }
 
     fn expected_message() -> Bytes {
@@ -127,7 +130,8 @@ mod tests {
                     }
                     let journal = Journal::abi_decode(&info.journal.bytes)?;
                     assert_eq!(
-                        from_wormhole_address(journal.emitterContract),
+                        from_wormhole_address(journal.emitterContract)
+                            .expect("Could not parse wormhole address"),
                         *contract.address()
                     );
                     assert_eq!(journal.encodedMessage, expected_message());

src/BeaconEmitter.sol

@@ -34,6 +34,7 @@ contract BeaconEmitter {
     }
 
     /// @notice Emits a Wormhole message containing the beacon block root for a specific slot.
+    /// @notice If `msg.value` is not the exact Wormhole fee, the call with revert.
     /// @dev Retrieves the beacon block root for the given slot using EIP-4788 and publishes
     /// it as a Wormhole message for cross-chain consumption.
     /// @param slot The beacon chain slot number to retrieve the block root for.

src/BlockRootOracle.sol

@@ -74,6 +74,7 @@ contract BlockRootOracle is AccessControl, ICommitmentValidator {
 
     error InvalidArgument();
     error InvalidPreState();
+    error InvalidPostState();
     error PermissibleTimespanLapsed();
     error UnauthorizedEmitterChainId();
     error UnauthorizedEmitterAddress();
@@ -122,12 +123,9 @@ contract BlockRootOracle is AccessControl, ICommitmentValidator {
     /// @param seal RISC Zero cryptographic proof validating the state transition
     function transition(bytes calldata journalData, bytes calldata seal) external {
         Journal memory journal = abi.decode(journalData, (Journal));
-        if (!_compareConsensusState(currentState, journal.preState)) {
-            revert InvalidPreState();
-        }
-        if (!_permissibleTransition(journal.preState)) {
-            revert PermissibleTimespanLapsed();
-        }
+        require(_compareConsensusState(currentState, journal.preState), InvalidPreState());
+        require(_validPostState(journal.postState), InvalidPostState());
+        require(_permissibleTransition(journal.preState), PermissibleTimespanLapsed());
 
         bytes32 journalHash = sha256(journalData);
         IRiscZeroVerifier(VERIFIER).verify(seal, imageID, journalHash);
@@ -143,12 +141,8 @@ contract BlockRootOracle is AccessControl, ICommitmentValidator {
         if (!valid) {
             revert(reason);
         }
-        if (vm.emitterChainId != EMITTER_CHAIN_ID) {
-            revert UnauthorizedEmitterChainId();
-        }
-        if (vm.emitterAddress != BEACON_EMITTER) {
-            revert UnauthorizedEmitterAddress();
-        }
+        require(vm.emitterChainId == EMITTER_CHAIN_ID, UnauthorizedEmitterChainId());
+        require(vm.emitterAddress == BEACON_EMITTER, UnauthorizedEmitterAddress());
 
         (uint64 slot, bytes32 root) = abi.decode(vm.payload, (uint64, bytes32));
 
@@ -177,16 +171,15 @@ contract BlockRootOracle is AccessControl, ICommitmentValidator {
     }
 
     function updateImageID(bytes32 newImageID) external onlyRole(ADMIN_ROLE) {
-        if (newImageID == imageID) revert InvalidArgument();
+        require(newImageID != imageID, InvalidArgument());
 
         emit ImageIDUpdated(newImageID, imageID);
         imageID = newImageID;
     }
 
     function updatePermissibleTimespan(uint24 newPermissibleTimespan) external onlyRole(ADMIN_ROLE) {
-        if (newPermissibleTimespan == permissibleTimespan) {
-            revert InvalidArgument();
-        }
+        require(newPermissibleTimespan != permissibleTimespan, InvalidArgument());
+
         permissibleTimespan = newPermissibleTimespan;
         emit PermissibleTimespanUpdated(newPermissibleTimespan);
     }
@@ -236,6 +229,15 @@ contract BlockRootOracle is AccessControl, ICommitmentValidator {
         return transitionTimespan <= uint256(permissibleTimespan);
     }
 
+    /// @notice Check if a post state is valid
+    /// @dev Ensures that the `postState.currentJustifiableCheckpoint` of the journal is set not set in the future
+    /// @param state The consensus state to check
+    /// @return Whether the `postState` is set in the past
+    function _validPostState(ConsensusState memory state) internal view returns (bool) {
+        uint256 epochTimestamp = Beacon.epochTimestamp(state.currentJustifiedCheckpoint.epoch, BEACON_CONFIG);
+        return epochTimestamp <= block.timestamp;
+    }
+
     /// @notice Generates a unique hash for a checkpoint at a given slot
     /// @dev Creates a unique identifier for block that was included in the chain at the given slot
     /// @param slot The slot number
@@ -253,7 +255,6 @@ contract BlockRootOracle is AccessControl, ICommitmentValidator {
     function _confirm(uint64 slot, bytes32 root, uint16 flag) internal {
         CheckpointAttestation storage attestation = attestations[_checkpointHash(slot, root)];
         attestation.confirmations = _confirm(attestation.confirmations, flag);
-        // TODO: Verify if blockroot collision is possible
         if (roots[slot] == UNDEFINED_ROOT) {
             roots[slot] = root;
         }
@@ -289,9 +290,7 @@ contract BlockRootOracle is AccessControl, ICommitmentValidator {
         returns (bool)
     {
         (uint240 blockId, uint16 version) = SteelEncoding.decodeVersionedID(commitment.id);
-        if (version != 2) {
-            revert Steel.InvalidCommitmentVersion(version);
-        }
+        require(version == 2, Steel.InvalidCommitmentVersion(version));
 
         return validateReceiverCommitment(SafeCast.toUint64(blockId), commitment.digest, confirmationLevel);
     }