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

Add ledger_key_hash to history_operations #237

Merged
merged 4 commits into from
Apr 15, 2024
Merged

Conversation

lucaszanotelli
Copy link
Contributor

This PR introduces a new field named ledger_key_hash to the history_operations export file. The field will only be included when the OperationType property is one of the following values:

  • InvokeHostFunction
  • ExtendFootprintTtl
  • RestoreFootprint

@lucaszanotelli lucaszanotelli requested a review from a team as a code owner April 11, 2024 18:41
Copy link
Contributor

@sydneynotthecity sydneynotthecity left a comment

Choose a reason for hiding this comment

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

Logic is correct; it just needs to be extended to cover all keys that are bumped/restored. The footprint can contain more than one key, so we will need to list all of them.

Comment on lines 1796 to 1801
for _, ledgerKey := range transactionEnvelope.Tx.Ext.SorobanData.Resources.Footprint.ReadOnly {
ledgerKeyHash := utils.LedgerKeyToLedgerKeyHash(ledgerKey)
if ledgerKeyHash != "" {
return ledgerKeyHash
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we just learned that restore/bump footprint ops will bump all keys in the footprint. Is it possible to store all keys in an array instead of saving only the first one?

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 last commit changes the ledgerKeyHashFromTxEnvelope function logic to store the ledger_key_hash in an array.

Here's an excerpt of an output from the export_operations command:

{
  "closed_at": "2024-02-06T18:42:32Z",
  "details": {
    "asset_balance_changes": [],
    "contract_code_hash": "4b9316721487281d8201e1c6044544400f120253487971e339eb23a465516935",
    "contract_id": "CAJRV7IFJ35V7W3NYDAUCG3BPQM32SBRX2R3OX7HQGTUJWOMJCA7VFKF",
    "function": "HostFunctionTypeHostFunctionTypeInvokeContract",
    "ledger_key_hash": [
      "980ea3840d17b86222a52d923a03cf9e918bd9851c00c0dd20a5f13aac18e523",
      "493f4eee738cf139a9d7690b355a0ce57f4bf36b489048edead7d1eba97a74e6",
      "8de7eeb5332495ed271a11d1849fc6f5ec970c73ca9cb5eb1a57126c4a278f56"
    ],
    "parameters": [
      {
        "type": "Address",
        "value": "AAAAEgAAAAETGv0FTvtf223AwUEbYXwZvUgxvqO3X+eBp0TZzEiB+g=="
      }...

Does that sound acceptable?

Copy link
Contributor

@sydneynotthecity sydneynotthecity left a comment

Choose a reason for hiding this comment

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

LGTM

@lucaszanotelli lucaszanotelli merged commit 395f1f8 into master Apr 15, 2024
4 checks passed
@sydneynotthecity sydneynotthecity deleted the add-ledger-key-hash branch April 17, 2024 20:31
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