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

SIMD-0194: Deprecate rent exemption threshold #194

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

deanmlittle
Copy link
Contributor

No description provided.

Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Love the change, just a few open questions. I'm not sure what the best deprecation process is for the field.

Comment on lines 44 to 45
Set the value of `DEFAULT_EXEMPTION_THRESHOLD` from `2.0` to `1.0`, and
deprecate it from the protocol.
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you propose deprecation? I'm curious which one hurts less post-deprecation: changing to 1u64 and preserving the account layout/size, or complete removal, changing the account size.

Copy link
Contributor Author

@deanmlittle deanmlittle Nov 15, 2024

Choose a reason for hiding this comment

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

We simply stop using it in any SDKs and thus any new programs going forward will be using u64 math, simply omitting the f64 value entirely, but it will remain in the rent account to not break the existing API and will be flagged as deprecated.

Comment on lines 93 to 94
* Allow users to make the assumption that `2.0` will remain stable and do `u64`
math themselves.
Copy link
Contributor

Choose a reason for hiding this comment

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

We could stablize this, but imo the field will soon become a misnomer, considering rent collection is being disabled. I think your proposed design makes more sense.

proposals/XXXX-deprecate-rent-exemption-threshold.md Outdated Show resolved Hide resolved
@deanmlittle deanmlittle changed the title Deprecate rent exemption threshold SIMD-0194: Deprecate rent exemption threshold Nov 16, 2024
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

The whole concept of Rent is going away. Since no accounts are rent-paying currently, the rent lamports can be considered a state bond. A big goal with state economics is to enable this state bond to be dynamic and reduce 100x or so. Under normal load this drops "rent" to be much much cheaper, while also allowing higher loads to exponentially increase fees to allocate state.

I think we'd want new sysvars/etc for programs to be able to read, so this change to the exemption threshold is probably fine.

the `lamports_per_byte` value. Any existing program using the current
implementation will be unaffected.

## Alternatives Considered
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like another alternative is to check if the bitpattern of exemption_threshold is 2.0 (bitwise comparison, not floating point), and then if true, do the math as described in this PR above.

Kinda hacky, I agree.

Copy link
Contributor Author

@deanmlittle deanmlittle Nov 22, 2024

Choose a reason for hiding this comment

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

Moving forward, we can actually skip the equivalence check entirely if we just decide that the best outcome for this SIMD is to ossify it at 2.0. The thing is, if we're already going to do that, it feels much cleaner to change it to 1.0 so it can be mathematically simplified out of the fee calculation formula as opposed to just being ignored.

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.

4 participants