Repository: paritytech/substrate
File: frame/support/src/traits/hooks.rs (Rust)

Code before:
			);

			errors.iter().for_each(|err| {
				log::error!(
					target: "try-runtime",
					"{:?}",
					err
				);
			});

			return Err("Detected multiple errors while executing `try_on_runtime_upgrade`, check the logs!".into())
		}

		Ok(weight)
	}

	/// `Executive` used to directly call AllPalletsWithSystem::post_upgrade() which is now a noop.
	/// If a runtime is running an old version of `Exeuctive`, return an error instead of silently
	/// doing nothing which can lead to confusion. See
	/// <https://github.com/paritytech/substrate/issues/13681>.
	#[cfg(feature = "try-runtime")]
	fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
		Err("If you see this error, the implementation of `pub fn try_runtime_upgrade` in your `Executive` pallet is probably old and `pre_upgrade` checks will *not* be executed. Please update your implementation of `Executive` `pub fn try_runtime_upgrade` to match Substrate master (https://github.com/paritytech/substrate/blob/master/frame/executive/src/lib.rs).".into())
	}

	/// `Executive` used to directly call AllPalletsWithSystem::post_upgrade() which is now a noop.
	/// If a runtime is running an old version of `Exeuctive`, return an error instead of silently
	/// doing nothing which can lead to confusion. See
	/// <https://github.com/paritytech/substrate/issues/13681>.
	#[cfg(feature = "try-runtime")]
	fn post_upgrade(_state: Vec<u8>) -> Result<(), TryRuntimeError> {
		Err("If you see this error, the implementation of `pub fn try_runtime_upgrade` in your `Executive` pallet is probably old and `post_upgrade` checks will *not* be executed. Please update your implementation of `Executive` `pub fn try_runtime_upgrade` to match Substrate master (https://github.com/paritytech/substrate/blob/master/frame/executive/src/lib.rs).".into())
	}
}

/// See [`Hooks::integrity_test`].
#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))]
#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
pub trait IntegrityTest {
	/// See [`Hooks::integrity_test`].
	fn integrity_test() {}
}

#[cfg_attr(doc, aquamarine::aquamarine)]
/// The pallet hooks trait. This is merely an umbrella trait for:
///
/// - [`OnInitialize`]
/// - [`OnFinalize`]
/// - [`OnRuntimeUpgrade`]
/// - [`crate::traits::misc::OffchainWorker`]

Review comment (refactor):
the impl for Tuple should not really depend from the implementation of `Executive`, which is higher level abstraction. 
if we go this direction I would just say that the default impl of `try_on_runtime_upgrade` already calls the `pre/post_upgrade`, which is the real reason here. `Executive` does not do anything specific, it just calls `try_on_runtime_upgrade`.

but I think the problem which is being addressed comes from the misuse of the trait. In my opinion these `pre/post_upgrade` funcs for Tuple should invoke the inner ones, and `try_on_runtime_upgrade` should not be used with `pre/post_upgrade`.

Code after:
		)*);

		if errors.len() == 1 {
			return Err(errors[0])
		} else if !errors.is_empty() {
			log::error!(
				target: "try-runtime",
				"Detected multiple errors while executing `try_on_runtime_upgrade`:",
			);

			errors.iter().for_each(|err| {
				log::error!(
					target: "try-runtime",
					"{:?}",
					err
				);
			});

			return Err("Detected multiple errors while executing `try_on_runtime_upgrade`, check the logs!".into())
		}

		Ok(weight)
	}

	/// [`OnRuntimeUpgrade::pre_upgrade`] should not be used on a tuple.
	///
	/// Instead, implementors should use [`OnRuntimeUpgrade::try_on_runtime_upgrade`] which
	/// internally calls `pre_upgrade` -> `on_runtime_upgrade` -> `post_upgrade` for each tuple
	/// member in sequence, enabling testing of order-dependent migrations.
	#[cfg(feature = "try-runtime")]
	fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
		Err("Usage of `pre_upgrade` with Tuples is not expected. Please use `try_on_runtime_upgrade` instead, which internally calls `pre_upgrade` -> `on_runtime_upgrade` -> `post_upgrade` for each tuple member.".into())
	}

	/// [`OnRuntimeUpgrade::post_upgrade`] should not be used on a tuple.
	///
	/// Instead, implementors should use [`OnRuntimeUpgrade::try_on_runtime_upgrade`] which
	/// internally calls `pre_upgrade` -> `on_runtime_upgrade` -> `post_upgrade` for each tuple
	/// member in sequence, enabling testing of order-dependent migrations.
	#[cfg(feature = "try-runtime")]
	fn post_upgrade(_state: Vec<u8>) -> Result<(), TryRuntimeError> {
		Err("Usage of `post_upgrade` with Tuples is not expected. Please use `try_on_runtime_upgrade` instead, which internally calls `pre_upgrade` -> `on_runtime_upgrade` -> `post_upgrade` for each tuple member.".into())
	}
}

/// See [`Hooks::integrity_test`].
#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))]
#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
pub trait IntegrityTest {
	/// See [`Hooks::integrity_test`].