SAC clawback should also emit a burn event

[chowbao]

Proposal

Add emitting a burn event on SAC clawback

Changes

/// Clawback `amount` from `from` account. `amount` is burned in the
/// clawback process.
///
/// # Arguments
///
/// * `from` - The address holding the balance from which the clawback will
///   take tokens.
/// * `amount` - The amount of tokens to be clawed back.
///
/// # Events
///
/// Emits an event with topics `["clawback", admin: Address, from: Address, sep0011_asset: String],
/// data = amount: i128`
+ /// Emits an event with topics `["burn", from: Address, sep0011_asset: String], data = amount:
+ /// i128`
fn clawback(env: Env, from: Address, amount: i128);

Reasoning

CAP-46-06 defines that a clawback should burn the amount from the clawback but current implementation does not actually emit a burn event. It only emits a new clawback event (seen in https://github.com/chowbao/stellar-protocol/blob/master/core/cap-0046-06.md#semantics-admin-interface)

[dmkozh]

We probably should just clarify the semantics of the clawback better and maybe not use the word 'burn' there. burn event is there for self-burning the token. clawback is for admin-controlled burns. I'm not sure it's a good idea to duplicate the events now - it will likely cause confusion in the downstream systems and cause them to double-count the burns.

[sisuresh]

Yeah I agree emitting a burn event with clawback info works as well. Changing the format of the current burn is more intrusive though because burn is already in SEP-41. The amount in the burn event is not a container unfortunately. Would we put the clawback info into the topic? Or specify a second burn event for backwards compatibility?

[dmkozh]

Can't we just append a value after amount? I think that's acceptable as a protocol change. That also has pretty low disruption potential (someone could be broken, but the fix is very simple).

[sisuresh]

Depends on if we're fine with making a breaking change like that. I agree the fix is simple. Existing tokens also wouldn't be able to adapt as well.

[dmkozh]

Anything we might do here (besides preserving the status quo and updating the docs) is a breaking change. I think it's acceptable at protocol boundaries, especially from something as underused as clawbacks are thus far.

[leighmcculloch]

If we think distinguishing clawback is important, then why not say that the token may be removed from supply either via burn, or via clawback?

👍🏻 After more thought I'm changing my opinion and think this is reasonable, that SEP-41 can get a clawback event, but no function to perform it similar to the mint proposal, and that clawback event should can be described in a way that is optional (without necessarily using that word, since most elements of the standard are ultimately optional). It can say that the event should be interpreted by external systems as a burn with a different cause, and that the event is intended to be useful to tokens that allow for burning by some action that is outside the control of the token holder, where there is value to signal to the community that token value has been reclaimed by the contract or token. Token implementations do not need to use it, but can if it improves the semantics and transparency of the contract's behaviour.

This pattern of continuing to heap on SEP-41 doesn't scale, but the SAC is the default SEP-41 token, so we should avoid significant divergence in their semantics and behaviours. And clawback is a reasonably generic behaviour that custom tokens that don't support it can just never generate an event for.

So I'm in favor of moving ahead with the other proposal instead:

• Custom Tokens emit Clawback events #1590

[chowbao]

Discussion has slowed.

The main conclusion is that we don't want to emit 2 events for a clawback action. But It's unclear what the conclusion for the CAP should be

• Do we want to get rid of clawback events?
  • No
• Instead of the clawback event do we want to append clawback info to a burn event to behave as the "clawback" event?
  • No
• Update CAP-46-06 to say what events actually should be emitted

[chowbao]

Closing discussion. Clawback event will not change. Admin will be removed