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

quickstart: add soroban high limit override for standalone networks #480

Closed

Conversation

tsachiherman
Copy link
Contributor

What ?

With the recent core build, the resource limits for soroban transactions were lowered dramatically.
On any of the public networks, there would be a process to raise these up to a reasonable values.
However, for standalone networks, there is no such "entity".

In order to bootstrap a network that is usable, we need to add the TESTING_SOROBAN_HIGH_LIMIT_OVERRIDE flag to any core node invocation.

This PR adds the option to enable setting the above flag on standalone network.
Note that it might be arguable that it's always desired on a standalone network - but that's TBD.

@tsachiherman tsachiherman self-assigned this Aug 31, 2023
@tsachiherman tsachiherman changed the title add soroban high limit override for standalone networks quickstart: add soroban high limit override for standalone networks Aug 31, 2023
start Outdated
@@ -53,6 +54,10 @@ function main() {
echo "--randomize-network-passphrase is only supported in the standalone network" >&2
exit 1
fi
if [ "$NETWORK" != "standalone" ] && [ "$ENABLE_SOROBAN_HIGH_LIMIT_OVERRIDE" = "true" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

might be more apt to validate usage with $ENABLE_SOROBAN_RPC" != "true"? regardless of network.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so. This feature is accessible independent of whether the soroban-rpc is running.

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.

the new --enable-soroban-high-limit-override flag should be mentioned on soroban rpc section in readme as well, so users are aware - https://github.com/stellar/quickstart#soroban-development

@leighmcculloch
Copy link
Member

@tsachiherman Had a new thought on this. Given that standalone is the dev+testing mode, and we already accelerate ledgers even for it, maybe we should enable this by default always and have no option.

Or, have an option to enable limits.

Either having no option, or one to enable limits, are probably a more useful default. We want people to have a great experience upfront which means running their contracts irrespective of limits. And then be able to become experts and know how to write small contracts and then test that they are sufficiently small.

@sreuland
Copy link
Contributor

maybe we should enable this by default always and have no option.

I was tempted to ask the same, and then figured that users will need to be able to replicate test cases under both modes correct? so, figured it was worth having the command line flag, like your other suggestion also of default high limit to enabled for seamless experience

Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

Thought more about this and I think it is a better default to reverse the default and have an option for disabling the option instead of enabling, such as --enable-soroban-limits.

@leighmcculloch
Copy link
Member

It just occurred to me that adding a setting won't be useful today, because the default limits are so low the soroban functionality is largely unusable. As such this flag will practically only be an on off switch for soroban rather than a control of limits. Apologies for flipping on this, but I think we should reduce this to a hardcoded value of on for standalone, since it's primary use case is testing.

Once the stellar-core team finish the below issue, converting the Python upgrade script into a more portable command, we should revisit adding an option, maybe an option that lets the user set pubnet limits rather than the default super low and largely not useful default limits.

@tsachiherman
Copy link
Contributor Author

I'm closing this PR in favor of #492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants