0xPolygonMiden / miden-client

Client library that facilitates interaction with the Miden rollup
MIT License
29 stars 25 forks source link

Add token mapping to the client to show assets in a more user-friendly way #258

Closed Dominik1999 closed 3 days ago

Dominik1999 commented 3 months ago

Feature description

Instead of sending of holding 50 0x12314 I would love to see 50 ETH in the CLI.

We could introduce a simple token mapping, or, assuming all faucets are on-chain, the client could get the token metadata from the faucet itself.

Why is this feature needed?

This is to be more user-friendly.

bobbinth commented 3 months ago

This may be related to (or dependent on) what we decide to do in https://github.com/0xPolygonMiden/miden-base/issues/140#issuecomment-2053579666.

mFragaBA commented 3 months ago

Indeed, if the ticker symbol is encoded within the account ID it get's pretty straightforward

bobbinth commented 2 months ago

Indeed, if the ticker symbol is encoded within the account ID it get's pretty straightforward

For a variety of reasons, we finally decided not to encode ticker symbol into the ID. So, now, we have broadly two ways to obtain the mapping:

  1. We can get the ticker symbol from account storage (in the current implementation of the basic faucet, this is stored in slot 1). There are two caveats to this: a. For public faucets this can be looked up directly on chain - however, for private faucets, account state would need to be provided via some kind of a side channel. b. We need to have some kind of a generic way of getting the ticker symbol (e.g., not assume that it is stored in slot 1). Maybe for this we should add a procedure to the basic faucet account code - something like get_ticker_symbol.
  2. The above approach would not provide an absolute mapping as two faucets could have the same exact ticker symbol stored in their storage (I'm curious if there is an accepted way to deal with this in Ethereum and other chains) - so, we may need to provide some kind of mapping manually (e.g., via a config file).
mFragaBA commented 2 months ago

Indeed, if the ticker symbol is encoded within the account ID it get's pretty straightforward

For a variety of reasons, we finally decided not to encode ticker symbol into the ID. So, now, we have broadly two ways to obtain the mapping:

1. We can get the ticker symbol from account storage (in the current implementation of the basic faucet, this is stored in slot 1). There are two caveats to this:
   a. For public faucets this can be looked up directly on chain - however, for private faucets, account state would need to be provided via some kind of a side channel.
   b. We need to have some kind of a generic way of getting the ticker symbol (e.g., not assume that it is stored in slot 1). Maybe for this we should add a procedure to the basic faucet account code - something like `get_ticker_symbol`.

2. The above approach would not provide an absolute mapping as two faucets could have the same exact ticker symbol stored in their storage (I'm curious if there is an accepted way to deal with this in Ethereum and other chains) - so, we may need to provide some kind of mapping manually (e.g., via a config file).

Instead of relying on a config file, couldn't we just introduce a faucet_alias table and add_alias/remove_alias functions in the client and commands on the CLI? It feels more of a client problem, rather than an account problem. Is there some use case where I'd want to use the ticket symbol from the account's storage?

bobbinth commented 2 months ago

Having it as a separate table may be fine - but one thing to consider is how users will add/maintain these mappings. Specifically, I'm imagining that at some point we'll have many such mappings (dozens or maybe even hundreds). Importing them manually one by one probably wont't be feasible. So, we probably need a way to import from a file of some sort (could be a simple text file with two columns - one column for account ID and the other one is for the symbol).

On the library side, we can probably have a method on the Client to set faucet mappings and this method could work as a "bulk update" of the relevant table in the store.

On the CLI side, if we are going to be importing from a file, it may make sense to make this file a "source of truth". For example, we could have faucet_symbols.txt file which gets parsed and the contents inserted into the client on startup. Though this may be too much extra work and maybe we should do something simpler here.

One other thing: we may want to do some basic validation (e.g., associating ETH symbol with a faucet that has BTC in defined as its symbol in account storage should result in an error) - but not sure how much complexity it would add.

bobbinth commented 2 months ago

One alternative approach is to leave faucet symbol mapping entirely to the CLI (i.e., the Client would work purely with faucet IDs). This may be a good way to do an initial implementation, and then we can later move it into the Client struct if we decide that this is a core part of the client functionality.

One potential benefit of this approach is that we leave the choice of how to map faucets to symbols to the specific users of the client (e.g., the way it works on the CLI may be different from how it works in the browser or in a mobile app etc.).

mFragaBA commented 2 months ago

One other thing: we may want to do some basic validation (e.g., associating ETH symbol with a faucet that has BTC in defined as its symbol in account storage should result in an error) - but not sure how much complexity it would add.

We can do that for accounts tracked by the client but not for external accounts though

mFragaBA commented 2 months ago

One alternative approach is to leave faucet symbol mapping entirely to the CLI (i.e., the Client would work purely with faucet IDs). This may be a good way to do an initial implementation, and then we can later move it into the Client struct if we decide that this is a core part of the client functionality.

One potential benefit of this approach is that we leave the choice of how to map faucets to symbols to the specific users of the client (e.g., the way it works on the CLI may be different from how it works in the browser or in a mobile app etc.).

Yeah, thinking about it, it does seem more of a CLI feature than a Client feature as I think the main use case is to avoid writing long IDs on CLI commands

bobbinth commented 2 months ago

One other thing: we may want to do some basic validation (e.g., associating ETH symbol with a faucet that has BTC in defined as its symbol in account storage should result in an error) - but not sure how much complexity it would add.

We can do that for accounts tracked by the client but not for external accounts though

This should be possible for any public account (we can always make a request to the GetAccountDetails endpoint to get account info). For private (off-chain) accounts, this would not work though.

bobbinth commented 3 days ago

Closed by #441.