stellar / stellar-core

Reference implementation for the peer-to-peer agent that manages the Stellar network.
https://www.stellar.org
Other
3.12k stars 967 forks source link

Asset issuance tracking #3324

Open jonjove opened 2 years ago

jonjove commented 2 years ago

Right now, the total issued amount of non-native assets are not tracked. This is an impediment to implementing new invariants (such as #1496) and new protocol features (such as SPEEDEX).

Asset issuance can be tracked by adding a new type of InternalLedgerEntry (which I will refer to as AmountIssuedEntry but I'm open to other names). AmountIssuedEntry contains an asset and an amount. Asset issuance always occurs in TrustLineWrapper::IssuerImpl, so this class must be modified to properly maintain AmountIssuedEntry.

Unlike other types of InternalLedgerEntry, AmountIssuedEntry cannot be ephemeral so it must be recorded in the database.

jayz22 commented 2 years ago

Asset issuance always occurs in TrustLineWrapper::IssuerImpl, so this class must be modified to properly maintain AmountIssuedEntry.

Besides asset issuance, asset clawback operation also changes the outstanding amount, and it is a NonIssuerImpl op?

sisuresh commented 2 years ago

Asset issuance always occurs in TrustLineWrapper::IssuerImpl, so this class must be modified to properly maintain AmountIssuedEntry.

Besides asset issuance, asset clawback operation also changes the outstanding amount, and it is a NonIssuerImpl op?

Jay and I went over this during our 1 on 1 and cleared things up. Clawback currently doesn't load the TrustLineWrapper for the account initiating the clawback (the issuer).

jayz22 commented 2 years ago

To lay out the scope of work for this issue.

  1. Add a new internal ledger entry field AssetAmountIssuedEntry:

    enum class InternalLedgerEntryType
    {
    ASSET_AMOUNT_ISSUED,
    };
    struct AssetAmountIssuedKey
    {
    Asset asset;
    };
    struct AssetAmountIssuedEntry
    {
    Asset asset;
    uint128_t amount;
    };

    and adapt the new type into LedgerTxn and SQL. The uint128_t amount is large enough to hold the maximum quantity of the asset being ever issued, and unsigned is good enough since this is for internal tracking.

  2. Workflows that affect amount of asset issued:

    • Payment (PathPaymentStrictSend and PathPaymentStrictReceive)
    • Manage Offer
    • Liquidity Pool Deposit / Withdraw
    • Clawback
    • Create Claimable balance
    • Claim Claimable Balance
    • Clawback Claimable Balance

Clawback and Clawback Claimable Balance currently doesn't load the TrustLineWrapper for the issuer (as Siddharth pointed out). We'll need to first add that part. Then all the AssetAmountIssued altering logic can be consolidated in the TrustLineWrapper::IssuerImpl.

  1. Handle the corner case where authorization is revoked for an trustline contributing to a liquidity pool (as Siddharth pointed out below).

  2. Initialize the AssetAmountIssued for all assets during catch up work. Right after applying buckets completes, we calculate the total amount issued by aggregating over all trustlines of a particular asset, and persist it.

  3. Add/update test cases in operations mentioned above.

  4. Depending on Speedex design, liabilities may affect the amount issued. For now we assume they don't, but need to verify. (Speedex offers are different from the DEX orders and won't affect the liabilities)

sisuresh commented 2 years ago

This plan looks good to me and I think you're ready to start. Just a few points -

  1. The InternalLedgerEntry change requires some LedgerTxn and SQL changes (I'm pretty sure you're already aware of this, just putting this here so we're on the same page).
  2. The manage offer and liquidity pool deposit/withdraw operations also affect amount issued. This shouldn't affect your current plan.
  3. I just thought of an edge case that we haven't discussed yet. When authorization is revoked for an trustline that has deposits in a liquidity pool, all pool shares involving that trustline are redeemed, and claimable balances are created for the individual assets. This means that if the other asset being redeemed is the issuers asset, then it will be burned because we don't create the claimable balance in this scenario - https://github.com/stellar/stellar-core/blob/76124d3eb0d3d21cdee754d2405c24a515b1e3ee/src/transactions/TransactionUtils.cpp#L1432
jayz22 commented 2 years ago

Thanks @sisuresh , I have updated the summary based on your points.