Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memorize hash #549

Closed
zhangsoledad opened this issue Apr 24, 2019 · 2 comments

Comments

@zhangsoledad
Copy link
Member

commented Apr 24, 2019

  • Cache hashes in the source structs (Header and Transaction) since they are constructed.
  • Do not calculate Header hash when fetching header from database.
  • Optimize ProposalShortId::hash().
  • Save hashes related to Transaction into database.
    So, we can just fetch them from database instead of compute them.
@yangby-cryptape

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

Summary

I think, only Header and Transaction are worth memorizing (I did).

All others can be found be a shell command:

yangby@laptop:ckb> grep -r 'fn [a-z_]*hash(' [^t]* | grep "[-]> [^&]*H256"
core/src/header.rs:    pub fn pow_hash(&self) -> H256 {
core/src/header.rs:    pub fn pow_hash(&self) -> H256 {
core/src/script.rs:    pub fn hash(&self) -> H256 {
core/src/block.rs:    pub fn cal_uncles_hash(&self) -> H256 {
core/src/uncle.rs:pub fn uncles_hash(uncles: &[UncleBlock]) -> H256 {
core/src/transaction.rs:    pub fn data_hash(&self) -> H256 {
core/src/transaction.rs:    pub fn hash(&self) -> H256 {
rpc/src/module/chain.rs:    fn get_block_hash(&self, _number: String) -> Result<Option<H256>>;
rpc/src/module/chain.rs:    fn get_block_hash(&self, number: String) -> Result<Option<H256>> {
shared/src/shared.rs:    fn block_hash(&self, number: BlockNumber) -> Option<H256> {
store/src/store.rs:    fn get_block_hash(&self, number: BlockNumber) -> Option<H256>;
store/src/store.rs:    fn get_block_hash(&self, number: BlockNumber) -> Option<H256> {
sync/src/types.rs:    pub fn block_hash(&self, number: BlockNumber) -> Option<H256> {
verification/src/tests/dummy.rs:    fn block_hash(&self, _height: u64) -> Option<H256> {

Further

I think we can optimize the hash of ProposalShortId.

ckb/core/src/transaction.rs

Lines 408 to 409 in de1cfc9

#[derive(Serialize, Deserialize, Copy, Clone, PartialEq, Eq, Default, Hash)]
pub struct ProposalShortId([u8; 10]);

ckb/core/src/transaction.rs

Lines 457 to 459 in de1cfc9

pub fn hash(&self) -> H256 {
blake2b_256(serialize(self).expect("ProposalShortId serialize should not fail")).into()
}

ckb/core/src/block.rs

Lines 90 to 98 in de1cfc9

pub fn cal_proposals_root(&self) -> H256 {
merkle_root(
&self
.proposals
.iter()
.map(ProposalShortId::hash)
.collect::<Vec<_>>(),
)
}

ckb/core/src/uncle.rs

Lines 43 to 51 in de1cfc9

pub fn cal_proposals_root(&self) -> H256 {
merkle_root(
&self
.proposals
.iter()
.map(ProposalShortId::hash)
.collect::<Vec<_>>(),
)
}

My suggest is:

pub fn hash(&self) -> H256 {
    let mut h = H256::zero();
    h.as_bytes_mut()[..self.0.len()].copy_from_slice(&self.0[..]);
    h
}
@TheWaWaR

This comment has been minimized.

Copy link
Member

commented May 7, 2019

Add assert_debug! in from_bytes_with_hash to check hash consistent with the header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.