Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTH Custom Memo Types #60

Closed

Conversation

dolanbernard
Copy link
Contributor

@dolanbernard dolanbernard commented Feb 3, 2023

# Unresolved questions
[unresolved-questions]: #unresolved-questions

- Would it be beneficial to have two different Custom Memo types: one for the payload `TxOut` and one for the change `TxOut`? Maybe the memo builder can accept a 'data' field and a 'change data' field. If the 'change data' field is not set, then the 'data' field is used for both outputs.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about this, the more I think it is a good idea. But should the type be 0x0400 or 0x0301?

Copy link
Contributor

@cbeck88 cbeck88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I get what you are trying to do, and I think the proposal is good in terms of achieving what it's trying to do.

But, I'm not sure this is actually a memo-type that we should standardize. Let me try to explain.

  1. Nothing can really stop clients from putting whatever they want in the memo field, but if they do that, then when users move their mnemonic between wallets, the new client may not understand other memos, or, a user e.g. in Moby sending funds to desktop wallet may result in the receiving client not having the memo.
  2. The reason we standardize memos in MCIPs is so that clients can achieve this interoperability, and there can be some authoritative spec for "if you get a memo with this type, this is how you should interpret it".

If we standardize "custom memo types" it basically means, we are giving up on this interoperability -- because there is no defined interpretation or handling for these memo types.

So I guess I don't see why it is useful to standardize this. Maybe as a template for how someone makes an authenticated memo?


Let me ask a meta question -- are you finding the process of standardizing memos too onerous? Is there something we can do to streamline this process or improve it? There is no reason it has to be tied to mobilecoin network release cycle or anything. I think it's also fine if memos are standardized and defined in clients and not necessarily implemented in mobilecoin.git.

I'd sort of prefer if instead of standardizing "custom memo types", we continue to try to assign memo type bytes for specific purposes with specific defined authentication and interpretation. But I'm willing to be convinced otherwise.

@dolanbernard
Copy link
Contributor Author

So, I get what you are trying to do, and I think the proposal is good in terms of achieving what it's trying to do.

But, I'm not sure this is actually a memo-type that we should standardize. Let me try to explain.

1. Nothing can really stop clients from putting whatever they want in the memo field, but if they do that, then when users move their mnemonic between wallets, the new client may not understand other memos, or, a user e.g. in Moby sending funds to desktop wallet may result in the receiving client not having the memo.

2. The reason we standardize memos in MCIPs is so that clients can achieve this interoperability, and there can be some authoritative spec for "if you get a memo with this type, this is how you should interpret it".

If we standardize "custom memo types" it basically means, we are giving up on this interoperability -- because there is no defined interpretation or handling for these memo types.

So I guess I don't see why it is useful to standardize this. Maybe as a template for how someone makes an authenticated memo?

I agree with your concerns, but I think there is more to consider.

We already have no guarantee of interoperability. To use your example, if a user has the same account key for both desktop wallet and Moby, transactions sent to their desktop wallet address won't show up in Moby. It won't show up in transaction history nor will Moby even be able to update the balance to show the money is there. The wallets will report two different balances. Furthermore, Fog wallets using different Fog services aren't interoperable already. So interoperability by mnemonic alone is not something we've ever been able to guarantee.

I would also argue that a standard, non-standard memo type offers a standard way to ignore data that your application does not understand.

@dolanbernard
Copy link
Contributor Author

Let me ask a meta question -- are you finding the process of standardizing memos too onerous? Is there something we can do to streamline this process or improve it? There is no reason it has to be tied to mobilecoin network release cycle or anything. I think it's also fine if memos are standardized and defined in clients and not necessarily implemented in mobilecoin.git.

I'd sort of prefer if instead of standardizing "custom memo types", we continue to try to assign memo type bytes for specific purposes with specific defined authentication and interpretation. But I'm willing to be convinced otherwise.

So, I don't necessarily think the MCIP process itself is usually particularly slow. And it is true that new memo types don't need new network releases. They are still blocked by several other things before they can make it into a client, however. After passing the MCIP process, the next blocker is MobileCoin engineering capacity. New memo types need to be implemented by a MobileCoin engineer and then pass the PR process. Then they need to wait for a MobileCoin release. At this point non-mobile clients can use them. But mobile clients still need to wait for SDK implementations. And after SDK implementations pass the PR process, they still are blocked until a new SDK release. I'm not really sure if/how this process can be optimized, but it is worth discussing.

@cbeck88
Copy link
Contributor

cbeck88 commented Feb 12, 2023

We already have no guarantee of interoperability. To use your example, if a user has the same account key for both desktop wallet and Moby, transactions sent to their desktop wallet address won't show up in Moby. It won't show up in transaction history nor will Moby even be able to update the balance to show the money is there. The wallets will report two different balances. Furthermore, Fog wallets using different Fog services aren't interoperable already. So interoperability by mnemonic alone is not something we've ever been able to guarantee.

So here are my thoughts on this:

We cannot have a guarantee of interoperability -- what we can have is, a series of rules that, if wallets play by the rules, they can interoperate.

It's true that moby has the parallel Moby Transaction History system, which they built to get around memo size limitations.

Henry and many others feel strongly that Moby SHOULD still write the normal, compatible RTH memos, in order to interoperate if you export your wallet to desktop wallet. To the extent that they aren't doing that, I consider that a bug.

It won't show up in transaction history nor will Moby even be able to update the balance to show the money is there.

To the extent that Moby lets you import your account from a mnemonic, they SHOULD still interpret the normal RTH memos that may be there, in case there is useful information. To the extent that they don't do that, that's a defect in the implementation of Moby, which is completely fixable.

To the extent that, if you import your account to desktop wallet, and it doesn't also attach proper encrypted fog hints to change outputs, that may cause the wrong balance. That's a bug, and we have a plan to fix that.

Furthermore, Fog wallets using different Fog services aren't interoperable already.

That's true. However, there's a "one fog" initiative where we are considering merging the fog databases and making it so that there is only one fog deployment. Then when there is only one Fog, I think the plan is that when you import from Desktop wallet, it will attempt to check your balance using the one Fog, and if it finds records, it decides that this must be a fog account. So I'm hopeful that this interoperability will improve.

I would also argue that a standard, non-standard memo type offers a standard way to ignore data that your application does not understand.

So like, if you're a client and you want to make a new memo type, the envisioned process is like, you make an MCIP PR reserving the type bytes for your purposes, and then any other apps that don't care about your memo type, simply don't implement support for that memo type. And then they can detect "this is a memo that I definitely don't understand", but later if they want to implement support for it, there is a clear path to doing so.

Like, essentially any memo type bytes that aren't listed in this repo somewhere represent a "nonstandard memo type" . From the client's point of view, any type bytes it doesn't know about / have a handler for is a "nonstandard memo type" where it can't interpret the data. So what exactly do we gain by codifying that some particular type bytes are "nonstandard" in the way proposed here?

@cbeck88
Copy link
Contributor

cbeck88 commented Feb 12, 2023

And it is true that new memo types don't need new network releases. They are still blocked by several other things before they can make it into a client, however. After passing the MCIP process, the next blocker is MobileCoin engineering capacity. New memo types need to be implemented by a MobileCoin engineer and then pass the PR process. Then they need to wait for a MobileCoin release. At this point non-mobile clients can use them. But mobile clients still need to wait for SDK implementations. And after SDK implementations pass the PR process, they still are blocked until a new SDK release. I'm not really sure if/how this process can be optimized, but it is worth discussing.

So like, if there were like a Moby or Signal-specific memo type that only made sense for a feature that only they have, I don't think it's necessary to follow this whole process. IMO it would be enough to make an MCIP saying "Hey, we want to use these bytes this way", and then either only implement it in Moby (or maybe SDK?). But it doesn't HAVE to go into the mobilecoin.git repo, or wait for a release of that, like that isn't a blocker IMO. The purpose of putting things in mobilecoin.git is to share them across many clients. And maybe like, later, someone might decide it's interesting for desktop wallet or some other client to also know about this memo type and be able to read it, so maybe at that point we might add it to mobilecoin.git.

But IMO, choosing to use the "custom memo type" for some new memo, should actually be just as easy as just making a new formal memo type instead. Like, it would still be fine to only write moby-specific code that writes and reads it, the only difference is that you make an MCIP documenting it, and reserving the type bytes so no one else accidentally uses the same ones. And then you actually have more data to play with potentially.

So like, one barrier to that is, we don't have a memo builder in mobliecoin.git right now that just lets you do whatever you want with the memo bytes. But that's mainly because no one asked for that. But I'd be happy to make a PR that creates that, or approve a PR that makes one like that.

LMK if that all makes sense and if you still think the custom memo is a good direction

@cbeck88
Copy link
Contributor

cbeck88 commented Feb 12, 2023

The other thing I want to say is that like, the reason we want to work for this interoperability is, cryptocurrency users expect to be able to move their mnemonic from wallet to wallet, and they are surprised and frustrated when it doesn't work. Like, the difficulties of porting your entropy around represent a pretty negative user experience right now, and while we indeed created the mess that we have right now, there are at least some in product that want to fix it among the various mobilecoin clients. So I don't think we're all aligned right now on just giving up on this interoperability, we actually want to try to go back and fix the problems.

When Alan was still here for instance, he was very much in the mindset of "let's build Moby in a way that works exactly the way we want it to and not really care about interoperability, and focus on shipping fast and getting to product market fit", and he had little interest in using the blockchain memos at all. And like, I'm not saying that thinking was necessarily wrong. But in the longer term I think product wants to have interoperability, because it's a bad user experience if these things don't interoperate.

@eranrund
Copy link
Contributor

eranrund commented Feb 13, 2023

So like, if there were like a Moby or Signal-specific memo type that only made sense for a feature that only they have, I don't think it's necessary to follow this whole process. IMO it would be enough to make an MCIP saying "Hey, we want to use these bytes this way", and then either only implement it in Moby (or maybe SDK?). But it doesn't HAVE to go into the mobilecoin.git repo, or wait for a release of that, like that isn't a blocker IMO. The purpose of putting things in mobilecoin.git is to share them across many clients. And maybe like, later, someone might decide it's interesting for desktop wallet or some other client to also know about this memo type and be able to read it, so maybe at that point we might add it to mobilecoin.git.

But IMO, choosing to use the "custom memo type" for some new memo, should actually be just as easy as just making a new formal memo type instead. Like, it would still be fine to only write moby-specific code that writes and reads it, the only difference is that you make an MCIP documenting it, and reserving the type bytes so no one else accidentally uses the same ones. And then you actually have more data to play with potentially.

So like, one barrier to that is, we don't have a memo builder in mobliecoin.git right now that just lets you do whatever you want with the memo bytes. But that's mainly cause no one asked for that. But I'd be happy to make a PR that creates that, or approve a PR that makes one like that.

LMK if that all makes sense and if you still think the custom memo is a good direction

I think this summarizes my thoughts on this - there's no need for all memo types to have an implementation in the main mobilecoin repository, but having one place that documents their format is very useful even if they only ever get implemented by a single client. This ensures thought goes into each memo type, and if someone comes in the future and needs to debug or understand something related to memos, they know there is a single source of truth for what each memo is supposed to be, and the thinking that went into adding it.

Copy link

@voloshyn voloshyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that memos have nearly limitless uses and support simplifying the process of creating new use cases with both hands.

there's no need for all memo types to have an implementation in the main mobilecoin repository, but having one place that documents their format is very useful

If we want to make a hard requirement for all new memos to have MCIPs we have to control it on the code level, and that is no different than what we have. If we add a memo builder that allows users to create their own memos, some would create MCIPs, and others would not. This approach would make the MCIPs optional, but I think it's fine as long as the interoperability is guaranteed.
The major purpose of the memos is to build the transaction history and a fully custom memo does not contribute to it, it would look as an anonymous transaction to other clients which was probably not the intention. I propose to remove the custom memo since it conflicts with RTH, but keep the Authenticated Sender With Data Memo & Destination With Data Memo pair.
Each client that does not support the custom data payload will be able to use the memo to properly build RTH and skip the additional metadata in the custom payload.

On a side note, Moby had implemented a heuristic approach to build the transaction history before RTH was deployed, since then Moby updated that approach to pull additional data from RTH (it also writes respective memos in sent TxOuts) and we agreed that it should be refactored to use RTH to its fullest potential as we move forward. So Moby is already compliant with RTH and Moby Transaction History (MTH) is used to provide additional bells and whistles like long comments, reactions, etc.

To summarize:

  • remove the custom memo
  • keep sender/destination with data memos
  • create a memo builder that would permit to populate the custom memo data
  • we can ask Moby team to always create MCIPs for the new memos they need, but it will have to be optional for external developers.

@voloshyn
Copy link

If instead of adding the new sender/destination with data memos, we modify the existing sender/destination memos (0x0100 & 0x0200), such change would be fully backwards compatible with all of the existing clients that take advantage of RTH.

@dolanbernard
Copy link
Contributor Author

Closing this MCIP as an alternative strategy has been proposed. See issue 3200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants