bug: tezos and eth eventstreams use invalid SHA256 invocation

[mcsaucy]

What happened?

https://github.com/search?q=repo%3Ahyperledger%2Ffirefly+sha256.New%28%29.Sum&type=code

sha256.New().Sum != sha256.Sum256. See https://go.dev/play/p/vSW0U3Hq4qk

I tried to make a PR but couldn't work out some test failures. Additionally, I don't have enough context to know whether this is a breaking change, so I'm just gonna file this issue instead.

What did you expect to happen?

I expected vars like "unique hash" to have unique hashes in them. Instead, they have the raw plaintext with SHA256("") appended to the end.

How can we reproduce it (as minimally and precisely as possible)?

Ya just gotta switch from sha256.New().Sum to sha256.Sum256. Note that you may need to throw that in a variable and then convert the byte array to byte slice with [:].

See kubernetes-sigs/bom#524 for a similar PR.

Anything else we need to know?

No response

OS version

# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here

[EnriqueL8]

Thanks for raising @mcsaucy

[EnriqueL8]

I think we do the correct thing here https://github.com/kaleido-io/firefly-common/blob/19e0a20127396cdb4e6a1e2e3015c9b6a2bc6c4c/pkg/fftypes/bytetypes.go#L47 @mcsaucy ?

[mcsaucy]

Yeah that invocation looks right.

[EnriqueL8]

Moving to using that then!

[EnriqueL8]

This change has quite a lot of migration implications unfortunately...

We use this hash to check for unique subscriptions for event streams against the connector, which means we will reprocess all the events from start again since this will create a new subscription with the new hash in the name! Which means we would need migration code to delete the old subscription and to be honest the value that is being hashed is the address of the contract which is already unique per chain so I think this change is not needed

[mcsaucy]

That makes sense. Given the problem space, it seemed likely this could be a load-bearing bug and it looks like that's the case. As you've stated, the incorrect usage is still perfectly accurate for comparisons.