Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 55 additions & 15 deletions polkadot/primitives/src/vstaging/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! Staging Primitives.
use core::fmt::Formatter;

use crate::{slashing::DisputesTimeSlot, ValidatorId, ValidatorIndex, ValidityAttestation};

// Put any primitives used by staging APIs functions here
Expand Down Expand Up @@ -66,8 +68,9 @@ pub enum CandidateDescriptorVersion {
}

/// A unique descriptor of the candidate receipt.
#[derive(PartialEq, Eq, Clone, Encode, Decode, DecodeWithMemTracking, TypeInfo, RuntimeDebug)]
#[derive(PartialEq, Eq, Clone, Encode, Decode, DecodeWithMemTracking, TypeInfo)]
#[cfg_attr(feature = "std", derive(Hash))]
#[cfg_attr(not(feature = "std"), derive(RuntimeDebug))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[cfg_attr(not(feature = "std"), derive(RuntimeDebug))]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we want to keep the empty implementation for when std is not enabled, similarly to what it was before this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slowly removing this everywhere, it just makes stuff more complicated when you want to debug stuff.

pub struct CandidateDescriptorV2<H = Hash> {
/// The ID of the para this is a candidate for.
para_id: ParaId,
Expand Down Expand Up @@ -99,6 +102,57 @@ pub struct CandidateDescriptorV2<H = Hash> {
/// The blake2-256 hash of the validation code bytes.
validation_code_hash: ValidationCodeHash,
}
impl<H> CandidateDescriptorV2<H> {
/// Returns the candidate descriptor version.
/// The candidate is at version 2 if the reserved fields are zeroed out
/// and the internal `version` field is 0.
pub fn version(&self) -> CandidateDescriptorVersion {
if self.reserved2 != [0u8; 64] || self.reserved1 != [0u8; 25] {
return CandidateDescriptorVersion::V1
}

match self.version.0 {
0 => CandidateDescriptorVersion::V2,
_ => CandidateDescriptorVersion::Unknown,
}
}
}

#[cfg(feature = "std")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[cfg(feature = "std")]

impl<H> core::fmt::Debug for CandidateDescriptorV2<H>
where
H: core::fmt::Debug,
{
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self.version() {
CandidateDescriptorVersion::V1 => f
.debug_struct("CandidateDescriptor")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.debug_struct("CandidateDescriptor")
.debug_struct("CandidateDescriptorV1")

.field("para_id", &self.para_id)
.field("relay_parent", &self.relay_parent)
.field("persisted_validation_hash", &self.persisted_validation_data_hash)
.field("pov_hash", &self.pov_hash)
.field("erasure_root", &self.erasure_root)
.field("para_head", &self.para_head)
.field("validation_code_hash", &self.validation_code_hash)
.finish(),
CandidateDescriptorVersion::V2 => f
.debug_struct("CandidateDescriptorV2")
.field("para_id", &self.para_id)
.field("relay_parent", &self.relay_parent)
.field("core_index", &self.core_index)
.field("session_index", &self.session_index)
.field("persisted_validation_data_hash", &self.persisted_validation_data_hash)
.field("pov_hash", &self.pov_hash)
.field("erasure_root", &self.pov_hash)
.field("para_head", &self.para_head)
.field("validation_code_hash", &self.validation_code_hash)
.finish(),
_ => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_ => {
CandidateDescriptorVersion::Unknown => {

write!(f, "Invalid CandidateDescriptorVersion")
},
}
}
}

impl<H: Copy> From<CandidateDescriptorV2<H>> for CandidateDescriptor<H> {
fn from(value: CandidateDescriptorV2<H>) -> Self {
Expand Down Expand Up @@ -589,20 +643,6 @@ impl<H: Copy> CandidateDescriptorV2<H> {
impl_getter!(pov_hash, Hash);
impl_getter!(validation_code_hash, ValidationCodeHash);

/// Returns the candidate descriptor version.
/// The candidate is at version 2 if the reserved fields are zeroed out
/// and the internal `version` field is 0.
pub fn version(&self) -> CandidateDescriptorVersion {
if self.reserved2 != [0u8; 64] || self.reserved1 != [0u8; 25] {
return CandidateDescriptorVersion::V1
}

match self.version.0 {
0 => CandidateDescriptorVersion::V2,
_ => CandidateDescriptorVersion::Unknown,
}
}

fn rebuild_collator_field(&self) -> CollatorId {
let mut collator_id = Vec::with_capacity(32);
let core_index: [u8; 2] = self.core_index.to_ne_bytes();
Expand Down
12 changes: 12 additions & 0 deletions prdoc/pr_9255.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Fix CandidateDescriptor debug logs

doc:
- audience: Node Dev
description: |
Implement core::fmt::Debug by hand when std is not enabled and print the
structure differently depending on the CandidateDescriptor version.

crates: [ polkadot-primitives ]
Loading