clean up disable_rent_fees_collection feature

[jstarry]

Problem

The disable_rent_fees_collection feature is activated on all clusters and can be cleaned up

Summary of Changes

• Remove disable_rent_fees_collection from svm feature set
• Remove obsolete tests
• Clean up rent collection code, leaving only rent epoch updates in place (those will be removed later)

Fixes #

[jstarry]

We haven't collected rent for many epochs so it's safe to assume that any loaded snapshot will have 0 collected rent.

[buffalojoec]

Are we just going to serialize a 0 from now on, since we can't get the field out of the deserialize side? Or can we?

[jstarry]

Oops I meant to put this comment in the deserialization path not the serialization path. Anyways, yes, since rent fee collection was turned off we have been only serializing 0 for quite awhile so explicitly serializing 0 isn't a behavior change. Because of always serializing 0 for awhile, we can safely ignore whatever value we deserialize here where I meant to write this comment.

-   collected_rent: AtomicU64::new(fields.collected_rent),

We can ignore it because fields.collected_rent should be 0 and even if it wasn't, we never used the collected_rent field after a bank was frozen anyways so it was kinda pointless to have put it in snapshots in the first place (other than for debugging).

[brooksprumo]

IMO we should remove collected_rent from BankFieldsToSerialize and hard code to 0 later on SerializableVersionedBank.

(And we can also rename SerializableVersionedBank::collected_rent to SerializableVersionedBank::unused_collected_rent.)

Is that too much of a pain for this PR? If yes, we can also do it as a follow-up.

[jstarry]

I got confused and thought this struct was serialized and needed to keep the field. It was simple to remove, thanks for calling out: 72546df

[jstarry]

These hashes changed because after removing the old rent collection path, we no longer update rent epoch for executable accounts which do not have a min rent exempt balance (this applies to builtin programs).

[buffalojoec]

Lgtm, just some questions!

[buffalojoec]

Are we just going to serialize a 0 from now on, since we can't get the field out of the deserialize side? Or can we?

[jstarry]

Force pushed to resolve a conflict in test_credit_debit_rent_no_side_effect_on_hash from #6561. Resolved the conflict by just removing that test since it's not relevant anymore.

[brooksprumo]

I didn't look at the SVM nor RPC files, but did scan the rest. I need to go back and do another pass on bank.rs and fee_distribution.rs too.

[brooksprumo]

IMO we should remove collected_rent from BankFieldsToSerialize and hard code to 0 later on SerializableVersionedBank.

(And we can also rename SerializableVersionedBank::collected_rent to SerializableVersionedBank::unused_collected_rent.)

Is that too much of a pain for this PR? If yes, we can also do it as a follow-up.

[HaoranYi]

lgtm. Thanks for cleaning it up.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.8%. Comparing base (3281f61) to head (72546df).
Report is 16 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #6622     +/-   ##
=========================================
  Coverage    82.8%    82.8%             
=========================================
  Files         849      849             
  Lines      379373   377938   -1435     
=========================================
- Hits       314300   313232   -1068     
+ Misses      65073    64706    -367     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[buffalojoec]

Lgtm from SVM side.

[brooksprumo]