Skip to content

Internal transaction position offers full ordering #226

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

Merged
merged 2 commits into from
Apr 25, 2025

Conversation

YordanPavlov
Copy link
Contributor

@YordanPavlov YordanPavlov commented Apr 25, 2025

  • In a previous change we moved away from using a global incrementing primary key to adding an 'internal transaction position'. That is when two transfers have identical
    (block number, transaction id, from, to) we would increment the internal transaction position to create uniqueness. The new field would remain 0 otherwise.
  • The above implementation is sufficient in the context of Clickhouse tables, also for generating ETH balances, however one usage I failed to consider is the ETH stacks job. In this job if an address' funds change twice inside single transaction we consider those two changes separately and do not aggregate. For this to work correctly we need ordering among the two changes. The above implementation does not define an ordering across the whole transaction. If we have
    (block number = 10, transaciton id = "tx id", from = "A", to = "B") and
    (block number = 10, transaciton id = "tx id", from = "C", to = "A")
    the two changes on the assets of A are in no relation.
  • In the new implementation part of this PR, we increment the internal tx position for all transfers within the same (block number, transaction id). The order matches the order received from the Node.

* The transaction hash is the longes field among the transfer data,
  do not store it also as a key.
* Assign incrementing internal tx position for all transfers within the
  same transaction. This creates full ordering within this transaction.
* What we did before was, increment this internal transaction position
  counter only when multiple transfers are present for same 'from-to'.
* The previous behavior does not give information on which transfer is
  to be handled first when one address interacts with more than one other
  addresses.
@@ -23,7 +23,7 @@ export function transactionOrder(a: ETHTransfer | EOB, b: ETHTransfer | EOB) {
return internalTxPositionA - internalTxPositionB
}

const ethTransferKey = (transfer: ETHTransfer) => `${transfer.blockNumber}-${transfer.transactionHash ?? ''}-${transfer.transactionPosition ?? ''}-${transfer.from}-${transfer.to}`
const ethTransferKey = (transfer: ETHTransfer) => `${transfer.blockNumber}-${transfer.transactionHash ?? ''}`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main change.

await this.sendData(events, 'transactionHash', signalRecord);
} else if (BLOCKCHAIN === 'eth') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we do not use the Kafka record key for now, storing the transaction id there is redundant and wastes spaces.

@YordanPavlov YordanPavlov merged commit 7f68556 into master Apr 25, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants