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

services/horizon: account operations endpoint does not include all operations affecting the account #5538

Open
leighmcculloch opened this issue Nov 24, 2024 · 3 comments

Comments

@leighmcculloch
Copy link
Member

leighmcculloch commented Nov 24, 2024

What version are you using?

22.0.1-dd8a9b473a303cfcdd383d1db45dace93ea0861c

What did you do?

Go to:

https://horizon.stellar.org/accounts/GDZAGXA74ZE2B54TYUXNUAOY3MTDL3ECP4EPYTRO3TKKKBRZF265UERA/operations?order=desc

Go to:

https://horizon.stellar.org/accounts/GDZAGXA74ZE2B54TYUXNUAOY3MTDL3ECP4EPYTRO3TKKKBRZF265UERA/effects?order=desc

What did you expect to see?

I expected to see present an operation and effects relating to:

https://horizon.stellar.org/operations/234143333304045569

The operation affects the GDZA account because a transfer is made to the account during the operation.

I expected this because the account operations endpoint shows payments from other source accounts that affect the GDZA account.

What did you see instead?

The operation is seen in the /effects endpoint, but not the /operations endpoint.


Related conversion: https://stellarfoundation.slack.com/archives/C03347FNAHK/p1732298224113449

cc @tomerweller

@aristidesstaffieri
Copy link

The specific query that spawned this conversation is used in Freighter here and is

const operationsData = await server
      .operations()
      .forAccount(pubKey)
      .order("desc")
      .join("transactions")
      .limit(TRANSACTIONS_LIMIT)
      .call();

@urvisavla
Copy link
Contributor

It seems that it was designed this way. In the operations processor, for InvokeHostFunction operations the comment says that only the source account is considered a participant. The account//operations endpoint relies on the history_operation_participants table which normally has both source and destination accounts for classic transactions like payments but not in Soroban transactions. On the other hand, in the effects processor is implemented to handle InvokeHostFunction. This likely explains why the account//effects endpoint is listing the operation but not account/<>/operations endpoint.

The following query is used by /account//operations endpoint:
SELECT hop.id, hop.transaction_id, hop.application_order, hop.type, hop.details, hop.source_account, hop.source_account_muxed, COALESCE(hop.is_payment, false) as is_payment, ht.transaction_hash, ht.tx_result, COALESCE(ht.successful, true) as transaction_successful FROM history_operations hop LEFT JOIN history_transactions ht ON ht.id = hop.transaction_id JOIN history_operation_participants hopp ON hopp.history_operation_id = hop.id WHERE hopp.history_account_id = ? AND hopp.history_operation_id > ? ORDER BY hopp.history_operation_id asc LIMIT 10

I don't have more context on why it was designed this way but @tamirms and @2opremio can provide more insight.

@tamirms
Copy link
Contributor

tamirms commented Dec 11, 2024

This is an oversight in the participants extraction logic. For other operations it is possible to determine the participants from the operation itself (for example, a payment operation contains both the sender and the recipient). However, it is not possible to determine participants for an invoke host operation by looking solely at the smart contract invocation. The code assumes that the only participant is the account performing the invoke host operation but that assumption is false.

A better way to determine participants for an invoke host operation is to use GetOperationChanges() to get all ledger entry changes which occurred as a side effect from executing the operation. We can then infer the participants by extracting the account ids from any accounts and trustlines included in the list of ledger entry changes.

I suspect this method is not perfect because there could be some corner cases where a participant is not present in the ledger entry changes. For example, if a smart contract invocation results in account A sending XLM to some destination and also account A receiving an equal amount of XLM. In this scenario, there is no change in the state of the account A ledger entry because it received an amount equal to what it sent and therefore account A will be missing from the ledger entry changes. Perhaps we can also look a the SAC events to extract any additional participants that are not present in the ledger entry changes.

Unfortunately, when we fix this issue we will need to reingest history to correct the missing data in the Horizon DB.

@tamirms tamirms moved this from Backlog to To Do in Platform Scrum Dec 17, 2024
@tamirms tamirms added this to the platform sprint 54 milestone Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To Do
Development

No branches or pull requests

5 participants