feat: various helper functions and fixes for platform contested resources PR #307
Conversation
…feat/QuerySumTree
QuantumExplorer
changed the title
fominok
force-pushed
the
feat/QuerySumTree
branch
from
8886695 to
adb8fd1
Compare
| pub fn as_sum_item_value(&self) -> Result<i64, Error> { | ||
| match self { | ||
| Element::SumItem(value, _) => Ok(*value), | ||
| _ => Err(Error::WrongElementType("expected a sum item")), | ||
| } | ||
| } | ||
|
|
||
| #[cfg(any(feature = "full", feature = "verify"))] | ||
| /// Decoded the integer value in the SumItem element type | ||
| pub fn into_sum_item_value(self) -> Result<i64, Error> { |
There was a problem hiding this comment.
what is the purpose of this method? as_sum_item_value is just fine because i64 is Copy
There was a problem hiding this comment.
still should be faster right as you are not copying memory. I agree it's not very important though.
There was a problem hiding this comment.
It returns from the function so the stack frame is cleared and memory will be copied from there anyways, we're not avoiding memory copies by using self unless we doing so to avoid doing explicit clonings
There was a problem hiding this comment.
Look, it really doesn't matter, we have a lot of into and as. While I could remove this, I just don't think it matters, so I prefer to keep it.
|
|
||
| #[cfg(any(feature = "full", feature = "verify"))] | ||
| /// Decoded the integer value in the SumTree element type | ||
| pub fn into_sum_tree_value(self) -> Result<i64, Error> { |
There was a problem hiding this comment.
as_sum_tree_value is fine
| @@ -54,6 +54,12 @@ pub enum ReferencePathType { | |||
| /// path [p, q] result = [a, b, p, q] | |||
| UpstreamRootHeightReference(u8, Vec<Vec<u8>>), | |||
|
|
|||
| /// This is very similar to the UpstreamRootHeightReference, however | |||
There was a problem hiding this comment.
i guess it needs a better explanation at least how it's done at UpstreamRootHeightReference
| ) | ||
| .unwrap_add_cost(&mut cost)?; | ||
|
|
||
| match maybe_item { |
There was a problem hiding this comment.
i think we shall stop with maybe_item there and reuse the following code intended for non-references, because it's the same and reference will be already resolved
There was a problem hiding this comment.
I didn't understand this comment.
| Err(e) => Err(e), | ||
| } | ||
| } else { | ||
| return Err(Error::CorruptedData( |
There was a problem hiding this comment.
doesn't seem like corrupted data but a wrong arg to macro
There was a problem hiding this comment.
Agreed, since this is just copied code we should make a new PR to fix this in all places.
Issue being fixed or feature implemented
What was done?
*Added helper methods to get element data more easily.
*If the query option is set to not error when parent paths are missing this will now work.
*Added some new conversions from return types from queries:
**to_last_path_to_keys_btree_map
**to_last_path_to_key_elements_btree_map
**to_last_path_to_elements_btree_map
**to_previous_of_last_path_to_keys_btree_map
*Added a new reference type: UpstreamRootHeightWithParentPathAdditionReference. This is very similar to the UpstreamRootHeightReference, however it appends to the absolute path when resolving the parent of the reference If the reference is stored at 15/9/80/7 then 80 will be appended to what we are referring to.
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only