use cost model to limit new account creation

[jeffwashington]

Problem

Summary of Changes

Fixes #

[t-nelson]

I think we also need to consider SystemInstruction::Allocate{,WithSeed} here, otherwise the checks can be sidestepped with a discrete Allocate -> Assign -> Transfer

[jeffwashington]

I added these 2 instructions.

[t-nelson]

@jackcmay what about the realloc instruction?

[jackcmay]

Realloc is not an instruction, it is an operation that a program can do on accounts they own, from within the program:

solana/sdk/program/src/account_info.rs

Line 130 in 7da2df7

pub fn realloc(&self, new_len: usize, zero_init: bool) -> Result<(), ProgramError> {

[t-nelson]

Realloc is not an instruction, it is an operation that a program can do on accounts they own, from within the program:

solana/sdk/program/src/account_info.rs

Line 130 in 7da2df7

pub fn realloc(&self, new_len: usize, zero_init: bool) -> Result<(), ProgramError> {

It doesn't seem like there's any runtime accounting of this operation? It'd be nice to consider its contributions to storage changes

[brooksprumo]

Curious because I do not know this part of the code yet; Is tracking realloc possible to do here?

[jackcmay]

We could probably add some kind of limit to the number of bytes realloc'd, not sure where that would come from

[brooksprumo]

Would this allow the creation of unlimited accounts without data? Since there is storage overhead per account, I think that should be factored into this model as well, as the size/size limit should reflect the requirements imposed on the validators instead of the account creator/holder.

Or does space in the SystemInstruction already account for that?

[jeffwashington]

Would this allow the creation of unlimited accounts without data? Since there is storage overhead per account, I think that should be factored into this model as well, as the size/size limit should reflect the requirements imposed on the validators instead of the account creator/holder.

Or does space in the SystemInstruction already account for that?

@taozhu-chicago tells me that there is already a # acct limit per slot.

[brooksprumo]

If max account data len is 100 MB, then getting to 1 TB would only take a bit over 1 hour, is that right? I don't know what the right number should be here though... If a DoS was happening, would 1 hour be long enough to prevent/handle it?

Math used: 10^12 B / 10^8 B * 0.4 s per slot / 60 s per min / 60 min per hour (double check in case I made dumb mistakes)

[jeffwashington]

@carllin ran these calculations in his head about 15 times last night.

[jeffwashington]

this comment was my invitation for him to comment ;-)

[carllin]

Yeah about an hour. I think if we want to survive longer, we'll have to require validators have a lot more disk. Maybe 10TB minimum. How much space does it take to store 1 billion 10 MB accounts 😄 , 10 petabytes?

Also each account costs about $40, so if we factor that in, we could set the disk requirements based on the maximum economic attack we want to handle. 1 billion account would be 40 bil

[sakridge]

We would want more like a week. How does this max compare with the current mainnet-beta account data average written per slot?

[jeffwashington]

I've added the data len to the metrics. I'm waiting on the ability to start a validator with this to get the metrics of mnb. Keep in mind the current metrics ONLY measure the data size of account creation; not updates.

[tao-stones]

number of writable accounts are counted into overall transaction cost, described in #20531.
There is a cost_weight multiplier to transaction cost, enables us to exponentially increase its value, should help in event a lot of account, regardless its size, to be created in a transaction.

[brooksprumo]

number of writable accounts are counted into overall transaction cost, described in #20531. There is a cost_weight multiplier to transaction cost, enables us to exponentially increase its value, should help in event a lot of account, regardless its size, to be created in a transaction.

@taozhu-chicago Is the MAX_WRITABLE_ACCOUNTS effectively the maximum number of new accounts per transaction?

solana/runtime/src/cost_model.rs

Line 14 in cd6f931

const MAX_WRITABLE_ACCOUNTS: usize = 256;

And is MAX_WRITABLE_ACCOUNT_UNITS what controls the maximum number of new accounts per block?

solana/runtime/src/block_cost_limits.rs

Line 58 in c4d6806

pub const MAX_WRITABLE_ACCOUNT_UNITS: u64 = MAX_BLOCK_REPLAY_TIME_US * COMPUTE_UNIT_TO_US_RATIO;

[brooksprumo]

There is a cost_weight multiplier to transaction cost, enables us to exponentially increase its value, should help in event a lot of account, regardless its size, to be created in a transaction.

@taozhu-chicago It looks like CostTracker::try_add() does factor in the cost weight (see line 87):

solana/runtime/src/cost_tracker.rs

Lines 82 to 91 in eb62faa

	pub fn try_add(
	&mut self,
	_transaction: &SanitizedTransaction,
	tx_cost: &TransactionCost,
	) -> Result<u64, CostTrackerError> {
	let cost = tx_cost.sum() * tx_cost.cost_weight as u64;
	self.would_fit(&tx_cost.writable_accounts, cost, tx_cost.account_data_size)?;
	self.add_transaction(&tx_cost.writable_accounts, cost, tx_cost.account_data_size);
	Ok(self.block_cost)
	}

But it doesn't look like cost weight is used by either add_transaction_cost() or would_transaction_fit():

solana/runtime/src/cost_tracker.rs

Lines 58 to 80 in eb62faa

	pub fn would_transaction_fit(
	&self,
	_transaction: &SanitizedTransaction,
	tx_cost: &TransactionCost,
	) -> Result<(), CostTrackerError> {
	self.would_fit(
	&tx_cost.writable_accounts,
	tx_cost.sum(),
	tx_cost.account_data_size,
	)
	}
	pub fn add_transaction_cost(
	&mut self,
	_transaction: &SanitizedTransaction,
	tx_cost: &TransactionCost,
	) {
	self.add_transaction(
	&tx_cost.writable_accounts,
	tx_cost.sum(),
	tx_cost.account_data_size,
	);
	}

Doing a quick grep tells me that neither add_transaction_cost() nor would_transaction_fit() are used anywhere, so maybe that's why this is OK. But this seems like a bug to me. Can you help me understand what I'm missing?

P.S. I know this is unrelated to this PR specifically.

[tao-stones]

@taozhu-chicago Is the MAX_WRITABLE_ACCOUNTS effectively the maximum number of new accounts per transaction?

solana/runtime/src/cost_model.rs

Line 14 in cd6f931

const MAX_WRITABLE_ACCOUNTS: usize = 256;

It is currently used as capacity used to create transactionCost.writable_accounts vec. It does not limits number of accounts.

[brooksprumo]

It is currently used as capacity used to create transactionCost.writable_accounts vec. It does not limits number of accounts.

@taozhu-chicago Gotcha. What I'm ultimately interested in is making sure that new accounts without data are also included in this tracking, since each account has storage overhead. (See #21369 (comment) for original comment.)

Can you point me to where the number of new accounts is limited?

[tao-stones]

And is MAX_WRITABLE_ACCOUNT_UNITS what controls the maximum number of new accounts per block?

solana/runtime/src/block_cost_limits.rs

Line 58 in c4d6806

pub const MAX_WRITABLE_ACCOUNT_UNITS: u64 = MAX_BLOCK_REPLAY_TIME_US * COMPUTE_UNIT_TO_US_RATIO;

For better parallelism, we discourage to packing too many transactions that write to same account. This limit is for this purpose. cost_tracker keeps tracking transaction cost per writable account in a block, transactions that would exceed any writable account limit are retried.

[tao-stones]

@taozhu-chicago It looks like CostTracker::try_add() does factor in the cost weight (see line 87):

solana/runtime/src/cost_tracker.rs

Lines 82 to 91 in eb62faa

	pub fn try_add(
	&mut self,
	_transaction: &SanitizedTransaction,
	tx_cost: &TransactionCost,
	) -> Result<u64, CostTrackerError> {
	let cost = tx_cost.sum() * tx_cost.cost_weight as u64;
	self.would_fit(&tx_cost.writable_accounts, cost, tx_cost.account_data_size)?;
	self.add_transaction(&tx_cost.writable_accounts, cost, tx_cost.account_data_size);
	Ok(self.block_cost)
	}

But it doesn't look like cost weight is used by either add_transaction_cost() or would_transaction_fit():

solana/runtime/src/cost_tracker.rs

Lines 58 to 80 in eb62faa

	pub fn would_transaction_fit(
	&self,
	_transaction: &SanitizedTransaction,
	tx_cost: &TransactionCost,
	) -> Result<(), CostTrackerError> {
	self.would_fit(
	&tx_cost.writable_accounts,
	tx_cost.sum(),
	tx_cost.account_data_size,
	)
	}
	pub fn add_transaction_cost(
	&mut self,
	_transaction: &SanitizedTransaction,
	tx_cost: &TransactionCost,
	) {
	self.add_transaction(
	&tx_cost.writable_accounts,
	tx_cost.sum(),
	tx_cost.account_data_size,
	);
	}

Doing a quick grep tells me that neither add_transaction_cost() nor would_transaction_fit() are used anywhere, so maybe that's why this is OK. But this seems like a bug to me. Can you help me understand what I'm missing?

P.S. I know this is unrelated to this PR specifically.

Oh yeah. The weight is newly added feature, it's main use is still in a pending PR #21220. But for now, it is used to set vote's weight to zero, to allow all vote transactions bypass cost_model.
try_add has become the main entry point of cost_tracker, it hosts additional logic on top of book keeping (such as applying weight and maybe logic in the future). would_fit and add_transaction are just the actual book keeping functions, they deal with whatever cost given. Perhaps they can be made private if no one is using them (They used by ledger-tool at some point)

[tao-stones]

Can you point me to where the number of new accounts is limited?

Mmm, it does not limits number of new accounts per se. Transaction cost is determined by instructions it has, and the number of writable accounts. These contributes to cost tracking.

[brooksprumo]

Overall this looks good to me. It is an incremental improvement, and further improvements can land in future PRs. I'd say mark this one as Ready For Review!

[codecov]

Codecov Report

Merging #21369 (e8e25b8) into master (0224a8b) will decrease coverage by 0.1%.
The diff coverage is 88.6%.

@@            Coverage Diff            @@
##           master   #21369     +/-   ##
=========================================
- Coverage    81.6%    81.4%   -0.2%     
=========================================
  Files         511      511             
  Lines      143320   143509    +189     
=========================================
- Hits       116976   116878     -98     
- Misses      26344    26631    +287     

[jeffwashington]

bench-tps #s I get locally are far lower than I expect, on 2 different colo machines, with both master and with this pr. The numbers seem similar, however. Will monitor overnight perf tests.