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

Set transaction size limit #4107

Merged
merged 21 commits into from
Mar 24, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,13 @@ metric_recorder = ["neard/metric_recorder"]
delay_detector = ["neard/delay_detector"]
rosetta_rpc = ["neard/rosetta_rpc"]
nightly_protocol = ["near-primitives/nightly_protocol", "near-jsonrpc/nightly_protocol"]
nightly_protocol_features = ["nightly_protocol", "neard/nightly_protocol_features", "protocol_feature_evm", "protocol_feature_block_header_v3", "protocol_feature_alt_bn128", "protocol_feature_access_key_nonce_range"]
nightly_protocol_features = ["nightly_protocol", "neard/nightly_protocol_features", "protocol_feature_evm", "protocol_feature_block_header_v3", "protocol_feature_alt_bn128", "protocol_feature_access_key_nonce_range", "protocol_feature_tx_size_limit"]
protocol_feature_forward_chunk_parts = ["neard/protocol_feature_forward_chunk_parts"]
protocol_feature_evm = ["neard/protocol_feature_evm", "testlib/protocol_feature_evm", "runtime-params-estimator/protocol_feature_evm"]
protocol_feature_alt_bn128 = ["neard/protocol_feature_alt_bn128", "testlib/protocol_feature_alt_bn128", "runtime-params-estimator/protocol_feature_alt_bn128"]
protocol_feature_block_header_v3 = ["near-primitives/protocol_feature_block_header_v3", "near-chain/protocol_feature_block_header_v3", "neard/protocol_feature_block_header_v3"]
protocol_feature_access_key_nonce_range = ["neard/protocol_feature_access_key_nonce_range"]
protocol_feature_tx_size_limit = ["near-primitives/protocol_feature_tx_size_limit", "node-runtime/protocol_feature_tx_size_limit", "neard/protocol_feature_tx_size_limit"]

# enable this to build neard with wasmer 1.0 runner
# now if none of wasmer0_default, wasmer1_default or wasmtime_default is enabled, wasmer0 would be default
Expand Down
2 changes: 1 addition & 1 deletion chain/chain/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub fn validate_chunk_proofs(chunk: &ShardChunk, runtime_adapter: &dyn RuntimeAd
}

/// Validates that the given transactions are in proper valid order.
/// See https://nomicon.io/BlockchainLayer/Transactions.html#transaction-ordering
/// See https://nomicon.io/ChainSpec/Transactions.html#transaction-ordering
pub fn validate_transactions_order(transactions: &[SignedTransaction]) -> bool {
let mut nonces: HashMap<(&AccountId, &PublicKey), Nonce> = HashMap::new();
let mut batches: HashMap<(&AccountId, &PublicKey), usize> = HashMap::new();
Expand Down
1 change: 1 addition & 0 deletions core/primitives-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ default = []
costs_counting = []
protocol_feature_evm = []
protocol_feature_alt_bn128 = []
protocol_feature_tx_size_limit = []
5 changes: 5 additions & 0 deletions core/primitives-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ pub struct VMLimitConfig {
pub max_length_returned_data: u64,
/// Max contract size
pub max_contract_size: u64,
/// Max transaction size
#[cfg(feature = "protocol_feature_tx_size_limit")]
pub max_transaction_size: u64,
Longarithm marked this conversation as resolved.
Show resolved Hide resolved
/// Max storage key size
pub max_length_storage_key: u64,
/// Max storage value size
Expand Down Expand Up @@ -149,6 +152,8 @@ impl Default for VMLimitConfig {
max_arguments_length: 4 * 2u64.pow(20), // 4 Mib
max_length_returned_data: 4 * 2u64.pow(20), // 4 Mib
max_contract_size: 4 * 2u64.pow(20), // 4 Mib,
#[cfg(feature = "protocol_feature_tx_size_limit")]
max_transaction_size: 4 * 2u64.pow(20), // 4 Mib

max_length_storage_key: 4 * 2u64.pow(20), // 4 Mib
max_length_storage_value: 4 * 2u64.pow(20), // 4 Mib
Expand Down
4 changes: 3 additions & 1 deletion core/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ protocol_feature_evm = ["near-primitives-core/protocol_feature_evm"]
protocol_feature_block_header_v3 = []
protocol_feature_alt_bn128 = ["near-primitives-core/protocol_feature_alt_bn128", "near-vm-errors/protocol_feature_alt_bn128"]
protocol_feature_access_key_nonce_range = []
nightly_protocol_features = ["nightly_protocol", "protocol_feature_forward_chunk_parts", "protocol_feature_rectify_inflation", "protocol_feature_evm", "protocol_feature_block_header_v3", "protocol_feature_access_key_nonce_range", "protocol_feature_alt_bn128"]
protocol_feature_tx_size_limit = ["near-primitives-core/protocol_feature_tx_size_limit"]

nightly_protocol_features = ["nightly_protocol", "protocol_feature_forward_chunk_parts", "protocol_feature_rectify_inflation", "protocol_feature_evm", "protocol_feature_block_header_v3", "protocol_feature_access_key_nonce_range", "protocol_feature_alt_bn128", "protocol_feature_tx_size_limit"]
nightly_protocol = []


Expand Down
9 changes: 9 additions & 0 deletions core/primitives/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ pub enum ActionsValidationError {
UnsuitableStakingKey { public_key: PublicKey },
/// The attached amount of gas in a FunctionCall action has to be a positive number.
FunctionCallZeroAttachedGas,
/// Transaction size exceeded the limit.
#[cfg(feature = "protocol_feature_tx_size_limit")]
TransactionSizeExceeded { size: u64, limit: u64 },
}

/// Describes the error for validating a receipt.
Expand Down Expand Up @@ -313,6 +316,12 @@ impl Display for ActionsValidationError {
f,
"The attached amount of gas in a FunctionCall action has to be a positive number",
),
#[cfg(feature = "protocol_feature_tx_size_limit")]
ActionsValidationError::TransactionSizeExceeded { size, limit } => write!(
f,
"Total transaction size {} exceeded the limit {}",
size, limit
),
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions core/primitives/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ impl Action {
_ => 0,
}
}
#[cfg(feature = "protocol_feature_tx_size_limit")]
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest for the transaction size to use the size of the transaction serialized as borsh. This will remove potential tricky angles of attack, e.g. it is still possible to create a heavy transaction that adds a very large number of function access keys with long method names. Also, that removes ambiguity in interpreting what transaction size is, so e.g. we can say that block size is sum of the transaction sizes plus header data.

Ideally, we would add a trait to Borsh called BorshSize that implements borsh_size() -> u64 method which we can use similarly to BorshSerialize and BorshDeserialize, like this: #[derive(BorshSize)]. AFAIR there might some places in our code where we measure the size of the objects by first serializing them in Borsh and then counting bytes, so it might be useful there too. It shouldn't be difficult to add to borsh and will probably be largely copy-paste of https://github.com/near/borsh-rs/blob/master/borsh/src/ser/mod.rs

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah agree with what Max said here

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!
Changed the way of size computation and added a separate issue for adding new trait.

pub fn get_size(&self) -> usize {
match self {
Action::FunctionCall(a) => a.args.len(),
Action::DeployContract(a) => a.code.len(),
_ => 0,
}
}
}

/// Create account action
Expand Down
6 changes: 5 additions & 1 deletion core/primitives/src/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ pub enum ProtocolFeature {
AltBn128,
#[cfg(feature = "protocol_feature_access_key_nonce_range")]
AccessKeyNonceRange,
#[cfg(feature = "protocol_feature_tx_size_limit")]
TransactionSizeLimit,
}

/// Current latest stable version of the protocol.
Expand All @@ -102,7 +104,7 @@ pub const PROTOCOL_VERSION: ProtocolVersion = 42;

/// Current latest nightly version of the protocol.
#[cfg(feature = "nightly_protocol")]
pub const PROTOCOL_VERSION: ProtocolVersion = 106;
pub const PROTOCOL_VERSION: ProtocolVersion = 107;

lazy_static! {
static ref STABLE_PROTOCOL_FEATURES_TO_VERSION_MAPPING: HashMap<ProtocolFeature, ProtocolVersion> =
Expand Down Expand Up @@ -142,6 +144,8 @@ lazy_static! {
(ProtocolFeature::AltBn128, 105),
#[cfg(feature = "protocol_feature_access_key_nonce_range")]
(ProtocolFeature::AccessKeyNonceRange, 106),
#[cfg(feature = "protocol_feature_tx_size_limit")]
(ProtocolFeature::TransactionSizeLimit, 107),
]
.into_iter()
.collect();
Expand Down
3 changes: 2 additions & 1 deletion neard/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ protocol_feature_evm = ["near-primitives/protocol_feature_evm", "node-runtime/pr
protocol_feature_alt_bn128 = ["near-primitives/protocol_feature_alt_bn128", "node-runtime/protocol_feature_alt_bn128"]
protocol_feature_block_header_v3 = ["near-epoch-manager/protocol_feature_block_header_v3", "near-primitives/protocol_feature_block_header_v3", "near-chain/protocol_feature_block_header_v3", "near-client/protocol_feature_block_header_v3"]
protocol_feature_access_key_nonce_range = ["near-primitives/protocol_feature_access_key_nonce_range", "node-runtime/protocol_feature_access_key_nonce_range", "near-client/protocol_feature_access_key_nonce_range"]
nightly_protocol_features = ["nightly_protocol", "near-primitives/nightly_protocol_features", "near-client/nightly_protocol_features", "near-epoch-manager/nightly_protocol_features", "near-store/nightly_protocol_features", "protocol_feature_forward_chunk_parts", "protocol_feature_rectify_inflation", "protocol_feature_evm", "protocol_feature_block_header_v3", "protocol_feature_alt_bn128", "protocol_feature_access_key_nonce_range"]
protocol_feature_tx_size_limit = ["near-primitives/protocol_feature_tx_size_limit", "node-runtime/protocol_feature_tx_size_limit"]
nightly_protocol_features = ["nightly_protocol", "near-primitives/nightly_protocol_features", "near-client/nightly_protocol_features", "near-epoch-manager/nightly_protocol_features", "near-store/nightly_protocol_features", "protocol_feature_forward_chunk_parts", "protocol_feature_rectify_inflation", "protocol_feature_evm", "protocol_feature_block_header_v3", "protocol_feature_alt_bn128", "protocol_feature_access_key_nonce_range", "protocol_feature_tx_size_limit"]
nightly_protocol = ["near-primitives/nightly_protocol", "near-jsonrpc/nightly_protocol"]

[[bin]]
Expand Down
1 change: 1 addition & 0 deletions runtime/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ protocol_feature_alt_bn128 = [
"near-vm-runner/protocol_feature_alt_bn128",
"near-vm-errors/protocol_feature_alt_bn128",
]
protocol_feature_tx_size_limit = []

[dev-dependencies]
tempfile = "3"
Expand Down
11 changes: 11 additions & 0 deletions runtime/runtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ pub fn safe_add_balance(a: Balance, b: Balance) -> Result<Balance, IntegerOverfl
a.checked_add(b).ok_or_else(|| IntegerOverflowError {})
}

#[cfg(feature = "protocol_feature_tx_size_limit")]
pub fn safe_add_usize(a: usize, b: usize) -> Result<usize, IntegerOverflowError> {
a.checked_add(b).ok_or_else(|| IntegerOverflowError {})
}

#[macro_export]
macro_rules! safe_add_balance_apply {
($x: expr) => {$x};
Expand Down Expand Up @@ -269,6 +274,12 @@ pub fn total_prepaid_gas(actions: &[Action]) -> Result<Gas, IntegerOverflowError
actions.iter().try_fold(0, |acc, action| safe_add_gas(acc, action.get_prepaid_gas()))
}

/// Get the total size of given actions.
#[cfg(feature = "protocol_feature_tx_size_limit")]
pub fn total_size(actions: &[Action]) -> Result<usize, IntegerOverflowError> {
actions.iter().try_fold(0, |acc, action| safe_add_usize(acc, action.get_size()))
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
56 changes: 56 additions & 0 deletions runtime/runtime/src/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ use near_store::{
get_access_key, get_account, set_access_key, set_account, StorageError, TrieUpdate,
};

#[cfg(feature = "protocol_feature_tx_size_limit")]
use crate::config::total_size;
use crate::config::{total_prepaid_gas, tx_cost, TransactionCost};
use crate::VerificationResult;
use near_primitives::checked_feature;
Expand Down Expand Up @@ -326,6 +328,18 @@ pub(crate) fn validate_actions(
});
}

#[cfg(feature = "protocol_feature_tx_size_limit")]
{
let total_size =
total_size(actions).map_err(|_| ActionsValidationError::IntegerOverflow)? as u64;
if total_size > limit_config.max_transaction_size {
Longarithm marked this conversation as resolved.
Show resolved Hide resolved
return Err(ActionsValidationError::TransactionSizeExceeded {
size: total_size,
limit: limit_config.max_transaction_size,
});
}
}

Ok(())
}

Expand Down Expand Up @@ -1445,6 +1459,48 @@ mod tests {
);
}

#[test]
#[cfg(feature = "protocol_feature_tx_size_limit")]
fn test_validate_actions_checking_tx_size_limit() {
let mut limit_config = VMLimitConfig::default();
let contract_size = 5;
let args_size = 3;

limit_config.max_transaction_size = 3;
assert_eq!(
validate_actions(
&limit_config,
&vec![Action::DeployContract(DeployContractAction {
code: vec![1; contract_size as usize]
}),]
)
.expect_err("Expected an error"),
ActionsValidationError::TransactionSizeExceeded {
size: contract_size,
limit: limit_config.max_transaction_size
},
);

limit_config.max_transaction_size = 10;
assert_eq!(
validate_actions(
&limit_config,
&vec![
Action::DeployContract(DeployContractAction {
code: vec![1; contract_size as usize]
}),
Action::FunctionCall(FunctionCallAction {
method_name: "test-method".to_string(),
args: vec![0; args_size as usize],
gas: 100,
deposit: 0
})
]
),
Ok(()),
);
}

// Individual actions

#[test]
Expand Down
1 change: 1 addition & 0 deletions test-utils/testlib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,4 @@ protocol_feature_alt_bn128 = [
"near-vm-errors/protocol_feature_alt_bn128",
]
protocol_feature_evm = ["near-evm-runner/protocol_feature_evm", "near-primitives/protocol_feature_evm", "neard/protocol_feature_evm", "node-runtime/protocol_feature_evm", "near-chain-configs/protocol_feature_evm", "near-chain/protocol_feature_evm"]
protocol_feature_tx_size_limit = ["near-primitives/protocol_feature_tx_size_limit", "node-runtime/protocol_feature_tx_size_limit"]