Set transaction size limit (#4107)

* Implement first version of validating summary tx size * Add feature flag for limiting tx size * Fix formatting * Add test checking exceeding tx size limit * Add new test * fix * Improve computation of transaction size * Add check for tx with valid size * Run cargo fmt * Add TODO * Introduce size field for SignedTransaction * Fix error in test * Minor fix Co-authored-by: Bowen Wang <[email protected]>
near · Mar 24, 2021 · 4088a98 · 4088a98
1 parent 9dcc926
commit 4088a98
Show file tree

Hide file tree

Showing 15 changed files with 115 additions and 12 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -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

diff --git a/chain/chain/src/validate.rs b/chain/chain/src/validate.rs
@@ -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();

diff --git a/chain/rosetta-rpc/src/lib.rs b/chain/rosetta-rpc/src/lib.rs
@@ -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(),

diff --git a/core/primitives-core/Cargo.toml b/core/primitives-core/Cargo.toml
@@ -28,3 +28,4 @@ costs_counting = []
 protocol_feature_add_account_versions = []
 protocol_feature_evm = []
 protocol_feature_alt_bn128 = []
+protocol_feature_tx_size_limit = []
diff --git a/core/primitives-core/src/config.rs b/core/primitives-core/src/config.rs
@@ -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,
@@ -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,
     /// Max storage key size
     pub max_length_storage_key: u64,
     /// Max storage value size
@@ -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

diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml
@@ -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 = []
 
 

diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs
@@ -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(
@@ -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
+            ),
         }
     }
 }

diff --git a/core/primitives/src/test_utils.rs b/core/primitives/src/test_utils.rs
@@ -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)
     }
 

diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs
@@ -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)
     }
 }
 
@@ -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, size) = self.transaction.get_hash_and_size();
+        self.hash = hash;
+        self.size = size;
     }
 
     pub fn get_hash(&self) -> CryptoHash {
         self.hash
     }
+
+    pub fn get_size(&self) -> u64 {
+        self.size
+    }
 }
 
 impl Hash for SignedTransaction {

diff --git a/core/primitives/src/version.rs b/core/primitives/src/version.rs
@@ -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.
@@ -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> =
@@ -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();

diff --git a/neard/Cargo.toml b/neard/Cargo.toml
@@ -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]]

diff --git a/runtime/runtime/Cargo.toml b/runtime/runtime/Cargo.toml
@@ -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"

diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs
@@ -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};

diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs
@@ -55,6 +55,19 @@ pub fn validate_transaction(
         return Err(InvalidTxError::InvalidSignature.into());
     }
 
+    #[cfg(feature = "protocol_feature_tx_size_limit")]
+    {
+        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))?;
 
@@ -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]

diff --git a/test-utils/testlib/Cargo.toml b/test-utils/testlib/Cargo.toml
@@ -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"]