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

docs: clarify tx-wide default max units explainer #24955

Merged
merged 1 commit into from
May 4, 2022

Conversation

t-nelson
Copy link
Contributor

@t-nelson t-nelson commented May 3, 2022

Problem

unclear description of how tx-wide compute budget will calculate the default maximum units

Summary of Changes

attempt to clarify it

@jackcmay
Copy link
Contributor

jackcmay commented May 3, 2022

I updated docs in the backports while fixing the merge errors but this looks great too!

Copy link
Member

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

Ah nice, this is much clearer, thanks!

exceed that value. The default number of maximum units allowed matches existing
values to avoid breaking existing client behavior.
transaction-wide default maximum number of units will be calculated as the product
of the instruction count and the existing per-instruction maximum units. This means
Copy link
Member

@jstarry jstarry May 4, 2022

Choose a reason for hiding this comment

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

Can you also mention that the calculated max is clamped at 1.4M compute units?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considered referencing the current constant values, but decided I didn't want to be responsible for keeping them up to date

Copy link
Member

Choose a reason for hiding this comment

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

Can we say something like "at the time of writing, this caps at 1.4M but it's subject to change" as a cop out? I think it's useful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see no precedent for citing other constant values in this file. Feel free to add them if you feel strongly!

@t-nelson t-nelson added the v1.10 label May 4, 2022
@t-nelson t-nelson merged commit 9bca909 into solana-labs:master May 4, 2022
@t-nelson t-nelson deleted the docs branch October 20, 2022 02:16
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