Extend the Change structure in the ledger_change_reader to include the operation that prompted the ledger entry change

[karthikiyer56]

A Change struct as represented here indicates that a ledger entry has changed.

A ledger entry can be changed because of a transaction or because of an operation.
Every successful operation will cause one or more ledger change entries.

For e.g a path payment operation will, at a minimum, will cause a ledger change entry for source account entry and destination account, vis-a-vis account balances. The same path payment operation can also cause one or more offers to be partially filled in the order book or take some liquidity of the liquidity pool.
So, in the operationMeta for a successful path payment operation, there will be ledger entry changes corresponding to - source account, destination account, offers that might have been touched by the operation, liqudity pool changes.

It is a useful construct to be able to tie "what caused this change" to the ledger change entry.

This task deals with extending the Change struct to include additional fields to indicate what operation, if any, caused this change

Note that not all changes are caused by operations.
For e.g a change can be caused by a transaction - for e.g The fees for all the operations in a transaction will be debited from the source account of the transaction. So, the Change struct that captures account balance changes for the soure account of the transaction, will not have any details about operation.

[sydneynotthecity]

➕ this would be immediately beneficial for Hubble. We are creating a tokens_transfer table that makes it easier for analysts to track token movement across the network. Eventually, this table will be loaded from the results of stellar/stellar-protocol#1553. In the interim, we are manually parsing this data from history_trades, history_operations and the ledger state tables, which is a bit of a mess. We're doing some fuzzy joins in BigQuery to associate an operation <> state table, but if the operation was included in the change struct, we would be able to predictably join these two tables.

So, the Change struct that captures account balance changes for the source account of the transaction, will not have any details about operation.

Would it make sense to also include the transaction id or transaction hash on the Change struct? While there is no operation that makes the change, there is a transaction that is associated with the change. For any type of change, regardless if an operation caused it, I think it's useful to also include a way to tie back to transaction id.

[karthikiyer56]

Would it make sense to also include the transaction id or transaction hash on the Change struct? While there is no operation that makes the change, there is a transaction that is associated with the change. For any type of change, regardless if an operation caused it, I think it's useful to also include a way to tie back to transaction id.

@sydneynotthecity - yes, this is a fair ask and one that is easily done.
I will include the Transaction Hash and Transaction id (They start from 0, instead of 1) within the LCM, in the Change entry struct

[chowbao]

Would it also make sense to include the ledger sequence? Or at what point is the normalization/denormalization of the data model reasonable for the change reader?

Does this change also include updates to the change compactor then?
Does it make sense to force the change compactor to only accept 1 ledger sequence at a time rather than a range of sequences?

Edit: PR shows added LedgerCloseMeta and LedgerTransaction which takes care of ledger sequence

[karthikiyer56]

The semantic of reading changes does not work with the change compactor.
Meaning, if you are using the ChangeCompactor, do not expect the sub-entries in the []Change to be accurate, wrto Txinfo etc etc.

I mention that in the comments over the Change struct

Tl;dr - if you want to get change information at this kind of grass root level, you should not be using the ChangeCompactor.
instead you should use the LedgerChangeReader...

Something like

changeReader, err = ingest.NewLedgerChangeReaderFromLedgerCloseMeta("network pass phrase", ledger)
var changes []ingest.Change
	for {
		change, err := reader.Read()
		if err != nil {
			changes = append(cchanges, change)
		}
	}
dosomethingWithChanges(changes)

[karthikiyer56]

Re

Does it make sense to force the change compactor to only accept 1 ledger sequence at a time rather than a range of sequences?
The changeCompactor works at the cadence of a single ledger and not a sequence of ledgrs.

If an entry undergoes multiple changes as a part of one or more operations within a transaction OR as a part of one or more transactions in a ledger, the change compactor simply spews a single Change struct that highlights the ledgerEntry state before ledger and after ledger.

Unless you meant something else?

[chowbao]

Re

Does it make sense to force the change compactor to only accept 1 ledger sequence at a time rather than a range of sequences?
The changeCompactor works at the cadence of a single ledger and not a sequence of ledgrs.

If an entry undergoes multiple changes as a part of one or more operations within a transaction OR as a part of one or more transactions in a ledger, the change compactor simply spews a single Change struct that highlights the ledgerEntry state before ledger and after ledger.

Unless you meant something else?

Well right now I think the change compactor can take more than 1 ledger to compact the changes. Stellar-etl is written in a way so it only compacts 1 ledger at a time to create our state tables. I'm asking if it is worth standardizing the practice of only passing one ledger at a time to the change compactor instead of a ledger range

NVM the change compactor accepts changes. It is up to the user to decide what changes to pass to the compactor

[sreuland]

confirmed this is done-done in standup.