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

Update bridge pallet to work with the (almost) lastest master #4672

Merged

Conversation

HCastano
Copy link
Contributor

I've rebased the hc-jp-bridge-module branch onto master from last week-ish, and now in this PR I've make some changes to make the pallet compile. The two main things are using the new decl_error! changes introduced by Basti, and updating dependencies to use their 2.0.0 versions.

@HCastano HCastano added the A0-please_review Pull request needs code review. label Jan 18, 2020
@HCastano HCastano requested a review from bkchr January 18, 2020 16:52
@parity-cla-bot
Copy link

It looks like @HCastano signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

)?;
);

// TODO: Remove once I can impl From<StorageError> for DispatchError
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally I would've like to have implemented this, but with the way decl_error! is written at the moment I'm not able to do this. If anybody has any better ideas than the map_storage_err function I'm using at the moment I'm all ears.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be just

let checker = checker.map_err(Self::map_storage_err)?;

I don't think that it's so bad that it requires a TODO line in the code. Feel free to create an issue for this though.

Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

lgtm!

)?;
);

// TODO: Remove once I can impl From<StorageError> for DispatchError
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be just

let checker = checker.map_err(Self::map_storage_err)?;

I don't think that it's so bad that it requires a TODO line in the code. Feel free to create an issue for this though.

.ok_or(Error::StorageValueUnavailable)?;

// TODO: Remove once I can impl From<StorageError> for DispatchError
let actual_validator_set = match checker.read_value(b":grandpa_authorities") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here:

let actual_validator_set = checker.read_value(MOVED_TO_CONSTANT).map_err(Self::map_storage_err)?
let actual_validator_set = actual_validator_set.ok_or(Error::<T>::StorageValueUnavailable))?;

}

Err(Error::InvalidAncestryProof)
// TODO: Remove once I can impl From<StorageError> for DispatchError
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO the function is fine.

Suggested change
// TODO: Remove once I can impl From<StorageError> for DispatchError

match e {
StorageError::StorageRootMismatch => Error::<T>::StorageRootMismatch.into(),
StorageError::StorageValueUnavailable => Error::<T>::StorageValueUnavailable.into(),
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}.into()

You could put into here to avoid duplication, but it's a minor thing.

let backend = <InMemory<Blake2Hasher>>::from(vec![
(None, b":grandpa_authorities".to_vec(), Some(encoded_set)),
let backend = <InMemoryBackend<Blake2Hasher>>::from(vec![
(None, vec![(b":grandpa_authorities".to_vec(), Some(encoded_set))]),
Copy link
Contributor

Choose a reason for hiding this comment

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

The string should come from the same constant.

]);
let root = backend.storage_root(std::iter::empty()).0;

// Generates a storage read proof
let proof = prove_read(backend, &[&b":grandpa_authorities"[..]]).unwrap();
let proof: StorageProof = prove_read(backend, &[&b":grandpa_authorities"[..]])
Copy link
Contributor

Choose a reason for hiding this comment

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

same here.

@HCastano HCastano merged commit 1131ecf into paritytech:hc-jp-bridge-module Jan 29, 2020
@HCastano HCastano deleted the hc-fix-error-handling branch January 29, 2020 05:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants