Remove the idea of virtual addresses and replace with alias(es) #259
Conversation
Signed-off-by: lukelee-sl <[email protected]>
lukelee-sl
requested review from
shemnon,
rbair23,
steven-sheehy,
tinker-michaelj and
Nana-EC
lukelee-sl
changed the title
There was a problem hiding this comment.
One additional thing to consider: since this PR is dealing with the changes introduced in #242, one of the changes previously introduced was to add evm_address field to transaction_record.proto.
I am not sure if we want to remove that as well, just mentioning it so that it is not accidentally left out.
| @@ -104,15 +104,6 @@ message AccountID { | |||
| * in that account, without creating anything, and with no creation fee being charged. | |||
| */ | |||
| bytes alias = 4; | |||
|
|
|||
| /** | |||
| * The ethereum account 20-byte EVM address to be used initially in place of the public key bytes. This EVM | |||
There was a problem hiding this comment.
This description was taken from the description of the alias field above, we should move it back there since we will be expecting EVM address values in the alias field.
| @@ -167,9 +167,4 @@ message CryptoCreateTransactionBody { | |||
| * simply be deposited in that account, without creating anything, and with no creation fee being charged. | |||
| */ | |||
| bytes alias = 18; | |||
|
|
|||
| /** | |||
| * EOA 20-byte address to create that is derived from the keccak-256 hash of a ECDSA_SECP256K1 primitive key. | |||
There was a problem hiding this comment.
Similar to the comment in basic_types.proto the description of the alias field above could be updated if we decide that we are going to support CryptoCreate with EVM address values in the alias field i.e. CryptoCreates for hollow accounts (still in discussion).
Signed-off-by: lukelee-sl <[email protected]>
@Nana-EC This is a good question. Should we update the name of the field to |
services/crypto_get_info.proto
Outdated
| bytes alias = 1; | ||
|
|
||
| /** | ||
| * Flag if this alias should now be set or is the default alias on the account. |
There was a problem hiding this comment.
Only one alias per account is allowed. We should not add this flag to the protobufs
From the HIP:
As with `HIP-32`, there can only
be a single `alias` per account (except for the `long-zero alias` which all accounts have), and it is immutable, and
therefore an account that has an `alias` cannot have it updated.
Signed-off-by: lukelee-sl <[email protected]>
We populate |
Yeah Mirror Node requested this so it explicitly had the value. This will now only be applicable when an ECDS key alias is set. I believe this prevented mirror node from having to duplicate the calculation logic in the services |
Nana-EC
requested review from
steven-sheehy,
stoqnkpL and
MiroslavGatsanoga
and removed request for
stoqnkpL and
MiroslavGatsanoga
|
For future reference, as claimed by |
Signed-off-by: lukelee-sl [email protected]
Description:
HIP 583 and HIP 631 introduce the idea of virtual addresses. Upon discussion, it was decided
to try to replace virtual addresses by extending the existing alias(es).
Related issue(s):
Notes for reviewer:
Checklist