Put accounts data len updates behind feature gate

[brooksprumo]

Problem

Updates to the Bank's accounts_data_len when collecting rent is not feature gated behind cap_accounts_data_len. This should've been feature-gated when it was first merged in #22412.

See here and here for existing usage of the feature.

Summary of Changes

Put the update behind the feature gate.

Related to #21604

[codecov]

Codecov Report

Merging #22918 (43cd01a) into master (75563f6) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #22918   +/-   ##
=======================================
  Coverage    81.3%    81.3%           
=======================================
  Files         560      560           
  Lines      152043   152045    +2     
=======================================
+ Hits       123623   123625    +2     
  Misses      28420    28420           

[jstarry]

@brooksprumo I was surprised that updating the accounts data len with a delta is only done for rent collection. Could it be used for transaction execution as well so that the feature switch can be moved inside there? Also, since transactions can be executed in parallel, isn't it possible that there is a data race when two txs load the account data len independently before storing it?

[brooksprumo]

@jstarry

I was surprised that updating the accounts data len with a delta is only done for rent collection. Could it be used for transaction execution as well so that the feature switch can be moved inside there?

So here's where the accounts data len is tracked/used within the invoke context:

solana/program-runtime/src/invoke_context.rs

Lines 494 to 499 in 28442aa

	if cap_accounts_data_len {
	let pre_data_len = pre_account.data().len() as i64;
	let post_data_len = account.data().len() as i64;
	let data_len_delta = post_data_len.saturating_sub(pre_data_len);
	self.accounts_data_meter.consume(data_len_delta)?;
	}

It has a feature gate around it there, and that cannot be removed since it could cause instructions to fail that exceed the accounts data budget.

Yes, I could move this PR's feature gate into update_accounts_data_len(). Would that be your preference? I purposely wanted to keep the update_accounts_data_len()/load_accounts_data_len()/store_accounts_data_len() functions as simple wrappers around the atomic with safe memory orderings. And then the places that pertain to tracking accounts data len (txn processing and rent collection) have the feature gate. Wdyt?

Also, since transactions can be executed in parallel, isn't it possible that there is a data race when two txs load the account data len independently before storing it?

This is good to know! Hah! I was observing discrepancies in the accounts_data_len vs the value computed during generate_index() (Discord link), so maybe this is the reason why. However, I currently see Bank::accounts_data_len as being always larger than generate_index(), and if there is a race, I would expect always-or-mostly lower. (Edit: I mixed up my columns, Bank::accounts_data_len is indeed always lower.) In either case, I'm working on a fix to get the delta and call update() instead of store().

[brooksprumo]

In either case, I'm working on a fix to get the delta and call update() instead of store().

PR is here: #22986

[jstarry]

In either case, I'm working on a fix to get the delta and call update() instead of store().

PR is here: #22986

Sweet, sounds good!