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

horizon: ingest Account Credited / Debited effects for smart contract asset transfers #4725

Closed
tamirms opened this issue Dec 22, 2022 · 5 comments · Fixed by #4806
Closed

Comments

@tamirms
Copy link
Contributor

tamirms commented Dec 22, 2022

The upcoming soroban release will implement single asset balance assets. This issue is a subtask of #4722 .

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.

prerequisite: #4747

@tamirms tamirms self-assigned this Dec 22, 2022
@tamirms tamirms moved this to Backlog in Platform Scrum Dec 22, 2022
@tamirms tamirms moved this from Backlog to In Progress in Platform Scrum Dec 22, 2022
@sreuland sreuland moved this from In Progress to Blocked in Platform Scrum Dec 22, 2022
@tamirms
Copy link
Contributor Author

tamirms commented Dec 22, 2022

The work for supporting single asset balances in horizon is more complicated than I anticipated. I thought we could simply infer that an account was credited or debited by looking at the trustline ledger entry changes in the tx meta for the invoke host function operation. However, this strategy will not work because it cannot generate effects on the issuer account whenever assets are issued / clawedback.

In stellar classic, an issuer can "mint" new assets by creating a payment operation from the issuer account to the recipient. Horizon will generate an "Account Credited" effect on the recipient and it will also generate an "Account Debited" effect on the issuer account. The problem is that the issuer account does not have a trustline for its own asset and so there is no ledger entry change on the issuer account corresponding to the "Account Debited" effect.

If we want to be consistent and have horizon emit "Account Debited" effects on the issuer account when a Stellar asset is minted via smart contract invocation, we would need to parse the smart contract events and be able to identify events which are emitted by the SAC (stellar asset contract). Horizon would need to be able to determine if a given contract id maps to a SAC and that would require Horizon to store contract data ledger entries in its database (which is a departure from the current policy of Horizon not storing any data about soroban smart contracts).

Here are a few options for how we can proceed:


Do not emit Account Credited / Debited effects for smart contract induced transfers

The downside of this approach is that anyone who depends on Account Credited / Debited effects would have an incomplete picture.


Remove Account Credited / Debited effects for issuer accounts

If we ignore Account Credited / Debited effects for issuer accounts we will be able to perform ingestion solely based on trustline ledger entry changes in tx meta. However, this would be a breaking change in our API.


Emit Account Credited / Debited effects from smart contract events

This would require Horizon to store contract data ledger entries in its database in order to identify which events are emitted by the SAC.

@Shaptic
Copy link
Contributor

Shaptic commented Dec 22, 2022

Some remarks:

  • Is it possible to infer that a contract data ledger entry belongs to a SAC from the entry itself? e.g. how it's possible to detect whether a particular Ethereum contract represents (or at least mimics) an ERC-20 token? Or do you run into an issue of "it looks like a SAC and acts like a SAC but it isn't one"?
  • If we decide to store entries, could/should we do it in a way that's distinct from the rest of our database, e.g. storing it in sqlite? This would let us heavily imply that we are storing this data simply for the purposes of persistence, rather than trying to make it a "full-fledged" part of Horizon (e.g. part of the overall DB schema, plumbed to endpoints, complex queries, scalability, etc.).
  • Should Horizon talk to Soroban RPC for this data? 😅

@tamirms
Copy link
Contributor Author

tamirms commented Dec 22, 2022

@Shaptic

Or do you run into an issue of "it looks like a SAC and acts like a SAC but it isn't one"?

Yes, that's exactly the problem we run into. Any smart contract can emit the same events as the SAC. In #4722 (comment) we discussed whether core could provide some information that could help distinguish contract ids which correspond to SAC but it doesn't seem like that's an option.

If we decide to store entries, could/should we do it in a way that's distinct from the rest of our database...

I'm not in favor of this approach because I think it would make it difficult for horizon to ensure 2 distinct databases are consistent. Whereas if the data was in postgres, we don't need to worry about that because Horizon will update all of its tables within a single database transaction during every round of ingestion.

Should Horizon talk to Soroban RPC for this data?

I think we should avoid having a circular dependency (currently, soroban depends on horizon for its endpoints). But even if we remove the soroban-rpc -> horizon dependency, I think there will be challenges in ensuring that soroban-rpc and horizon are at the same ledger when it comes to ingestion.

@Shaptic
Copy link
Contributor

Shaptic commented Dec 22, 2022

Thoughtful answers, thanks! I agree with you on all counts, just needed some rationale.

Horizon/RPC ingestion sync can be mitigated by sharing a Captive Core, but I think you're right that the dependency might get hairy. My intent was to try to keep the SC and Classic worlds as decoupled as possible, but that doesn't seem possible 😕

On the first point, is there a way to generate meta that must be unique to the SAC? For example, one could have a method that returns a signature signed by the issuing account. Such a blob couldn't be returned by a non-SAC SC, since it'd need the issuer's secret. Though I guess in that case, the issuer itself could mimic the SAC and that method, but can an issuer deploy its own token twice (once via SAC and once via custom)??

Anyway, that's just a thought - maybe there's a clever construction there that Core can pull off and we can leverage to glean information.

@tomerweller
Copy link
Contributor

tomerweller commented Dec 22, 2022

I think we should embrace the events approach. and I agree there's no escape from saving some minimal contract data information (just mapping SAC contract ids to assets, right?)

This is not special to Horizon. Any soroban downstream systems that ingests events will need to "know" about the capabilities of the emitting contract before being able to process it's events.

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

Successfully merging a pull request may close this issue.

4 participants