Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Make functionality to read relay state proof entries public #1135

Merged
merged 6 commits into from
Apr 11, 2022
Merged
Changes from all 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
28 changes: 28 additions & 0 deletions pallets/parachain-system/src/relay_state_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ pub struct MessagingStateSnapshot {
pub enum Error {
/// The provided proof was created against unexpected storage root.
RootMismatch,
/// The entry cannot be read.
ReadEntry(ReadEntryErr),
/// The optional entry cannot be read.
ReadOptionalEntry(ReadEntryErr),
/// The slot cannot be extracted.
Slot(ReadEntryErr),
/// The upgrade go-ahead signal cannot be read.
Expand Down Expand Up @@ -273,4 +277,28 @@ impl RelayChainStateProof {
)
.map_err(Error::UpgradeRestriction)
}

/// Read an entry given by the key and try to decode it. If the value specified by the key according
/// to the proof is empty, the `fallback` value will be returned.
///
/// Returns `Err` in case the backend can't return the value under the specific key (likely due to
/// a malformed proof), in case the decoding fails, or in case where the value is empty in the relay
/// chain state and no fallback was provided.
pub fn read_entry<T>(&self, key: &[u8], fallback: Option<T>) -> Result<T, Error>
Copy link
Member

Choose a reason for hiding this comment

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

Why not directly expose the functions to read the babe randomness? As we do for all the other values?

Copy link
Contributor Author

@4meta5 4meta5 Apr 5, 2022

Choose a reason for hiding this comment

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

Then we will need functions for every value we would like to get. This will be at least:

read_current_block_randomness
read_one_epoch_ago_randomness
read_two_epochs_ago_randomness
read_epoch_index

It is less code to write one read_entry<T> which is generic over the successful result value returned.

Why not directly expose the functions to read the babe randomness? As we do for all the other values?

This approach also requires adding a new function if we ever want to read a new value from the RelayChainStateProof and that comes with a release delay so it is not preferred.

Copy link
Member

Choose a reason for hiding this comment

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

I get what you say, but currently it also requires that you add these values before on the client side :P

We can add this function, I don't really care, but then please add some docs :)

where
T: Decode,
{
read_entry(&self.trie_backend, key, fallback).map_err(Error::ReadEntry)
}

/// Read an optional entry given by the key and try to decode it.
///
/// Returns `Err` in case the backend can't return the value under the specific key (likely due to
/// a malformed proof) or if the value couldn't be decoded.
pub fn read_optional_entry<T>(&self, key: &[u8]) -> Result<Option<T>, Error>
where
T: Decode,
{
read_optional_entry(&self.trie_backend, key).map_err(Error::ReadOptionalEntry)
}
}