Add memo checker

[abelliumnt]

Summary of Bug

Now exchange use just one address to receive all user deposit atom token. And users have to specify additional tag in memo to identify account information. However, sometimes users may forget to fill tag into memo which will result in token losing. Can we add a memo checker in checkTx to reject transactions without correct memo? By default, the memo checker should not be called. Only when the receiver address require memo, then the memo checker will be executed.

EOS and XRP provides customized validation functions for each address. EOS allows addresses to force a memo with predefined number of letter to be added in order to make a successful transfer. XRP also support such validation https://bithomp.com/explorer/FAEE963AF009E90BE14B8845D3D0A6CFB38128D3FAC1B81D046853FA7ED8BC6B.

Version

Steps to Reproduce

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[fedekunze]

Do you mean adding something like a MemoRequired boolean to the Account that checks for the mandatory memo?

I wonder if this should be something that should be validated on the client side instead... Implementing the above solution will increase the bytes stored on accounts which is not ideal.

[abelliumnt]

Do you mean adding something like a MemoRequired boolean to the Account that checks for the mandatory memo?

Yes.

I wonder if this should be something that should be validated on the client side

In fact, taking transfer token transaction for example, not all receiving addresses require memo. Only exchange deposit addresses require this. So wallet can't force users to fill memo, and it can just warn users to fill memo when send token to exchange deposit addresses.

So I think adding this checker on server side is better.

[fedekunze]

@ValarDragon thoughts?

[alexanderbez]

There isn't too much storage overhead to this. In addition, this should probably run in the AnteHandler.

I don't think something like this is too controversial. It consists of only three trivial updates: modifying the account, updating the AnteHandler, and creating a message w/ a handler.

Thoughts @jackzampolin

[ValarDragon]

Is this a near term requirement? I dislike the trend of this (tomorrow perhaps we add a new field per account for minimum memo length, not just a boolean) imo modifying account should be controversial, that should be expected to comprise the majority of state. I do see the need for this to be on chain though.

I'd prefer if there was a use case being planned for. I imagine anyone who requires a memo will want the memo to follow a certain format. This can be checked on chain with a FSM/Regex/something low polytime. (Fsms and regexs are linear time) I'd prefer the latter solution as its solving the problems I imagine.

I don't mind a boolean for memo checking if it is implemented with the proviso that it will be replaced with something more dynamic.

[alexanderbez]

I believe a regex would be nice, but any such validation on an account would have to be provided by the account owner and I can't imagine a fluid/easy to use UX for that :-/

[zmanian]

Go Regex's have bad worst case performance compared to engines in other languages. https://github.com/golang/go/issues/11646

But I was thinking a glob matcher might be a good reduced functionality subset. https://github.com/gobwas/glob

I think the downside would be unlike a RegEx you can't enforce properties on the memo like length.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[zmanian]

One thing related to this that pops into my head is that in ICS20 transaction the "memo" is set by the relayer and not the "sender". Exchanges that use "memo" to route transactions to their users account will run into problem if anyone send an ICS20 packet directly to them.

[aghamir]

Hi, Any progress in this issue?

[alexanderbez]

No, but we're open to reviewing a concise ADR 😃

[ValarDragon]

The way I'd architect this is making a new auth account extension interface, that an account type can optionally implement. Then add a new account type.