`add_account` should track the tag associated with the `Address`

[mmagician]

Address-derived vs. AccountId-derived NoteTag

The tag derived from an Address is (potentially) different than the tag derived from an AccountId.
Currently, when we add a new account to the client store, we fall back to AccountId-derived tag (via Account -> NoteTagRecord conversion): https://github.com/0xMiden/miden-client/blob/ba14ca5e765c7c81c45a56a7114b0a156374387b/crates/rust-client/src/account/mod.rs#L155

But, client.add_account doesn't provide any option to specify the Address as a parameter.
There are two paths:

1. Easy path: inside add_account, derive the "default" Address::AccountIdAddress by passing in the AccountId and AddressInterface::Unspecified.
2. New interface: let users add Addresses, for example by exposing a new add_address method, similar to add_account. Potentially deprecate add_account.
3. Change the interface of add_account to accept AccountIdAddress instead of AccountId. This is akin to what @PhilippGackstatter suggested in create_p2id_note should take Address as parameter (or should it?) protocol#1837 (comment).
4. Other ideas?

These approaches would further require adding a new enum variant Address to NoteTagSource (or potentially change NoteTagSource::Account to NoteTagSource::AccountIdAddress if we choose option 3).

---

Query the store for NoteTags based on the source.

This part could be potentially to be split out into a separate issue, depending on the decisions above^.

It would also be great if we could query the store for tags coming from a specific source.
This would allow us to limit the amount of notes downloaded from the note transport layer to only Address-derived tags. So in Fetch notes section here, instead of:

let note_tags: BTreeSet<NoteTag> = self.store.get_unique_note_tags().await?;
// query the gRPC node for these note_tags AND query the transport layer for these tags

we could define a new store-querying source interface: NoteTagStoreSource:

pub enum NoteTagStoreSource { // name TBD
    Address(AddressSource),
    Note(NoteSource),
    User(UserSource),
}

pub enum AddressSource {
    All,
    Exact(Address)
}

// etc. for NoteSource, UserSource

And then query as:

let public_note_tags: BTreeSet<NoteTag> = self.store.get_unique_note_tags().await?; // this doesn't change, gets all note tags
// query the gRPC node for public_note_tags

let public_note_tags: BTreeSet<NoteTag> = self.store.get_unique_note_tags_from_source(
    NoteTagStoreSource::Address(AddressSource:All)
).await?;
// query the transport layer for private note tags

[bobbinth]

• New interface: let users add Addresses, for example by exposing a new add_address method, similar to add_account. Potentially deprecate add_account.
• Change the interface of add_account to accept AccountIdAddress instead of AccountId. This is akin to what @PhilippGackstatter suggested in create_p2id_note should take Address as parameter (or should it?) miden-base#1837 (comment).

We can't really deprecate Client::add_account() or make it take AccountIdAddress because this method take a full account object. We could modify and also add add_address() method to ensure we handle addresses properly (see below).

In general, to handle addresses properly, I think we'll need to make them "first-class citizens" in the client. This implies the following:

• We need to have a way to associate multiple addresses with a single account in the client's database. This probably requires some new methods on the store to retrieve/add/remove account addresses.
  • For this, we'd probably need modify AccountRecord to include addresses field into it.
• This would also require changes in the client interface. For example:
  • We may want to modify Client::add_account() to take a list of addresses to associate with the account.
  • We may want to add Client::add_address() method that would add an address to an existing account.
  • We may want to add Client::remove_address() method that would remove an address from an existing account.

When an address gets added to an account, we would need to add the corresponding tag to the set of tracked note tags for that account (and the opposite would need to happen when we remove an address from an account).

These changes can be implemented gradually over a couple of PRs. For example:

• At first, we'd make the underlying database/store changes. At this point, all accounts would get a single default address (with the Unspecified interface) upon creation.
• Then, we'd update the client to allow modifying addresses attached to an account - i.e., Client::add_address() and Client::remove_address() methods.
• Then we'd update the Client::add_account() method to also take a list of addresses to associate with the account upon creation (and here we can also decide whether we still want to have the default address with the Unspecified interface created).

One thing I think we should try to maintain at this point is one-to-many relationship between accounts and addresses/tags. That is the same address (and the resulting tag) should always map to a single account in the client. The question to answer here is how do we deal with the situation where two accounts have the same 16 bit prefixes.

---

It would also be great if we could query the store for tags coming from a specific source.
This would allow us to limit the amount of notes downloaded from the note transport layer to only Address-derived tags. So in Fetch notes section here,

Is the idea here that if a tag was not derive from an address, even if we get a note with this tag, we won't be able to decrypt it (or otherwise make sense of it)? Or is the main idea that we want to have different tags for public and private notes?

[mmagician]

We can't really deprecate Client::add_account() or make it take AccountIdAddress because this method take a full account object

Yes good point 👍🏼 I agree with this progressive approach. I'd also skip updating the Client::add_account() to take in a list: the same functionality can be achieved by chaining add_account() and a few add_address() calls, if needed. I've created sub-issues for tracking the progress.

---

Is the idea here that if a tag was not derive from an address, even if we get a note with this tag, we won't be able to decrypt it

Due to how our transport layer API is defined, the assumption is that all (encrypted) notes downloaded from the transport layer are going to have address-derived tags (AccountIdAddress for now, in the future other variants).

A user should never expect a user-defined tag to appear on the transport layer, so if there was a tag added manually for tracking in the client, and that same tag appears on the transport layer, then it's only by chance - it would not be decryptable, and so the client should not fetch it in the first place.

---

The question to answer here is how do we deal with the situation where two accounts have the same 16 bit prefixes.

The easiest solution is to disallow adding two such accounts to the client in the first place. I can't think of any downsides to this - and if someone really wants to have two such accounts, they could simply instantiate two different clients.

[bobbinth]

Yes good point 👍🏼 I agree with this progressive approach. I'd also skip updating the Client::add_account() to take in a list: the same functionality can be achieved by chaining add_account() and a few add_address() calls, if needed. I've created sub-issues for tracking the progress.

Yes, agreed! I think I was thinking of not assigning default addresses at some point in the future (which would require specifying address on account creation) - but this could also be achieved in other ways.

Due to how our transport layer API is defined, the assumption is that all (encrypted) notes downloaded from the transport layer are going to have address-derived tags (AccountIdAddress for now, in the future other variants).

A user should never expect a user-defined tag to appear on the transport layer, so if there was a tag added manually for tracking in the client, and that same tag appears on the transport layer, then it's only by chance - it would not be decryptable, and so the client should not fetch it in the first place.

I'm thinking this may be highlighting a different issue. Is there really a need for for a user to define "custom" tags? All tags should be derived from addresses in one way or the other. Right now we have AccountIdAddress, but to allow users to specify custom tags, we'd need to add something like CustomAddress variant.

So, maybe as we add add_address() and remove_address(), add_tag() and remove_tag() functionality should go away. @igamigo - what do you think?

The easiest solution is to disallow adding two such accounts to the client in the first place. I can't think of any downsides to this - and if someone really wants to have two such accounts, they could simply instantiate two different clients.

Yes, agreed. I think this could happen randomly when a user tries to create several new accounts - but it is probably easier to handle this edge case at account creation time rather than all the time.

[mmagician]

All tags should be derived from addresses in one way or the other.

I suppose it's up to us to make that call.

I'm trying to come up with use cases for user defined tags. For example: PaymentRequest. There, the user would specify a custom (likely random) tag. While in this specific use case, the client would ideally watch the network for note ID, rather than a concrete tag. And even if not, we could easily have a PaymentRequest-derived NoteTag (so introducing another variant NoteTagStoreSource::PaymentRequest, or even making an Address::PaymentRequest variant).

So based on the outcome of the above little thought experiment, I tend to agree we could deprecate {add\remove}_tag unless there's a counter use case.

[igamigo]

While in this specific use case, the client would ideally watch the network for note ID, rather than a concrete tag.

Could a valid use-case here be sending swap notes to a specific address that signals (for example) USDC/MIDEN swaps?

So, maybe as we add add_address() and remove_address(), add_tag() and remove_tag() functionality should go away. @igamigo - what do you think?

In terms of client API it's always nice to remove calls as it simplifies things. The concept of addresses and how they link to accounts has been confusing for users already but if they are more at the forefront it might be easier to grasp. However I'm not sure it's worth it if we have to add a CustomAddress variant. Also how would that work with the "one-to-many" account<->address relation?

One thing I think we should try to maintain at this point is one-to-many relationship between accounts and addresses/tags. That is the same address (and the resulting tag) should always map to a single account in the client. The question to answer here is how do we deal with the situation where two accounts have the same 16 bit prefixes.

I do agree that there are virtually no downsides to disallowing 2 accounts with conflicting prefixes, but I'm not too sure why this is a problem in the first place.

[mmagician]

Could a valid use-case here be sending swap notes to a specific address that signals (for example) USDC/MIDEN swaps?

So this is not a payment per-se, nor an interaction with a specific account, but rather a use-case-derived tag: the user is signaling an action they want to perform, and the recipient could be any account willing to perform SWAPs. Yes I think that covers it actually 👍🏼

[bobbinth]

Is this issue now mostly covered by #1270 and #1271? Or is there anything not captured by these? Maybe:

• Renaming NoteTagSource::Account to NoteTagSource::Address (or maybe this would be a part of one of the above issues?).
• Disallowing the client from tracking accounts with similar account ID prefixes - though this will probably be tricky.
• Anything else?

Aside from these, we should probably close this issue.

[mmagician]

We still have the second part of the OP regarding Query the store for NoteTags based on the source

Renaming NoteTagSource::Account to NoteTagSource::Address (or maybe this would be a part of one of the above issues?).

I don't think it would be covered by #1270 or #1271 , but it would be covered by the mentioned second part^

Disallowing the client from tracking accounts with similar account ID prefixes - though this will probably be tricky.

Created an issue for this: #1290

---

We can either use this issue as a tracking issue and/or to address Query the store for NoteTags based on the source