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

Conversation

@bkchr
Copy link
Member

@bkchr bkchr commented Nov 27, 2019

Currently the runtime interface enforces H256 as hash type, but in the
future people could use whatever they want as hash type. The hash type
always needs to match between the runtime and the node, but that is
already required.

Currently the runtime interface enforces `H256` as hash type, but in the
future people could use whatever they want as hash type. The hash type
always needs to match between the runtime and the node, but that is
already required.
@bkchr bkchr added the A0-please_review Pull request needs code review. label Nov 27, 2019
@bkchr bkchr added this to the polkadot-0.7.0 milestone Nov 27, 2019
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

looks good as far as I can tell

key: &[u8]
) -> Result<Option<Vec<u8>>, String> {
let root = self.storage(storage_key)?
.and_then(|r| Decode::decode(&mut &r[..]).ok())
Copy link
Contributor

Choose a reason for hiding this comment

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

why doesn't it returns an error here ?

let mut root = match self.storage(storage_key) {
Ok(value) => value.unwrap_or(default_root.clone()),
Ok(value) =>
value.and_then(|r| Decode::decode(&mut &r[..]).ok()).unwrap_or(default_root.clone()),
Copy link
Contributor

Choose a reason for hiding this comment

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

this decode fails only on corrupted storage if I understand correctly, maybe it worth a warn or at least a comment

@gavofyork gavofyork added A7-looksgoodtestsfail and removed A0-please_review Pull request needs code review. labels Nov 27, 2019
@gavofyork gavofyork merged commit b9e7f09 into master Nov 28, 2019
@gavofyork gavofyork deleted the bkchr-runtime-interface-storage-hashing branch November 28, 2019 00:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants