[CAP-67] Classic Ops emit Transfer/Mint/Burn/Clawback events

[leighmcculloch]

What

I propose we unify transfer events on the network in smart contract events, by exploring emitting transfer contract events, like those defined by SEP-41, CAP-46-6, and those potentially defined by #1534, for classic transfers (payments, path payments, etc).

Why

To create a singe unified data stream of all movements of assets / tokens on Stellar.

Today there is no single way to consume transfer events on the network. Transfers are divided by the classic vs smart contract divide, and then also on a separate dimension by stellar assets and custom tokens.

The RPC is the way developers are intended to stream and ingest live events occurring on Stellar. The RPC exposes smart contract events. However any application using the RPC and wanting to ingest transfers must implement complex logic merging the results of streaming smart contract events and transactions containing payments, path payments, offers, so on.

The shortest paths to a payments implementation today is to go back to Horizon's /effects endpoints, or Horizon's /payments endpoint, however the migration path we're wanting to create is in the opposite direction from Horizon to the RPC. There should be a way for developers to ingest all transfers from RPC, otherwise the path for migrating from Horizon to RPC is not realistic.

We could explore ways to add payment information to RPC, such as a similar Horizon-like /effects endpoint or /payments endpoint, however I think that is solving the problem at the wrong layer and repeating mistakes of the past. We've made that mistake before with Horizon, by solving all indexing problems at the Horizon layer which encouraged folks to build on Horizon rather than innovate on new and or better data sources.

RPC's API is designed to provide a raw view into the network and live / current network data. It exposes raw transaction XDR, and raw events. RPC doesn't contain "business logic" or even "indexer logic" for understanding network data. This is critical because if you build on RPC, you're building on the same data that's also available from Galexie, other CDP tooling, or raw meta. You have access to the same data, so applications can select between them without being disadvantaged, and without artificial / convenience forcing functions causing folks to interact with one when another would be more appropriate.

What's critical is that network data be a good foundation to build on. In many ways it is, but for understanding transfers between accounts it is unfortunately not. Soroban transfers have resolved this problem for transfers originating on Soroban through Soroban events, and if classic adopted the same events network data would tell a great coherent story about transfers.

If we solve the problem at the network layer, then events on Stellar can support a unified view into the movement of value on the network.

How

All transfers / payments on Stellar classic are the result of interactions with Stellar Assets. All Stellar Assets have a reserved contract address and a reserved installation of the built-in Stellar Asset Contract. For example, on mainnet (pubnet) the native asset has contract address CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA. Stellar classic operations can emit Soroban events using the asset's reserved contract address, even if the reserved contract address hasn't been deployed to yet for that contract. All classic transfers of value are mappable to Soroban events, because the Soroban Stellar Asset Contract was modelled on the Stellar asset classic behaviours.

A proposed mapping exists at: https://docs.google.com/document/d/1511nhYdUiaGAJ-CT7TzVeAdBaTm4HLBgEFndfjno4jc

And is being discussed here: #1553 (comment)

Links

Discussion on Discord: https://discord.com/channels/897514728459468821/1285736582426394760

[Shaptic]

Stellar classic operations could emit Soroban events using that contract address

Define "emit"? I don't think this is even logistically possible from Core's perspective. Since smart contracts operate in their own land of execution and thus emit events into metadata via host functions, how would classic operations replicate this? Are you suggesting they inject ContractEvents into the meta despite (a) not coming from a contract, (b) possibly referring to a nonexistent contract address, and (c) having no understanding of ScVals? I should let them speak to its feasibility, though.

Don't get me wrong, this sounds really nice, but I just can't see how it's possible.

[leighmcculloch]

Are you suggesting they inject ContractEvents into the meta despite (a) not coming from a contract

"Not coming from a contract" is an implementation detail. The Stellar Asset Contract is a code API into existing classic data. Events are data about how data has changed, and transfer events provide information about how classic data has changed. The proposal here is to emit events about changes in the same data.

possibly referring to a nonexistent contract address

This is unfortunate, but I think a trade-off that's worth the benefits. It's not a nonexistent contract address in anyway that is unsafe. Contract addresses are reserved. The event contains enough information to identify the asset reliably, so even if the SAC contract isn't deployed, the asset is still identifiable.

Because of state archival, it's always possible for a contract to emit an event, and shortly after for that event to reference a contract that's not "live" on chain, so problems that arise from a SAC not existing would be similar to the problems that arise from state archival anyway.

having no understanding of ScVals

ScVal's are just an XDR type. Core knows about all XDR types. For example: https://github.com/search?q=repo:stellar/stellar-core%20ScVal&type=code

this sounds really nice

💯 ❤️

[Shaptic]

Great points all around. Couple more points to follow up:

• I'd like to suggest renaming ContractEvent for XDR clarity since it wouldn't always come from contracts.
• When you say "transfer events," you mean the exact same format as the SAC today, right?
• Does this proposal need a detailed enumeration of the things that happen on the native operations' side that would result in a transfer event?

[leighmcculloch]

Yup a formal proposal / CAP would need a detailed enumeration of classic happenings to events.

[dmkozh]

While this seems easy enough to support for simple payments, I wonder what should we do for things like path payments, DEX trades, claimable balances etc. In Soroban world these would contain a number of transfer events, as well as maybe some other events. I don't think that's feasible for classic, as we wouldn't want to have an operation to emit a lot of events (these won't have a fee, unlike in Soroban case).

I wonder if a subset of operations that move balances is sufficient to support. Basically, it would be nice to identify the requirements/use cases that we can support with certain 'classic' events.

[leighmcculloch]

I wonder if a subset of operations that move balances is sufficient to support. Basically, it would be nice to identify the requirements/use cases that we can support with certain 'classic' events.

+1. If there's general consensus that this is a direction to explore, then I think the next step would be to do some assessment of all possible movements of value on classic, and mapping / excluding, so that we could talk about the implications more specifically.

[Shaptic]

I don't think that's feasible for classic, as we wouldn't want to have an operation to emit a lot of events (these won't have a fee, unlike in Soroban case).

Yeah, this separates it from Horizon's /payments which are 1:1 and is more like its /effects which are 1:N, where we derive all of the side-effects for an operation (e.g., account_debited, offer_updated, account_credited for a simple path payment). N is typically > 1 but I don't think long paths are very common which is the only operation that can have a large number of events. So isn't it generally okay that there won't be fees since there's no VM cost being incurred and the events can't get arbitrarily large?

Alternatively +1 to exploring mapping / excluding.

[dmkozh]

The event fee actually goes towards the meta storage (we don't want it to be possible to just spam meta with arbitrary events for free), but I guess the missing fee is probably acceptable given that the event size will be proportional to the diffs we emit anyway. But I'm still not sure if implementing very granular event reporting is worth the effort, which is why I'd like to understand what's actually useful.

[mollykarcher]

RPC's API is designed to provide a raw view into the network and live / current network data. It exposes raw transaction XDR, and raw events. RPC doesn't contain "business logic" for understanding network data.

Just wanted to big ➕ this! There have been a handful of conversations lately (internal + external) that have assumed or implied that we will be rebuilding or recreating the unified data models that we built out in Horizon, but now in RPC. Repeating this behavior would only recreate a lot of the issues we saw with Horizon, and allow RPC to become the defacto backend/monolith for any and all applications that want on-chain data. So yes, I'd wholeheartedly agree that the right layer to solve this at feels like the source-of-truth of the events data (ie. Core).

I wonder if a subset of operations that move balances is sufficient to support

I think this is the right bit to start with as there's generally a lot of interest around a unified understanding of "what is a payment" or "what moves value" (lots of interest/talk around an open-source payment indexer, for example).

[JakeUrban]

I'm a big fan of this proposal. I want to point out that there are two layers in this unification effort -- the source of the data and the format of the data.

Providing a stream for all stellar asset transactions at the protocol level via events, as opposed to at the application level (like Horizon's /payments endpoint), makes sources of this data interchangeable. Clients can use RPC, a txmeta lake (produced by galaxie), captive core, or some other source of txmeta.

However, I think it is equally important that we unify the format of the data provided. We could add events for all stellar asset transfers, providing a unified the stream for this data, but if the format of these events vary, clients will inevitably support a subset of these formats. We have this same problem right now with Horizon's /payments endpoint -- most clients support the simple payment format, but don't support the path payment or SAC transfer formats.

Unfortunately I think we've already introduced multiple data formats in cases where a single format could have sufficed. For example, mint, burn, and clawback are all effectively transfers, but clients may unintentionally omit support for these events in their implementation because the first topic is the name of the contract function, rather than "transfer". If clients wanted to know more about the nature of the transfer (i.e. whether it was a mint, burn, clawback, simple transfer, path payment, etc.) they could inspect other information included in the associated TransactionMetaV3.

So lets take path payments for example. I would advocate that the event(s) emitted as a result of a path payment operation use the ["transfer", from, to, asset]: amount format. Clients interested in this event can inspect the associated txmeta to determine if it was a path payment if they'd like.

I'd like to see events in this format emitted for the following classic operations:

• payment
• both path payment types
• create account
• merge account

Now there are some classic operations where using the same format may not be the best path because there is not an obvious from and to address.

• create claimable balance
• claim claimable balance
• amm deposits
• amm withdraws

In these cases, we may need to use a different format. The good news is that most applications won't need to care about these events because they don't use claimable balances or native AMMs.

Finally, I think we should also consider emitting events for transaction fees. These are a kind XLM transfer, and anyone using events to update balances won't be able to maintain an accurate XLM balance without this event.

[dmkozh]

These are a kind XLM transfer, and anyone using events to update balances won't be able to maintain an accurate XLM balance without this event.

This is a very important point that I've alluded to above. If we want to make it possible to track the exact balances only via events, then the things will become much more complicated, as we'd need to cover every balance movement at once. It's not obvious to me that this should be a requirement. I think an argument can be made that getting the balance is a very common operation and in general a user shouldn't ingest each and every event concerning a given account (and also interpret these correctly), just to figure out something as simple as balance is. Thus I'm not sure this should be a requirement.

[leighmcculloch]

Just to clarify some history and why on these things:

mint, burn, and clawback are all effectively transfers, but clients may unintentionally omit support for these events in their implementation because the first topic is the name of the contract function, rather than "transfer"

Transfers is the movement of an amount between two addresses and SEP-41 does use "transfer" as the topic of any event that moves an amount between accounts. For example, the transfer_from function generates a "transfer" event.

The mint, burn, clawback events are separate because they create and destroy amounts, they do not move amounts between the issuer and the holder, even though they do affect the balance of a holder. On Classic stellar these operations were represented by transfers, but in Soroban it was decided not to represent those happenings as transfers. An application can still ingest these events also though.

[Shaptic]

Just to clarify

On Classic stellar these operations were represented by transfers

More accurately, Horizon interprets the side effects of these operations as payments and presents them as such, where a payment is literally just a concept we made up. Just to echo the general sentiment that standardization of a transfer at the source of truth would go a long way 👍

[leighmcculloch]

I've written up a document taking the list that @JakeUrban posted at #1553 (comment) and documenting what I think each of the events would need to be emitted to align the events with the events that the Stellar Asset Contract already emits today.

https://docs.google.com/document/d/1511nhYdUiaGAJ-CT7TzVeAdBaTm4HLBgEFndfjno4jc

Please leave comments about specifics directly on the document ☝🏻, or more general broad comments here 👇🏻.

cc @JakeUrban appreciate your 👀 again.

cc @sisuresh @dmkozh @anupsdf @MonsieurNicolas

Cross-posted to Discord: https://discord.com/channels/897514728459468821/1285736582426394760/1300244089102532718

[dmkozh]

All proposed events are the same size as existing events that get published by the Stellar Asset Contract. Events are afaik designed to be as small as possible.

Keep in mind that classic events are free, unlike soroban events. There is also no limit for the size of events emitted by an operation.

Could this be the push towards multiple categories of meta, or optional/configurable data within meta, where an operator can enable "events meta" only, and disable other types of meta. Meta may become smaller rather than capturing the full transaction meta that exists today to derive the same events off-chain.

Yeah, that seems reasonable. I'm not sure if the sizes are going to be always be smaller, but events should have 'asymptotically' the same size as the ledger diffs, so we should be fine if we were to emit only events or only diffs most of the time.

Ideally, an event emitted by core would describe both the state transition (i.e. the transfer) and the new state (the updated balances). This is what we're aiming to do with the "replace ledger entry changes with events".

This sounds suspiciously similar to the current ledger diffs. If the end state (and maybe the start state) of the entry are necessary, let's use the appropriate meta format (ledger diffs). Balance is just a single example of this; there are also account/asset properties beyond the balance (e.g. authorization). The Soroban approach to this is to use events to track the updates and getters to get the interesting data (such as balance).

[sisuresh]

Another issue with emitting a Balance event is that it would be unique to classic. Tracking balances for smart wallets will require a different method, leading to the fragmentation we want to avoid.

[leighmcculloch]

Tracking balances for smart wallets will require a different method

Why would that be? Custom tokens that share a concept of a balance could emit an event communicating changes in balance.

[leighmcculloch]

This proposal is focused on bringing consistency between Stellar classic and Soroban so that classic operations that move assets get emitted in the same way that asset movement on Soroban does. This proposal makes no changes to Soroban emitted events, and only brings classic into consistency.

I think it's been helpful for us to zoom out to see if the scope should be broader, but so far the ideas for tracking balances have all discussed what could be implemented as additive features, so I think we should explore them as a separate proposal.

As part of that separate proposal process, we should consider why other ecosystems have not needed balance events. When looking at what data providers do for balance tracking in EVM chains, there's some evidence (see below) that in other ecosystems this is done via tracking transfer events which contain no balance information. So we may be solving a problem at a layer that's not necessary to solve at this layer.

---

Dune does balance tracking via tracking ERC20 transfer events:

• For ERC20: Analysis is based on token transfer events detailed in the tokens_ethereum_base_transfers. The address balance is updated based on the to/from addresses in the transfer events. If the token is a non-standard ERC20 token, the balance might not be updated until we detect a standard ERC20 transfer event. Known issues currently include ERC4626 tokens and rebasing tokens like stETH or $ampleforth. We are working on improving the range of balance changes that we detect.
• For ERC721 and ERC1155: Inspection of NFT transfer activities via the nft_ethereum_transfers, focusing on the to/from addresses.

Ref: https://docs.dune.com/data-catalog/curated/evm/asset-tracking/balances#balance-calculation-methodology

[sisuresh]

Tracking balances for smart wallets will require a different method

Why would that be? Custom tokens that share a concept of a balance could emit an event communicating changes in balance.

Yeah sorry I wasn't clear. This proposal is about adding events to classic ops, so I meant if that wasn't done for smart wallets, then we'd have to figure something out. We could add that event to the token standard (although any existing non-upgradeable tokens would be out of luck).

[JakeUrban]

@jopmiddelkamp I'd be curious on your take from Beans' perspective. Or if you'd continue using a third-party indexer, maybe you can ask your indexer to take a look at this?

@earrietadev what is xBull's solution for indexing value transfers? Would this be helpful?

I know from Freighter and Vibrant's perspective, this would be quite helpful because it would make our implementation much simpler.

Today Vibrant listens for classic payments and path payments using RPC's getTransactions method, and it doesn't support SAC transfers. If we wanted to support them, we'd need to use getEvents in addition to getTransactions for the classic ops. Vibrant also doesn't index some of the transfer types that are unlikely but still possible to occur, like account merges. Unifying all transfers using events would ensure they don't miss anything without putting a lot of thought into ensuring all cases are considered.

Its a similar situation for Freighter.

[earrietadev]

For xBull because of the different ways to know a transfer of funds was made (payments, create account, merge account, path payments, trades, etc), I ended up using account effects instead (which also includes SAC transfers) and just sending users to stellar.expert if they want to see more information.

At the end of the day, most users just want to know how much left their accounts and how much is entering their accounts, they don't really care "how" or "why" the transfer was executed (especially if they are keeping business payment records).

I think using just events will be similar to just using effects so in my opinion that makes everything easier.

A note regarding fees, it will be extremely useful if the amount of fee paid is also an event because that's also an amount of funds leaving the account which with the current tools we need to make a separate call (like getting the transaction and filter if the same account paid the fee) just to show that or when you try to see a historical variation of the XLMs amounts in an account.

[leighmcculloch]

I think using just events will be similar to just using effects so in my opinion that makes everything easier.

Events will also have transfers for custom tokens, unlike effects which currently don't.

[jopmiddelkamp]

@jopmiddelkamp I'd be curious on your take from Beans' perspective. Or if you'd continue using a third-party indexer, maybe you can ask your indexer to take a look at this?

@earrietadev what is xBull's solution for indexing value transfers? Would this be helpful?

I know from Freighter and Vibrant's perspective, this would be quite helpful because it would make our implementation much simpler.

Today Vibrant listens for classic payments and path payments using RPC's getTransactions method, and it doesn't support SAC transfers. If we wanted to support them, we'd need to use getEvents in addition to getTransactions for the classic ops. Vibrant also doesn't index some of the transfer types that are unlikely but still possible to occur, like account merges. Unifying all transfers using events would ensure they don't miss anything without putting a lot of thought into ensuring all cases are considered.

Its a similar situation for Freighter.

I’ve reached out to BlockDeamon to see if one of their Stellar developers could review this, as we’re utilizing them as our third-party indexer.

[piyalbasu]

Just to provide a little context from Freighter's side: we hoped to use an indexer to help us parse transfer events, but it's been tough to find the right fit for us. Currently, we're opting to construct token transfers in an account's history using the XDR from Horizon's transaction's endpoint and checking to see if it's a transfer or a mint. If it is, we parse out the params for display (to, from, amount, symbol, etc). This only works for sent token transfers, as there's no equivalent transaction for received.

Our current approach requires quite a bit of manual effort to get this to work at all. I think this type of manual data parsing is a big stumbling block for new wallet developers, especially ones that want to support both classic and Soroban. The logic becomes pretty hairy very quickly and devs end up spending time maintaining logic that was "out of the box" with Horizon rather than innovating. This proposal would be a big win for wallet engineers.

I'd also add that from Freighter's perspective, I'd lean more towards having as much context around the event as possible. While it's true that users probably care most about the account effect (change of balance), for a wallet like Freighter that's geared towards power users, we tend to want to show all the context so they can know exactly what happened. Is it reasonable to have the emitted event be verbose by default and offer maybe a helper in an SDK to parse that down to something simpler that's closer to how account effects work now?

[leighmcculloch]

Hi folks, conversation seems to have slowed in this proposal, suggesting some consensus exists, with the discussions on balances having identified a new problem we should think about and potentially solve independently.

One of the most critical decisions with this proposal is where we solve the problem.

The proposal as it is has the problem being solved in Stellar-Core.

Alternatives that were considered:

• RPC
  • no because that would create artificial motivators for people to use the RPC over other data sources
• Some new service that sits very close to core and generates events ingested by all existing systems that ingest meta
  • no because the events that core already emits are what we'd want more of, and already speak directly into the desired domain, and this proposal is a tweak to emit them in all cases rather than only the contract subset
  • maybe because decoupling event production from core might help maintainers from an implementation level iterate faster
  • no because it would be difficult to decouple as the data would have to be provided by core and so this decoupling would actually be coupled transformation

Does anyone have alternative views on this?

For folks who think this is being solved in the right place, please leave a positive emoji of some kind.

[sisuresh]

Reviving this thread - It sounds like there's support for adding the events described in this doc to stellar-core. We still need to figure out exactly what's required. Do the unified events need to be emitted retroactively? Also, the soroban events vector in tx meta can currently be verified because it's hashed into the ledger, so these classic events (which won't be hashed into the ledger), will need to either be a different field in meta, or put into the diagnostic events vector.

Related to this, we need to make a change to make sure the events are semantically correct for transfers involving the issuer in the SAC - discussion. This will result in different logic to consume the events between protocol versions (a transfer call from an issuer will emit a mint event in one protocol, and then a transfer event in the next protocol).

[dmkozh]

I'm not sure it makes sense, given that events are not sufficient to infer the balance if not tracked from the genesis. It's possible to optimize ingestion by bootstrapping if necessary - Core can catchup in any ledger range and there is nothing special to do about it. The only benefit of explicitly cutting the backfill is to not deal with some quirks of the old protocols, but I'm not sure if these are bad enough (and Core needs to maintain them anyway).

[sisuresh]

Back to the question of hashing the new events - @MonsieurNicolas suggested not hashing the new events until we decide what we want to do w.r.t stellar/stellar-core#4624. This would make it simpler to add the other events and hash them later.

[sisuresh]

I realized the CAP is missing a detail that we've briefly discussed here - CAP-0067 changes the SAC to stop emitting the admin on mint/clawback events, and transfers involving the issuer on one side will instead result in a mint or clawback event. The CAP only specifies that these changes will be made for the new protocol version. Due to the fact that the soroban events are already hashed into the ledger, we still need to produce those hashes as they were in those older protocol versions. To allow for the verification of those hashes, we should emit the events as they were emitted. What this means for consumers is that they will still need to be aware of how to process these events as they appeared in these old protocols. I think the consensus here is that this is not acceptable.

One of the ideas mentioned here was to emit the new format of events for older protocols in a different part of meta (maybe diagnostics? We'd have to remove the old protocol events) so consumers don't need to track parsing logic by protocol versions. Another option might be to enable a flag where core will transform the events on replay, but that doesn't sound like a good idea.

Thoughts @dmkozh @leighmcculloch @chowbao?

[dmkozh]

This is the backfill issue I brought up several times when discussing changes to the Soroban events. I don't think there is a way around this that doesn't require any changes from the consumers; they either need to look for the events in a different location depending on the protocol version, or parse the events differently. FWIW I don't think it's necessary to do any protocol-based logic to parse the events both with and without the admin; the parsing code may just be modified to only read the first topic. This actually might be easier to implement for everyone vs moving the events around the meta structure.

[sisuresh]

It's technically possible for us to provide the events as a single diagnostic vector that should always be consumed on replay, but not sure if that's a good idea. I'd be happy to leave CAP-0067 as is. I agree the admin issue isn't a big deal, but the transfer issuer mint/burn change could be burdensome if that scenario has been hit on pubnet.

[sisuresh]

Here's the initial CAP - https://github.com/stellar/stellar-protocol/blob/master/core/cap-0067.md. There are some notable points that'll probably change -

1. The CAP specifies that the events will be emitted at the Transaction layer, and not the Operation layer. I'll think about alternative solutions that emit the events at the Operation layer.
2. The CAP doesn't emit the set_authorized event on trustline flag changes, but the Stellar Asset Contract currently does. While the set_authorized event is not necessary to track balance changes, it makes classic consistent with soroban, so we may add that event to this CAP.
3. The CAP still needs to specify how retroactive events will work, and go through any edge cases due to older protocol behavior. Note that the retroactive events cannot be hashed into the ledger.

[chowbao]

What's the process for comments in the initial CAP that you merged? Like there is no PR anymore to comment on. Is the expectation that all discussion should be done in this discussion even if it was something like a rewording suggestion or a typo fix suggestion?

[sisuresh]

Yeah discussions should be done here. For grammar or typo fixes, you can either open a PR fixing them, or you can just let me know if it's one or two changes and I can add them to my next update.

[Shaptic]

I noticed the following line in the proposed CAP-67 for this discussion after #1617:

By using the existing events<> vector in SorobanTransactionMeta. We can avoid making any xdr changes. This does have some tradeoffs, mainly that all events for a given transaction will be emitted in a single vector, making it impossible to distinguish which operation emitted a specific event. The alternative would be to move Soroban meta from the transaction layer into the operation layer of transaction meta, but that would be a breaking change.

(Also noted by @sisuresh in (1), above.) This seems prohibitive to downstream ingesters, unless the events themselves are expanded to make it completely unnecessary to inspect the operation itself (which would require more details in each classic event's data, imo).

Could we make events<> a nested vector, instead, in a binary-compatible way, or will that not work because of a prepended length byte? I'm not 100% sure how XDR encodes things. We could also add classic events into a new SorobanTransactionMetaExt (V2).

[sisuresh]

I think a cleaner solution would be to introduce a TransactionMetaV4, and pull anything related to events out of SorobanTransactionMeta. The events could be in an OperationMetaV2 or something like that under TransactionMetaV4. What do you think?

[Shaptic]

Wouldn't that still be breaking to SorobanTransactionMeta if events aren't in it anymore? Or do you mean introducing a V2 of it for MetaV4?

[sisuresh]

Yeah we'd need a SorobanTransactionMetaV2 as well. This path will require some work for downstream consumers, but the meta will make more sense.

[Shaptic]

MetaV4 introduces more opportunities to 💥 in downstream integrations, but if we're thorough I think we can mitigate this. Would Soroban transactions still use V3 (which would increase backwards compatibility) or is V4 the universal payload?

[chowbao]

I like the MetaV4 (and other +V*). I would also prefer V4 the universal payload. Basically going with a "clean slate" going forward approach

[Shaptic]

So I had a thought I want to throw out there. If the whole purpose of this event stream is for people downstream to track balances, couldn't the event itself just include the current balance for to and from (where appropriate) within the data payload of the event alongside amount? It could be omitted from custom tokens where the balance isn't stored in a standardized way. This would help downstream avoid painful things like:

• catch-up from arbitrary ledgers: they can start anywhere (no need for a history archive boundary) and have a safe starting point
• slippage bad financial math like casting to float
• missing events for whatever reason (e.g., downtime)

Preemptive potential reasons why this might be a bad idea:

• Event size bloat / Other performance implications?
• Core doesn't know the balance at "event generating time" because they're atomically accumulated elsewhere?
• We shouldn't try to accommodate people doing things wrong (see Horizon's "helpful" txqueue for an example of this going poorly)

[sisuresh]

I think we could use the ContractEvent xdr ExtensionPoint to turn it into a union with another type that can hold a list of events. I haven't thought about this much, but it would let you group events together.

[dmkozh]

Depends on what we consider 'small'. Events have an extension point, so we could just add an 'invocation id' there (just a number that increases by 1 with every invocation). This is simple to do in the new protocol, but backfill is an issue as usual, though it's not that different from adding the new events (everything new would need to go into the non-hashed event stream).

[leighmcculloch]

If we can do the array solution, that seems like a better solution in the sense that we don't introduce any new data id to reason about.

[dmkozh]

What do you mean by the 'array solution'? If you mean extending topics/data of the events, I don't think that's a good idea, as these are meant to be populated by the contract logic, not by the protocol (and contract logic doesn't know about the invocations). Of course, we could 'hack' the SEP spec further and add 'linking' to only the token events, but I'm not sure if that's a great idea (and it's still non-trivial for a custom token to do).

[leighmcculloch]

I was referring to @sisuresh's most recent comment above.

[sreuland]

@dmkozh ' suggestion of converging all event types down to a single balance_change event(if other context needed, can dive into the tx meta at that point per earlier comment ) and the other suggestion for using map for event data model seem complementary, the two ideas seem compelling to be combined for a single asset movement event model.

On its own, map may be considered tedious for ingestion apps as it requires manual out-of-band comprehension of key/value docs and then translating that into application ingestion code(usually with hard-coded key names). Could the event data be defined in IDL terms, such as similar XDR Ext arm pattern, i.e. BalanceChangeDataExtV, providing ingestion apps a more intuitive type-safe development approach to parsing events.

[sisuresh]

I've been thinking about how to deal with old protocol bugs related to the native asset. More specifically, when it was possible to mint/burn the native asset in specific scenarios due to a cached account. From stellar-cores perspective, we'd ideally be able to go off of meta and emit an event that has a 1-to-1 mapping with a LedgerEntryChange like a balance_change(address, token, before, after) instead of using transfer events (@dmkozh mentioned this event in a slightly different context above), but this doesn't fit in with how we currently emit events for the SAC. Even if we went that route, we would still need to make minted/burned vs transferred amounts distinct based on our current requirements.

Instead, we can use logic similar to the ConservationOfLumens invariant to identify when the asset has been minted/burned, tie that diff to a specific account, and emit the correct event. The bug should've only been hit in very specific scenarios, so it shouldn't be that difficult to special case this.

[dmkozh]

From stellar-cores perspective, we'd ideally be able to go off of meta and emit an event that has a 1-to-1 mapping with a LedgerEntryChange like a balance_change(address, token, before, after) instead of using transfer event

FWIW this event has been proposed in the context of including balance into events. I wasn't proposing it in the context of generating events directly from the ledger entry diffs.

[dmkozh]

There is one thing that seems to have escaped our attention when discussing this: memos and muxed accounts. It's arguably not necessary for the high level purpose of 'just tracking the balances', but I think we generally want the events to be more self-sufficient and compatible with Soroban. But for Soroban the 'classic' ways of handling memos by parsing both the transaction and the meta is rather annoying. The annoyance will be much worse for Soroban, if we ever want it to support multiplexing. So I think we might need to try specifying something useful for the classic memos/muxed accounts and then hopefully make it also possible in Soroban. I don't have any concrete proposal yet; looking for some ideas/opinions first.

[dmkozh]

Let's not overload transfer. Doing so complicates the narrative for the token interface, and increases surface area that memos mess with and that developers encounter, because of the need for the additional MuxedAddress type, and a third type that wraps the Address|MuxedAddress. Those new concepts as types in the SDK come at a cost.

I think the transfer overload makes a lot of sense to the users and wallets and I think this should be higher on our priority list than the contract complexity, given that only a few well-audited contracts will ever need to deal with this. I think it's worth of additional complexity on the host/SDK side (unless it's completely impossible to implement at all, that is).

The Address|MuxedAddress type would also pollute every token. Because the transfer function is the same name, we wouldn't be able to make it an extension trait in Rust, or a composable component with the existing Token interface.

I don't think that's unsolvable. We could have two different interfaces. The 'real' contract interface is defined in terms of host types, and these allow variant types via Val. But even in the worst case it doesn't seem too bad to me if every token implementation supports this, because there is no need for too many token implementations. The same can probably be said about transfer_with_memo - exchange support is probably something that a token implementation will generally want.

Exchanges and Wallets can still use M addresses in their products with transfer_with_memo. An exchange displays the M address, the wallet unpacks the M into G+memo when making the transfer call. The main win of M addresses is their user-facing value, not in any value on the backend.

I think we might be underestimating the downstream complexity here. Every other contract function does not behave in the way that we want transfer_with_memo to behave, so some hacking will be necessary both for the 'proper' function argument and signature payload display. Also functions would need to be picked dynamically depending on the address format (or the wallet would need to default to transfer_with_memo, but what if token doesn't support it?). If a block explorer doesn't support this properly, then the transaction won't look like what happened in the wallet. I think the probability of the mistakes and misinterpretations is much higher vs doing the simple ScAddress<->strkey mapping.

There is also a whole another level of complexity added if transfer_with_memo is ever not a top-level contract call (which is apparently something we want to support at least for batching the transfers together). It's just impossible to render the addresses 'properly' as there is no standard for passing the memos around, so devs will end up with a bespoke memo passing approach every time its necessary.

Having memos isolated in one place like a transfer_with_memo fn, that only a subset of devs care about,

My question is why have memos at all? Again, in case of supporting muxed addresses only a subset of the token contract devs needs to worry about it, while in case of transfer_with_memo the full set of wallet devs needs to worry about it.

I think you answered my question on overloading: if we use the same name, there is no easy way to know if the token implements that extension so we need some new meta information?

What exactly are you trying to learn from the meta here? If that's the list of SEPs the contract implements, then it's something that doesn't exist today, but is something that we want to generally add to the spec. If that's the interface, then it's something we already support since contract spec contains the input and output types. It's really not that different from checking if a transfer_with_memo function exists, and I think neither check is really necessary outside of, maybe, something like block explorers. If the token is listed on the exchange it's reasonable to expect that it supports muxed transfers, and if a muxed address is accidentally passed to a method that doesn't support it (or transfer_with_memo is called on a token that doesn't have it), the call will just fail in simulation. So I'd say meta is a non-issue in either approach, even without the new SEP specs.

[Shaptic]

Imo if we go this way, we could at least make the fields combined:

-transfer_with_memo(from:Address, from_id:Option<u64>, to:Address, to_id:Option<u64>, amount:u64)
+muxed_transfer(from: ScAddress|MAddress, to: MAddress, amount: u64)

[dmkozh]

Imo if we go this way, we could at least make the fields combined:

Yep, that's a possible option as well and it seems like a reasonable complexity tradeoff (wallets only need to know when to use muxed_transfer, for contract developers this is just a new functions).

[sisuresh]

Decisions we still need to make -

• Using the existing transfer function that can take an M address vs a new transfer_with_memo function.
  • If we go with transfer_with_memo should it take G + memo or from: ScAddress|MAddress, to: MAddress (what @Shaptic suggested above).
    • Shaptics suggestion sounds like a good tradeoff between both @dmkozh's and @leighmcculloch's arguments from the protocol meeting today. A new function allows you to keep the transfer function as is, and we won't be propagating the memo concept any further.
• Reuse the transfer event or a new transfer_with_memo event that gets emitted along with the current transfer event.
  • I am leaning towards reusing the transfer event, and making the data field a map. Even though it's a breaking change, it makes logical sense, and I would not be surprised if add more data to the transfer event in the future. The only concern I have with using a map is the increased size of the events.
• For classic events, what do we do about the tx memo and muxed accounts?
  • The idea being floated currently is forwarding the muxed account info to the event if the muxed account is set, and if not, use the tx memo if it is set. The tx memo is a union that can be more than a u64, so we'll allow the memo in the event to be any of the types of the xdr MEMO. If we go with a G + memo function parameter solution, there's some discussion on whether the function should take either any of the memo types, or just u64. I think u64 is sufficient. @dmkozh @leighmcculloch

[chowbao]

This thread has been continued here, here and here

[leighmcculloch]

RE: https://docs.google.com/document/d/1511nhYdUiaGAJ-CT7TzVeAdBaTm4HLBgEFndfjno4jc/edit?disco=AAABbyoARVA
RE: https://docs.google.com/document/d/1511nhYdUiaGAJ-CT7TzVeAdBaTm4HLBgEFndfjno4jc/edit?disco=AAABcA1qEq8

The trades within a path payment are conceptually between the source account and the owner of the offers. Those are the addresses that'll appear on the event pairs specified above. At the end of all the trades, we need to emit one more transfer (or burn if the destination is the issuer) event to indicate a transfer from the source account to the destination account. The amount will be equivalent to the sum of the destination asset received on the trades of the final hop.

Ref: https://github.com/stellar/stellar-protocol/blob/68f8c230ea5a17713bc409b6f9e87acf79fbfa58/core/cap-0067.md

What's the status of how events will be published for path payments that result in multiple assets exchanging between accounts?

@sisuresh Are we confident that the the above statements from CAP-67 about what transfer events that path payments will emit are how it will be implemented?

Early on in those discussions @MonsieurNicolas suggested that events might get generated for net movement. I think everyone else involved in the discussion (@tomerweller @Shaptic @tamirms @leighmcculloch) seemed to prefer the pattern that's described in CAP-67, that is also consistent with how Horizon presents the trades.

Just want to confirm this is closed out, or something that's still under consideration.

[sisuresh]

I believe the justification for net movements was due to bugs in the older protocol where XLM could be minted/burned. The CAP specifies a way to handle those bugs, so the events as specified in the CAP currently should be acceptable.

As for my confidence on what the final implementation will look like - I'd say we're somewhat confident, but hoping we'll come to consensus after this weeks protocol meeting.

[dmkozh]

As has been discussed during the protocol meeting, here is a rough sketch for how the muxed address support could look like: stellar/soroban-examples#363. I've briefly described the pieces of the sketch in the doc. Some concerns that I recall have been raised regarding this approach during the meeting:

• How does one figure out if a given contract supports muxed address inputs? - That's represented in the contract spec.
• How to get around the lack of overloads in Rust? - One way to solve this is with a separate trait, which isn't too pretty, but seems easy enough to use on the developer side
• Increase in the token complexity - this is kind of hard to quantify. The changes in the token example above are fairly minimal. I would imagine more changes necessary for a shared/modular implementation, like OpenZeppelin, but OTOH for this the solution with some code generation might be more appropriate. At least, we can definitely avoid adding any complexity for the token clients as the tokens are still compatible with the SEP-41 interface.

[leighmcculloch]

The downside is that clients would still have to pick the right function to be called based on the arguments.

I don't think this is really an issue given the expected use cases of muxed accounts.

We might have a different idea about the expected use cases - the ideal client-side scenario is being able to simply put any valid strkey as a transfer destination.

+1 because we are not aiming to support composability, a separate function isn't an issue.

The goal is to support / not support:

✅ single payments to an exchange where transfer_muxed is the top-level call

✅ batched payments to an exchange where transfer_muxed is not the top-level call, but is bundling transfers that otherwise do not need to interact with each other

❌ composing transfers to muxed inside other contract calls that need to know about address and muxed addresses

The merging of muxed id and address, and merging the two functions, really on become a huge benefit if the goal is composability.

the ideal client-side scenario is being able to simply put any valid strkey as a transfer destination

This approach seems to hinge on this ideal client-side scenario being worth solving. The client side is not the biggest win of this approach imo, because it will be relatively easy for client SDKs to build the transfer with a muxed value. The biggest win is that calls to transfer with an M address could be rendered as the M address in developer tooling, like block explorers. But because events won't render as M because we want the G+memo split there, I'm not sure even that is meaningful.

[dmkozh]

The dual interface is a unique concept that is not intuitive.

Yeah, I agree this could be confusing, though I think we might be overestimating the level of confusion this might cause, given that only a very narrow slice of contracts would care about enabling this, and the default behavior is not different from the current state. Still, I wouldn't insist on keeping this - a separate transfer_muxed function seems very reasonable as well. Then I think MuxedAddress type can be exposed explicitly in the client interface.

The spec XDR would be extended to support a type being an "any-of", and the code generation would turn that any-of into a generic type that all the any-of types are compatible.

Yeah, that seems interesting in general. What I did here is pretty bespoke, but variant type support might generally be useful. This has just come up within the related discussion here - if we wanted to pass String|Bytes|u64 to transfer_with_memo, we'd have to use just Val within the current spec limitations. That said, if this is too complicated, bespoke mux address type support seems ok to me, given that it extends the same ScVal type (unlike user-defined type generics).

We expect consumers will want to filter on the base address only.

Yeah, I've replied in the PR, this is was just the quick stab at the muxed address support, so I didn't do much for the events. Emitting id separately would be easy.

Is the muxed value storable?

Yes, it's storable in the initial PR. I haven't implemented this check yet, but it shouldn't be hard to do. Though I guess if developer really insists on observing this, they could e.g. use XDR conversion, and I wouldn't go that deep in trying to special-case this.

The biggest win is that calls to transfer with an M address could be rendered as the M address in developer tooling, like block explorers. But because events won't render as M because we want the G+memo split there, I'm not sure even that is meaningful.

FWIW events can also be rendered with M-addresses if/when necessary, especially in the user-relevant contexts like wallet transaction history. I think the main value here really is about providing a mostly consistent (modulo events) multiplexing framework without involving the separate memos.

[dmkozh]

For the sake of completeness, I've created the SDK change and example that only support transfer_muxed, so there is no hacking with interfaces and generated clients:

• SDK change: Muxed address support sketch for Soroban SDK, transfer_muxed edition rs-soroban-sdk#1447
• Example change: Demo token implementation supporting muxed address sketch, using transfer_muxed function. soroban-examples#365

I've also sketched the storage 'protection' support - it's really pretty cheap and simple: stellar/rs-soroban-env@93cab9a

[leighmcculloch]

@dmkozh Would the MuxedAddressObject contain all muxed/memo types? I don't see any reason why internally to the host that couldn't be the case, but it wasn't obvious looking at the spike if that was the case. For handling all existing cases I think it's important it does. For example, the MuxedAddressObject should be able to hold an M / G+memo(id), or a G+memo(text|hash|return). As an input type to contract calls, only the M version would be accessible. We just need a sensible XDR representation for outputting the wider value in events.

[dmkozh]

The host object doesn't need anything but muxed id address (as you've pointed out here). As for the actual XDR, depends on the event representation. If we land on the 'full muxed address in the event data field` option, then I agree we can extend the address type. If we land on 'only mux-id in the data field' option, then ScAddress XDR doesn't need support for the non-id cases.

[leighmcculloch]

For comparison with the spike @dmkozh posted (#1553 (comment)) I've posted an example of what it would look like to implement @MonsieurNicolas's idea (#1553 (reply in thread)) (not too different to also what was posted at #1534), which I'm pondering on because of its simplicity, and how easy it can be isolated, and because of its very low future maintenance burden:

• Spike: Token Muxed Extension rs-soroban-sdk#1443

• Spike: Example Use of Token Muxed Extension soroban-examples#364

Trade-offs with this approach:

• It would be unlikely we'd add support to pass an M address via developer tools like the Lab, CLI that prefer not to write contract specific mapping logic, so while exchanges, anchors, and wallets would display M addresses and most users would experience their benefits, developers hacking on the command line would need to decode the M address (using tools) into their parts when using the Lab, CLI, or viewing on stellar.expert. This is the main advantage I see with the spike @dmkozh posted, that M addresses could render in all the places. But also, because we don't want M addresses to render in events, I think this tradeoff is okay, and maybe even preferred.

[leighmcculloch]

2. You use a new event instead of extending transfer. I know we haven't decided on this yet, but it sounded like we were leaning towards reusing transfer.

I've updated the spike to use the same event. 👍🏻

[leighmcculloch]

just plain G+memo format seems to be the least demanding for all the developers

👍🏻

the cost of footgun-y UX for the end user

The end user can still use M addresses. Exchange displays M address. Wallet accepts M address. Wallet gives M address to Client SDK fn, SDK fn decodes M address and builds transfer_muxed with two components.

[dmkozh]

The end user can still use M addresses. Exchange displays M address. Wallet accepts M address. Wallet gives M address to Client SDK fn, SDK fn decodes M address and builds transfer_muxed with two components.

Sure, we have discussed this option multiple times. My point is that it still requires special handling of M-addresses, just at different layers. Any sort of invocation/signature preview would need special handling, events would need to be reconciled into M-address in the contexts where it's M-addresses were used as input (which is a universal issue of the approaches where we break the address 2 parts in the events), block explorers may or may not to bother reconciling the components into a single address. The 'purest' option is really to not to ever bother reconciling the address parts into one - this makes the invocations, signatures and events consistent with each other everywhere (and also happens to naturally support non-integer memos, though I don't think we need to support them in Soroban). I don't like this option due to the UX implications, but just putting it out there in case if we don't want to take any complexity compromises at all.

[leighmcculloch]

because we don't want M addresses to render in events

I know we've been saying this from the start, but should we consider what it would look like if we did? Downstream consumers would need to provide a way to filter on the underlying G address.

Another approach to the event is to emit the G in the topic, and the M in the data. It would be redundant data, but it would continue to use the topic as a value intended for indexing, whilst still providing the muxed value in a single unit rather than the separated ID.

[dmkozh]

. It would be redundant data, but it would continue to use the topic as a value intended for indexing, whilst still providing the muxed value in a single unit rather than the separated ID.

Yeah, this is an interesting option. That's about +36/bytes per event, for almost consistent event representation. There is still a bit of room for confusion (e.g. mixing up source/destination addresses), but probably less than for the 'raw' ids that definitely require some bespoke processing to be displayed as M-strkeys.

On a side note, we seem to take the muxed transfer source as granted in all the options, even though the use case for it is not really clear. If only destination can have muxed address, then there is less room for confusion in every option we've considered thus far.

[leighmcculloch]

I think we should answer the question:

What percentage of tokens will need to know about memos/muxed?

cc @tomerweller @johncanneto

We've been discussing different approaches, where we introduce new first-class types (#1553 (comment), #1553 (reply in thread)), and other approaches that require less effort/maintenance (#1553 (comment)). All spikes have been extensions to SEP-41, rather than a modification of SEP-41.

If we're targeting a small number of contracts using memos, an extension makes sense.

But if we expect a lot of tokens to add mux capabilities, then I think we should consider changing SEP-41 instead. To add it to SEP-41, then @dmkozh's original proposal to bring MuxedAddress to the existing transfer function would be simplified if it didn't need to be an extension supporting both the old and the new interfaces. A lot of the complexity in that spike came from attempting to make it an extension where devs could choose one or the other. It would be simplified because SEP-41's transfer function can be changed to use the MuxedAddress and that would be the only interface the SDK publishes.

The new interface would still be backwards compatible if the SDK auto-upgraded Address to MuxedAddress, which @dmkozh's first spike did. Any new token that uses MuxedAddress could receive either Address or MuxedAddress as function arguments. Any old token that uses Address could only receive Address.

[tomerweller]

But if we expect a lot of tokens to add mux capabilities

I think that it's safe to assume that pretty much every token will be interested in being listed in an exchange which would require mux capabilities

[leighmcculloch]

👍🏻. In that case, I think @dmkozh's proposal of a first-class type, if integrated directly into SEP-41 without the burden of being an extension, seems to tell the best complete narrative. The features that stand out to me are, reusing transfer function in a way that is graceful to existing contracts with auto-upgrade behavior from Address to MuxedAddress, and where MuxedAddress never exposes the muxed ID to users and MuxedAddress is not storable, and the transfer event has the M address in the event data and G address in the event topic.

If this finds consensus, the outstanding issue is how to capture all memo types, not just ID (u64), in the transfer event.

[MonsieurNicolas]

overall approach of having a single transfer sounds good to me if we can make it work.

The thing I am not quite sure I understand is the backward compat: how would this really look like?
I think there are two separate questions:

• auto-upgrade, is this really safe (compared to just require fairly simple manual update)?
  • if contract logic assumes Address and now uses MuxedAddress but "everything compiles" (because Rust code makes heavy use of type inference), bad things are likely to happen? I'm thinking allow/deny lists, generating storage keys, or in general anywhere contract logic is just using generic vals.
  • or is the "auto upgrade" done so that the contract still only supports Address somehow (by exposing MuxedAddress at the interface level, but still binding the variable into Address maybe)?
  • in general if token contracts "panic" when given a MuxedAddress (and not an Address), how SDKs/wallet know which type to use when calling a token?
  • what about return values or other methods that take Address is this auto-upgrade only applied to transfer?
• What happens when calling already deployed contracts (that don't get recompiled) that assume arguments are Address with a MuxedAddress?
  • Are we assuming that the currently generated code will crash because everyone uses the Soroban SDK and therefore arguments are bound to Address (which would panic from what I understand)? ---> similar question from above on how does Wallets/SDKs know that those contracts need to be called with an Address and not the muxed variant?

Depending on how we answer those questions, having transfer and transfer_mux seems a lot more surgical and less likely to trigger unexpected or surprising behavior.

[dmkozh]

auto-upgrade, is this really safe (compared to just require fairly simple manual update)?

I'm not sure what exactly did Leigh mean by auto-upgrade; my understanding is that he just referred to seamless compatibility between Address and MuxedAddress (but not vice versa). I.e. as long as addresses are passed around, nothing changes for any existing contracts, but a muxed address requires a contract to explicitly be written to accept it. But that's just my understanding.

in general if token contracts "panic" when given a MuxedAddress (and not an Address), how SDKs/wallet know which type to use when calling a token?

The type is in the spec, it's the same way how they know it's an address at all (and not a string or integer).

Are we assuming that the currently generated code will crash because everyone uses the Soroban SDK and therefore arguments are bound to Address (which would panic from what I understand)?

I'd say we're more generally assuming that contracts type-check their arguments. I think it's a fair expectation, as if the contract accepts arbitrary types in place of Address, then it's already broken.

similar question from above on how does Wallets/SDKs know that those contracts need to be called with an Address and not the muxed variant?

The spec will have the 'legacy' address type.

I think the flows can be separated in a few categories, and none of them seem broken to me:

• 'Hardcoded' call flow (I'd imagine that's what a wallet would do): client uses call spec based on SEP-41 -> client expects strkey as a user input of the destination -> client converts strkey to ScVal -> (optionally) client checks that scval is compatible with the token spec (if not supported, fail on the wallet side) -> client sends the call to simulation (call fails in simulation if muxed address is not supported and there is no wallet check).
• 'Dynamic' call flow (e.g. CLI, or generated JS/TS clients): client parses the spec to come up with the argument types for the function -> arguments are parsed against the generated spec ->unsupported muxed addresses fail during the type check in the same way as any unsupported type would fail
• Static cross-contract call flow: contract uses the spec or SDK interface to generate token client -> SDK ensures static type checks -> static type checks ensure that the user needs to pass MuxedAdress type into contracts that support it and Address to these that don't
• Dynamically dispatched cross-contract flow: contract uses env.call(transfer, args) -> the called contract performs runtime argument type check -> unsupported muxed address will fail the type check in the called contract at runtime.

[leighmcculloch]

What I'm proposing is we do not make use any Rust inference, so from a Rust type perspective the Rust types Address and MuxedAddress are incompatible. There will be no use of traits or types to unify them.

But when an SDK loads a Val, coming from a function argument, into either an Address or MuxedAddress SDK type, the following will happen:

• Address SDK type will successfully unpack only a Val with tag Address. If a Val with tag MuxedAddress is passed, then it should error.
• MuxedAddress SDK type will successfully unpack:
  • A Val with tag Address
  • A Val with tag MuxedAddress

And when the Host is taking in values:

• It should always build the most compatible value possible. So:
• If an Address is in the tx, the Host should generate a Val with tag = Address.
• If a MuxedAddress is in the tx with an ID, the host should generate a Val with tag = MuxedAddress.

In terms of host functions:

• Any functions on MuxedAddress should be made to also work with Val with tag = Address, so that outside of the Rust SDK, in other SDKs, we build an expectation that MuxedAddress is a superset of Address.

• Address and now uses MuxedAddress but "everything compiles"

For existing users who uprgade their SDK:

Implementors:

• If they implement the Rust TokenInterface trait, they'll see a compile error, and will need to adopt MuxedAddress.
• If they do not implement the Rust TokenInterfact trait, they won't see a compile error, their contract will continue to use Address, if an AddresswithID i.e. MuxedAddress is passed to the contract, it will panic.

Clients:

• If they import a contract that uses Address, the generated client will accept only an Address.
• If they import a contract that uses MuxedAddress, the generated client will accept only a MuxedAddress.
• If they use the SDK TokenClient that today uses Address, tomorrow it will use MuxedAddress, and compile errors will be seen on SDK upgrade.
  • If they pass a MuxedAddress to the client, and the called contract only supports Address, the call will succeed if the underlying type of the MuxedAddress is an Address. It will fail if the underlying type is actually MuxedAddress.

• what about return values or other methods that take Address is this auto-upgrade only applied to transfer?

Auto-upgrade will need to be done on the MuxedAddress type itself. So someone could start using MuxedAddress for other things, it won't be limited to the transfer function. Same upgrade rules will apply since it occurs at the type level not the use case. Same with return values. If a contract returns an Address, and a caller unpacks it into a MuxedAddress that'll work, although most devs will unpack into a type as defined by the generated contract client, so likely a caller will expect a MuxedAddress. Both cases are okay.

[leighmcculloch]

Assuming we do #1553 (reply in thread), then we need to define how the event will render for all memo types.

1. Break apart consistently

G/C ->

topics: ["transfer", from:G|C, to:G|C]
data: { amount: i128 } 

M ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, from_memo:u64|String|BytesN<32>, to_memo:u64|String|BytesN<32> }

G+memo(id|text|hash|return) ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, to_memo:u64|String|BytesN<32> }

2. M where-ever possible

G/C ->

topics: ["transfer", from:G|C, to:G|C]
data: { amount: i128 } 

M ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, from:M, to:M }

G+memo(id) ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, to:M }

G+memo(text|hash|return) ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, to_memo:String|BytesN<32> }

3. A new M-strkey that can store all types of memos

G/C ->

topics: ["transfer", from:G|C, to:G|C]
data: { amount: i128 } 

M ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, from:M, to:M }

G+memo(id|text|hash|return) ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, to:M }

4. Embed in ScAddress but no new strkey

G/C ->

topics: ["transfer", from:G|C, to:G|C]
data: { amount: i128 } 

M ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, from:M, to:M }

G+memo(id) ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, to:M }

G+memo(text|hash|return) ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, to:{addr:G, memo:String|BytesN<32>} }

Thanks @sisuresh @dmkozh for contributing to these ideas.

[dmkozh]

mux seems ok to me as well.

[leighmcculloch]

from_muxed_id? I don't love 'mux', but muxed_id sounds like it connects clearly to the muxed address.

[leighmcculloch]

id on its own is mildly ambiguous, but not significantly.

[dmkozh]

muxed_id makes sense too.

[leighmcculloch]

Okay so, updating further:

G/C ->

topics: ["transfer", from:G|C, to:G|C]
data: { amount: i128 } 

M ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, from_muxed_id:u64, to_muxed_id:u64 }

G+memo(id|text|hash|return) ->

topics: ["transfer", from:G, to:G]
data: { amount: i128, to_muxed_id:u64|String|BytesN<32> }

[leighmcculloch]

In the protocol meeting today we discussed a bit what would happen with transfer events that had a mix of muxed accounts and transaction memos.

Are we leaning towards doing the following?

1. from_memo is set to the muxed ID of the op/tx source account if the op/tx source account is a muxed account? i.e. from_memo can only be None or U64.

2. to_memo is set to, in order of priority listed here:

   • the muxed ID of the to field if set in the to value
   • the memo if set in the tx

[leighmcculloch]

we shouldn't do anything about the payload of the events emitted by the contracts. This is surprising for the developers and is generally really sketchy.

💯

[dmkozh]

One thing that comes to mind is that if we prohibit memos, but folks still want to tag transactions for non-multiplexing reasons, we could add either a new tx field, or a new memo variant, like transaction_tag_not_to_be_used_for_identifying_payment_destination (I might be slightly exaggerating here) and explicitly state that it's never included into any events and also doesn't need soroban authorizers to know about it. This surely might backfire, but maybe there is a way to make the probability of misuse low enough?

[leighmcculloch]

If someone needs it, they can propose it, with more context on what they're doing. For Soroban transactions folks can introduce any context they want by wrapping function calls in other function calls that output any other event data they need without constraints.

[leighmcculloch]

@sisuresh The CAP should probably be very explicit about how these mapping occur for every operation they are implemented for. For example, path payments, claiming claimable balances?, movements of values with liquidity pools? What will happen in all these cases with memos or muxed? Especially cases where the source account is actually the destination of value.

[dmkozh]

If someone needs it, they can propose it, with more context on what they're doing.

Yeah, I'm not suggesting to address that in this CAP, rather that maybe prohibiting the regular memos wouldn't mean that there is no other way to tag transactions. Non-protocol solution might be ok here as well.

[Shaptic]

I hate to necro a topic but a lot has changed since we last discussed it so I wanted to get a temp check on it.

How do we feel about ScAddress extending LP and CB IDs? LPs and CBs are only ever used in the output of events, so there are tradeoffs:

• This means significant downstream impact because devs will have to handle additional union arms in both contracts, dApps, and backend code for scenarios that will never occur.
• However, the alternative is a partially-redundant and new ScVal arm that either has all of the address types (meaning new ones would be added to both places) or only the non-ScAddress, event-only ones (which feels really weird).

In a similar vein, I can see an argument made for MuxedAccounts not being part of ScAddress, but those can, in contrast, at least be the inputs to the new transfer signature so it does make sense to handle for contract devs. However, the point still stands that people will have to filter those out of other contract invocations, authorization entries, contract data entries, etc.

Prior discussion (not "required reading" but included for completeness):

• CAP-0067 update after 02/27 protocol meeting #1667 (comment)
• CAP-0067 update after 02/27 protocol meeting #1667 (comment)

[leighmcculloch]

especially if there is an easy strkey<->ScAddress mapping for the 'unsupported' address types

What does this mean?

This might create an incentive for creating some redundant test coverage everywhere one deals with strkeys and ScAddresses (e.g. this has happened to the expired entries - even though they may never reach the contract, people still want to be able to cover the host behavior with expired entries, which kind of makes sense given that it's a theoretically valid input given the data structures).

Most tests devs write for contracts use the SDK types, and the Address SDK type won't be able to represent the other types.

Even if they are testing with ScAddress values, I don't think this is a meaningful con. Anyone who tries to invoke a contract with the other address types that don't support invocation would see an error. There's nothing ambiguous, and this is no different.

We're trying to capture the validity of inputs using the type system, which is always limited. It's normal that in some contexts an input be invalid, and in another it be valid.

As I posted at #1667 (comment), this is a tradeoff where adding a second ScVal arm for addresses has a moderate impact to developer tooling not being able to convert strkeys to ScVal's reliably. I don't think the type system gains outweigh the cost of no longer being able to convert ScVal <> strkeys.

[dmkozh]

What does this mean?

I mean that if you deal with a client that accepts strkeys and converts these to ScAddress in order to build a transaction (as per contract spec) (say, CLI, or a wallet), then you will constantly encounter unsupported strkeys.

this is a tradeoff where adding a second ScVal arm for addresses has a moderate impact to developer tooling not being able to convert strkeys to ScVal's reliably.

I actually think that it's a feature, not a bug. LP/CB strkey->ScAddress (or ScVal::Address) is a really sketchy conversion as it doesn't seem to have any use cases (OTOH if your interface is string->ScVal, then it doesn't really matter where LPs/CBs go). LP/CB ScAddress->strkey conversion seems useful for e.g. presenting the unified events to the user, but I'm not sure if it's problematic to use a separate ScVal type for this, given that it's likely that at the presentation layer ScVal->string interface will be used. Basically since this is an output-only type, it seems useful to exclude it from all the input code paths, and for the output code paths the complexity difference doesn't seem meaningful to me.

I don't think the type system gains outweigh the cost of no longer being able to convert ScVal <> strkeys.

ScVal<->strkey conversion is not affected in either option. Strkey library itself also is not affected in either option as it doesn't operate on the contract types at all. So only ScAddress<->strkey conversion is affected, and that's not necessarily a bad thing.

[sisuresh]

I don't feel too strongly for or against either solution, but we do have to decide between the two. I do see the positives of using a separate type, but my intuition says that a single type will still be better. Does anyone feel strongly one way or another?

[leighmcculloch]

my intuition says that a single type will still be better

+1

I wouldn't worry too much about filtering them out, unsupported type will be caught sooner or later in transaction submission process.

+1

I think the confusion introduced by a separation workaround is larger than the confusion on handling these extra variants downstream.

+1

---

I think we should run with the single ScVal::Address arm for all address types.

[dmkozh]

Ok, I still feel like the complexity of supporting two disjoint address types in the ingestion systems might be exaggerated here (notably we're doing the same thing to the event data payload in this CAP as well and no-one seems to be too concerned about it bit overly difficult to support) - I would be surprised if XDR was stored in the DB as is instead of doing some conversion. But I don't want to block this anymore, so if everyone is fine dealing with special addresses then let's keep everything in ScAddress and close this discussion.

[anupsdf]

Should we add a new ContractEventType calling it native or classic or something else instead of reusing contract type for emitting classic events?

native: asset, topics: ... instead of contract: asset, topics: ...

[anupsdf]

If we did change the type, downstream services might be impacted if they are setup to parse for contract type.

[anupsdf]

Note that the contract field for these events corresponds to the Stellar Asset Contract address for the respective asset. The Stellar Asset Contract instance is not required to be deployed for the asset. The events will be published using the reserved contract address regardless of deployment status.

Just read this in the CAP which clarifies that classic assets will publish events using reserved contract address. So in future is a Stellar Asset Contract is deployed, it will make it easy to reconcile.

[dmkozh]

This might be going against the idea of the event unification, especially in the cases of the simple transfers (like payments). These conceptually are the same as the respective SAC transfers and it's not clear why the events should be different. If the type of the tx that produced the event is important, than consumer could look at e.g. TransactionResult to figure out which operation has produced it (or of course the transaction itself). But token events themselves by design don't try to provide context for the execution environment had produced them (for better or worse).

[leighmcculloch]

token events themselves by design don't try to provide context for the execution environment had produced them

+1.

[dmkozh]

This can be considered closed now.

[dmkozh]

I've probably missed the discussion regarding the fee event, but do we really need a separate event instead of using burn? Technically XLM is not completely burned, but put into the fee pool. However, I'm not sure if that's relevant for the event consumers, while the need to ingest a special event type is definitely relevant for them (and it creates some additional complexity in understanding the events).

[dmkozh]

Using the burn event makes an assumption.

Which assumption? The token is effectively burned for all intents and purposes relevant for the token holder.

easy to understand in the context of other blockchains

I couldn't quickly find a chain that has a SAC analog, so it's kind of hard to compare. But my main concern here really is that this event is still a SAC event (i.e. not a special system event for the fee), so I'm not sure why do we go just half way there and still require users to do some seemingly unnecessary special-casing.

[leighmcculloch]

Using the burn event for fee payments makes an assumption that downstream systems, and developers, don't care about distinguishing fees.

A cursory look around shows that blockchain analytics products are usually interested in fees independent of asset movements, as well as in relation to other events. For example: Dune indexes fees, Etherscan indexes fees relative to other events, Artemis tracks fees separately from burning in their operating templates.

For systems that locate fees elsewhere in the ledger close meta, and also process asset events, having to marry the fees with burn events seems like a poor experience vs marrying them with unique fee event.

it creates some additional complexity in understanding the events

If we also think about the developer experience, viewing events for a transfer and seeing burns of dust would be confusing, compared to seeing events for fee payments. The narrative for developers is much clearer. Developers know what fees are, seeing a fee event won't significantly increase the complexity of understanding.

Burns are a significant event where the why matters, which is why clawback was a separate event, and why I think fees should be separate.

cc @chowbao or @sydneynotthecity who might be able to provide more detail here.

[dmkozh]

Using the burn event for fee payments makes an assumption that downstream systems, and developers, don't care about distinguishing fees.

Distinguishing fees is still easy when necessary:

• Fee events have a separate location in the meta structure
• XLM can't be burned otherwise, so it's safe to assume that every XLM burn is a fee payment.

Burns are a significant event where the why matters, which is why clawback was a separate event, and why I think fees should be separate.

I don't disagree here, but I can't say that we have a consistent policy for the event reasons. E.g. transfer events don't contain any 'reasoning' information. I think burn is justifiable here, because similarly to the token burn operation (or a transfer to the issuer) the event comes from an action that the user has explicitly authorized themselves. This is not the case for clawback, which represents a unique case where someone takes token from the user without the user ever authorizing that.

That said, I don't feel too strongly here. If a slightly simpler event model is not worth the tradeoff of identifying the fees indirectly, then we can keep the bespoke event as well. This seems a bit arbitrary to me as I've mentioned above (e.g. we seemed to agree that #1553 (comment) suggestion is adding some unnecessary specificity to the events), but I can live with the separate event approach as well.

[chowbao]

The fee event is nice otherwise we would need to parse through xdr to find out if a burn was a fee.

• Fee events have a separate location in the meta structure

RPC getEvents() endpoint is a flat stream. The fee event is in a different location in xdr but the output event won't know this.

• XLM can't be burned otherwise, so it's safe to assume that every XLM burn is a fee payment.

Personally I like parsing this info with a clear fee instead of needing to know that if I want fees I have to know to look for burn and asset == native.

[dmkozh]

Ok, let's keep this as is and close this discussion.

[dmkozh]

While working on the prototype for the ScAddress::MuxedAccount support I've noticed that putting the whole MuxedAccount struct into ScAddress is rather annoying, because now there are two ways of building exactly the same address for a non-muxed G-account. Events won't contain this type at all. I assume we would just prohibit the non-muxed variant of MuxedAccount, but it still might be rather confusing for the users. Thus I'm suggesting to use

case SC_ADDRESS_TYPE_MUXED_ACCOUNT:
struct
    {
        uint64 id;
        uint256 ed25519;
    } muxedAccount;

instead. This way there is no ambiguity built into the data structure. FWIW this seems consistent with the strkey library representation as well. Any objections?

[dmkozh]

Yeah, I can make it a named struct as well, but we shouldn't modify the MuxedAccount.

[Shaptic]

Could you re-use MuxedAccountMed25519 for this?

[dmkozh]

No, there is no such struct in XDR. It's unfortunately defined as an anonymous struct, and I assume the MuxedAccountMed25519 names comes from the generated code.

[dmkozh]

I've created a new named struct for this and we might eventually reuse it in the MuxedAccount as well. But I think this discussion can be closed as the change is non-controversial otherwise.
Updated in #1680

[Shaptic]

Gotcha, thanks for the clarification.

[leighmcculloch]

I see change #1686 occurring adding the following statement:

To make sure the events are emitted in a way where the balances don't go negative and make sense, any XLM mint should be emitted first in OperationMetaV2, and burns should be emitted last.

Does this only affect past operations? or also new operations that execute in P23?

Are these events from within a single operation only? or can this reordering span operations in a single transaction?

@sisuresh Can you share a list of all the operations the reordering can happen within and some examples?

[sisuresh]

Looks like I should reorganize the section a bit to make this clear. That paragraph and the one before it is about a bug pre protocol 8 where xlm was minted and burned. We need to emit events for these bugs so balances can be built using events. I mention the order because the mint/burn bug doesn't happen as the intended result of the operation, so it may not be clear when it should be emitted in the context of an operation.

The actual logic that I believe should be sufficient is this - if the ledger is pre-protocol 8 and we identify a mint/burn of XLM that isn't due to inflation, the operation should be one of -

1. ACCOUNT_MERGE - emit mint to destination source. Example
2. PAYMENT - emit mint to source. Example
3. PATH_PAYMENT_STRICT_RECEIVE - emit burn from source. Example

[karthikiyer56]

@sisuresh, is example 1 correct?
I think in the account merge example, also, it is a mint to the source and not to the destination.
Basically, to take into account the pre protocol 8 bug, and to maintain the invariant, the mint/burn happens from/to the source account of the operation strictly.

more accurately, for example 1 - ACCOUNT_MERGE - emit mint to destination Example, it shud be mint to source instead.

Rationale:::

• for operation-0 - it is an account-merge from GAZ... to GDN...
  so a transfer event from: GAZ..., to: GDN... for amount 999799999600 will happen, since it is an accountMerge operation. After this operation, GAZ... should ideally have been deleted.
  So exactly one event - transfer, should be emitted, and it would look something like

{
  "meta": {
    "ledgerSequence": 8597093,
    "closedAt": "2017-01-15T03:12:26Z",
    "txHash": "6de6776a6a391ad4d0d1c1a6d8cfc6a85b4a37f76c2728f3ec97d9ccecb76c57",
    "transactionIndex": 0,
    "operationIndex": 0,
    "contractAddress": "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA"
  },
  "asset": {
    "native": true
  },
  "transfer": {
    "from": "GAZABMZS7VKMX7SRL3DDA6YW7VGAZJOKPVBMSK5FH66BZMHZDG66ASWM",
    "to": "GDN3RK562Z442M662Z2EDCK7KR7ICXEXD6VJ5H3Z7CX36T2AZ2CWQS6Y",
    "amount": "999799999600"
  }
}

• for operation 1 - it is an account-merge - again from GAZ... to GDN.... This is where the bug is exercised, since account GAZ... was deleted in the prior operation. So, a transfer from GAZ... to GDN... should appear because thats what we intend to emit for an AccountMerge operation. But before that, we need to issue a mint for GAZ.., i.e. the source account of the operation. This is how we have defined we wud be emitting events for the source account of the operation according to this section in the CAP
  
  So, the events should look like so - first mint, then transfer:

{
  "meta": {
    "ledgerSequence": 8597093,
    "closedAt": "2017-01-15T03:12:26Z",
    "txHash": "6de6776a6a391ad4d0d1c1a6d8cfc6a85b4a37f76c2728f3ec97d9ccecb76c57",
    "transactionIndex": 0,
    "operationIndex": 1,
    "contractAddress": "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA"
  },
  "asset": {
    "native": true
  },
  "mint": {
    "to": "GAZABMZS7VKMX7SRL3DDA6YW7VGAZJOKPVBMSK5FH66BZMHZDG66ASWM",
    "amount": "999799999600"
  }
}

{
  "meta": {
    "ledgerSequence": 8597093,
    "closedAt": "2017-01-15T03:12:26Z",
    "txHash": "6de6776a6a391ad4d0d1c1a6d8cfc6a85b4a37f76c2728f3ec97d9ccecb76c57",
    "transactionIndex": 0,
    "operationIndex": 1,
    "contractAddress": "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA"
  },
  "asset": {
    "native": true
  },
  "transfer": {
    "from": "GAZABMZS7VKMX7SRL3DDA6YW7VGAZJOKPVBMSK5FH66BZMHZDG66ASWM",
    "to": "GDN3RK562Z442M662Z2EDCK7KR7ICXEXD6VJ5H3Z7CX36T2AZ2CWQS6Y",
    "amount": "999799999600"
  }
}

• Similarly, operation-2 does the same thing as operation-1, i.e. try to merge the previously deleted account GAZ..., so we issue a mint for GAZ.., followed by a transfer from GAZ... to GDN...., as described for operation-1 above

{
  "meta": {
    "ledgerSequence": 8597093,
    "closedAt": "2017-01-15T03:12:26Z",
    "txHash": "6de6776a6a391ad4d0d1c1a6d8cfc6a85b4a37f76c2728f3ec97d9ccecb76c57",
    "transactionIndex": 0,
    "operationIndex": 2,
    "contractAddress": "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA"
  },
  "asset": {
    "native": true
  },
  "mint": {
    "to": "GAZABMZS7VKMX7SRL3DDA6YW7VGAZJOKPVBMSK5FH66BZMHZDG66ASWM",
    "amount": "999799999600"
  }
}

{
  "meta": {
    "ledgerSequence": 8597093,
    "closedAt": "2017-01-15T03:12:26Z",
    "txHash": "6de6776a6a391ad4d0d1c1a6d8cfc6a85b4a37f76c2728f3ec97d9ccecb76c57",
    "transactionIndex": 0,
    "operationIndex": 2,
    "contractAddress": "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA"
  },
  "asset": {
    "native": true
  },
  "transfer": {
    "from": "GAZABMZS7VKMX7SRL3DDA6YW7VGAZJOKPVBMSK5FH66BZMHZDG66ASWM",
    "to": "GDN3RK562Z442M662Z2EDCK7KR7ICXEXD6VJ5H3Z7CX36T2AZ2CWQS6Y",
    "amount": "999799999600"
  }
}

Examples 2 and 3 are accurate, in that - in example 2, it will be a MINT to source account. In example 3, it will be a BURN from the source account.

[sisuresh]

Yeah that was a mistake. It's a mint to the source.

[leighmcculloch]

I see that there's a change for CAP-63 that says that refunds of fees will occur later outside of individual transaction applies:

• CAP-0063 - specify refund logic #1673

And that there's a related change in CAP-67 that says that fee refunds will not have their own event, rather a single fee event per transaction will be emitted that is the net result of the initial amount debited minus the refund:

• CAP-0067 - clarify refund #1669

I think these changes will create a subtle inconsistency in the balance of an account when viewed from different perspectives.

When viewed from the perspective of a contract that executes during the ledger, they will see the balance of the account as the account balance minus the full fee.

When viewed from the perspective of an off-chain system ingesting events, they will see the balance of the account as the account balance minus the net fee.

Contracts will see the account balance in a state that off-chain systems will never see it in.

I'm not convinced this problem will be material, or that it's worth having a separate refund event to exactly represent the changes in value. It'd be nice if the transition was also hidden from contracts, but I don't think that's possible.

[leighmcculloch]

Does this put a burden on event consumers, or does it lighten it?

Event consumers have three options for consuming events via RPCs, so I don't think getEvents is a burden to consumers, or rather if it was, and they needed per transaction event blocks they could do that with other methods:

• getEvents - chronological endless event stream with no concept of tx or ledger boundaries. Events stand on their own.
• getTransactions - get events for a single transaction.
• getLedger - get events for a whole ledger.

[tamirms]

it's not clear if the fee and fee refund events should be included in getTransactions() because the fee events are applied before any transactions are executed

[leighmcculloch]

it's not clear if the fee and fee refund events should be included in getTransactions()

getTransactions resultMetaXdr field exposes all events including transaction fee / fee-refund events. Is that not the case?

[tamirms]

the tx meta does include the fee events but the ordering of events requires that all fee events in a ledger be processed before all operation events in a ledger. So, if we include the fee events in the getTransactions response it's a bit misleading because the consumer would assume all the events in the response are consecutive without any gaps. But, in actuality that is not the case.

[Shaptic]

I don't think that discussion belongs to this CAP's discussion

Great point; I agree 👍 we've diverged from the "me trying to understand event ordering from a meta perspective" to a "how should RPC present this" perspective. I opened stellar/stellar-rpc#414 for us to track the work, so interested parties should discuss there instead of in this thread.

[sisuresh]

@jayz22 pointed out this early success case in PaymentOp where we return early if you're paying yourself. Should we emit an event this specific case? I don't think it's necessary, but wanted to make this known because the TTP probably emits an event in this scenario. A path payment that sends and receives the same amount of XLM would emit an event, so I could see the argument for consistency, but I'm not a fan of doing additional work to emit an event that has no value. @karthikiyer56 @leighmcculloch

If we decide not to add the event, then we should update the CAP to mention that an operation might not emit an event if no movement of value occurs.

[leighmcculloch]

I think we should consider how to prevent it, because this is a very surprising for downstream events processing events in an event stream fashion, where a balance will go negative.

Another reason to consider this is it may not only be about downstream systems. If the protocol ever emits balance events (#1591), or emits balances in token events, those balances will be negative.

It's also fair to say that this is just a feature of classic, and events should just model how balances can go negative. It might also be more correct to say that incremental asset movements are more accurate and this is just the trade off.

We should get more perspective on this issue. cc @tomerweller @JakeUrban

[tomerweller]

Yeah, this is a pickle. I think that having transfer events that break an implicit "balance is non-negative" invariant would be idiosyncratic to the rest of the blockchain ecosystem and has the potential to break downstream systems.

My mental model of this is to think of the DEX as a smart contract. If it were to be a contract invocation that processes multi-hop trades then I would imagine it would do a bunch of internal accounting and then execute the net transfers (IIRC this is what the uniswap router does for multi hop trades). So from an events perspective this would look like net transfers, which would be my vote.

[sisuresh]

Net transfers in the DEX sounds complex. I'm guessing in the Uniswap case, the router contract itself is the counter party on transfers, which doesn't work with the DEX. I need to think about this.

[tomerweller]

OK. I think net transfers are preferred, but not at the cost of a meaningful delay for p23

[Shaptic]

unless all consumers batch events by transaction

Imo we should instead make this clear from the get-go: Stellar has invariants that holds across transactions, not events. Flash loans are one example of this, but I wouldn't be surprised if there were other cases where balances go negative during the course of a transaction but are positive at the end of it. Even if that is the only possible case, it's still a good rule of thumb: operations are atomic, all other bets are off.

[sisuresh]

Currently for Soroban transactions, the diagnostics vector contains all events (diagnostics, events from failed invocations, and events for successful invocations). This was done because the diagnostics might be interleaved with non-diagnostic events, and having all of them in a single vector would let you consume all events in order.

For classic transactions, the current implementation does not duplicate operation events into the diagnostics vector, and will only exist in the non-diagnostic operation level vector. We don't emit diagnostics for classic operations, so there's no interleaving. Is this an issue? I believe there might be some consumers that solely go off of the diagnostics vector with the expectation that it will contain all events.

@Shaptic @mollykarcher @dmkozh @leighmcculloch

[leighmcculloch]

In a single list I think the structure would need changing slightly to include an operation index. Or could events be linked to operations another way?

[Shaptic]

I would prefer the per-operation breakdown and the way things are structured today. If diagnostic events didn't duplicate Soroban events, that'd be slightly better (less meta redundancy despite us needing to make minor code changes), but doesn't matter much from our perspective as long as the choice is documented.

While RPC may just continue to flatten this list for getEvents purposes, that isn't the only place events are consumed. We shouldn't rely on implicit ordering via some operationIndex within a flat event list, that's janky imo. A 1:1 relationship between operations and their events alongside a transaction-level array makes perfect sense and matches existing meta conventions.

Imo diagnostic events are a red herring in this discussion. I agree with the sentiment that these shouldn't even be exposed in the getEvents stream, so keeping them separate in meta (duped or not duped is a Core choice, I don't think people generally care about interleaved contract vs. diagnostic ordering but maybe they do) creates a clear "these are ephemeral, non-standard, non-protocol structures meant for humans to read" notion.

[karthikiyer56]

I would prefer the per-operation breakdown and the way things are structured today

+1 to what @Shaptic said.
I too would prefer to have a list of operationEvents - one list per operation in the tx. It is easier to reason from a platform team/go mono repo perspective to have a list of lists.
Besides, this is consistent with what we have today for operationMeta. i.e within TransactionMeta, there is a list of OperationMeta, i,e one list for per operation of all ledgerEntryChanges impacted by that operation within the transaction.
That, imo, intuitively makes more sense, structure wise than having a single flattened list and then injecting an additional opIndex field in the event stream.

From what I understand, (and I could be wrong), the choice to include smart-contract related events at the transaction level instead of at the operation level might hinder, if we ever choose to have more than 1 smart contract invocation in a Tx.
And this change to include smart contract events under OperationMetaV2 is to, in addition to account for classic operations, will help us do just that.

Re:

Soroban contract events are still emitted for the second time in diagnostic events

can we simply, going forward as of P23, not include contract events (mint/burn/transfer....) at all, in TransactionMetaV4. diagnosticEvents.
Whats stopping us from doing that and documenting it in the release notes?
From what I understand, the diagnostic events are used to convey information about - "what might have caused my soroban transaction to fail", and including legit contract events with diagnostic events (as we do it today) might even be misleading.

Re:

We could keep this as is and revisit the event design in the future protocol.

I have a differing opinion on this. P23 is already shaping up to be pretty big because of unified events. It might be best to bite the bullet now and reconcile the contractEvents/diagnosticEvent dichotomy in P23 itself

[dmkozh]

From what I understand, (and I could be wrong), the choice to include smart-contract related events at the transaction level instead of at the operation level might hinder, if we ever choose to have more than 1 smart contract invocation in a Tx.

FWIW that already happens with classic events, so it doesn't really matter if we ever have more than a single smart contract operation.

can we simply, going forward as of P23, not include contract events (mint/burn/transfer....) at all, in TransactionMetaV4. diagnosticEvents.

That's an option, but I'm not sure if it's in the scope of this CAP. It's also not a protocol change, so we might address this after p23 release if necessary.

I have a differing opinion on this. P23 is already shaping up to be pretty big because of unified events. It might be best to bite the bullet now and reconcile the contractEvents/diagnosticEvent dichotomy in P23 itself

I'm not sure I follow the logic here. P23 is indeed big and we need to converge on something if we want to ever ship it. If current solution is good enough, let's just go on with it and update later if necessary. I realize the current XDR structure may not be perfect, but I'm not sure if it's bad enough to spend more time on the overhaul and downstream integration. This also has nothing to do with CAP-67.

[karthikiyer56]

re:

FWIW that already happens with classic events, so it doesn't really matter if we ever have more than a single smart contract operation.

Not following this part. What happens with classic events? There are no classic events atm.

re:

This also has nothing to do with CAP-67.

Word...
In that case, lets not change anything and mark this discussion as complete?
Meaning, lets retain the one event list per operation way of working as shown in the current version of CAP-67

[karthikiyer56]

-- Thread about Muxed Info for Mint and Burn events ---

Reading through this section in CAP-67, the impression i get is that the from_muxed_id and to_muxed_id, shud they appear, will only appear when eventType = transfer.

What about when eventType = mint or burn?

Consider the case of a simple payment Op - where operationSource = MuxedAccount-A, and destination = AssetIssuer
This will show up as a Burn event.
WiIl there be a from_muxed_id in the BURN event then?
This might be the case where an exchange wants to BURN some asset from one of it's user's account (hence the use of muxed-account in the operationSource)

Same question for mint.
Consider the case of a simple payment Op - where operationSource = AssetIssuer, and destination = MuxedAccount-B
This will show up as a Mint Event.
will there be a to_muxed_id in the MINT event?
This might be the case where a mint is happening from an asset issuer to some exchange user's account (hence the use of muxed-account in the destination)

And what if txMemo is set, instead of the destination being a MuxedAccount in the case of the mint?

In all of these cases, IMO, the appropriate muxed (either from_ or to_) information shud be included, no?

cc @sisuresh @leighmcculloch @dmkozh

[karthikiyer56]

Re:

However, in this specific case....I think it's ok to drop them altogether from our events.

@tomerweller - Just to clarify.
Your recommendation is to remove all mux information from all events altogether, i.e including transfer events,
or
did you mean that we shudnt worry about including it in burn/mint/clawback events whilst retaining them in transfer events?

[leighmcculloch]

@karthikiyer56 We're proposing removing source / from muxed info, not all muxed info. Destination / to muxed info will still be included.

[karthikiyer56]

Ack.
My bad. I misinterpreted it to mean removal of muxed info altogether.
If we decide to remove source muxed info, that would certainly simplify path payment related events .
As in, the destination mux info will only be present in the last event,if at all.
+1 to removing source muxed info.

The only question then remains - do we include destination mux info in mint events as well?

[sisuresh]

It sounds like we have some consensus on -

1. Removing from_muxed_id from all events, and updating the transfer function to take an Address instead of a MuxedAddress as input for from.
2. Allow to_muxed_id in the mint event and follow the same rules as the transfer event.

I'll work on updating the CAP and implementation.

[karthikiyer56]

Shall we marked this thread as closed please then? with the consensus decribed ⬆️
cc @chowbao

[leighmcculloch]

@sisuresh I can't find in CAP-67 a clear description of when the fees are deducted and credited to the fee paying account, and therefore when the fee events should be interpreted to have had an effect on the balance. I know somewhere in the threads above it was mentioned that:

• the initial fee is always charged before all txs start being applied
• <p23 the refund occurs immediately after the tx applies, before the next tx applies
• >=p23 the refund occurs after all txs have finished being applied

Is that correct? If that's not captured in CAP-67 can it be?

cc @tamirms (prior conversation offline is here)

[sisuresh]

The p23 refund change is documented here in CAP-0063. The ordering description is missing from CAP-0067. I'll add it.