Add blake2 as built in hash function and make HashingAlgorithm non-exhaustive
#881
Conversation
ameba23
changed the title
HashingAlgorithm non-exaustiveHashingAlgorithm non-exhaustive
| @@ -16,6 +16,7 @@ At the moment this project **does not** adhere to | |||
| - Add a way to change program modification account ([#843](https://github.com/entropyxyz/entropy-core/pull/843)) | |||
| - Add support for `--mnemonic-file` and `THRESHOLD_SERVER_MNEMONIC` ([#864](https://github.com/entropyxyz/entropy-core/pull/864)) | |||
| - Add validator helpers to cli ([#870](https://github.com/entropyxyz/entropy-core/pull/870)) | |||
| - Add blake2 as built in hash function and make HashingAlgorithm non-exhaustive ([#881](https://github.com/entropyxyz/entropy-core/pull/881) | |||
There was a problem hiding this comment.
Im not sure if this counts as a breaking change. I think old JSON encoded signature requests will still work. But the shared type HashingAlgorithm has changed so strictly speaking this probably is breaking.
There was a problem hiding this comment.
Yeah this would be breaking for the entropy-shared crate since: a) we added a new field to a public enum and b) with the addition of non_exhaustive existing callers now need to handle the wildcard case.
We should also add this PR to the Added section.
| @@ -16,6 +16,7 @@ At the moment this project **does not** adhere to | |||
| - Add a way to change program modification account ([#843](https://github.com/entropyxyz/entropy-core/pull/843)) | |||
| - Add support for `--mnemonic-file` and `THRESHOLD_SERVER_MNEMONIC` ([#864](https://github.com/entropyxyz/entropy-core/pull/864)) | |||
| - Add validator helpers to cli ([#870](https://github.com/entropyxyz/entropy-core/pull/870)) | |||
| - Add blake2 as built in hash function and make HashingAlgorithm non-exhaustive ([#881](https://github.com/entropyxyz/entropy-core/pull/881) | |||
There was a problem hiding this comment.
Yeah this would be breaking for the entropy-shared crate since: a) we added a new field to a public enum and b) with the addition of non_exhaustive existing callers now need to handle the wildcard case.
We should also add this PR to the Added section.
No. It looks like the |
Blake2 is used to hash ECDSA signer payloads on substrate / polkadot. So i think it makes sense to have it as one of our built in hash functions. This PR does that.
Ive also marked the
HashingAlgorithmenum asnon_exhaustive. This means we handle the case that it was some unknown variant and return a user error if it was. The idea is to future proof - meaning we can add extra variants without breaking validators who didn't yet update their software. There are probably arguments against doing this, so im willing to take it out if people think its a bad idea.