PR reviews

substrate/frame/revive/src/exec.rs

@@ -20,6 +20,7 @@ use crate::{
 	gas::GasMeter,
 	limits,
 	primitives::{ExecReturnValue, StorageDeposit},
+	pure_precompiles::{self, is_precompile},
 	runtime_decl_for_revive_api::{Decode, Encode, RuntimeDebugNoBound, TypeInfo},
 	storage::{self, meter::Diff, WriteOutcome},
 	tracing::if_tracing,
@@ -1372,13 +1373,88 @@ where
 		}
 		Some(System::<T>::block_hash(&block_number).into())
 	}
-}
 
-/// Determine if the given address is a precompile.
-/// For now, we consider that all addresses between 0x1 and 0xff are reserved for precompiles.
-fn is_precompile(address: &H160) -> bool {
-	let bytes = address.as_bytes();
-	bytes.starts_with(&[0u8; 19]) && bytes[19] != 0
+	fn run_precompile(
+		&mut self,
+		precompile_address: H160,
+		is_delegate: bool,
+		is_read_only: bool,
+		value_transferred: U256,
+		input_data: &[u8],
+	) -> Result<(), ExecError> {
+		if_tracing(|tracer| {
+			tracer.enter_child_span(
+				self.caller().account_id().map(T::AddressMapper::to_address).unwrap_or_default(),
+				precompile_address,
+				is_delegate,
+				is_read_only,
+				value_transferred,
+				&input_data,
+				self.gas_meter().gas_left(),
+			);
+		});
+
+		self.transient_storage.start_transaction();
+		let mut do_transaction = || -> ExecResult {
+			if !is_delegate {
+				Self::transfer_from_origin(
+					&self.origin,
+					&self.caller(),
+					&T::AddressMapper::to_fallback_account_id(&precompile_address),
+					value_transferred,
+				)?;
+			}
+
+			pure_precompiles::Precompiles::<T>::execute(
+				precompile_address,
+				self.gas_meter_mut(),
+				input_data,
+			)
+			.map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })
+		};
+
+		let transaction_outcome =
+			with_transaction(|| -> TransactionOutcome<Result<_, DispatchError>> {
+				let output = do_transaction();
+				match &output {
+					Ok(result) if !result.did_revert() =>
+						TransactionOutcome::Commit(Ok((true, output))),
+					_ => TransactionOutcome::Rollback(Ok((false, output))),
+				}
+			});
+
+		let (success, output) = match transaction_outcome {
+			Ok((success, output)) => {
+				if_tracing(|tracer| {
+					let gas_consumed = top_frame!(self).nested_gas.gas_consumed();
+					match &output {
+						Ok(output) => tracer.exit_child_span(&output, gas_consumed),
+						Err(e) => tracer.exit_child_span_with_error(e.error.into(), gas_consumed),
+					}
+				});
+
+				(success, output)
+			},
+			Err(error) => {
+				if_tracing(|tracer| {
+					let gas_consumed = top_frame!(self).nested_gas.gas_consumed();
+					tracer.exit_child_span_with_error(error.into(), gas_consumed);
+				});
+
+				(false, Err(error.into()))
+			},
+		};
+
+		if success {
+			self.transient_storage.commit_transaction();
+		} else {
+			self.transient_storage.rollback_transaction();
+		}
+
+		output.map(|output| {
+			self.top_frame_mut().last_frame_output = output;
+		})
+	}
 }
 
 impl<'a, T, E> Ext for Stack<'a, T, E>
@@ -1411,9 +1487,11 @@ where
 		*self.last_frame_output_mut() = Default::default();
 
 		let try_call = || {
+			// Enable read-only access if requested; cannot disable it if already set.
+			let is_read_only = read_only || self.is_read_only();
+
 			if is_precompile(dest_addr) {
-				log::debug!(target: crate::LOG_TARGET, "Unsupported precompile address {dest_addr:?}");
-				return Err(Error::<T>::UnsupportedPrecompileAddress.into());
+				return self.run_precompile(*dest_addr, false, is_read_only, value, &input_data);
 			}
 
 			let dest = T::AddressMapper::to_account_id(dest_addr);
@@ -1434,9 +1512,6 @@ where
 					_ => None,
 				});
 
-			// Enable read-only access if requested; cannot disable it if already set.
-			let is_read_only = read_only || self.is_read_only();
-
 			if let Some(executable) = self.push_frame(
 				FrameArgs::Call { dest: dest.clone(), cached_info, delegated_call: None },
 				value,
@@ -1494,6 +1569,10 @@ where
 		address: H160,
 		input_data: Vec<u8>,
 	) -> Result<(), ExecError> {
+		if is_precompile(&address) {
+			return self.run_precompile(address, true, false, 0u32.into(), &input_data);
+		}
+
 		// We reset the return data now, so it is cleared out even if no new frame was executed.
 		// This is for example the case for unknown code hashes or creating the frame fails.
 		*self.last_frame_output_mut() = Default::default();

substrate/frame/revive/src/exec/tests.rs

@@ -180,8 +180,8 @@ impl Executable<Test> for MockExecutable {
 		&self.code_hash
 	}
 
-	fn code_deposit(&self) -> BalanceOf<Test> {
-		self.code_info.deposit()
+	fn code_info(&self) -> &CodeInfo<Test> {
+		&self.code_info
 	}
 }
 

substrate/frame/revive/src/lib.rs

@@ -26,8 +26,8 @@ mod benchmarking;
 mod exec;
 mod gas;
 mod limits;
-mod precompiles;
 mod primitives;
+mod pure_precompiles;
 mod storage;
 mod transient_storage;
 mod wasm;

substrate/frame/revive/src/precompiles.rs → substrate/frame/revive/src/pure_precompiles.rs

@@ -14,28 +14,33 @@
 // 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.
-mod ecrecover;
 
-use crate::{
-	exec::{ExecResult, Ext},
-	Config, Error, H160,
-};
+use crate::{exec::ExecResult, Config, Error, GasMeter, H160};
+
+mod ecrecover;
 pub use ecrecover::*;
 
+/// Determine if the given address is a precompile.
+/// For now, we consider that all addresses between 0x1 and 0xff are reserved for precompiles.
+pub fn is_precompile(address: &H160) -> bool {
+	let bytes = address.as_bytes();
+	bytes.starts_with(&[0u8; 19]) && bytes[19] != 0
+}
+
 /// The `Precompile` trait defines the functionality for executing a precompiled contract.
 pub trait Precompile<T: Config> {
 	/// Executes the precompile with the provided input data.
-	fn execute<E: Ext<T = T>>(ext: &mut E, input: &[u8]) -> ExecResult;
+	fn execute(gas_meter: &mut GasMeter<T>, input: &[u8]) -> ExecResult;
 }
 
 pub struct Precompiles<T: Config> {
 	_phantom: core::marker::PhantomData<T>,
 }
 
 impl<T: Config> Precompiles<T> {
-	pub fn execute<E: Ext<T = T>>(addr: H160, ext: &mut E, input: &[u8]) -> ExecResult {
+	pub fn execute(addr: H160, gas_meter: &mut GasMeter<T>, input: &[u8]) -> ExecResult {
 		if addr == ECRECOVER {
-			ECRecover::execute(ext, input)
+			ECRecover::execute(gas_meter, input)
 		} else {
 			Err(Error::<T>::UnsupportedPrecompileAddress.into())
 		}

substrate/frame/revive/src/precompiles/ecrecover.rs → substrate/frame/revive/src/pure_precompiles/ecrecover.rs

@@ -15,10 +15,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 use super::Precompile;
-use crate::{
-	exec::{ExecResult, Ext},
-	Config, ExecReturnValue, RuntimeCosts,
-};
+use crate::{exec::ExecResult, Config, ExecReturnValue, GasMeter, RuntimeCosts};
 use hex_literal::hex;
 use pallet_revive_uapi::ReturnFlags;
 use sp_core::H160;
@@ -28,8 +25,8 @@ pub const ECRECOVER: H160 = H160(hex!("0000000000000000000000000000000000000001"
 pub struct ECRecover;
 
 impl<T: Config> Precompile<T> for ECRecover {
-	fn execute<E: Ext<T = T>>(ext: &mut E, i: &[u8]) -> ExecResult {
-		ext.gas_meter_mut().charge(RuntimeCosts::EcdsaRecovery)?;
+	fn execute(gas_meter: &mut GasMeter<T>, i: &[u8]) -> ExecResult {
+		gas_meter.charge(RuntimeCosts::EcdsaRecovery)?;
 
 		let mut input = [0u8; 128];
 		let len = i.len().min(128);

substrate/frame/revive/src/wasm/mod.rs

@@ -387,7 +387,7 @@ where
 		&self.code_hash
 	}
 
-	fn code_deposit(&self) -> BalanceOf<T> {
-		self.code_info.deposit
+	fn code_info(&self) -> &CodeInfo<T> {
+		&self.code_info
 	}
 }