Replace Double-SHA256 with blake2b and implement domain seperation #1245
Conversation
|
Rebuild after update in master |
…h when there is 1 branch left
|
Rebuild after update in master |
2 similar comments
|
Rebuild after update in master |
|
Rebuild after update in master |
app/appmessage/p2p_msgtx.go
Outdated
| payloadHash = *hashes.HashData(payload) | ||
| writer := hashes.NewPayloadHashWriter() | ||
| writer.InfallibleWrite(payload) | ||
| payloadHash = *writer.Finalize() |
There was a problem hiding this comment.
Similar sets of three lines keep appearing in the code.
I would've preferred if you could bundle them together into convenience methods and used those instead.
domain/consensus/model/pow/pow.go
Outdated
| err = serialization.WriteElement(writer, timestamp) | ||
| writer := hashes.NewPoWHashWriter() | ||
| writer.InfallibleWrite(prePowHash[:]) | ||
| err := serialization.WriteElement(writer, timestamp) | ||
| if err != nil { | ||
| panic(errors.Wrap(err, "this should never happen. SHA256's digest should never return an error")) |
There was a problem hiding this comment.
Please update this error message.
domain/consensus/model/pow/pow.go
Outdated
| if err != nil { | ||
| panic(errors.Wrap(err, "this should never happen. SHA256's digest should never return an error")) | ||
| } | ||
| writer.InfallibleWrite(zeroes[:]) | ||
| err = serialization.WriteElement(writer, nonce) | ||
| if err != nil { | ||
| panic(errors.Wrap(err, "this should never happen. SHA256's digest should never return an error")) |
There was a problem hiding this comment.
Please update this error message.
| @@ -90,12 +82,12 @@ func merkleRoot(hashes []*externalapi.DomainHash) *externalapi.DomainHash { | |||
| merkles[offset] = nil | |||
|
|
|||
| // When there is no right child, the parent is generated by | |||
| // hashing the concatenation of the left child with itself. | |||
| // hashing the concatenation of the left child with zeros. | |||
There was a problem hiding this comment.
What are the benefits of this over concatenating the child with itself?
There was a problem hiding this comment.
This prevents having 2 separate inclusion proofs for the same thing in the same merkle tree
There was a problem hiding this comment.
And it is also the standard for how merkle trees are built, see: https://tools.ietf.org/html/rfc6962#section-2.1
| buf, err := vm.dstack.PopByteArray() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| vm.dstack.PushByteArray(hashes.HashData(buf)[:]) | ||
| hash := sha256.Sum256(buf) |
|
Rebuild after update in master |
This solves the following issues: #1204, #1188, #1186.
We chose Blake2b because of its speeds (See #1188 (comment)) we use blake2b's MAC for domain separation because golang x/crypto currently doesn't expose the
personalizationfeature (See golang/go#32447).We also fix the merkle tree bug by doing 2 things:
There's now a new file called
domains.gothat contains all the constructors for the domain separated writers. which should be the only way to hash in kaspad.if we ever need to hash something new we need to write a new constructor with the appropriate domain