ADR 001: coin cross-chain transfer source tracing

[fedekunze]

Description

closes: #6649

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added relevant godoc comments.
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer
• Review Codecov Report in the comment section below once CI passes

[alexanderbez]

Thanks for the elaborate and well throughout ADR @fedekunze, but I can see that I'll already have concerns with this because we're conflating concerns and use-cases w/ IBC. Specifically, I do not believe trace should be a first-class citizen field of Coin. Please consider instead using some notion of metadata.

[aaronc]

I agree that we should use metadata and not add this to Coin itself. @fedekunze I have some ideas on how to address this more generally and will write something up.

[aaronc]

So I realize what I have in mind will be a larger ADR that also covers supply. I'm hoping to get to that relatively soon as these are things I've thought about for some of Regen's use case which allows for some modules to arbitrarily create approved denom's.

Anyway the gist of it is that from the perspective of the bank module, IBC transfer is just minting or burning coins for some denom which it has the right to manage.

I propose we use prefixes for these denoms such as ibc: or ibc/ and then IBC assigns an auto-generated ID that looks something like ibc:2g8sd6h. Or maybe ibc:atom:2g8sd6h to be more descriptive if the source denom is atom.

Then the Trace information is something that gets managed as metadata by the ibc transfer module. So the ibc transfer keeper would then have a method like DescribeIBCDenom(denom string) IBCDenomMetadata to retrieve the Trace data.

One problem that I pointed out in the call is that balances are indexed by denom. So adding Trace would break that in addition polluting any balances with all of that metadata. Instead we can just treat denoms as dumb identifiers which are fully described by their metadata.

To take it a step further, I would restrict which modules can mint/burn which denoms. So x/mint would get rights to atom, x/ibc-transfer would mange the whole ibc:* prefix, etc. But that gets into supply which I'd love to address too, but later after stargate.

[cwgoes]

This makes sense, although for now (IBC 1.0) I think we should do the simplest thing which solves our main problem, and hash the denominations in the 20-transfer module to allow for arbitrary port/channel paths, and keep a lookup table so that we can lookup the port(s) & channel(s) from the denomination.

[anilcse]

Great work, added some minor comments

[AdityaSripal]

Looks good, Mostly minor changes requested

[colin-axner]

Overall looks amazing to me!! 🎉 🙌 Thanks for doing all this work @fedekunze !! I think we should copy over some of this into transfer docs.

Left some small nits on wording, but the general flow aligns with my mental model

[anilcse]

lgtm

[cwgoes]

Agree with @colin-axner's comments. Otherwise this looks fine to me.

addressed