Propose to remove new account creation data size check from cost model

[tao-stones]

Problem

As part of effort to define detailed costs for accounts (such as costs to load read/write/executable/lookup accounts, and costs of loaded account data size, etc), I would like to clean up this block of code that limits how many new account data size each block can allocate, utilizing cost_model.

For context, it was implemented as a quick way to limit data size accounts can take, adding preventive check at leader side was the quickest way. However, due to the fact that cost_model functions before transactions being processed, the implementation was limited to check only instructions that create accounts.

Since then more comprehensive approaches were consisted and implemented: (thanks to @brooksprumo for documenting)

Capping allocations per transaction

• Feature Gate: Cap Accounts Data Allocations Per Transaction #27375
• Set cap for new allocations per transaction #27385

Capping total loaded account data size

• Feature Gate: Limit loaded data per transaction to a fixed cap #27839
• Limit loaded data per transaction to a fixed cap #29743

IMO, with these two changes, the block of code in question can/should be removed. POC here. It would not only clean up the code, gain some perf back, also make it easier to define cost for data size accounts loads.

Some other resources/contexts:

Capping total accounts data size

• Feature Gate: Cap Total Accounts Data Size #24135
• Set cap for total accounts data size #21604

Capping allocations per block

• Feature Gate: Cap Accounts Data Allocations Per Block #25517

Proposed Solution

• remove the implementation meant for short-term
• If all possible, land this before add cost tracking for each bank during replay #30035

[mvines]

seems reasonable. does #30205 need a feature gate as well, or no since the behavior change only affects the current slot leader?

[tao-stones]

• land this before add cost tracking for each bank during replay #30035

No need feature gate if "land this before #30035", otherwise need to be gated. Another motivation for do it now 😄

[tao-stones]

Also checked the metrics, mainnet-beta seems never hit this check anyway:

SELECT max("would_exceed_account_data_block_limit") AS "max_would_exceed_account_data_block_limit" FROM "mainnet-beta"."autogen"."banking_stage-leader_slot_transaction_errors" WHERE time > :dashboardTime: AND time < :upperDashboardTime: GROUP BY time(:interval:) FILL(null)

[brooksprumo]

These checks in the cost model are the only way to add limits on new allocations within a block. We'll be able to limit new allocations per transaction with #27375, but we won't be able to limit allocations per block (#25517) elsewhere.

As noted, the current checks are not comprehensive in what they can—and do—check, so it may be appropriate to remove them. I'm not sure I am willing to be that person though 😸

[tao-stones]

Good point to distinguish per-transaction and per-block. I wonder if limiting allocation per block has actual use. Transactions from multiple blocks are replayed simultaneously anyway. Before being able to limit-per-transaction, set cap per block helps a bit. Since we can set limit per transaction now (limits on both new allocation per tx, and total data loaded per tx), hence time to reevaluate the need of limit-allocations-per-block.

[t-nelson]

does #27385 account for both system program allocations and the realloc syscall?

[brooksprumo]

does #27385 account for both system program allocations and the realloc syscall?

Yep!

solana/sdk/src/transaction_context.rs

Lines 996 to 1010 in 27f18f9

	if self
	.transaction_context
	.is_cap_accounts_data_allocations_per_transaction_enabled
	{
	// The resize can not exceed the per-transaction maximum
	let length_delta = (new_length as i64).saturating_sub(old_length as i64);
	if self
	.transaction_context
	.accounts_resize_delta()?
	.saturating_add(length_delta)
	> MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION
	{
	return Err(InstructionError::MaxAccountsDataAllocationsExceeded);
	}
	}