From 9cdf3d999e9de7996495c891af7085f55acc695a Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Fri, 30 Aug 2024 20:55:57 +0800 Subject: [PATCH] Add more logs for AcceptanceCheckErr (#5513) # Description The error message should be logged out when the check method returns an error. Because specific information is lost when `UmpAcceptanceCheckErr`, `ProcessedDownwardMessagesAcceptanceErr`, `HrmpWatermarkAcceptanceErr`, `OutboundHrmpAcceptanceErr` are converted to `AcceptanceCheckErr`, a log is added to each check. ## Integration ## Review Notes # Checklist * [ ] My PR includes a detailed description as outlined in the "Description" and its two subsections above. * [ ] My PR follows the [labeling requirements]( https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process ) of this project (at minimum one label for `T` required) * External contributors: ask maintainers to put the right label on your PR. * [ ] I have made corresponding changes to the documentation (if applicable) * [ ] I have added tests that prove my fix is effective or that my feature works (if applicable) --- .../runtime/parachains/src/inclusion/mod.rs | 79 +++++++++++++------ prdoc/pr_5513.prdoc | 10 +++ 2 files changed, 67 insertions(+), 22 deletions(-) create mode 100644 prdoc/pr_5513.prdoc diff --git a/polkadot/runtime/parachains/src/inclusion/mod.rs b/polkadot/runtime/parachains/src/inclusion/mod.rs index fbf13339dfca..b1d22996fd12 100644 --- a/polkadot/runtime/parachains/src/inclusion/mod.rs +++ b/polkadot/runtime/parachains/src/inclusion/mod.rs @@ -34,7 +34,6 @@ use alloc::{ }; use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec}; use codec::{Decode, Encode}; -#[cfg(feature = "std")] use core::fmt; use frame_support::{ defensive, @@ -381,7 +380,7 @@ pub mod pallet { const LOG_TARGET: &str = "runtime::inclusion"; /// The reason that a candidate's outputs were rejected for. -#[cfg_attr(feature = "std", derive(Debug))] +#[derive(Debug)] enum AcceptanceCheckErr { HeadDataTooLarge, /// Code upgrades are not permitted at the current time. @@ -439,7 +438,6 @@ pub(crate) enum UmpAcceptanceCheckErr { IsOffboarding, } -#[cfg(feature = "std")] impl fmt::Debug for UmpAcceptanceCheckErr { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { match *self { @@ -828,23 +826,20 @@ impl Pallet { let prev_context = Self::para_most_recent_context(¶_id); let check_ctx = CandidateCheckContext::::new(prev_context); - if check_ctx - .check_validation_outputs( - para_id, - relay_parent_number, - &validation_outputs.head_data, - &validation_outputs.new_validation_code, - validation_outputs.processed_downward_messages, - &validation_outputs.upward_messages, - BlockNumberFor::::from(validation_outputs.hrmp_watermark), - &validation_outputs.horizontal_messages, - ) - .is_err() - { + if let Err(err) = check_ctx.check_validation_outputs( + para_id, + relay_parent_number, + &validation_outputs.head_data, + &validation_outputs.new_validation_code, + validation_outputs.processed_downward_messages, + &validation_outputs.upward_messages, + BlockNumberFor::::from(validation_outputs.hrmp_watermark), + &validation_outputs.horizontal_messages, + ) { log::debug!( target: LOG_TARGET, - "Validation outputs checking for parachain `{}` failed", - u32::from(para_id), + "Validation outputs checking for parachain `{}` failed, error: {:?}", + u32::from(para_id), err ); false } else { @@ -1307,9 +1302,10 @@ impl CandidateCheckContext { ) { log::debug!( target: LOG_TARGET, - "Validation outputs checking during inclusion of a candidate {:?} for parachain `{}` failed", + "Validation outputs checking during inclusion of a candidate {:?} for parachain `{}` failed, error: {:?}", backed_candidate_receipt.hash(), u32::from(para_id), + err ); Err(err.strip_into_dispatch_err::())?; }; @@ -1365,10 +1361,49 @@ impl CandidateCheckContext { para_id, relay_parent_number, processed_downward_messages, + ) + .map_err(|e| { + log::debug!( + target: LOG_TARGET, + "Check processed downward messages for parachain `{}` on relay parent number `{:?}` failed, error: {:?}", + u32::from(para_id), + relay_parent_number, + e + ); + e + })?; + Pallet::::check_upward_messages(&self.config, para_id, upward_messages).map_err( + |e| { + log::debug!( + target: LOG_TARGET, + "Check upward messages for parachain `{}` failed, error: {:?}", + u32::from(para_id), + e + ); + e + }, )?; - Pallet::::check_upward_messages(&self.config, para_id, upward_messages)?; - hrmp::Pallet::::check_hrmp_watermark(para_id, relay_parent_number, hrmp_watermark)?; - hrmp::Pallet::::check_outbound_hrmp(&self.config, para_id, horizontal_messages)?; + hrmp::Pallet::::check_hrmp_watermark(para_id, relay_parent_number, hrmp_watermark) + .map_err(|e| { + log::debug!( + target: LOG_TARGET, + "Check hrmp watermark for parachain `{}` on relay parent number `{:?}` failed, error: {:?}", + u32::from(para_id), + relay_parent_number, + e + ); + e + })?; + hrmp::Pallet::::check_outbound_hrmp(&self.config, para_id, horizontal_messages) + .map_err(|e| { + log::debug!( + target: LOG_TARGET, + "Check outbound hrmp for parachain `{}` failed, error: {:?}", + u32::from(para_id), + e + ); + e + })?; Ok(()) } diff --git a/prdoc/pr_5513.prdoc b/prdoc/pr_5513.prdoc new file mode 100644 index 000000000000..0eda8b02cde0 --- /dev/null +++ b/prdoc/pr_5513.prdoc @@ -0,0 +1,10 @@ +title: Add more logs to trace AcceptanceCheckErr + +doc: + - audience: Runtime Dev + description: | + `Debug` was derived on `AcceptanceCheckErr` to print the error. This PR adds more logs to trace the error. + +crates: + - name: polkadot-runtime-parachains + bump: patch