Skip to content

BoundedString to BoundedVec #622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

BoundedString to BoundedVec #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions node/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use parity_scale_codec::MaxEncodedLen;
use parity_scale_codec::{Decode, Encode};
use scale_info::TypeInfo;
use serde::{Deserialize, Serialize};
use sidechain_domain::byte_string::{BoundedString, SizedByteString};
use sidechain_domain::byte_string::SizedByteString;
use sidechain_domain::{
CrossChainPublicKey, DelegatorKey, MainchainKeyHash, PermissionedCandidateData,
RegistrationData, ScEpochNumber, ScSlotNumber, StakeDelegation, StakePoolPublicKey, UtxoId,
Expand Down Expand Up @@ -461,13 +461,15 @@ impl AsCardanoSPO for BlockAuthor {
}
}

pub const MAX_METADATA_URL_LENGTH: u32 = 512;
parameter_types! {
pub const MaxMetadataUrlLength: u32 = 512;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks quite long, should we short it?

}

#[derive(
Clone, Debug, MaxEncodedLen, Encode, Decode, Serialize, Deserialize, PartialEq, Eq, TypeInfo,
)]
pub struct BlockProducerMetadataType {
pub url: BoundedString<MAX_METADATA_URL_LENGTH>,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is our own reference runtime implementation, not some code that anyone other than us is forced to use. We can make any assumptions that are convenient for us to do here. I went with a string-like type, because that's easy and intuitive for us to work with.

pub url: BoundedVec<u8, MaxMetadataUrlLength>,
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this change reflected in RPC?

I would like to see "before" and "after" before approving such a change.

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 think it is hard to answer clearly that one, it actually depends on what will be the source of data. If type will be created from runtime (i.e. Decode trait will be involved), then in theory it may contain non utf-8 characters, so further serialization of this type with serde::Serialize may cause permanent error.
Other scenario will be if type will be created with serde::Deserialize, then we will have validation being involved (utf-8 check), but later there are no guarantees that data will remain utf-8 (code before changes).
From the user perspective providing of the data should stay the same, user will provide some arbitrary blob of bytes that may or may not decode to string, point here is to not make any assumptions regarding what is inside (due we not validate either utf-8 data or url format).
After the changes serialization of the data back to readable format may change, proposed changes will try to save them in bytes format, without attempt to provide "readable" utf-8 form, due underlying data may be invalid

pub hash: SizedByteString<32>,
}

Expand Down
58 changes: 0 additions & 58 deletions toolkit/sidechain/domain/src/byte_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
use core::fmt::{Debug, Display};

use alloc::vec::Vec;
use byte_string_derive::byte_string;
use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;
use serde::de::Error as DeError;
use serde::ser::Error as SerError;
use serde::{Deserialize, Serialize};
use sp_core::bounded::BoundedVec;
use sp_core::ConstU32;

/// Wrapper for bytes that is serialized as hex string
/// To be used for binary data that we want to display nicely but
Expand Down Expand Up @@ -52,54 +45,3 @@ impl<const N: usize> Default for SizedByteString<N> {
Self([0; N])
}
}

/// Byte-encoded text string with bounded length
#[derive(Eq, Clone, PartialEq, TypeInfo, Default, Encode, Decode, MaxEncodedLen)]
pub struct BoundedString<const N: u32>(pub BoundedVec<u8, ConstU32<N>>);

impl<const N: u32> TryFrom<Vec<u8>> for BoundedString<N> {
type Error = <BoundedVec<u8, ConstU32<N>> as TryFrom<Vec<u8>>>::Error;

fn try_from(value: Vec<u8>) -> Result<Self, Self::Error> {
Ok(Self(value.try_into()?))
}
}

impl<'a, const N: u32> Deserialize<'a> for BoundedString<N> {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'a>,
{
Ok(Self(
BoundedVec::try_from(
alloc::string::String::deserialize(deserializer)?.as_bytes().to_vec(),
)
.map_err(|_| D::Error::custom("Size limit exceeded"))?,
))
}
}

impl<const N: u32> Serialize for BoundedString<N> {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
let str = alloc::string::String::from_utf8(self.0.to_vec())
.map_err(|_| S::Error::custom("String is not valid UTF-8"))?;
serializer.serialize_str(&str)
}
}

impl<const N: u32> Display for BoundedString<N> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.write_str(
&alloc::string::String::from_utf8(self.0.to_vec()).map_err(|_| core::fmt::Error)?,
)
}
}

impl<const N: u32> Debug for BoundedString<N> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.write_str(&alloc::format!("BoundedString<{}>({:?})", N, self.0))
}
}
Loading