Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[epic] horizon: support single balance assets #4722

Closed
tsachiherman opened this issue Dec 19, 2022 · 14 comments
Closed

[epic] horizon: support single balance assets #4722

tsachiherman opened this issue Dec 19, 2022 · 14 comments
Assignees

Comments

@tsachiherman
Copy link
Contributor

tsachiherman commented Dec 19, 2022

horizon: support single balance assets

Acceptance Criteria: Horizon updates to support the new aspects:

  • Confirm if Payment Effect should be generated, related to token contract asset transfers.
  • depends on release: update core docker image #4721 , integration tests may not pass until reference is changed to latest soroban core release pkg

Child tasks:

@paulbellamy
Copy link
Contributor

@paulbellamy
Copy link
Contributor

paulbellamy commented Dec 19, 2022

Should we emit "payment" effects for smart-contract-induced transfers? Probably yes, but that'll be interesting, but we can get those from events in the txmeta emitted by the built-in asset contract. Coordinate with @sisuresh to make sure those events are emitted as expected.

@tamirms tamirms self-assigned this Dec 20, 2022
@tamirms
Copy link
Contributor

tamirms commented Dec 21, 2022

The good news is that the horizon accounts endpoint should already display the correct asset balances. We won't need to modify ingestion to update trustline balances because smart-contract-induced transfers will generate tx meta reflecting the changes in the trustlines. All tx-meta, regardless of whether it is caused by a smart-contract operation or a stellar classic operation, is ingested via the trustline processor.

However, there are still two horizon endpoints that need to be updated in order to support single balance assets: /assets and /effects.


/assets

For a any given asset, the /assets endpoint shows information about how many accounts and claimable balances hold the asset and the total amount of the asset:

      {
        "_links": {
          "toml": {
            "href": "https://stablecoin.anchorusd.com/.well-known/stellar.toml"
          }
        },
        "asset_type": "credit_alphanum4",
        "asset_code": "USD",
        "asset_issuer": "GDUKMGUGDZQK6YHYA5Z6AY2G4XDSZPSZ3SW5UN3ARVMO6QSRDWP5YLEX",
        "paging_token": "USD_GDUKMGUGDZQK6YHYA5Z6AY2G4XDSZPSZ3SW5UN3ARVMO6QSRDWP5YLEX_credit_alphanum4",
        "num_accounts": 45829,
        "num_claimable_balances": 2,
        "num_liquidity_pools": 15,
        "amount": "1929494.0800967",
        "accounts": {
          "authorized": 45829,
          "authorized_to_maintain_liabilities": 0,
          "unauthorized": 0
        },
        "claimable_balances_amount": "45.0183495",
        "liquidity_pools_amount": "244.4334277",
        "balances": {
          "authorized": "1929494.0800967",
          "authorized_to_maintain_liabilities": "0.0000000",
          "unauthorized": "0.0000000"
        },
        "flags": {
          "auth_required": false,
          "auth_revocable": false,
          "auth_immutable": false,
          "auth_clawback_enabled": false
        }
      }

We could add two extra fields in the response num_smart_contracts, which is the total number of smart contracts that hold an asset, and smart_contracts_amount, which is the total amount held of the asset held by all the smart contracts combined.

In order to implement these changes we would need to ingest contract data ledger entries into the horizon db.

We would also need to modify the asset stats processor to process contract data ledger entry changes.

Specifically, we would need to identify if a contract data ledger entry change originates from the builtin token contract. A soroban smart contract can either be compiled from wasm or it can be an instantiation of the builtin token contract. The builtin token contract is a special smart contract which represents a Stellar Classic asset.

To determine if a soroban smart contract is a builtin token contract, you can derive a ledger key corresponding to the contract data ledger entry that stores the code for the smart contract. Using that ledger key, we can lookup the contract data ledger entry from the horizon db.

struct ContractDataEntry {
    Hash contractID;
    SCVal key;
    SCVal val;
};

With the ledger entry we can parse the val field and determine if it corresponds to a builtin token contract.

If we have identified that the smart contract is a builtin token, we would need to check if the contract data ledger entry is storing a token balance. If so, we would need to update the deltas in the asset stats processor for smart contract balances.


Effects

We will need to emit "Account Credited" / "Account Debited" effects whenever a Stellar asset is transferred or clawed back from a Stellar account via a smart contract invocation. This will require modifications to the effects ingestion processor. The changes should be pretty straight forward because we can iterate through the tx meta produced by the invoke host function operation and see if there are any ledger entry changes for trustlines.

We could also introduce a new set of effects for when a smart contract receives or spends a Stellar asset ("Smart Contract Credited" / "Smart Contract Debited"). Supporting these new effects will require either processing contract data ledger entry changes (the same workflow described above in the asset stats processor) or processing transfer events emitted by the smart contract.

We would also need to implement SEP-23: Add a contract strkey in all the stellar sdks so that the horizon clients could parse the smart contract credited / debited effects from horizon.


Payments

@bartekn raised a good point: horizon has a payments endpoint which returns all operations that result in a payment to / from an account. Since stellar assets can be transferred via smart contract invocations it would make sense to include those operations in the payments endpoint.


Test Cases

We should have integration tests which cover the following scenarios:

  • transfer stellar assets via smart contract invocation between Stellar accounts
  • transfer stellar assets via smart contract invocation between smart contracts
  • transfer stellar assets via smart contract invocation between a Stellar account and a smart contract
  • mint stellar asset via smart contract invocation to Stellar account
  • mint stellar asset via smart contract invocation to smart contract
  • burn stellar asset via smart contract invocation from Stellar account
  • burn stellar asset via smart contract invocation from smart contract
  • clawback stellar assets via smart contract invocation from a Stellar account
  • clawback stellar assets via smart contract invocation from a smart contract

In all these test cases we should verify that the /accounts, /assets and effects endpoints are matching expectations.

@sreuland
Copy link
Contributor

Does the contract data ledger entry for built-in token contract have a well-known/published model(expressed in rust data types) that can be used to locate the asset balances within?

@tamirms
Copy link
Contributor

tamirms commented Dec 21, 2022

@sisuresh we discussed supporting single balance assets in horizon during our team meeting today and we realized that it would simplify our ingestion code significantly if there was an easy way to identify if a contract data ledger entry belongs to a token contract (as opposed to a custom contract with WASM). Would it be possible to encode the contract id in such a way that we could differentiate between token contracts and normal wasm contracts? If that's not possible, would it make sense to add another field to the ContractDataEntry struct which can be used to identify token contracts?

@Shaptic
Copy link
Contributor

Shaptic commented Dec 21, 2022

Something noteworthy: as of today, Horizon's account_[credited|debited] doesn't provide the creditor/debitor account (see this one, for example). This can simplify things for us in regards to providing the same level of detail. With that in mind, regarding "Contract Debited", /payments, etc. I don't think it's unreasonable to ask developers to run Soroban RPC if they want more details on how things happen.

@sisuresh
Copy link

@tamirms the ContractDataEntry for the SAC contains information that says it's a token contract. The val will be ScContractCode::Token.

@tamirms
Copy link
Contributor

tamirms commented Dec 21, 2022

@sisuresh I think that only applies for the ContractDataEntry corresponding to the ScVal::Static(ScStatic::LedgerKeyContractCode) ledger key. But, what about the ContractDataEntry corresponding to the asset balance?

@sisuresh
Copy link

@tamirms Ah yeah I misread your comment. Making the contractID for the SAC special has come up in the past, but it's something I'd like to avoid to keep the experience between using the SAC and a WASM token the same. We could add additional information to the entries, but this isn't ideal and I'm not convinced it's necessary. Wouldn't the consumer already be aware of the ContractDataEntry for the token contract, and tie that to the balance entry?

@tamirms
Copy link
Contributor

tamirms commented Dec 22, 2022

Wouldn't the consumer already be aware of the ContractDataEntry for the token contract, and tie that to the balance entry?

@sisuresh that would require you to store extra information so you can identify all contract ids which correspond to a SAC. it's doable but I was jut wondering if there was a way to do it without having to record all contract ids which are SAC.

@sreuland
Copy link
Contributor

the test development was carved out to a separate ticket #4726

@sisuresh
Copy link

@tamirms We could add another field to ContractDataEntry that only the SAC can write to if we think that's necessary, but the use case sounds too specific. There's also this discord discussion you may find interesting where Orbitlens proposes a different type of contractID for the SAC that contains the issuer and asset code. I want to avoid this, but the idea would help you as well, so it's worth thinking about from our end.

@tamirms tamirms removed their assignment Jan 23, 2023
@jcx120
Copy link

jcx120 commented Jan 23, 2023

Design needs to be finalized (what Horizon needs to exposed for contracts).
moving to blocked for now

@tamirms tamirms removed their assignment Feb 2, 2023
@mollykarcher mollykarcher changed the title horizon: support single balance assets [epic] horizon: support single balance assets Feb 13, 2023
@mollykarcher
Copy link
Contributor

I am going to zero out the points on this, and we'll need to point the individual/child tasks within it. I believe that we pointed this quite a long time ago when the scope of this ticket was less well known and before we had actually broken it out into multiple tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

8 participants