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

feat!: add domain consensus hashing to all previous uses of default consensus hashing #4522

Merged

Conversation

hansieodendaal
Copy link
Contributor

@hansieodendaal hansieodendaal commented Aug 23, 2022

Description

  • Added domain hashing to all previous uses of default consensus hashing.
  • Changed output_mr, witness_mr, kernel_mr and input_mr types in pub struct BlockHeader to be FixedHash instead of variable length BlockHash.

Motivation and Context

As per issue #4396.

How Has This Been Tested?

  • Passed all unit tests
  • Passed all cucumber tests

base_layer/core/src/blocks/block_header.rs Outdated Show resolved Hide resolved
base_layer/core/src/blocks/block_header.rs Show resolved Hide resolved
base_layer/core/src/blocks/block_header.rs Outdated Show resolved Hide resolved
@@ -364,18 +368,67 @@ pub(crate) mod hash_serializer {
}
}

pub(crate) mod fixed_hash_serializer {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange place for this. Shouldn't this live with FixedHash ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in the same place as pub(crate) mod hash_serializer

When trying to move:
Module tari_core::blocks::block_header::fixed_hash_serializer uses module tari_core::blocks::block_header which will be inaccessible after move

base_layer/core/src/blocks/block_header.rs Outdated Show resolved Hide resolved
base_layer/core/src/chain_storage/blockchain_database.rs Outdated Show resolved Hide resolved
base_layer/core/src/proto/block_header.rs Outdated Show resolved Hide resolved
- Added domain hashing to all previous uses of default consensus hashing.
- Changed `output_mr`, `witness_mr `kernel_mr` and `input_mr` types in
  `pub struct BlockHeader` to be `FixedHash` instead of variable length
  `BlockHash`.
- Fixed unit tests.
- Fixed cucumber tests.
@hansieodendaal hansieodendaal changed the title [wip] feat!: add domain consensus hashing to all previous uses of default consensus hashing feat!: add domain consensus hashing to all previous uses of default consensus hashing Aug 23, 2022
@stringhandler stringhandler merged commit 1885509 into tari-project:development Aug 23, 2022
@hansieodendaal hansieodendaal deleted the ho_consensus_default branch August 23, 2022 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants