Rename hash module to merkle_root
#1334
Conversation
17d4fea to
bc845d0
Compare
|
concept ACK. If #418 gets in before this one, those new functions should also live in the new |
|
Oops, yes. I blame github's # autocomplete :). |
|
How about module being named |
|
I like it! |
bc845d0 to
e410555
Compare
|
Renamed as suggested. |
|
CI failure is actually in master, sorry |
|
No sweat, I'll fix it :) |
|
Sure -- though I have a fix that I'll push as soon as my local tests pass. (If you beat me to it, fine, I'll just test and ACK yours -- it's only a couple lines :)) |
|
#1346 whichever goes first :) |
e410555 to
a7e74fc
Compare
|
Rebased on master, no other changes. |
| /// | ||
| /// # Returns | ||
| /// - `None` if `hashes` is empty. The merkle root of an empty tree of hashes is undefined. | ||
| /// - `Some(hash)` if `hashes` contains one element. A single hash is by definition the merkle root. | ||
| /// - `Some(merkle_root)` if length of `hashes` is greater than one. | ||
| pub fn bitcoin_merkle_root_inline<T>(hashes: &mut [T]) -> Option<T> | ||
| pub fn calculate_root_inline<T>(hashes: &mut [T]) -> Option<T> |
There was a problem hiding this comment.
Now that I see this I realize it'd be nice to explicitly state that the hashes are overwritten in the process. And perhaps state that the state we leave it in is undefined and must not be relied on.
There was a problem hiding this comment.
Agreed. Though FWIW taking a mutable slice implies that we'll overwrite stuff.
There was a problem hiding this comment.
Added some extra docs as an additional patch.
bitcoin/src/util/mod.rs
Outdated
| @@ -97,6 +96,34 @@ pub use crate::bip32; | |||
| #[deprecated(since = "0.30.0", note = "Please use crate::bip158")] | |||
| pub use crate::bip158; | |||
|
|
|||
| /// The `hash` module was renamed to `../merkle_tree`. | |||
There was a problem hiding this comment.
Although GH says "renamed" and we think of it as renaming, a more sensible representation is "removed and the merkle tree functions were moved to merkle_tree"
There was a problem hiding this comment.
Noted, all thees "deprecated" messages should go away in a version or two so unless I re-spin for some other reason I'll leave this as is.
There was a problem hiding this comment.
Changed to: /// Functions from the hash module were renamed and moved to ../merkle_tree.
| merkle_tree::calculate_root(hashes) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Very good choice here finding them could be hard otherwise.
The `util::hash` module provides two functions for computing a merkle root from a list/iterator of hashes. Rename the module to `merkle_root` and move it to the crate root, deprecate the original functions. Done as part of flattening the `util` module.
Recently we renamed the `hash` module to `merkle_root`, this makes the public functions provided stutter if used with one layer of path as is Rust convention: `merkle_root::bitcoin_merkle_root` We can improve on this by renaming the functions to 'calculate', then we get - `merkle_root::calculate()` - `merkle_root::calculate_inline()`
The function call `calculate_root_inline` calculates the merkle root using the input array as a scratch buffer, i.e., we trash the data during recursive calls to `merkle_root_r`. Add explicit documentation to the function so its super clear not to use the hashes again after calling this function.
a7e74fc to
29df410
Compare
|
Changes in force push:
|
Done as part of flattening
util.The
util::hashmodule only provides two functions, both to calculate the merkle root of a list of hashes.util::hash->crate::merkle_rootcalculate[_inline]so usage becomesmerkle_root::calculateDone as two separate patches so we can bikeshed the names, can squash if needed.