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

ingest/ledgerbackend: Add version check for protocol 21 #5346

Merged
merged 4 commits into from
Jun 20, 2024

Conversation

urvisavla
Copy link
Contributor

@urvisavla urvisavla commented Jun 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 shared functions to get the stellar-core build version and protocol version:
    GetCoreBuildVersion
    GetCoreProtocolVersion

  • Updated ledgerexporter and captivecorebackend to use these shared functions to get stellar-core build version.

  • Implemented a version check for protocol 21 when creating a new captive-core instance.

Why

We previously had the core build for Soroban in several places. Now that we're on protocol 21, we should check the protocol version instead of the core build version. For more details, see #5333.

Known limitations

Any system using captive-core will not be compatible with stellar-core versions older than 21.

@urvisavla urvisavla force-pushed the remove-core-version-check branch 2 times, most recently from 4c36e8e to b32aad8 Compare June 14, 2024 00:39
@urvisavla urvisavla changed the title Remove support for stellar classic ingest/ledgerbackend: Remove support for core versions older than Protocol 20 Jun 14, 2024
@urvisavla urvisavla marked this pull request as ready for review June 14, 2024 01:59
@tamirms
Copy link
Contributor

tamirms commented Jun 14, 2024

you can also remove protocol 20 from the integration tests matrix https://github.com/stellar/go/blob/master/.github/workflows/horizon.yml#L16

@tamirms
Copy link
Contributor

tamirms commented Jun 14, 2024

@urvisavla I think you meant to say "Removing checks for core versions older than protocol 21," right?

@urvisavla urvisavla changed the title ingest/ledgerbackend: Remove support for core versions older than Protocol 20 ingest/ledgerbackend: Remove support for core versions older than Protocol 21 Jun 14, 2024
@urvisavla
Copy link
Contributor Author

@urvisavla I think you meant to say "Removing checks for core versions older than protocol 21," right?

yes, thank you.

@urvisavla urvisavla force-pushed the remove-core-version-check branch 3 times, most recently from e9de3a2 to 2fbbde2 Compare June 17, 2024 23:45
@urvisavla urvisavla changed the title ingest/ledgerbackend: Remove support for core versions older than Protocol 21 ingest/ledgerbackend: Add cccccbgfruhfecevjdlgbndninktkcnbkdtccbnuelcea version check for protocol 21 Jun 18, 2024
@urvisavla urvisavla changed the title ingest/ledgerbackend: Add cccccbgfruhfecevjdlgbndninktkcnbkdtccbnuelcea version check for protocol 21 ingest/ledgerbackend: Add version check for protocol 21 Jun 18, 2024
…ol version and update ledgerexporter and captivecorebackend to use these shared functions. Implement a version check for protocol 21 when creating a new captive-core instance.
Copy link
Contributor

@sreuland sreuland left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for considering the suggeestions.

@urvisavla urvisavla merged commit 0b7df85 into stellar:master Jun 20, 2024
23 checks passed
@urvisavla urvisavla deleted the remove-core-version-check branch June 20, 2024 22:53
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.

3 participants