SEP-41: Add muxed address support to token interface - #1775

Continuing from the SDK PR discussion regarding my concerns about over-specifying memos in SEP-41:

This is the reason we added the clawback event to SEP-41, even though SEP-41 has no self defined mechanics for clawback. Without defining clawback in SEP-41 it is impossible to get a complete picture of the movement of value of a Stellar asset when implementing a consumer of SEP-41. The same principle applies here? The muxed ID is part of the destination identity.

I think there is difference between clawbacks and muxed ids (even though I agree that arguably even clawback in SEP-41 is a bit ugly). If one ignores the muxed information completely, they still can get accurate value transfer information for the on-chain entities. Muxed ids define bespoke off-chain entities that are hard to impossible to interpret if you are not the one managing them.
Maybe we should have gone with a more granular approach to this. In SEP-41:

• Specify all the value moving events (no change here)
• Allow any value moving event to have a map field as data, with amount field representing the event amount
• Allow any map contain additional fields, specified either by the future SEP-41 changes, by separate SEPs, or even arbitrarily defined by the custom tokens

Then the muxed identifiers could go into a separate SEP that would say that if SEP-41 event contains a to_muxed_id identifer, then it identifies the event receiver.

I'm not saying we should necessarily go with this proposal, but I would at least spend a bit of time considering something like it. While I realize this seems somewhat more complex in the short term, there may be some general benefit of making the spec more extensible. We still could make some significant changes to SEP-41 because majority of the tokens on-chain are still SAC, which we can provide backfill for (which is still a hack though). But if there are useful custom tokens around, then it will be tricky to make any adjustments to the events in the future in case if all the consumers expect a very rigid payload, even though it's not necessary to track value movement.

Of course, if we're sure that SEP-41 will stay as is and won't ever have any more changes, then we can hardcode whatever we have now. Also I guess we can always make SEP-41 more abstract in the future and ask consumers to update.

At the very least I think we should make it more explicit that the data fields of any events with data fields can be extendable with additional fields. I think this was our goal, but it is not explicitly stated. I don't think this change should make that statement for all events, just so that this change is narrowly scoped. But I think we could start a discussion making that proposal generically across all the SEP-41 events, where any event's data can be the single value (if currently defined as so), vec (if currently defined as so), or a map of named elements.

I've pushed a commit stating that the data entries are not rigid but can be extended by implementations.

Because the SAC writes the string and bytes memos into the to_muxed_id field, I still think they need to be defined here, otherwise the SAC would be incompatible with the definition here due to its use of different types in an already defined field. In hindsight if CAP-67 had written the memo data to a separate field then that behaviour would have fit more comfortably as an extension. We could state the to_muxed_id field contains a Val, but that seems more problematic than calling out the legacy case explicitly.

Because the SAC writes the string and bytes memos into the to_muxed_id field, I still think they need to be defined here, otherwise the SAC would be incompatible with the definition here

SAC doesn't write strings/bytes to to_muxed_id. Only classic events do so. Classic events generally can't be fully specified in Soroban-only terms, so this is just 'yet another' difference. The tradeoff here is between keeping things more streamlined via using just i64 ids and explicit types in Soroban, or bringing all the classic legacy to Soroban for the sake of being able to do a bit more things that are possible in classic events (which also don't have an obvious utility).

SAC doesn't write strings/bytes to to_muxed_id. Only classic events do so.

From an events consumer perspective there is no distinction between SAC and Classic, the event is from the SAC contract ID, so I think what code path generated it is an implementation detail, what matters is consumers ingesting events. Part of the goal of CAP-67 was to finally address the one-way-to-consume-memo usage, so I think it's critical in service of that to not introduce more fragmentation for event consumers.

so I think what code path generated it is an implementation detail, what matters is consumers ingesting events

I don't really disagree here. What I'm saying that that there is a disconnect between the Soroban interface that uses i64-id muxed addresses and only C/G addresses, and the 'consumer' interface, that may need to care about the classic event intricacies as well (additional address types, additional memo types, fee events, maybe something else). I've had the same comment for the SDK, but basically I don't see a fundamental reason for these to be tightly coupled. I feel like the coupling might lead to confusion, e.g. it's not obvious from reading SEP-41 what an Address is (and to make things worse, it actually means two different things in the interface and event sections). Fee events are also not defined in SEP-41 and it would be weird to define them here.

Part of the goal of CAP-67 was to finally address the one-way-to-consume-memo usage, so I think it's critical in service of that to not introduce more fragmentation for event consumers.

Sure, but does that mean that we also have to expose all these details to the event producers? Event consumption can be unified, but event production from the Soroban side can't really be fully consistent with what consumers have to expect. Basically SEP-41 was historically oriented towards the contract developers, so it's important for them to know what is the minimum set of features that a token should support. But it is unfortunately not sufficient for properly ingesting the unified events.

Any objections to merging this change?

Any objections to merging this change?

No objections from me, we can do the updates as a followup.