Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Prevent Rent-reward recipients from ending up RentPaying #30130

Merged

Conversation

CriesofCarrots
Copy link
Contributor

@CriesofCarrots CriesofCarrots commented Feb 4, 2023

Problem

There is still one way accounts can end up newly rent-paying: a validator identity account with a very small or zero balance may receive a sub-rent-exemption rent distribution here:

if account.checked_add_lamports(rent_to_be_paid).is_err() {

Summary of Changes

Add feature-gate that, when active, will burn attempted rent distributions that do not meet rent-state transition requirements
Piggy-backs on #16847 (which in retrospect, should have included a test. I wasted some time thinking the test_distribute_rent_to_validators_overflow was testing the credit-overflow checked in that PR)

@CriesofCarrots CriesofCarrots marked this pull request as ready for review February 4, 2023 01:37
@CriesofCarrots
Copy link
Contributor Author

cc @jeffwashington

Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

lgtm, just nits.

Copy link
Contributor

@HaoranYi HaoranYi 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!

@HaoranYi
Copy link
Contributor

HaoranYi commented Feb 4, 2023

How about adding a log or stat for rent credit that result in rent paying account when the feature is not enabled, so that we can gather such info on mnb now?

@CriesofCarrots CriesofCarrots force-pushed the rent-exempt-in-dist-rent branch 2 times, most recently from 898209a to 0e89393 Compare February 4, 2023 20:11
@CriesofCarrots CriesofCarrots force-pushed the rent-exempt-in-dist-rent branch from 0e89393 to ffadad0 Compare February 4, 2023 20:12
@CriesofCarrots
Copy link
Contributor Author

@HaoranYi @t-nelson , Added a warn! log and metric, and bettered the test. Please take a look and re-✅ if all looks good. Thanks!
Also... v1.14 backport?

@CriesofCarrots CriesofCarrots added the v1.15 (abandoned) The v1.15 branch has been abandoned label Feb 4, 2023
Copy link
Contributor

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

Nice and clean. thanks!

Copy link
Contributor

@t-nelson t-nelson 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 the changes!

@CriesofCarrots CriesofCarrots merged commit a14473e into solana-labs:master Feb 6, 2023
mergify bot pushed a commit that referenced this pull request Feb 6, 2023
@jeffwashington
Copy link
Contributor

@CriesofCarrots thanks!
@behzadnouri you perhaps should take a look at this.

CriesofCarrots pushed a commit that referenced this pull request Feb 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
v1.15 (abandoned) The v1.15 branch has been abandoned
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants