rent collector improvments

[ParthDesai]

Problem

1. Rent collector copies account struct everytime .update is called, which is in-efficient
2. We are not charging rent for zero data account

Summary of Changes

1. Rent collector now takes mutable account reference
2. Rent collection now takes rent in a format of (base rent + rent per data byte), where base rent could come from genesis.

Fixes #6864

tag: @mvines @aeyakovenko @rob-solana

[codecov]

Codecov Report

Merging #6888 into master will decrease coverage by 5.6%.
The diff coverage is 90.9%.

@@           Coverage Diff            @@
##           master   #6888     +/-   ##
========================================
- Coverage    78.7%   73.1%   -5.7%     
========================================
  Files         222     222             
  Lines       42939   46215   +3276     
========================================
- Hits        33829   33817     -12     
- Misses       9110   12398   +3288

[mvines]

What if we expressed this in terms of DEFAULT_LAMPORTS_PER_BYTE_YEAR? Say the base account overhead is whatever the current size of the Account structure currently is * DEFAULT_LAMPORTS_PER_BYTE_YEAR. That'll still be zero currently so no functional change to this PR

[ParthDesai]

@mvines We decided to detach this from current size of the Account structure right? Otherwise, we need to do hard fork in case size changes in future.

[mvines]

Yes. I was just suggesting we use the current size of the Account structure as the value we hard code here (rather than randomly picking a value)

[ParthDesai]

@mvines hmm. This could work out. But, do you think hard coding it is a good idea compared to keeping it as default, and allowing us to overwrite it in future via gensis?

[mvines]

I just mean this:

-pub const DEFAULT_BASE_RENT_PER_YEAR: u64 = 0;
+pub const DEFAULT_BASE_RENT_PER_YEAR: u64 = 128 * DEFAULT_LAMPORTS_PER_BYTE_YEAR;

128 seems is approximately the current Account size.

[mvines]

maybe that's too much for a base rent 🤷‍♂

[ParthDesai]

ahh, I misunderstood. Cool, this works.

[ParthDesai]

@mvines done.

[ParthDesai]

hmm. On visualising, this design is less intuitive, instead we could do this:

1. Define ACCOUNT_STORAGE_OVERHEAD, and assign it to some arbitrary value.
2. Modify calculation in a way that by using lamports per byte per year, we can calculate how much rent is due for an account for bytes of (ACCOUNT_STORAGE_OVERHEAD + data.len())

[rob-solana]

hmm. On visualising, this design is less intuitive, instead we could do this:

1. Define ACCOUNT_STORAGE_OVERHEAD, and assign it to some arbitrary value.
2. Modify calculation in a way that by using lamports per byte per year, we can calculate how much rent is due for an account for bytes of (ACCOUNT_STORAGE_OVERHEAD + data.len())

if you take approach 1), there are 2 variables that need to change in order to turn on rent... approach 2) means you only have to change lamports_per_byte_year

[rob-solana]

this is what I meant. the old test had an implicit "1" in the calculation that I've pulled out for more clarity

[rob-solana]

lgtm

Pull request has been modified.