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/ingest: add SKIP_TXMETA #5208

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

sreuland
Copy link
Contributor

@sreuland sreuland commented Feb 14, 2024

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with name of package that is most changed in the PR, ex.
    services/friendbot, or all or doc if the changes are broad or impact many
    packages.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated any docs (developer docs, .md
    files, etc... affected by this change). Take a look in the docs folder for a given service,
    like this one.

Release planning

  • I've updated the relevant CHANGELOG (here for Horizon) if
    needed with deprecations, added features, breaking changes, and DB schema changes.
  • I've decided if this PR requires a new major/minor version according to
    semver, or if it's mainly a patch change. The PR is targeted at the next
    release branch if it's not a patch change.

What

Added new SKIP_TXMETA. Defaults to FALSE. When TRUE the transaction model for classic or soroban tx's won't have tx meta populated anymore, on database, history_transactions.tx_meta column will have serialized xdr that equates to empty for any protocol version, such as for protocol 20,xdr.TransactionMeta.V3, Operations, TxChangesAfter, TxChangesBefore will be empty arrays and SorobanMeta will be nil and API responses with transaction model will have same empty/nil.

Removed DISABLE_SOROBAN_INGEST configuration parameter and it's cohort by reverting #5176, it did similar tx-meta removal but was limited to just soroban specific, SKIP_TXMETA parameter instead and removed parsing effects/ops aspects derived from tx-meta, which wasn't needed.

Why

to prevent extended storage space needs on db.
Closes #5189

Known limitations

Comment on lines 41 to 42
// TODO, generate TxChangesAfter also
//assert.Greater(t, len(txMetaResult.MustV2().TxChangesAfter), 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you know of any operations that would trigger TxChangesAfter, i tried several and FeeBumps, but only generated TxChangesBefore.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think soroban refunds can appear in TxChangesAfter:

https://github.com/stellar/stellar-protocol/blob/8571ed1e9f9a5f26e92d2469316d7997d61ec093/core/cap-0046-07.md?plain=1#L242

but if this case is difficult to produce, I think you can just delete the commented out code

@sreuland sreuland merged commit 0188e74 into stellar:release-horizon-v2.28.3 Feb 14, 2024
28 checks passed
sreuland added a commit to sreuland/go that referenced this pull request Feb 20, 2024
* Revert "services/horizon: Add DISABLE_SOROBAN_INGEST flag to skip soroban ingestion processing (stellar#5176)"

This reverts commit bfaf9e1.

* stellar#5189: added optional SKIP_TXMETA parameter to not persist tx meta in transaction model, removed DISABLE_SOROBAN_INGEST, use SKIP_META instead
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