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

fix(core)!: consensus hashing without extraneous length prefixes for each write #4420

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Aug 8, 2022

Description

  • Removes extraneous length prefixes for each write done in consensus encoding.
  • cucumber: Corrects hashing for metadata, kernel and script sigs in cucumber tests
  • cucumber: Adds missing minimumValuePromise value from metadata hash
  • cucumber: fix use of hex string instead of buffer in hash
  • new genesis for esme and igor
  • new faucet utxos for esme

Motivation and Context

Because the H.chain(a).chain(b) != H(a||b) property does not hold in domain-separated hashes, we cannot allow writes to call Digest::update on a domain hasher.

Prefixing the length for every element is redundant with ConsensusEncoding as it is canonical and well-defined (either fixed length, length prefixed, bit-prefixed (varint)).

The PR uses a hash domain (impl DomainSeparation) to prepend a domain label, but uses consensus encoding directly after that so that H.chain(a).chain(b) != H(a||b) holds.

Implementing the hashing in other languages, as in the cucumber tests, now becomes fairly trivial.

How Has This Been Tested?

Clear out mempool cucumber used for testing and passes confirming that all javascript hashes match the rust backend.

@sdbondi sdbondi force-pushed the integration-tests-fix-hashing branch from ceaab70 to c0ba271 Compare August 8, 2022 15:53
stringhandler
stringhandler previously approved these changes Aug 8, 2022
@aviator-app aviator-app bot added the mq-failed label Aug 8, 2022
@sdbondi sdbondi changed the title fix(core): consensus hashing without extraneous length prefixes for each write fix(core)!: consensus hashing without extraneous length prefixes for each write Aug 9, 2022
@sdbondi sdbondi force-pushed the integration-tests-fix-hashing branch from 59a8528 to 60a0126 Compare August 9, 2022 05:27
@aviator-app aviator-app bot removed the mq-failed label Aug 10, 2022
@aviator-app aviator-app bot merged commit 16ddc4e into tari-project:development Aug 10, 2022
@sdbondi sdbondi deleted the integration-tests-fix-hashing branch August 10, 2022 09:11
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