[Wallet repos] Prevent indexing accounts by name

[altafan]

At the moment the name of an account is used in the wallet repos as (primary) key to identify them in the storage.

This basically prevents the user to be allowed to change the name of an account but this is a feature we might want to add in combo with wallet restoration: in such a situation, it's possible that the user doesn't know or remember some or all the wallet accounts, therefore, ocean would assign some default name to the restored accounts. I think it would be handful for the user to have an rpc like ChangeAccountName({ old_name, new_name }) so the account's name can be changed just like it was nothing more than a label.

At the moment this is not possible because in every wallet repo implementation the account's name is used as its identifier, making such change hard to implement. The idea would be to use only the account index as identifier and "downgrade" the name as info just like the derivation path or any other domain.WalletAccount field.

[altafan]

@tiero @sekulicd do you have any thoughts?

[tiero]

We had oroginally picked the name, as a way to decouple "accounts" frlm derivation paths.

If we treat the name as "namespace" is evident is not a user choice to change since its binded to the use case/app is requesting keys.

What if we add an additional.field called label and keep name as primary key? (Namespace would be easier to understand)

[altafan]

If we treat the name as "namespace" is evident is not a user choice to change since its binded to the use case/app is requesting keys.

I didn't really get this.

The name of the account is chosen by the user at creation time and he uses the name to refer to the account instead of its derivation index. From an external POV, the account name is already used as a label because it lets the user refer to an account by something different than its derivation index, which is the only field that really makes an account unique.

From an internal POV, instead, I believe we made a mistake because we decided to treat the account name just like the derivation index, ie. something unique and immutable that won't ever change.

IMO it should only be something unique to prevent different accounts from sharing the same name, but we could allow the user to change an account's name if needed.

[sekulicd]

We can make account_index primary key autoincrement starting from 0 or 1, and set name as unique. This change will impact not just db layer as now index would be identifier and not account name, which means we would need to update from RPC layer to db layer.

[tiero]

user refer to an account by something different than its derivation index

This was the indeed reason we decided to do this, to NOT bind an "account" to a specific "BIP44-ish account", because not every script will have a standard BIP that defines a derivation path based on a BIP44 account. The account in this context is more of a "user-defined namespace", not a specific child in the bip32 node.

Think of external apps may want to load/create "accounts" at runtime, telling us the script template they want to use. They could tell us the base derivation path they want to use, if not we can default to the SLIP13 derivation using indeed the "namespace" to deterministically retrieve the base derivation path for each app/service. (what now are we calling names).

On this topic, we made further discussion after the initial Ocean spec, in Marina where we developed more in-depth the idea https://github.com/vulpemventures/marina/issues/250

A namespace is NOT a user-given description/label, which can be introduced eventually if we want as additional field for each account. Think of a namespace like the website ie. vulpem.com or a specific app like tdex-network/tdexd@v1 is requesting to create an account that is more of a "group of keys" used by that app. The user may want to archive it to not be shown in listing RPCs, but we are never going to rename it since can potentially lead to a different derivation path and therefore entirely different group of keys. User can create new "accounts" with a different versions for example

[altafan]

We can make account_index primary key autoincrement starting from 0 or 1, and set name as unique. This change will impact not just db layer as now index would be identifier and not account name, which means we would need to update from RPC layer to db layer.

I don't see any advantage in making the account index an autoincrement field, which, btw, is a very specific thing of relational dbs (I am speaking in a very general way when i speak of repos instead).

The next account account index is an info holded by the wallet domain and I do think it's ok just like it is and doesn't need to be modified.

If it's not a problem for ALL repo types to prevent indexing accounts by their names I think we should make this change sooner than later to basically allow the user to change account name at will.

We'll be able to see the benefit of these changes mainly for wallet restoration, where the user might not remember all the accounts and recognizes them after the restoration by looking at their balances. At that moment he'll be able to replace any default account name given by ocean with the preferred ones.

[altafan]

This was the indeed reason we decided to do this, to NOT bind an "account" to a specific "BIP44-ish account", because not every script will have a standard BIP that defines a derivation path based on a BIP44 account. The account in this context is more of a "user-defined namespace", not a specific child in the bip32 node.

Exactly, and that's what we've done, but I'm just trying to say that we maybe made an error in making the account name a field that cannot mutate in the future.

A namespace is NOT a user-given description/label, which can be introduced eventually if we want as additional field for each account. Think of a namespace like the website ie. vulpem.com or a specific app like tdex-network/tdexd@v1 is requesting to create an account that is more of a "group of keys" used by that app. The user may want to archive it to not be shown in listing RPCs, but we are never going to rename it since can potentially lead to a different derivation path and therefore entirely different group of keys. User can create new "accounts" with a different versions for example

Well, to be honest, the account name field was meant to be more a label than a "namespace", at least in my mind when I added the feature. In fact, it must be set by the user at account creation time, just like a label, instead of being calculated by ocean following some standard (SLIP13).

At the same time, I do see that except for this very last thing, from a different perspective the account name, as used now, sounds more like a "namespace" rather than a label.

What if we add an additional.field called label and keep name as primary key? (Namespace would be easier to understand)

This sounds like the best trade-off: let's add a new field Namespace (and calculate it programmatically with SLIP13). Namespace is the immutable field that must be used by the repo impls for indexing, while Name, instead, is mutable one that references to a key. This way there are no breaking changes, at least from the user POV (the matter requires breaking changes internally).

If the user doesn't specify a label for an account the default one account<account_index> is assigned and returned in the response message (requires proto changes), and can then be used to refer to it.

@tiero @sekulicd do you agree?

[tiero]

instead of being calculated by ocean following some standard (SLIP13).

Its not calculated, a namecpace it's chosen by the user anyway and it's used in the future to reference it other RPCs.

There will be most likely default "hardcoded" namespaces, like the one following the bips, which should likely have the namespacr as the name of the bip. Ie. "CreateBip84Acount" will default to m/84 base derivation path for example, the namespace its default and would not be chosen by user, in this case since "bag of keys" is derived from a known BIP. It can be called something like bip84-testnet.

We can make another default accounts , like the "tdex-fee-account@v0orbip84-account0@v0 whivh was based on assumption of a bip84 base derivation path and specific indexes for accounts.

Also, there is no production/release with ocean, fine to rename fields now etc...

If the user doesn't specify a label for an account the default one

Thr thing with label/descriptions is that it can be empty and would be totally fine, compared to namespace which is either chosen or picked by default.

[altafan]

"CreateBip84Acount" will default to m/84 base derivation path for example, the namespace its default and would not be chosen by user, in this case since "bag of keys" is derived from a known BIP. It can be called something like bip84-testnet.

We can make another default accounts , like the "tdex-fee-account@v0orbip84-account0@v0 whivh was based on assumption of a bip84 base derivation path and specific indexes for accounts.

CreateBip84Account is not needed since it is already supported by ocean, just in a different way: a BIP84 account is just a a BIP44 one with a very specific root path: m/84'/<network>'. Ocean has an env var OCEAN_ROOT_PATH that allows the user to change the wallet root path from the default one m/44'/<network>'. It's just enough to start ocean with for example OCEAN_ROOT_PATH=m/84'/1'to achieve the goal.

Thr thing with label/descriptions is that it can be empty and would be totally fine, compared to namespace which is either chosen or picked by default.

Yes exactly, and the user should be able to refer to an account by label, if set, or namespace otherwise. In the previous comment, by saying "calculated" I was literally meaning "picked by default". My idea is to allow the user to assign a mutable label to the account, while keeping the namespace immutable and determined by the wallet in a programmatic way. What do you think?