Skip to content

Commit

Permalink
Removed without_storage_info from messages and schema pallets (#1702)
Browse files Browse the repository at this point in the history
# Goal
The goal of this PR is to remove `without_storage_info` which is
necessary to be able to calculate max encoded length for PoV.

Closes <!-- issue # -->
#1696 

# Changes
- Since we can not submit a `MaxEncodedLen` PoV for messages due to
oversized PoV we would need to use `measured` until we reworked the
messages pallet.
- Reduced the `MessagesMaxPayloadSizeBytes` from 50K to 3K to reduce the
chance of Oversized PoV even for measured mode until the rework of
messages pallet.
- Reduced the `MessagesMaxPerBlock` from 7000 to 200 to reduce the
chance of Oversized PoV even for measured mode until the rework of
messages pallet.

# Checklist
- [X] Chain spec updated
- [X] Weights updated

# Notes
I checked all the submitted messages on rococo and main-net and there
are no messages that violate these restrctions
Here is list of all messages on rococo and there are none on main-net.
The biggest one has less than 700 bytes


[messages.txt](https://github.com/LibertyDSNP/frequency/files/13033855/messages.txt)

# Merge order
This most probably will get merged after node upgrade PR unless
something changes
  • Loading branch information
aramikm authored Oct 23, 2023
1 parent 831337e commit 4736b69
Show file tree
Hide file tree
Showing 14 changed files with 244 additions and 180 deletions.
2 changes: 1 addition & 1 deletion e2e/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 13 additions & 10 deletions pallets/messages/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use frame_support::{assert_ok, pallet_prelude::DispatchResult};
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use sp_runtime::traits::One;

const AVERAGE_NUMBER_OF_MESSAGES: u32 = 499;
const IPFS_SCHEMA_ID: u16 = 50;
const IPFS_PAYLOAD_LENGTH: u32 = 10;

Expand All @@ -23,7 +22,7 @@ fn onchain_message<T: Config>(schema_id: SchemaId) -> DispatchResult {
let payload = Vec::from(
"{'fromId': 123, 'content': '232323', 'fromId': 123, 'content': '232323'}".as_bytes(),
);
let bounded_payload: BoundedVec<u8, T::MaxMessagePayloadSizeBytes> =
let bounded_payload: BoundedVec<u8, T::MessagesMaxPayloadSizeBytes> =
payload.try_into().expect("Invalid payload");
MessagesPallet::<T>::add_message(
provider_id.into(),
Expand All @@ -40,7 +39,7 @@ fn ipfs_message<T: Config>(schema_id: SchemaId) -> DispatchResult {
let payload =
Vec::from("bafkreidgvpkjawlxz6sffxzwgooowe5yt7i6wsyg236mfoks77nywkptdq".as_bytes());
let provider_id = ProviderId(1);
let bounded_payload: BoundedVec<u8, T::MaxMessagePayloadSizeBytes> =
let bounded_payload: BoundedVec<u8, T::MessagesMaxPayloadSizeBytes> =
payload.try_into().expect("Invalid payload");

MessagesPallet::<T>::add_message(
Expand All @@ -63,8 +62,10 @@ fn create_schema<T: Config>(location: PayloadLocation) -> DispatchResult {
}

benchmarks! {
// this is temporary to avoid massive PoV sizes which will break the chain until rework on messages
#[pov_mode = Measured]
add_onchain_message {
let n in 0 .. T::MaxMessagePayloadSizeBytes::get() - 1;
let n in 0 .. T::MessagesMaxPayloadSizeBytes::get() - 1;
let message_source_id = DelegatorId(2);
let caller: T::AccountId = whitelisted_caller();
let schema_id = 1;
Expand All @@ -77,19 +78,21 @@ benchmarks! {
assert_ok!(T::MsaBenchmarkHelper::set_delegation_relationship(ProviderId(1), message_source_id.into(), [schema_id].to_vec()));

let payload = vec![1; n as usize];

for j in 1 .. AVERAGE_NUMBER_OF_MESSAGES {
let average_messages_per_block: u32 = T::MaxMessagesPerBlock::get() / 2;
for j in 1 .. average_messages_per_block {
assert_ok!(onchain_message::<T>(schema_id));
}
}: _ (RawOrigin::Signed(caller), Some(message_source_id.into()), schema_id, payload)
verify {
assert_eq!(
MessagesPallet::<T>::get_messages(
BlockNumberFor::<T>::one(), schema_id).len(),
AVERAGE_NUMBER_OF_MESSAGES as usize
average_messages_per_block as usize
);
}

// this is temporary to avoid massive PoV sizes which will break the chain until rework on messages
#[pov_mode = Measured]
add_ipfs_message {
let caller: T::AccountId = whitelisted_caller();
let cid = "bafkreidgvpkjawlxz6sffxzwgooowe5yt7i6wsyg236mfoks77nywkptdq".as_bytes().to_vec();
Expand All @@ -99,16 +102,16 @@ benchmarks! {
assert_ok!(create_schema::<T>(PayloadLocation::IPFS));
}
assert_ok!(T::MsaBenchmarkHelper::add_key(ProviderId(1).into(), caller.clone()));

for j in 1 .. AVERAGE_NUMBER_OF_MESSAGES {
let average_messages_per_block: u32 = T::MaxMessagesPerBlock::get() / 2;
for j in 1 .. average_messages_per_block {
assert_ok!(ipfs_message::<T>(IPFS_SCHEMA_ID));
}
}: _ (RawOrigin::Signed(caller),IPFS_SCHEMA_ID, cid, IPFS_PAYLOAD_LENGTH)
verify {
assert_eq!(
MessagesPallet::<T>::get_messages(
BlockNumberFor::<T>::one(), IPFS_SCHEMA_ID).len(),
AVERAGE_NUMBER_OF_MESSAGES as usize
average_messages_per_block as usize
);
}

Expand Down
13 changes: 6 additions & 7 deletions pallets/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ mod types;

use frame_support::{ensure, pallet_prelude::Weight, traits::Get, BoundedVec};
use sp_runtime::DispatchError;
use sp_std::{convert::TryInto, prelude::*};
use sp_std::{convert::TryInto, fmt::Debug, prelude::*};

use codec::Encode;
use common_primitives::{
Expand Down Expand Up @@ -105,7 +105,7 @@ pub mod pallet {

/// The maximum size of a message payload bytes.
#[pallet::constant]
type MaxMessagePayloadSizeBytes: Get<u32> + Clone;
type MessagesMaxPayloadSizeBytes: Get<u32> + Clone + Debug + MaxEncodedLen;

#[cfg(feature = "runtime-benchmarks")]
/// A set of helper functions for benchmarking.
Expand All @@ -117,7 +117,6 @@ pub mod pallet {
}

#[pallet::pallet]
#[pallet::without_storage_info]
pub struct Pallet<T>(_);

/// A permanent storage for messages mapped by block number and schema id.
Expand All @@ -131,7 +130,7 @@ pub mod pallet {
BlockNumberFor<T>,
Twox64Concat,
SchemaId,
BoundedVec<Message<T::MaxMessagePayloadSizeBytes>, T::MaxMessagesPerBlock>,
BoundedVec<Message<T::MessagesMaxPayloadSizeBytes>, T::MaxMessagesPerBlock>,
ValueQuery,
>;

Expand Down Expand Up @@ -226,7 +225,7 @@ pub mod pallet {
let provider_key = ensure_signed(origin)?;
let cid_binary = Self::validate_cid(&cid)?;
let payload_tuple: OffchainPayloadType = (cid_binary, payload_length);
let bounded_payload: BoundedVec<u8, T::MaxMessagePayloadSizeBytes> = payload_tuple
let bounded_payload: BoundedVec<u8, T::MessagesMaxPayloadSizeBytes> = payload_tuple
.encode()
.try_into()
.map_err(|_| Error::<T>::ExceedsMaxMessagePayloadSizeBytes)?;
Expand Down Expand Up @@ -281,7 +280,7 @@ pub mod pallet {
) -> DispatchResult {
let provider_key = ensure_signed(origin)?;

let bounded_payload: BoundedVec<u8, T::MaxMessagePayloadSizeBytes> =
let bounded_payload: BoundedVec<u8, T::MessagesMaxPayloadSizeBytes> =
payload.try_into().map_err(|_| Error::<T>::ExceedsMaxMessagePayloadSizeBytes)?;

if let Some(schema) = T::SchemaProvider::get_schema_by_id(schema_id) {
Expand Down Expand Up @@ -341,7 +340,7 @@ impl<T: Config> Pallet<T> {
pub fn add_message(
provider_msa_id: MessageSourceId,
msa_id: Option<MessageSourceId>,
payload: BoundedVec<u8, T::MaxMessagePayloadSizeBytes>,
payload: BoundedVec<u8, T::MessagesMaxPayloadSizeBytes>,
schema_id: SchemaId,
current_block: BlockNumberFor<T>,
) -> Result<bool, DispatchError> {
Expand Down
25 changes: 17 additions & 8 deletions pallets/messages/src/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use common_primitives::{
schema::*,
};

use codec::{Encode, MaxEncodedLen};
use frame_support::{
dispatch::DispatchResult,
parameter_types,
Expand Down Expand Up @@ -77,26 +78,34 @@ parameter_types! {
// should have this set large enough to accommodate the largest possible CID.
// Take care when adding new tests for on-chain (not IPFS) messages that the payload
// is not too big.
pub const MaxMessagePayloadSizeBytes: u32 = 73;
pub const MessagesMaxPayloadSizeBytes: u32 = 73;
}

impl std::fmt::Debug for MaxMessagePayloadSizeBytes {
impl std::fmt::Debug for MessagesMaxPayloadSizeBytes {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
f.debug_struct("MaxMessagePayloadSizeBytes")
.field("v", &MaxMessagePayloadSizeBytes::get())
f.debug_struct("MessagesMaxPayloadSizeBytes")
.field("v", &MessagesMaxPayloadSizeBytes::get())
.finish()
}
}

impl PartialEq for MaxMessagePayloadSizeBytes {
impl PartialEq for MessagesMaxPayloadSizeBytes {
fn eq(&self, _other: &Self) -> bool {
true
}
}

impl Clone for MaxMessagePayloadSizeBytes {
impl Clone for MessagesMaxPayloadSizeBytes {
fn clone(&self) -> Self {
MaxMessagePayloadSizeBytes {}
MessagesMaxPayloadSizeBytes {}
}
}

impl Encode for MessagesMaxPayloadSizeBytes {}

impl MaxEncodedLen for MessagesMaxPayloadSizeBytes {
fn max_encoded_len() -> usize {
u32::max_encoded_len()
}
}

Expand Down Expand Up @@ -213,7 +222,7 @@ impl pallet_messages::Config for Test {
type SchemaProvider = SchemaHandler;
type WeightInfo = ();
type MaxMessagesPerBlock = MaxMessagesPerBlock;
type MaxMessagePayloadSizeBytes = MaxMessagePayloadSizeBytes;
type MessagesMaxPayloadSizeBytes = MessagesMaxPayloadSizeBytes;

/// A set of helper functions for benchmarking.
#[cfg(feature = "runtime-benchmarks")]
Expand Down
2 changes: 1 addition & 1 deletion pallets/messages/src/tests/other_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ fn retrieved_ipfs_message_should_always_be_in_base32() {
#[test]
fn get_messages_by_schema_with_ipfs_payload_location_should_fail_bad_schema() {
new_test_ext().execute_with(|| {
let bad_message: Message<MaxMessagePayloadSizeBytes> = Message {
let bad_message: Message<MessagesMaxPayloadSizeBytes> = Message {
payload: BoundedVec::try_from(
vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16].to_vec(),
)
Expand Down
11 changes: 6 additions & 5 deletions pallets/messages/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
use codec::{Decode, Encode};
use codec::{Decode, Encode, MaxEncodedLen};
use common_primitives::{
messages::MessageResponse, msa::MessageSourceId, node::BlockNumber, schema::PayloadLocation,
};
use frame_support::{traits::Get, BoundedVec};
use multibase::Base;
use scale_info::TypeInfo;
use sp_std::prelude::*;
use sp_std::{fmt::Debug, prelude::*};

/// Payloads stored offchain contain a tuple of (bytes(the payload reference), payload length).
pub type OffchainPayloadType = (Vec<u8>, u32);

/// A single message type definition.
#[derive(Default, Encode, Decode, PartialEq, Debug, TypeInfo, Eq)]
#[derive(Default, Encode, Decode, PartialEq, Debug, TypeInfo, Eq, MaxEncodedLen)]
#[scale_info(skip_type_params(MaxDataSize))]
#[codec(mel_bound(MaxDataSize: MaxEncodedLen))]
pub struct Message<MaxDataSize>
where
MaxDataSize: Get<u32>,
MaxDataSize: Get<u32> + Debug,
{
/// Data structured by the associated schema's model.
pub payload: BoundedVec<u8, MaxDataSize>,
Expand All @@ -32,7 +33,7 @@ where

impl<MaxDataSize> Message<MaxDataSize>
where
MaxDataSize: Get<u32>,
MaxDataSize: Get<u32> + Debug,
{
/// Helper function to handle response type [`MessageResponse`] depending on the Payload Location (on chain or IPFS)
pub fn map_to_response(
Expand Down
Loading

0 comments on commit 4736b69

Please sign in to comment.