atomicDEX-API icon indicating copy to clipboard operation
atomicDEX-API copied to clipboard

Refactor eth code: eliminate duplicate address to string conversion functions

Open dimxy opened this issue 11 months ago • 4 comments

Updated according to the discussion:

In eth.rs we have two public address to string conversion functions: eth_addr_to_hex() and display_eth_address(). PR #2261 also adds a trait AddrToString for the same purpose. Also, display_eth_address() provides address format with checksum, basically for user input verification.

The required changes:

  • Let's use the trait for getting string address for internal purposes and eliminate usage of functions eth_addr_to_hex and display_eth_address (make them private if we still need them).
  • Let's use display_eth_address() to return addresses in the API result (to the GUI side). Maybe it's good to implement it as a trait.
  • Also eliminate code like: format!("{:#02x}", token_contract_address) and use the unified approach with a trait.

dimxy avatar Feb 07 '25 09:02 dimxy

eliminate one of two functions eth_addr_to_hex and display_eth_address. (I think display_eth_address is what we need. Also let's make it private and use as implementation for the AddrToString::addr_to_string).

We shouldn't use display_eth_address instead of eth_addr_to_hex everywhere when we only need to convert eth Address type to a string. Using display_eth_address in such cases is inefficient, as it unnecessarily computes checksum_address each time, wasting resources.

eth_addr_to_hex and display_eth_address have their own purposes: https://github.com/KomodoPlatform/komodo-defi-framework/blob/39515a9f3ea1089bb462e99c8cafb1049a920dbd/mm2src/coins/eth.rs#L6577-L6587

So they are not duplicate conversion functions.

But it still worth making eth_addr_to_hex private and call addr_to_string (AddrToString trait method) instead.

laruh avatar Feb 10 '25 06:02 laruh

Using display_eth_address in such cases is inefficient, as it unnecessarily computes checksum_address each time, wasting resources.

Okay, let's ensure we use display_eth_address only for returned results and make this consistently (not like use both addresses with and w/o checksum in results)

BTW I can see the ethereum_types lib has implementation for 'Address' to string conversion. Let's use it maybe.

dimxy avatar Feb 10 '25 07:02 dimxy

BTW I can see the ethereum_types lib has implementation for 'Address' to string conversion. Let's use it maybe.

can you provide code part or screenshot so I can find it.

laruh avatar Feb 10 '25 08:02 laruh

BTW I can see the ethereum_types lib has implementation for 'Address' to string conversion. Let's use it maybe.

can you provide code part or screenshot so I can find it.

https://github.com/paritytech/parity-common/blob/d54ac847cc5447941390b5e9e5e3baef436e1451/fixed-hash/src/hash.rs#L232

dimxy avatar Feb 10 '25 08:02 dimxy