Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use cost model to limit new account creation #21369

Merged
merged 8 commits into from
Dec 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ pub enum DbTransactionErrorCode {
WouldExceedMaxBlockCostLimit,
UnsupportedVersion,
InvalidWritableAccount,
WouldExceedMaxAccountDataCostLimit,
}

impl From<&TransactionError> for DbTransactionErrorCode {
Expand Down Expand Up @@ -356,6 +357,9 @@ impl From<&TransactionError> for DbTransactionErrorCode {
TransactionError::WouldExceedMaxBlockCostLimit => Self::WouldExceedMaxBlockCostLimit,
TransactionError::UnsupportedVersion => Self::UnsupportedVersion,
TransactionError::InvalidWritableAccount => Self::InvalidWritableAccount,
TransactionError::WouldExceedMaxAccountDataCostLimit => {
Self::WouldExceedMaxAccountDataCostLimit
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/banking_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -947,8 +947,8 @@ impl BankingStage {
bank.prepare_sanitized_batch_with_results(txs, transactions_qos_results.into_iter());
lock_time.stop();

// retryable_txs includes AccountInUse, WouldExceedMaxBlockCostLimit and
// WouldExceedMaxAccountCostLimit
// retryable_txs includes AccountInUse, WouldExceedMaxBlockCostLimit
// WouldExceedMaxAccountCostLimit, and WouldExceedMaxAccountDataCostLimit
let (result, mut retryable_txs) = Self::process_and_record_transactions_locked(
bank,
poh,
Expand Down
11 changes: 11 additions & 0 deletions core/src/qos_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ impl QosService {
self.metrics.retried_txs_per_account_limit_count.fetch_add(1, Ordering::Relaxed);
Err(TransactionError::WouldExceedMaxAccountCostLimit)
}
CostTrackerError::WouldExceedAccountDataMaxLimit => {
self.metrics.retried_txs_per_account_data_limit_count.fetch_add(1, Ordering::Relaxed);
Err(TransactionError::WouldExceedMaxAccountDataCostLimit)
}
}
}
})
Expand Down Expand Up @@ -165,6 +169,7 @@ struct QosServiceMetrics {
selected_txs_count: AtomicU64,
retried_txs_per_block_limit_count: AtomicU64,
retried_txs_per_account_limit_count: AtomicU64,
retried_txs_per_account_data_limit_count: AtomicU64,
}

impl QosServiceMetrics {
Expand Down Expand Up @@ -204,6 +209,12 @@ impl QosServiceMetrics {
.swap(0, Ordering::Relaxed) as i64,
i64
),
(
"retried_txs_per_account_data_limit_count",
self.retried_txs_per_account_data_limit_count
.swap(0, Ordering::Relaxed) as i64,
i64
),
);
}
}
Expand Down
11 changes: 6 additions & 5 deletions runtime/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1032,11 +1032,12 @@ impl Accounts {
let keys: Vec<_> = txs
.zip(results)
.filter_map(|(tx, res)| match res {
Err(TransactionError::AccountInUse) => None,
Err(TransactionError::SanitizeFailure) => None,
Err(TransactionError::AccountLoadedTwice) => None,
Err(TransactionError::WouldExceedMaxBlockCostLimit) => None,
Err(TransactionError::WouldExceedMaxAccountCostLimit) => None,
Err(TransactionError::AccountInUse)
| Err(TransactionError::SanitizeFailure)
| Err(TransactionError::AccountLoadedTwice)
| Err(TransactionError::WouldExceedMaxBlockCostLimit)
| Err(TransactionError::WouldExceedMaxAccountCostLimit)
| Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => None,
_ => Some(tx.get_account_locks(demote_program_write_locks)),
})
.collect();
Expand Down
5 changes: 3 additions & 2 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl ExecuteTimings {
}

type BankStatusCache = StatusCache<Result<()>>;
#[frozen_abi(digest = "32EjVUc6shHHVPpsnBAVfyBziMgyFzH8qxisLwmwwdS1")]
#[frozen_abi(digest = "GcfJc94Hb3s7gzF7Uh4YxLSiQf1MvUtMmtU45BvinkVT")]
pub type BankSlotDelta = SlotDelta<Result<()>>;
type TransactionAccountRefCells = Vec<(Pubkey, Rc<RefCell<AccountSharedData>>)>;

Expand Down Expand Up @@ -3467,7 +3467,8 @@ impl Bank {
Some(index)
}
Err(TransactionError::WouldExceedMaxBlockCostLimit)
| Err(TransactionError::WouldExceedMaxAccountCostLimit) => Some(index),
| Err(TransactionError::WouldExceedMaxAccountCostLimit)
| Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => Some(index),
jeffwashington marked this conversation as resolved.
Show resolved Hide resolved
Err(_) => None,
Ok(_) => None,
})
Expand Down
3 changes: 3 additions & 0 deletions runtime/src/block_cost_limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,6 @@ pub const MAX_BLOCK_UNITS: u64 =
/// limit is to prevent too many transactions write to same account, threrefore
/// reduce block's paralellism.
pub const MAX_WRITABLE_ACCOUNT_UNITS: u64 = MAX_BLOCK_REPLAY_TIME_US * COMPUTE_UNIT_TO_US_RATIO;

/// max len of account data in a slot (bytes)
pub const MAX_ACCOUNT_DATA_LEN: u64 = 100_000_000;
Comment on lines +62 to +63
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@carllin carllin Nov 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

108 changes: 107 additions & 1 deletion runtime/src/cost_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use {
execute_cost_table::ExecuteCostTable,
},
log::*,
solana_sdk::{pubkey::Pubkey, transaction::SanitizedTransaction},
solana_sdk::{
instruction::CompiledInstruction, program_utils::limited_deserialize, pubkey::Pubkey,
system_instruction::SystemInstruction, system_program, transaction::SanitizedTransaction,
},
std::collections::HashMap,
};

Expand All @@ -27,6 +30,7 @@ pub struct TransactionCost {
// `cost_weight` is a multiplier could be applied to transaction cost,
// if set to zero allows the transaction to bypass cost limit check.
pub cost_weight: u32,
pub account_data_size: u64,
}

impl Default for TransactionCost {
Expand All @@ -38,6 +42,7 @@ impl Default for TransactionCost {
data_bytes_cost: 0u64,
execution_cost: 0u64,
cost_weight: 1u32,
account_data_size: 0u64,
}
}
}
Expand Down Expand Up @@ -118,6 +123,7 @@ impl CostModel {
tx_cost.data_bytes_cost = self.get_data_bytes_cost(transaction);
tx_cost.execution_cost = self.get_transaction_cost(transaction);
tx_cost.cost_weight = self.calculate_cost_weight(transaction);
tx_cost.account_data_size = self.calculate_account_data_size(transaction);

debug!("transaction {:?} has cost {:?}", transaction, tx_cost);
tx_cost
Expand Down Expand Up @@ -201,6 +207,59 @@ impl CostModel {
}
}

fn calculate_account_data_size_on_deserialized_system_instruction(
instruction: SystemInstruction,
) -> u64 {
match instruction {
SystemInstruction::CreateAccount {
lamports: _lamports,
space,
owner: _owner,
} => space,
SystemInstruction::CreateAccountWithSeed {
base: _base,
seed: _seed,
lamports: _lamports,
space,
owner: _owner,
} => space,
SystemInstruction::Allocate { space } => space,
SystemInstruction::AllocateWithSeed {
base: _base,
seed: _seed,
space,
owner: _owner,
} => space,
jeffwashington marked this conversation as resolved.
Show resolved Hide resolved
_ => 0,
jeffwashington marked this conversation as resolved.
Show resolved Hide resolved
}
}

fn calculate_account_data_size_on_instruction(
program_id: &Pubkey,
instruction: &CompiledInstruction,
) -> u64 {
if program_id == &system_program::id() {
if let Ok(instruction) = limited_deserialize(&instruction.data) {
return Self::calculate_account_data_size_on_deserialized_system_instruction(
instruction,
);
}
}
0
}

/// eventually, potentially determine account data size of all writable accounts
/// at the moment, calculate account data size of account creation
fn calculate_account_data_size(&self, transaction: &SanitizedTransaction) -> u64 {
transaction
.message()
.program_instructions_iter()
.map(|(program_id, instruction)| {
Self::calculate_account_data_size_on_instruction(program_id, instruction)
})
.sum()
}

fn calculate_cost_weight(&self, transaction: &SanitizedTransaction) -> u32 {
if is_simple_vote_transaction(transaction) {
// vote has zero cost weight, so it bypasses block cost limit checking
Expand Down Expand Up @@ -272,6 +331,53 @@ mod tests {
);
}

#[test]
fn test_cost_model_data_len_cost() {
let lamports = 0;
let owner = Pubkey::default();
let seed = String::default();
let space = 100;
let base = Pubkey::default();
for instruction in [
SystemInstruction::CreateAccount {
lamports,
space,
owner,
},
SystemInstruction::CreateAccountWithSeed {
base,
seed: seed.clone(),
lamports,
space,
owner,
},
SystemInstruction::Allocate { space },
SystemInstruction::AllocateWithSeed {
base,
seed,
space,
owner,
},
] {
assert_eq!(
space,
CostModel::calculate_account_data_size_on_deserialized_system_instruction(
instruction
)
);
}
assert_eq!(
0,
CostModel::calculate_account_data_size_on_deserialized_system_instruction(
SystemInstruction::TransferWithSeed {
lamports,
from_seed: String::default(),
from_owner: Pubkey::default(),
}
)
);
}

#[test]
fn test_cost_model_simple_transaction() {
let (mint_keypair, start_hash) = test_setup();
Expand Down
Loading