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 19 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", "testlib/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", "protocol_feature_add_account_versions", "testlib/nightly_protocol_features"]
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_add_account_versions", "protocol_feature_tx_size_limit", "testlib/nightly_protocol_features"]
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"]
protocol_feature_add_account_versions = ["near-primitives/protocol_feature_add_account_versions"]

# enable this to build neard with wasmer 1.0 runner
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
2 changes: 1 addition & 1 deletion chain/rosetta-rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ async fn construction_payloads(
actions,
};

let transaction_hash = unsigned_transaction.get_hash().clone();
let (transaction_hash, _) = unsigned_transaction.get_hash_and_size().clone();

Ok(Json(models::ConstructionPayloadsResponse {
unsigned_transaction: unsigned_transaction.into(),
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 @@ -28,3 +28,4 @@ costs_counting = []
protocol_feature_add_account_versions = []
protocol_feature_evm = []
protocol_feature_alt_bn128 = []
protocol_feature_tx_size_limit = []
7 changes: 7 additions & 0 deletions core/primitives-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ pub struct VMConfig {
}

/// Describes limits for VM and Runtime.
/// TODO #4139: consider switching to strongly-typed wrappers instead of raw quantities
#[derive(Debug, Serialize, Deserialize, Clone, Hash, PartialEq, Eq)]
#[serde(default)]
pub struct VMLimitConfig {
/// Max amount of gas that can be used, excluding gas attached to promises.
pub max_gas_burnt: Gas,
Expand Down Expand Up @@ -68,6 +70,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 +154,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
3 changes: 2 additions & 1 deletion core/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ 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_alt_bn128", "protocol_feature_access_key_nonce_range", "protocol_feature_add_account_versions"]
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_alt_bn128", "protocol_feature_access_key_nonce_range", "protocol_feature_add_account_versions", "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 @@ -143,6 +143,9 @@ pub enum InvalidTxError {
Expired,
/// An error occurred while validating actions of a Transaction.
ActionsValidation(ActionsValidationError),
/// The size of serialized transaction exceeded the limit.
#[cfg(feature = "protocol_feature_tx_size_limit")]
TransactionSizeExceeded { size: u64, limit: u64 },
}

#[derive(
Expand Down Expand Up @@ -441,6 +444,12 @@ impl Display for InvalidTxError {
write!(f, "Transaction actions validation error: {}", error)
}
InvalidTxError::NonceTooLarge { tx_nonce, upper_bound } => { write!(f, "Transaction nonce {} must be smaller than the access key nonce upper bound {}", tx_nonce, upper_bound) }
#[cfg(feature = "protocol_feature_tx_size_limit")]
InvalidTxError::TransactionSizeExceeded { size, limit } => write!(
f,
"Size of serialized transaction {} exceeded the limit {}",
size, limit
),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/primitives/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl Transaction {
}

pub fn sign(self, signer: &dyn Signer) -> SignedTransaction {
let signature = signer.sign(self.get_hash().as_ref());
let signature = signer.sign(self.get_hash_and_size().0.as_ref());
SignedTransaction::new(signature, self)
}

Expand Down
19 changes: 14 additions & 5 deletions core/primitives/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ pub struct Transaction {
}

impl Transaction {
/// Computes a hash of the transaction for signing
pub fn get_hash(&self) -> CryptoHash {
/// Computes a hash of the transaction for signing and size of serialized transaction
pub fn get_hash_and_size(&self) -> (CryptoHash, u64) {
let bytes = self.try_to_vec().expect("Failed to deserialize");
hash(&bytes)
(hash(&bytes), bytes.len() as u64)
}
}

Expand Down Expand Up @@ -205,22 +205,31 @@ pub struct SignedTransaction {
pub signature: Signature,
#[borsh_skip]
hash: CryptoHash,
#[borsh_skip]
size: u64,
}

impl SignedTransaction {
pub fn new(signature: Signature, transaction: Transaction) -> Self {
let mut signed_tx = Self { signature, transaction, hash: CryptoHash::default() };
let mut signed_tx =
Self { signature, transaction, hash: CryptoHash::default(), size: u64::default() };
signed_tx.init();
signed_tx
}

pub fn init(&mut self) {
self.hash = self.transaction.get_hash();
let hash_and_size = self.transaction.get_hash_and_size();
Longarithm marked this conversation as resolved.
Show resolved Hide resolved
self.hash = hash_and_size.0;
self.size = hash_and_size.1;
}

pub fn get_hash(&self) -> CryptoHash {
self.hash
}

pub fn get_size(&self) -> u64 {
self.size
}
}

impl Hash for SignedTransaction {
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 @@ -97,6 +97,8 @@ pub enum ProtocolFeature {
DeleteActionRestriction,
#[cfg(feature = "protocol_feature_add_account_versions")]
AccountVersions,
#[cfg(feature = "protocol_feature_tx_size_limit")]
TransactionSizeLimit,
}

/// Current latest stable version of the protocol.
Expand All @@ -105,7 +107,7 @@ pub const PROTOCOL_VERSION: ProtocolVersion = 43;

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

lazy_static! {
static ref STABLE_PROTOCOL_FEATURES_TO_VERSION_MAPPING: HashMap<ProtocolFeature, ProtocolVersion> =
Expand Down Expand Up @@ -148,6 +150,8 @@ lazy_static! {
(ProtocolFeature::AccessKeyNonceRange, 106),
#[cfg(feature = "protocol_feature_add_account_versions")]
(ProtocolFeature::AccountVersions, 107),
#[cfg(feature = "protocol_feature_tx_size_limit")]
(ProtocolFeature::TransactionSizeLimit, 108),
]
.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 @@ -73,7 +73,8 @@ protocol_feature_alt_bn128 = ["near-primitives/protocol_feature_alt_bn128", "nod
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"]
protocol_feature_add_account_versions = ["near-primitives/protocol_feature_add_account_versions"]
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_add_account_versions"]
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_add_account_versions", "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
5 changes: 5 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
63 changes: 63 additions & 0 deletions runtime/runtime/src/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,19 @@ pub fn validate_transaction(
return Err(InvalidTxError::InvalidSignature.into());
}

#[cfg(feature = "protocol_feature_tx_size_limit")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This check should only be done once. There is no reason to check transactions for which you already checked. For example, inside prepare_transactions it doesn't make sense to check again because this check should have been done when the transaction is received. To further reduce the overhead, you could change NetworkClientMessage::Transaction to include the transaction size. It probably even makes sense to implement custom borsh deserializer that would fail when the a larger-than-allowed transaction arrives on the network (this can potentially be done in a separate PR)

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems that at least one serialization is already happening inside SignedTransaction::new, to save Transaction hash. So I saved size there in the same fashion and started to use it, which shouldn't give significant overhead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah that makes sense

{
let transaction_size = signed_transaction.get_size();
let max_transaction_size = config.wasm_config.limit_config.max_transaction_size;
if transaction_size > max_transaction_size {
return Err(InvalidTxError::TransactionSizeExceeded {
size: transaction_size,
limit: max_transaction_size,
}
.into());
}
}

validate_actions(&config.wasm_config.limit_config, &transaction.actions)
.map_err(|e| InvalidTxError::ActionsValidation(e))?;

Expand Down Expand Up @@ -1177,6 +1190,56 @@ mod tests {
);
}

#[test]
#[cfg(feature = "protocol_feature_tx_size_limit")]
fn test_validate_transaction_exceeding_tx_size_limit() {
let (signer, mut state_update, gas_price) =
setup_common(TESTING_INIT_BALANCE, 0, Some(AccessKey::full_access()));

let transaction = SignedTransaction::from_actions(
1,
alice_account(),
bob_account(),
&*signer,
vec![Action::DeployContract(DeployContractAction { code: vec![1; 5] })],
CryptoHash::default(),
);
let transaction_size = transaction.get_size();

let mut config = RuntimeConfig::default();
let max_transaction_size = transaction_size - 1;
config.wasm_config.limit_config.max_transaction_size = transaction_size - 1;

assert_eq!(
verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
false,
None,
PROTOCOL_VERSION,
)
.expect_err("expected an error"),
RuntimeError::InvalidTxError(InvalidTxError::TransactionSizeExceeded {
size: transaction_size,
limit: max_transaction_size
}),
);

config.wasm_config.limit_config.max_transaction_size = transaction_size + 1;
verify_and_charge_transaction(
&config,
&mut state_update,
gas_price,
&transaction,
false,
None,
PROTOCOL_VERSION,
)
.expect("valid transaction");
}

// Receipts

#[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,5 +54,6 @@ 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"]
nightly_protocol_features = ["nightly_protocol", "neard/nightly_protocol_features"]
nightly_protocol = ["neard/nightly_protocol"]