Getting AccountId/ContractId from an evm address is confusing after HIP-583 about hedera-sdk-reference HOT 12 OPEN

1. Agreeing with above opinions that fromSolidityAddress should be dropped in favor of fromEvmAddress overloads.
2. Also consider the following code. I assume developers are going to be doing this themselves a lot now? Why don't we do it for them with a single SDK call?

// identifying prefix for account-num alias (aka long-zero)
const LONG_ZERO_PREFIX = '0x000000000000000000000000'
let contractId;
// given a 20 byte string contractAddress of form 0x....
if (contractAddress.startsWith(constants.LONG_ZERO_PREFIX)) {
    // assumes 0 value shard and realm and converts adddress to num
    contractId = ContractId.fromSolidityAddress(contractAddress);
} else {
    // uses hex address instead of num in id
    contractId = ContractId.fromEvmAddress(0, 0, contractAddress);
}

from hedera-sdk-reference.

My thoughts

1. Add a fromEvmAddress(address) and fromEvmAddress(shard, realm, address) - former assumes 0 shard and realm. Both cases set a null not a 0 num
2. Deprecate the other 2 (fromEvmAddress() and fromSolidityAddress()) after some time
3. Explore a future Mirror Node query using api/v1/accounts/{address}. To be clear I'm not saying as part of fromEvmAddress but rather as a MAPI query, similar to HAPI queries.

from hedera-sdk-reference.

In my opinion,

1. fromAddress is a bit too generic and might confuse the user. I agree with @Nana-EC, it should be fromEvmAddress and we should deprecate fromSolidityAddress. fromEvmAddress should return 0.0.Number and then 0.0.evmAddress if the first one if not possible
2. I think we need to have a way for the user to convert EVM address to account/contract id through SDK. Maybe we can implement client.fromEvmAddress so you don't need to worry about specifying the network for now.

SDK talking to the mirror node is becoming unavoidable. The mirror node currently supports smart contract state queries now so I think that in the future the SDK should switch to query smart contract state from the mirror node instead of the consensus node so the users don't need to pay for the transaction fee.

from hedera-sdk-reference.

Gentle nudge on the conversation: Not all languages can do overloads (Namely Rust in my case, Go too out of the SDKs we have) nor default parameter arguments (again Rust and likely Go), although #128 (comment) does seem to at least partially address this.

from hedera-sdk-reference.

From an API design standpoint if we've addressed the issue and it scales I think we're good as is and should just deprecate fromSolidityAddress() since as you said fromEvmAddress() can handle both evm formats now.

from hedera-sdk-reference.

In my opinion, fromAddress is a bit too generic in term of naming. It would also cause a confusion.

from hedera-sdk-reference.

Note: This PR does the second sub point to the first issue! It only adds the checks inside the present methods and navigates the flow based on the input.

• Next steps should be to decide whether or not to create the generic method fromAddress() (and if we will deprecate the present ones), since the present methods have different inputs and how do we handle that?

• Also the fact that fromEvmAddress() sets a default 0 value for the num field, despite the fact that if you query the mirror node for the same evm address, you will get the actual num value.

from hedera-sdk-reference.

I am in favor of the one size fits all fromAddress that you talked about as the third option and then depreciation of the other methods. It simplifies everything and just works. Here here.

from hedera-sdk-reference.

One thing to consider in general is that in JS we do not have method overloading. So this is the reason why we ended up with so many different method signatures. The same thing should be considered here plus the fact that (I assume) we do not want to make any breaking changes, meaning that present methods fromEvmAddress() and fromSolidityAddress() should stay the same and only add handlers there if any and add new method for example fromAddress() and design it in a way that would handle all cases (because we are kind of running out of free method signatures already 😄)

@david-bakin-sl On your second point -> this check is already added to both methods fromEvmAddress() and fromSolidityAddress() so that even if users pass the wrong format address, the flow will be passed to the correct function

On the topic about getting the actual account num from the mirror node - I would agree that we should enhance and make a query which calls the mirror node and gets the actual num of the AccountId @Nana-EC @pathornteng

from hedera-sdk-reference.

• if we have generalized method fromAddress(evmAddress, shard = 0, realm = 0)
  note: input parameters should be reordered if we want for shard and realm to default to 0 because if they are before evmAddress (in JS) they cannot be optional and users should always pass a value for them

  • if evmAddress is a long-zero format → pass to fromSolidityAddress() method which parses the bytes to a proper shard, realm, num format for AccountId
  • otherwise generate AccountId with shard and realm from the input (if not set - assign 0 as default value) and 0 for num field and evmAddress from the input
    • log a message or add to method description that a 0 will be set for the num field and the user should query the mirror node for the actual num field. We can even throw an exception if a user tries to use the generated AccountId without the proper num value i.e "Account num is null, please query the mirror node and get the actual value"?

• design the Mirror node call

  • can take evmAddress as a string or AccountId object and query the mirror node with execute(client) so that we can get the network from the client and query the proper mirror node and after getting the result → automatically set the num from the mirror node to the num field of the passed AccountId object. An open question here would be if we add this functionality to the AccountId/ContractId objects as a method or a separate call... IMO an object method would be better

@mgarbs @Nana-EC @pathornteng @david-bakin-sl

from hedera-sdk-reference.

(w.r.t. reordering the parameters so shard+realm can be defaulted to 0: I suggest they be reversed from the standard definition to be address then realm then shard. It's already the case that the three components are all integers, the order is easily confused, IMO reverse is better than rotate.)

(Although this JS consideration of no overloading yet default-to-zero is painful in this regard: this signature with 3 consecutive longs, where we have to reverse it from the standard. Typing doesn't help so you can't just say it works fine in TypeScript. High possibility for confusion in the future. IDE hints or not. Gosh. Maybe two separate names is better, just not the ones we've got. And we don't even have shards+realms yet. Sigh. Something like fromAddress (1 arg version) and fromFullAddress or fromFullyQualifiedAddress (3 arg version)? (Whatever the name we give officially to the "full" shard+realm+number if we need to distinguish it from the current 0+0+number.) I dunno. This might be something ... requires even more consideration of the user POV over time?)

(Yet another possibility: use shard+realm+number in that order and everyone always has to type ...(0,0,addr) and that's just the way it is. Why not? Everything else we document and put into production (e.g., hashscan) always uniformly puts the 0.0.n version out there, never elides the 0.0. prefix. How hard would it be to just type the 0,0, when you need it? It would sure eliminate this confusion.)

(Proves once again that little-endian is the proper choice for almost everything, networking precedent aside ...)

from hedera-sdk-reference.

@pathornteng @Nana-EC @izik1 @david-bakin-sl @mgarbs
Continuing the conversation from above, currently, we do have:

• handlers in both methods so that the flow can be passed to the correct method
• MAPI call that queries the mirror node so that a user can get his actual num field when AccountId/ContractId is generated from an actual evmAddress

Shall we add a generic method fromAddress()?
It would be pretty hard to unify this method in languages throughout all SDKs, reasons:

No overloads -> JS, Rust, Go
No default/optional parameters -> Rust, Go

In JS we can design it like this: fromAddress(address, shard = 0, realm = 0) which will handle both evm address formats in Hedera..

Or we can leave it like this and just deprecate fromSolidityAddress() since fromEvmAddress() can handle both evm formats now!

from hedera-sdk-reference.