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

Use stellar-core upgrade config set command #513

Merged
merged 28 commits into from
Nov 1, 2023

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Oct 18, 2023

What

Use stellar-core upgrade config set command to set Soroban config instead of using the high limit override.

Why

The high limit override is good, but it is a blunt configuration that only allows setting a single high limit. When it is off the limit is so low it is unusable. There is no way to configure custom limits, or limits that mirror pubnet/testnet/etc.

While this PR doesn't make it configurable, it makes it possible for us to change quickstart's limits, and in a follow up PR we can add options like --enable-unlimited-limits or --enable-testnet-limits. At the moment the PR simply uses testnet limits from a .json file.

Close #508

@leighmcculloch

This comment was marked as outdated.

@leighmcculloch
Copy link
Member Author

@sisuresh @tsachiherman Looking for your review of this PR. I'm not marking it as ready because I don't want it merged yet because it's still using an unreleased version of stellar-core from @sisuresh's fork. Before merging I'll want to update it to use a patch release of stellar-core.

But what I'm looking for review is your thoughts. This PR doesn't leave this in a state that I think is perfect. It's somewhat brittle, although I'd done my best to expose statuses, logs, and check for success as best as possible, without introducing dependencies on more services being up at this point of the upgrade being done.

Not entirely proud of it, but I think this is an area we can improve on down the line and I think this is probably just stable enough for dev/testing.

@leighmcculloch
Copy link
Member Author

leighmcculloch commented Oct 28, 2023

This change introduces a ~6 second delay into friendbot startup, because friendbot startup is delayed until after network config completes, because network config and friendbot use the same account.

I think we'll accept that as an acceptable slow down that'll get better once we can reduce the 4 async steps of config down to 2 via a built-in deployer contract or a built-in config updater contract. cc @dmkozh

Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

Overall, I think the change is good.
I'd rather seeing us using release core versions rather then private branches.

start Outdated Show resolved Hide resolved
@leighmcculloch
Copy link
Member Author

leighmcculloch commented Oct 28, 2023

I'd rather seeing us using release core versions rather then private branches.

The PR is temporarily using @sisuresh's PR to validate that it works end to end before we merge and release stellar-core. We need to do more early validation imo.

This PR won't be merged until the change in the form is released, which will be hopefully soon in a patch 🙏.

@leighmcculloch leighmcculloch marked this pull request as ready for review October 30, 2023 21:06
@leighmcculloch
Copy link
Member Author

I'd rather seeing us using release core versions rather then private branches.

This change is using stellar-core v20.0.0-rc.2.2 now.

@leighmcculloch leighmcculloch merged commit 0674b5c into master Nov 1, 2023
66 checks passed
@leighmcculloch leighmcculloch deleted the upgrade-config-set branch November 1, 2023 23:32
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.

Remove use of soroban high limit override and replace with actual config upgrade
2 participants