Skip to content

Commit

Permalink
fix: single allocation for block header consensus encoding (#4101)
Browse files Browse the repository at this point in the history
Description
---
- implements `ConsensusEncodingSized` for `BlockHeader` to allow for single allocation when encoding to bytes
- uses `to_consensus_bytes` in `tari_mining_helper_ffi::inject_nonce`
- Use consensus encoding impl for EpochTime

Motivation and Context
---
Previously an unallocated Vec was passed into `consensus_encode` which leads to a number of allocations.
It is best practice, if the size is known, to allocate the vec once. This is implemented generically in `to_consensus_bytes`.

How Has This Been Tested?
---
Existing tests pass.
  • Loading branch information
sdbondi committed May 16, 2022
1 parent 6163d7b commit aa85738
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 14 deletions.
8 changes: 5 additions & 3 deletions base_layer/core/src/blocks/block_header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ use thiserror::Error;
#[cfg(feature = "base_node")]
use crate::blocks::{BlockBuilder, NewBlockHeaderTemplate};
use crate::{
consensus::{ConsensusDecoding, ConsensusEncoding, ConsensusHashWriter},
consensus::{ConsensusDecoding, ConsensusEncoding, ConsensusEncodingSized, ConsensusHashWriter},
proof_of_work::{PowAlgorithm, PowError, ProofOfWork},
};

Expand Down Expand Up @@ -398,7 +398,7 @@ impl ConsensusEncoding for BlockHeader {
let mut written = self.version.consensus_encode(writer)?;
written += self.height.consensus_encode(writer)?;
written += copy_into_fixed_array_lossy::<_, 32>(&self.prev_hash).consensus_encode(writer)?;
written += self.timestamp.as_u64().consensus_encode(writer)?;
written += self.timestamp.consensus_encode(writer)?;
written += copy_into_fixed_array_lossy::<_, 32>(&self.output_mr).consensus_encode(writer)?;
written += copy_into_fixed_array_lossy::<_, 32>(&self.witness_mr).consensus_encode(writer)?;
written += self.output_mmr_size.consensus_encode(writer)?;
Expand All @@ -413,13 +413,15 @@ impl ConsensusEncoding for BlockHeader {
}
}

impl ConsensusEncodingSized for BlockHeader {}

impl ConsensusDecoding for BlockHeader {
fn consensus_decode<R: Read>(reader: &mut R) -> Result<Self, io::Error> {
let version = u16::consensus_decode(reader)?;
let mut header = BlockHeader::new(version);
header.height = u64::consensus_decode(reader)?;
header.prev_hash = <[u8; 32] as ConsensusDecoding>::consensus_decode(reader)?.to_vec();
header.timestamp = EpochTime::from(u64::consensus_decode(reader)?);
header.timestamp = EpochTime::consensus_decode(reader)?;
header.output_mr = <[u8; 32] as ConsensusDecoding>::consensus_decode(reader)?.to_vec();
header.witness_mr = <[u8; 32] as ConsensusDecoding>::consensus_decode(reader)?.to_vec();
header.output_mmr_size = u64::consensus_decode(reader)?;
Expand Down
17 changes: 6 additions & 11 deletions base_layer/tari_mining_helper_ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use std::{convert::TryFrom, ffi::CString, slice};
use libc::{c_char, c_int, c_uchar, c_uint, c_ulonglong};
use tari_core::{
blocks::BlockHeader,
consensus::{ConsensusDecoding, ConsensusEncoding},
consensus::{ConsensusDecoding, ToConsensusBytes},
proof_of_work::sha3_difficulty,
};
use tari_crypto::tari_utilities::hex::Hex;
Expand Down Expand Up @@ -236,15 +236,7 @@ pub unsafe extern "C" fn inject_nonce(header: *mut ByteVector, nonce: c_ulonglon
},
};
block_header.nonce = nonce;
let mut header_bytes = Vec::new();
match block_header.consensus_encode(&mut header_bytes) {
Ok(_) => {},
Err(e) => {
error = MiningHelperError::from(InterfaceError::Conversion(e.to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
},
};
(*header).0 = header_bytes;
(*header).0 = block_header.to_consensus_bytes();
}

/// Returns the difficulty of a share
Expand Down Expand Up @@ -357,7 +349,10 @@ pub unsafe extern "C" fn share_validate(
mod tests {
use libc::c_int;
use tari_common::configuration::Network;
use tari_core::blocks::{genesis_block::get_genesis_block, Block};
use tari_core::{
blocks::{genesis_block::get_genesis_block, Block},
consensus::ConsensusEncoding,
};

use super::*;
use crate::{inject_nonce, public_key_hex_validate, share_difficulty, share_validate};
Expand Down

0 comments on commit aa85738

Please sign in to comment.