Skip to content

Commit

Permalink
Remove some unwrap/expect in chain crate (#2621)
Browse files Browse the repository at this point in the history
* Return Result instead of calling expect in root(). It would kill peer's thread. Perhaps we should ban this peer as malicious.
* Remove some unwraps
  • Loading branch information
hashmap authored and ignopeverell committed Feb 25, 2019
1 parent 2df633b commit e71eca1
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 10 deletions.
2 changes: 1 addition & 1 deletion chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ impl Chain {

/// Return a merkle proof valid for the current output pmmr state at the
/// given pos
pub fn get_merkle_proof_for_pos(&self, commit: Commitment) -> Result<MerkleProof, String> {
pub fn get_merkle_proof_for_pos(&self, commit: Commitment) -> Result<MerkleProof, Error> {
let mut txhashset = self.txhashset.write();
txhashset.merkle_proof(commit)
}
Expand Down
3 changes: 3 additions & 0 deletions chain/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ pub enum ErrorKind {
/// We cannot process data once the Grin server has been stopped.
#[fail(display = "Stopped (Grin Shutting Down)")]
Stopped,
/// Internal Roaring Bitmap error
#[fail(display = "Roaring Bitmap error")]
Bitmap,
}

impl Display for Error {
Expand Down
3 changes: 2 additions & 1 deletion chain/src/txhashset/rewindable_kernel_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ impl<'a> RewindableKernelView<'a> {
/// fast sync where a reorg past the horizon could allow a whole rewrite of
/// the kernel set.
pub fn validate_root(&self) -> Result<(), Error> {
if self.pmmr.root() != self.header.kernel_root {
let root = self.pmmr.root().map_err(|_| ErrorKind::InvalidRoot)?;
if root != self.header.kernel_root {
return Err(ErrorKind::InvalidTxHashSet(format!(
"Kernel root at {} does not match",
self.header.height
Expand Down
11 changes: 6 additions & 5 deletions chain/src/txhashset/txhashset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,11 @@ impl TxHashSet {
}

/// build a new merkle proof for the given position.
pub fn merkle_proof(&mut self, commit: Commitment) -> Result<MerkleProof, String> {
let pos = self.commit_index.get_output_pos(&commit).unwrap();
PMMR::at(&mut self.output_pmmr_h.backend, self.output_pmmr_h.last_pos).merkle_proof(pos)
pub fn merkle_proof(&mut self, commit: Commitment) -> Result<MerkleProof, Error> {
let pos = self.commit_index.get_output_pos(&commit)?;
PMMR::at(&mut self.output_pmmr_h.backend, self.output_pmmr_h.last_pos)
.merkle_proof(pos)
.map_err(|_| ErrorKind::MerkleProof.into())
}

/// Compact the MMR data files and flush the rm logs
Expand Down Expand Up @@ -1590,6 +1592,5 @@ pub fn input_pos_to_rewind(
current = batch.get_previous_header(&current)?;
}

let bitmap = bitmap_fast_or(None, &mut block_input_bitmaps).unwrap();
Ok(bitmap)
bitmap_fast_or(None, &mut block_input_bitmaps).ok_or_else(|| ErrorKind::Bitmap.into())
}
6 changes: 3 additions & 3 deletions core/src/core/pmmr/rewindable_pmmr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ where

/// Computes the root of the MMR. Find all the peaks in the current
/// tree and "bags" them to get a single peak.
pub fn root(&self) -> Hash {
pub fn root(&self) -> Result<Hash, String> {
if self.is_empty() {
return ZERO_HASH;
return Ok(ZERO_HASH);
}
let mut res = None;
for peak in self.peaks().iter().rev() {
Expand All @@ -106,7 +106,7 @@ where
Some(rhash) => Some((*peak, rhash).hash_with_index(self.unpruned_size())),
}
}
res.expect("no root, invalid tree")
res.ok_or_else(|| "no root, invalid tree".to_owned())
}

/// Returns a vec of the peaks of this MMR.
Expand Down

0 comments on commit e71eca1

Please sign in to comment.