diff --git a/examples/erc20-counter/methods/guest/src/bin/balance_of.rs b/examples/erc20-counter/methods/guest/src/bin/balance_of.rs index 6f1ce2c7..5bde0da1 100644 --- a/examples/erc20-counter/methods/guest/src/bin/balance_of.rs +++ b/examples/erc20-counter/methods/guest/src/bin/balance_of.rs @@ -55,7 +55,10 @@ fn main() { // Execute the view call; it returns the result in the type generated by the `sol!` macro. let call = IERC20::balanceOfCall { account }; - let returns = Contract::new(contract, &env).call_builder(&call).call(); + let returns = Contract::new(contract, &env) + .call_builder(&call) + .call() + .unwrap(); // Check that the given account holds at least 1 token. assert!(returns._0 >= U256::from(1)); diff --git a/examples/erc20/README.md b/examples/erc20/README.md index 8b638b24..f8c485a2 100644 --- a/examples/erc20/README.md +++ b/examples/erc20/README.md @@ -70,7 +70,7 @@ fn main() { // Execute the view call; it returns the result in the type generated by the `sol!` macro. let contract = Contract::new(CONTRACT, &env); - let returns = contract.call_builder(&CALL).from(CALLER).call(); + let returns = contract.call_builder(&CALL).from(CALLER).call().unwrap(); println!("View call result: {}", returns._0); } ``` diff --git a/examples/erc20/methods/guest/src/main.rs b/examples/erc20/methods/guest/src/main.rs index 22c060f6..bfb30a73 100644 --- a/examples/erc20/methods/guest/src/main.rs +++ b/examples/erc20/methods/guest/src/main.rs @@ -57,6 +57,6 @@ fn main() { // Execute the view call; it returns the result in the type generated by the `sol!` macro. let contract = Contract::new(CONTRACT, &env); - let returns = contract.call_builder(&CALL).from(CALLER).call(); + let returns = contract.call_builder(&CALL).from(CALLER).call().unwrap(); println!("View call result: {}", returns._0); } diff --git a/examples/token-stats/methods/guest/src/main.rs b/examples/token-stats/methods/guest/src/main.rs index 86403326..2c64a449 100644 --- a/examples/token-stats/methods/guest/src/main.rs +++ b/examples/token-stats/methods/guest/src/main.rs @@ -36,10 +36,12 @@ fn main() { let utilization = contract .call_builder(&CometMainInterface::getUtilizationCall {}) .call() + .unwrap() ._0; let supply_rate = contract .call_builder(&CometMainInterface::getSupplyRateCall { utilization }) .call() + .unwrap() ._0; // The formula for APR in percentage is the following: diff --git a/steel/CHANGELOG.md b/steel/CHANGELOG.md index 828c567c..1f112665 100644 --- a/steel/CHANGELOG.md +++ b/steel/CHANGELOG.md @@ -12,6 +12,8 @@ All notable changes to this project will be documented in this file. ### 🚨 Breaking Changes +- `CallBuilder::call` in the guest now returns an error that needs to be handled, before it just panicked. + ## [0.13.0](https://github.com/risc0/risc0-ethereum/releases/tag/steel-v0.13.0) - 2024-09-10 ### ⚡️ Features diff --git a/steel/src/beacon.rs b/steel/src/beacon.rs index ca7f0d6e..bc336a0a 100644 --- a/steel/src/beacon.rs +++ b/steel/src/beacon.rs @@ -273,7 +273,7 @@ mod host { /// Returns the header, with `parent_root` equal to `parent.root`. /// /// It iteratively tries to fetch headers of successive slots until success. - /// TODO: use `eth/v1/beacon/headers?parent_root`, once all the nodes support it. + // TODO(#242): use `eth/v1/beacon/headers?parent_root` when more clients support it. async fn get_child_beacon_header( client: &BeaconClient, parent: GetBlockHeaderResponse, @@ -297,6 +297,7 @@ mod host { } } // return the last error, if all calls failed + // safe unwrap: there must have been at least one error when we reach this line let err = anyhow::Error::from(request_error.unwrap()); Err(err.context("no valid response received for the 32 consecutive slots")) } diff --git a/steel/src/block.rs b/steel/src/block.rs index 819ea61d..78f95d5f 100644 --- a/steel/src/block.rs +++ b/steel/src/block.rs @@ -28,7 +28,7 @@ pub struct BlockInput { } impl BlockInput { - /// Converts the input into a [EvmEnv] for a verifiable state access in the guest. + /// Converts the input into a [EvmEnv] for verifiable state access in the guest. pub fn into_env(self) -> GuestEvmEnv { // verify that the state root matches the state trie let state_root = self.state_trie.hash_slow(); @@ -55,7 +55,6 @@ impl BlockInput { previous_header = ancestor; } - // TODO(victor): When do we check that the storage tries are ok? let db = StateDb::new( self.state_trie, self.storage_tries, @@ -95,6 +94,7 @@ pub mod host { H: EvmBlockHeader + TryFrom<::HeaderResponse>, ::HeaderResponse>>::Error: Display, { + // safe unwrap: env is never returned without a DB let mut db = env.db.unwrap(); assert_eq!( db.inner().block_hash(), diff --git a/steel/src/contract.rs b/steel/src/contract.rs index d366d0d7..e27d5752 100644 --- a/steel/src/contract.rs +++ b/steel/src/contract.rs @@ -67,7 +67,7 @@ use revm::{ /// // Guest: /// let evm_env = evm_input.into_env(); /// let contract = Contract::new(contract_address, &evm_env); -/// contract.call_builder(&get_balance).call(); +/// contract.call_builder(&get_balance).call().unwrap(); /// /// # Ok(()) /// # } @@ -201,6 +201,7 @@ mod host { /// list. This does *not* set the access list as part of the transaction (as specified in /// EIP-2930), and thus can only be specified during preflight on the host. pub async fn prefetch_access_list(self, access_list: AccessList) -> Result { + // safe unwrap: env is never returned without a DB let db = self.env.db.as_mut().unwrap(); db.add_access_list(access_list).await?; @@ -219,11 +220,11 @@ mod host { self.tx.to ); - let cfg = self.env.cfg_env.clone(); - let header = self.env.header.inner().clone(); - // we cannot clone the database, so it gets moved in and out of the task + // as mutable references are not possible, the DB must be moved in and out of the task let db = self.env.db.take().unwrap(); + let cfg = self.env.cfg_env.clone(); + let header = self.env.header.inner().clone(); let (result, db) = tokio::task::spawn_blocking(move || { let mut evm = new_evm(db, cfg, header); let result = self.tx.transact(&mut evm); @@ -232,8 +233,9 @@ mod host { (result, db) }) .await - .context("EVM execution panicked")?; + .expect("EVM execution panicked"); + // restore the DB before handling errors, so that we never return an env without a DB self.env.db = Some(db); result.map_err(|err| anyhow!("call '{}' failed: {}", C::SIGNATURE, err)) @@ -249,6 +251,7 @@ mod host { /// [EvmEnv]: crate::EvmEnv pub async fn call_with_prefetch(self) -> Result { let access_list = { + // safe unwrap: env is never returned without a DB let db = self.env.db.as_mut().unwrap(); let tx = ::TransactionRequest::default() @@ -285,14 +288,15 @@ where /// Executes the call with a [EvmEnv] constructed with [Contract::new]. /// /// [EvmEnv]: crate::EvmEnv - pub fn call(self) -> C::Return { + pub fn call(self) -> Result { + // safe unwrap: env is never returned without a DB let state_db = self.env.db.as_ref().unwrap(); let mut evm = new_evm::<_, H>( WrapStateDb::new(state_db), self.env.cfg_env.clone(), self.env.header.inner(), ); - self.tx.transact(&mut evm).unwrap() + self.tx.transact(&mut evm) } } diff --git a/steel/src/ethereum.rs b/steel/src/ethereum.rs index 0dd9156d..6b74613e 100644 --- a/steel/src/ethereum.rs +++ b/steel/src/ethereum.rs @@ -94,6 +94,7 @@ impl EvmBlockHeader for EthBlockHeader { // technically, this is only valid after EIP-4399 but revm makes sure it is not used before blk_env.prevrandao = Some(header.mix_hash); if let Some(excess_blob_gas) = header.excess_blob_gas { + // safe conversion: according to the consensus spec, excess_blob_gas is always an uint64 blk_env.set_blob_excess_gas_and_price(excess_blob_gas.try_into().unwrap()) }; } diff --git a/steel/src/host/db/alloy.rs b/steel/src/host/db/alloy.rs index 124d068e..c6002eac 100644 --- a/steel/src/host/db/alloy.rs +++ b/steel/src/host/db/alloy.rs @@ -79,8 +79,17 @@ impl> ProviderDb fo } } +/// Errors returned by the [AlloyDb]. +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("RPC error")] + RPC(#[from] TransportError), + #[error("block not found")] + BlockNotFound, +} + impl> Database for AlloyDb { - type Error = TransportError; + type Error = Error; fn basic(&mut self, address: Address) -> Result, Self::Error> { let f = async { @@ -143,11 +152,11 @@ impl> Database for AlloyDb Result { - let block = self + let block_response = self .handle .block_on(self.provider.get_block_by_number(number.into(), false))?; - // TODO: return proper error - let block = block.unwrap(); + let block = block_response.ok_or(Error::BlockNotFound)?; + Ok(block.header().hash()) } } diff --git a/steel/src/host/db/proof.rs b/steel/src/host/db/proof.rs index 4a343a51..5a6b9111 100644 --- a/steel/src/host/db/proof.rs +++ b/steel/src/host/db/proof.rs @@ -161,14 +161,19 @@ impl> ProofDb> ProofDb, { - /// Converts the environment into a [EvmInput] committing to a Ethereum Beacon block root. + /// Converts the environment into a [EvmInput] committing to an Ethereum Beacon block root. /// /// This function assumes that the /// [mainnet](https://github.com/ethereum/consensus-specs/blob/v1.4.0/configs/mainnet.yaml) diff --git a/steel/src/lib.rs b/steel/src/lib.rs index 2e8a25a2..cc452af1 100644 --- a/steel/src/lib.rs +++ b/steel/src/lib.rs @@ -73,6 +73,7 @@ pub struct EvmEnv { impl EvmEnv { /// Creates a new environment. + /// /// It uses the default configuration for the latest specification. pub(crate) fn new(db: D, header: Sealed) -> Self { let cfg_env = CfgEnvWithHandlerCfg::new_with_spec_id(Default::default(), SpecId::LATEST); @@ -87,6 +88,8 @@ impl EvmEnv { } /// Sets the chain ID and specification ID from the given chain spec. + /// + /// This will panic when there is no valid specification ID for the current block. pub fn with_chain_spec(mut self, chain_spec: &config::ChainSpec) -> Self { self.cfg_env.chain_id = chain_spec.chain_id(); self.cfg_env.handler_cfg.spec_id = chain_spec diff --git a/steel/tests/common/mod.rs b/steel/tests/common/mod.rs index 9c340b69..cc87650d 100644 --- a/steel/tests/common/mod.rs +++ b/steel/tests/common/mod.rs @@ -70,7 +70,7 @@ where let result = { let contract = Contract::new(address, &env); - options.apply(contract.call_builder(&call)).call() + options.apply(contract.call_builder(&call)).call().unwrap() }; assert_eq!( result, preflight_result, diff --git a/steel/tests/corruption.rs b/steel/tests/corruption.rs index f4decb86..f3b31e01 100644 --- a/steel/tests/corruption.rs +++ b/steel/tests/corruption.rs @@ -114,10 +114,12 @@ fn mock_anvil_guest(input: EthEvmInput) -> Commitment { let env = input.into_env().with_chain_spec(&ANVIL_CHAIN_SPEC); Contract::new(ANVIL_CONTRACT_ADDRESS, &env) .call_builder(&sol::Pair::aCall {}) - .call(); + .call() + .unwrap(); Contract::new(ANVIL_CONTRACT_ADDRESS, &env) .call_builder(&sol::Pair::bCall {}) - .call(); + .call() + .unwrap(); env.into_commitment() } @@ -319,7 +321,7 @@ async fn corrupt_ancestor() { #[should_panic(expected = "Invalid commitment")] async fn corrupt_header_block_commitment() { let input = anvil_input().await.unwrap(); - let exp_commitment = input.clone().into_env().into_commitment(); + let exp_commit = input.clone().into_env().into_commitment(); // get the JSON representation of the block header for the state let mut input_value = to_value(&input).unwrap(); @@ -331,8 +333,12 @@ async fn corrupt_header_block_commitment() { *header_value = to_value(header).unwrap(); // executing this should lead to an Invalid commitment - let commitment = mock_anvil_guest(from_value(input_value).unwrap()); - assert_eq!(commitment, exp_commitment, "Invalid commitment"); + let commit = mock_anvil_guest(from_value(input_value).unwrap()); + assert_eq!(commit.blockID, exp_commit.blockID, "Commitment changed"); + assert_eq!( + commit.blockDigest, exp_commit.blockDigest, + "Invalid commitment" + ); } #[test(tokio::test)] @@ -343,15 +349,15 @@ async fn corrupt_header_beacon_commitment() { }) .await .unwrap(); - let exp_commitment = input.clone().into_env().into_commitment(); + let exp_commit = input.clone().into_env().into_commitment(); // get the JSON representation of the block header for the state let mut input_value = to_value(&input).unwrap(); let header_value = &mut get_block_input_mut(&mut input_value)["header"]; - // corrupt the header by modifying its timestamp + // corrupt the header by modifying its number let mut header: EthBlockHeader = from_value(header_value.clone()).unwrap(); - header.inner_mut().timestamp = 0xdeadbeaf; + header.inner_mut().number = 0xdeadbeaf; *header_value = to_value(header).unwrap(); // executing this should lead to an Invalid commitment @@ -359,8 +365,14 @@ async fn corrupt_header_beacon_commitment() { let env = input.into_env().with_chain_spec(&ANVIL_CHAIN_SPEC); Contract::new(USDT_ADDRESS, &env) .call_builder(&USDT_CALL) - .call(); - assert_eq!(env.into_commitment(), exp_commitment, "Invalid commitment"); + .call() + .unwrap(); + let commit = env.into_commitment(); + assert_eq!(commit.blockID, exp_commit.blockID, "Changed commitment"); + assert_eq!( + commit.blockDigest, exp_commit.blockDigest, + "Invalid commitment" + ); } #[test(tokio::test)] @@ -371,7 +383,7 @@ async fn corrupt_beacon_proof() { }) .await .unwrap(); - let exp_commitment = input.clone().into_env().into_commitment(); + let exp_commit = input.clone().into_env().into_commitment(); // get the JSON representation of the block header for the state let mut input_value = to_value(&input).unwrap(); @@ -385,6 +397,12 @@ async fn corrupt_beacon_proof() { let env = input.into_env().with_chain_spec(&ANVIL_CHAIN_SPEC); Contract::new(USDT_ADDRESS, &env) .call_builder(&USDT_CALL) - .call(); - assert_eq!(env.into_commitment(), exp_commitment, "Invalid commitment"); + .call() + .unwrap(); + let commit = env.into_commitment(); + assert_eq!(commit.blockID, exp_commit.blockID, "Commitment changed"); + assert_eq!( + commit.blockDigest, exp_commit.blockDigest, + "Invalid commitment" + ); }