docs: Add evmAddress type to generateKey

[rwalworth]

This PR adds an evmAddress type to generateKey, which will allow the TCK to utilize the SDKs to generate EVM addresses for use in testing. It also removes protobufBytes as a parameter as the evmAddress type will be used to generate an alias instead.

Related issue(s):

Fixes #210

[thenswan]

I am confused what happens if type or fromKey is not provided, I understand that random key type should be returned then, but like what would be the use case for that?

[rwalworth]

There isn't a particular use case necessarily, it's mostly for developer convenience. If a test just requires any valid key but it doesn't matter if it's public, private, ED25519, or ECDSAsecp256k1, then the developer won't need to fill in those params but they'll get a usable key. fromKey helps in the case you need to get the associate public key of a private key, or the EVM address from an ECDSAsecp256k1 key. If fromKey is not provided, then the SDK server should assume a new key or EVM address should be generated.

[thenswan]

Understood.

then the developer won't need to fill in those params but they'll get a usable key

I am for deterministic way of testing in TCK, meaning that in this case there should be no randomness, only things specified by developer.

Would like to have opinions from other team members as well 🙂

[gsstoykov]

I agree with Rob on the fact that this can be there for developer convenience most of the times. But Nikita also has a point here because it makes sense to have a function called which does not depend on randomness when used in TCK scope. If a scenario exists in which this randomness can affect us we would have to remove this(So maybe it would be less risky to not have it).

(Example)
The developer gets used to that function providing him with a working key and gets lazy in specification. Tries to create TCK specifications for EthereumTransaction and signs with a random key(not ECDSA).

So if there is no risk to begin with maybe we would be better off.

[rwalworth]

If we want to remove the randomness that's fine, I mostly just wanted something where we didn't have to specify a type every time we call generateKey if it doesn't matter what the key is. Maybe we can just default to an ed25519PrivateKey if a type isn't specified?

[agadzhalov]

I would suggest to remove all the randomness and keep up with the deterministic way of testing. In my opinion everything in the TCK should be set explicitly.

[rwalworth]

If we add in utility.md that generateKey without a specified "type" will return an ed25519PrivateKey, do we think that would be explicit enough?

[agadzhalov]

I would go with setting the type explicitly every time. I know it's not convenient but it's way clearer and straightforward, for example you can easily read whether it's ECDSA or ED25519, without having to check the default type.

[rwalworth]

If that's what everyone prefers we can go that way, I'll add it to this.

Just a note, the functionality of returning a random key if no type was provided wasn't added in this PR, but in #200. There was even a question about it which was marked resolved (#200 (comment)) and the work was approved. I would ask going forward that these discussions/change requests take place in the PR the specifications were added. I've already done some work with the assumption that this was how we wanted to do it (since it was approved), so now I'm going to have to go back and "undo" that work. It isn't too much, but going forward we should definitely try to iron out these inefficiencies.

[agadzhalov]

@rwalworth that's a valid point you've raised. Just keep in mind we are four people reviewing TCK PRs and sometimes it's possible to miss some details or comments. We will be aware of this in the future!

[thenswan]

LGTM, thank you @rwalworth

[agadzhalov]

LGTM, tyvm @rwalworth