diff --git a/chain/chain/Cargo.toml b/chain/chain/Cargo.toml index 510b35d0af3..f460c02f7ab 100644 --- a/chain/chain/Cargo.toml +++ b/chain/chain/Cargo.toml @@ -81,6 +81,10 @@ no_cache = ["near-store/no_cache"] protocol_feature_reject_blocks_with_outdated_protocol_version = [ "near-primitives/protocol_feature_reject_blocks_with_outdated_protocol_version", ] +protocol_feature_relaxed_chunk_validation = [ + "near-primitives/protocol_feature_relaxed_chunk_validation", + "node-runtime/protocol_feature_relaxed_chunk_validation", +] nightly = [ "near-async/nightly", @@ -98,6 +102,7 @@ nightly = [ "nightly_protocol", "node-runtime/nightly", "protocol_feature_reject_blocks_with_outdated_protocol_version", + "protocol_feature_relaxed_chunk_validation", ] nightly_protocol = [ "near-async/nightly_protocol", diff --git a/chain/chain/src/chain.rs b/chain/chain/src/chain.rs index b59fcc90310..f8b462b7202 100644 --- a/chain/chain/src/chain.rs +++ b/chain/chain/src/chain.rs @@ -3198,6 +3198,17 @@ impl Chain { prev_block_header: &BlockHeader, chunk: &ShardChunk, ) -> Result<(), Error> { + let protocol_version = + self.epoch_manager.get_epoch_protocol_version(block.header().epoch_id())?; + + if checked_feature!( + "protocol_feature_relaxed_chunk_validation", + RelaxedChunkValidation, + protocol_version + ) { + return Ok(()); + } + if !validate_transactions_order(chunk.transactions()) { let merkle_paths = Block::compute_chunk_headers_root(block.chunks().iter_deprecated()).1; @@ -3214,8 +3225,6 @@ impl Chain { return Err(Error::InvalidChunkProofs(Box::new(chunk_proof))); } - let protocol_version = - self.epoch_manager.get_epoch_protocol_version(block.header().epoch_id())?; if checked_feature!("stable", AccessKeyNonceRange, protocol_version) { let transaction_validity_period = self.transaction_validity_period; for transaction in chunk.transactions() { diff --git a/chain/chain/src/runtime/mod.rs b/chain/chain/src/runtime/mod.rs index 2e375167af0..e829f5907be 100644 --- a/chain/chain/src/runtime/mod.rs +++ b/chain/chain/src/runtime/mod.rs @@ -1400,11 +1400,18 @@ fn chunk_tx_gas_limit( fn calculate_transactions_size_limit( protocol_version: ProtocolVersion, runtime_config: &RuntimeConfig, - last_chunk_transactions_size: usize, + mut last_chunk_transactions_size: usize, transactions_gas_limit: Gas, ) -> u64 { // Checking feature WitnessTransactionLimits if ProtocolFeature::StatelessValidation.enabled(protocol_version) { + if near_primitives::checked_feature!( + "protocol_feature_relaxed_chunk_validation", + RelaxedChunkValidation, + protocol_version + ) { + last_chunk_transactions_size = 0; + } // Sum of transactions in the previous and current chunks should not exceed the limit. // Witness keeps transactions from both previous and current chunk, so we have to limit the sum of both. runtime_config diff --git a/chain/chain/src/stateless_validation/chunk_validation.rs b/chain/chain/src/stateless_validation/chunk_validation.rs index 64d77fce36a..d269cc78729 100644 --- a/chain/chain/src/stateless_validation/chunk_validation.rs +++ b/chain/chain/src/stateless_validation/chunk_validation.rs @@ -18,6 +18,7 @@ use near_epoch_manager::EpochManagerAdapter; use near_pool::TransactionGroupIteratorWrapper; use near_primitives::apply::ApplyChunkReason; use near_primitives::block::Block; +use near_primitives::checked_feature; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::merkle::merklize; use near_primitives::receipt::Receipt; @@ -245,46 +246,54 @@ pub fn pre_validate_chunk_state_witness( ))); } - // Verify that all proposed transactions are valid. - let new_transactions = &state_witness.new_transactions; - if !new_transactions.is_empty() { - let transactions_validation_storage_config = RuntimeStorageConfig { - state_root: state_witness.chunk_header.prev_state_root(), - use_flat_storage: true, - source: StorageDataSource::Recorded(PartialStorage { - nodes: state_witness.new_transactions_validation_state.clone(), - }), - state_patch: Default::default(), - }; + let epoch_id = last_chunk_block.header().epoch_id(); + let protocol_version = epoch_manager.get_epoch_protocol_version(&epoch_id)?; + if !checked_feature!( + "protocol_feature_relaxed_chunk_validation", + RelaxedChunkValidation, + protocol_version + ) { + // Verify that all proposed transactions are valid. + let new_transactions = &state_witness.new_transactions; + if !new_transactions.is_empty() { + let transactions_validation_storage_config = RuntimeStorageConfig { + state_root: state_witness.chunk_header.prev_state_root(), + use_flat_storage: true, + source: StorageDataSource::Recorded(PartialStorage { + nodes: state_witness.new_transactions_validation_state.clone(), + }), + state_patch: Default::default(), + }; - match validate_prepared_transactions( - chain, - runtime_adapter, - &state_witness.chunk_header, - transactions_validation_storage_config, - &new_transactions, - &state_witness.transactions, - ) { - Ok(result) => { - if result.transactions.len() != new_transactions.len() { + match validate_prepared_transactions( + chain, + runtime_adapter, + &state_witness.chunk_header, + transactions_validation_storage_config, + &new_transactions, + &state_witness.transactions, + ) { + Ok(result) => { + if result.transactions.len() != new_transactions.len() { + return Err(Error::InvalidChunkStateWitness(format!( + "New transactions validation failed. \ + {} transactions out of {} proposed transactions were valid.", + result.transactions.len(), + new_transactions.len(), + ))); + } + } + Err(error) => { return Err(Error::InvalidChunkStateWitness(format!( - "New transactions validation failed. {} transactions out of {} proposed transactions were valid.", - result.transactions.len(), - new_transactions.len(), + "New transactions validation failed: {}", + error, ))); } - } - Err(error) => { - return Err(Error::InvalidChunkStateWitness(format!( - "New transactions validation failed: {}", - error, - ))); - } - }; + }; + } } let main_transition_params = if last_chunk_block.header().is_genesis() { - let epoch_id = last_chunk_block.header().epoch_id(); let shard_layout = epoch_manager.get_shard_layout(&epoch_id)?; let congestion_info = last_chunk_block .block_congestion_info() diff --git a/chain/client/Cargo.toml b/chain/client/Cargo.toml index 0c098578637..7a35a758379 100644 --- a/chain/client/Cargo.toml +++ b/chain/client/Cargo.toml @@ -73,6 +73,11 @@ near-primitives = { workspace = true, features = ["clock", "solomon", "rand"] } near-actix-test-utils.workspace = true [features] +protocol_feature_relaxed_chunk_validation = [ + "near-chain/protocol_feature_relaxed_chunk_validation", + "near-primitives/protocol_feature_relaxed_chunk_validation", +] + # if enabled, we assert in most situations that are impossible unless some byzantine behavior is observed. byzantine_asserts = ["near-chain/byzantine_asserts"] shadow_chunk_validation = ["near-chain/shadow_chunk_validation"] @@ -120,6 +125,7 @@ nightly = [ "near-telemetry/nightly", "near-vm-runner/nightly", "nightly_protocol", + "protocol_feature_relaxed_chunk_validation", ] sandbox = [ "near-client-primitives/sandbox", diff --git a/chain/client/src/stateless_validation/state_witness_producer.rs b/chain/client/src/stateless_validation/state_witness_producer.rs index 51b55b2bf22..1f3e1a5a5bc 100644 --- a/chain/client/src/stateless_validation/state_witness_producer.rs +++ b/chain/client/src/stateless_validation/state_witness_producer.rs @@ -1,6 +1,6 @@ -use std::collections::HashMap; -use std::sync::Arc; - +use super::partial_witness::partial_witness_actor::DistributeStateWitnessRequest; +use crate::stateless_validation::chunk_validator::send_chunk_endorsement_to_block_producers; +use crate::Client; use near_async::messaging::{CanSend, IntoSender}; use near_chain::{BlockHeader, Chain, ChainStoreAccess}; use near_chain_primitives::Error; @@ -20,11 +20,8 @@ use near_primitives::stateless_validation::stored_chunk_state_transition_data::{ use near_primitives::types::{AccountId, EpochId, ShardId}; use near_primitives::validator_signer::ValidatorSigner; use near_primitives::version::ProtocolFeature; - -use crate::stateless_validation::chunk_validator::send_chunk_endorsement_to_block_producers; -use crate::Client; - -use super::partial_witness::partial_witness_actor::DistributeStateWitnessRequest; +use std::collections::HashMap; +use std::sync::Arc; /// Result of collecting state transition data from the database to generate a state witness. /// Keep this private to this file. @@ -126,6 +123,7 @@ impl Client { let chunk_header = chunk.cloned_header(); let epoch_id = self.epoch_manager.get_epoch_id_from_prev_block(chunk_header.prev_block_hash())?; + let protocol_version = self.epoch_manager.get_epoch_protocol_version(&epoch_id)?; let prev_chunk = self.chain.get_chunk(&prev_chunk_header.chunk_hash())?; let StateTransitionData { main_transition, @@ -135,17 +133,26 @@ impl Client { contract_updates, } = self.collect_state_transition_data(&chunk_header, prev_chunk_header)?; - let new_transactions = chunk.transactions().to_vec(); - let new_transactions_validation_state = if new_transactions.is_empty() { - PartialState::default() + let (new_transactions, new_transactions_validation_state) = if checked_feature!( + "protocol_feature_relaxed_chunk_validation", + RelaxedChunkValidation, + protocol_version + ) { + (Vec::new(), PartialState::default()) } else { - // With stateless validation chunk producer uses recording reads when validating transactions. - // The storage proof must be available here. - transactions_storage_proof.ok_or_else(|| { - let message = "Missing storage proof for transactions validation"; - log_assert_fail!("{message}"); - Error::Other(message.to_owned()) - })? + let new_transactions = chunk.transactions().to_vec(); + let new_transactions_validation_state = if new_transactions.is_empty() { + PartialState::default() + } else { + // With stateless validation chunk producer uses recording reads when validating + // transactions. The storage proof must be available here. + transactions_storage_proof.ok_or_else(|| { + let message = "Missing storage proof for transactions validation"; + log_assert_fail!("{message}"); + Error::Other(message.to_owned()) + })? + }; + (new_transactions, new_transactions_validation_state) }; let source_receipt_proofs = diff --git a/core/primitives-core/Cargo.toml b/core/primitives-core/Cargo.toml index e4efb9a0896..40fd9e1fefa 100644 --- a/core/primitives-core/Cargo.toml +++ b/core/primitives-core/Cargo.toml @@ -38,6 +38,7 @@ protocol_feature_fix_staking_threshold = [] protocol_feature_fix_contract_loading_cost = [] protocol_feature_reject_blocks_with_outdated_protocol_version = [] protocol_feature_nonrefundable_transfer_nep491 = [] +protocol_feature_relaxed_chunk_validation = [] nightly = [ "nightly_protocol", @@ -45,6 +46,7 @@ nightly = [ "protocol_feature_fix_staking_threshold", "protocol_feature_nonrefundable_transfer_nep491", "protocol_feature_reject_blocks_with_outdated_protocol_version", + "protocol_feature_relaxed_chunk_validation", ] nightly_protocol = [ diff --git a/core/primitives-core/src/version.rs b/core/primitives-core/src/version.rs index 97856e07b8c..3fb1f51ea77 100644 --- a/core/primitives-core/src/version.rs +++ b/core/primitives-core/src/version.rs @@ -183,6 +183,12 @@ pub enum ProtocolFeature { /// to sync the current epoch's state. This is not strictly a protocol feature, but is included /// here to coordinate among nodes CurrentEpochStateSync, + /// Relaxed validation of transactions included in a chunk. + /// + /// Chunks no longer become entirely invalid in case invalid transactions are included in the + /// chunk. Instead the transactions are discarded during their conversion to receipts. + #[cfg(feature = "protocol_feature_relaxed_chunk_validation")] + RelaxedChunkValidation, } impl ProtocolFeature { @@ -262,6 +268,8 @@ impl ProtocolFeature { ProtocolFeature::ShuffleShardAssignments => 143, ProtocolFeature::CurrentEpochStateSync => 144, ProtocolFeature::SimpleNightshadeV4 => 145, + #[cfg(feature = "protocol_feature_relaxed_chunk_validation")] + ProtocolFeature::RelaxedChunkValidation => 146, ProtocolFeature::BandwidthScheduler => 147, // Place features that are not yet in Nightly below this line. } diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index 975fd8e35ce..047638bb33a 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -67,6 +67,9 @@ protocol_feature_reject_blocks_with_outdated_protocol_version = [ protocol_feature_nonrefundable_transfer_nep491 = [ "near-primitives-core/protocol_feature_nonrefundable_transfer_nep491", ] +protocol_feature_relaxed_chunk_validation = [ + "near-primitives-core/protocol_feature_relaxed_chunk_validation", +] nightly = [ "near-fmt/nightly", @@ -78,6 +81,7 @@ nightly = [ "protocol_feature_fix_staking_threshold", "protocol_feature_nonrefundable_transfer_nep491", "protocol_feature_reject_blocks_with_outdated_protocol_version", + "protocol_feature_relaxed_chunk_validation", ] nightly_protocol = [ diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 51c38db549f..2178db63775 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -56,8 +56,7 @@ impl From for TxExecutionError { pub enum RuntimeError { /// An unexpected integer overflow occurred. The likely issue is an invalid state or the transition. UnexpectedIntegerOverflow(String), - /// An error happened during TX verification and account charging. It's likely the chunk is invalid. - /// and should be challenged. + /// An error happened during TX verification and account charging. InvalidTxError(InvalidTxError), /// Unexpected error which is typically related to the node storage corruption. /// It's possible the input state is invalid or malicious. diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 2cc5fd93c04..1da15cc4c8b 100644 --- a/integration-tests/Cargo.toml +++ b/integration-tests/Cargo.toml @@ -101,6 +101,9 @@ protocol_feature_reject_blocks_with_outdated_protocol_version = [ protocol_feature_nonrefundable_transfer_nep491 = [ "near-primitives/protocol_feature_nonrefundable_transfer_nep491", ] +protocol_feature_relaxed_chunk_validation = [ + "near-primitives-core/protocol_feature_relaxed_chunk_validation", +] nightly = [ "near-actix-test-utils/nightly", @@ -131,6 +134,7 @@ nightly = [ "protocol_feature_fix_contract_loading_cost", "protocol_feature_nonrefundable_transfer_nep491", "protocol_feature_reject_blocks_with_outdated_protocol_version", + "protocol_feature_relaxed_chunk_validation", "testlib/nightly", ] nightly_protocol = [ diff --git a/integration-tests/src/test_loop/tests/mod.rs b/integration-tests/src/test_loop/tests/mod.rs index 026776f9450..7be430d582e 100644 --- a/integration-tests/src/test_loop/tests/mod.rs +++ b/integration-tests/src/test_loop/tests/mod.rs @@ -1,19 +1,19 @@ -pub mod bandwidth_scheduler_protocol_upgrade; +mod bandwidth_scheduler_protocol_upgrade; mod chunk_validator_kickout; -pub mod congestion_control; -pub mod congestion_control_genesis_bootstrap; -pub mod contract_distribution_cross_shard; -pub mod contract_distribution_simple; +mod congestion_control; +mod congestion_control_genesis_bootstrap; +mod contract_distribution_cross_shard; +mod contract_distribution_simple; mod create_delete_account; -pub mod epoch_sync; -pub mod fix_min_stake_ratio; -pub mod in_memory_tries; -pub mod max_receipt_size; -pub mod multinode_stateless_validators; -pub mod multinode_test_loop_example; -pub mod protocol_upgrade; +mod epoch_sync; +mod fix_min_stake_ratio; +mod in_memory_tries; +mod max_receipt_size; +mod multinode_stateless_validators; +mod multinode_test_loop_example; +mod protocol_upgrade; mod resharding_v3; -pub mod simple_test_loop_example; -pub mod state_sync; -pub mod syncing; -pub mod view_requests_to_archival_node; +mod simple_test_loop_example; +mod state_sync; +mod syncing; +mod view_requests_to_archival_node; diff --git a/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs b/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs index 7bb3bb42e3d..051e732fbf5 100644 --- a/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs +++ b/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs @@ -2,7 +2,7 @@ use crate::tests::client::process_blocks::produce_blocks_from_height; use assert_matches::assert_matches; use near_async::messaging::CanSend; use near_chain::orphan::NUM_ORPHAN_ANCESTORS_CHECK; -use near_chain::{Error, Provenance}; +use near_chain::{ChainStoreAccess as _, Error, Provenance}; use near_chain_configs::{Genesis, NEAR_BASE}; use near_chunks::metrics::PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER; use near_client::test_utils::{create_chunk_with_transactions, TestEnv}; @@ -260,7 +260,22 @@ fn test_chunk_transaction_validity() { .persist_and_distribute_encoded_chunk(chunk, merkle_paths, receipts, validator_id) .unwrap(); let res = env.clients[0].process_block_test(block.into(), Provenance::NONE); - assert_matches!(res.unwrap_err(), Error::InvalidTransactions); + match res.as_deref() { + Ok([h]) => { + let Ok(block) = env.clients[0].chain.get_block(&h) else { + panic!("did not find block from result!"); + }; + for chunk_hdr in block.chunks().iter_raw() { + let hash = chunk_hdr.chunk_hash(); + let Ok(chunk) = env.clients[0].chain.mut_chain_store().get_chunk(&hash) else { + continue; + }; + assert_eq!(chunk.prev_outgoing_receipts().len(), 0); + } + } + Err(Error::InvalidTransactions) => (), + e => panic!("unexpected result {e:?}"), + } } #[test] diff --git a/integration-tests/src/tests/client/features/adversarial_behaviors.rs b/integration-tests/src/tests/client/features/adversarial_behaviors.rs index 490d5238196..104c8e77eea 100644 --- a/integration-tests/src/tests/client/features/adversarial_behaviors.rs +++ b/integration-tests/src/tests/client/features/adversarial_behaviors.rs @@ -379,6 +379,7 @@ fn test_banning_chunk_producer_when_seeing_invalid_chunk() { #[test] #[cfg(feature = "test_features")] +#[cfg_attr(feature = "protocol_feature_relaxed_chunk_validation", ignore)] fn test_banning_chunk_producer_when_seeing_invalid_tx_in_chunk() { init_test_logger(); let mut test = AdversarialBehaviorTestData::new(); diff --git a/integration-tests/src/tests/client/features/stateless_validation.rs b/integration-tests/src/tests/client/features/stateless_validation.rs index 7b67f79bcdd..9f46489ed91 100644 --- a/integration-tests/src/tests/client/features/stateless_validation.rs +++ b/integration-tests/src/tests/client/features/stateless_validation.rs @@ -2,37 +2,36 @@ use crate::tests::client::features::wallet_contract::{ create_rlp_execute_tx, view_balance, NearSigner, }; use assert_matches::assert_matches; -use near_client::{ProcessTxResponse, ProduceChunkResult}; +use near_chain::Provenance; +use near_chain_configs::{Genesis, GenesisConfig, GenesisRecords}; +use near_client::test_utils::TestEnv; +use near_client::ProcessTxResponse; +use near_crypto::{InMemorySigner, KeyType, SecretKey}; use near_epoch_manager::{EpochManager, EpochManagerAdapter}; +use near_o11y::testonly::init_integration_logger; use near_primitives::account::id::AccountIdRef; use near_primitives::account::{AccessKeyPermission, FunctionCallPermission}; use near_primitives::action::{Action, AddKeyAction, TransferAction}; -use near_primitives::stateless_validation::state_witness::ChunkStateWitness; -use near_primitives::version::ProtocolFeature; -use near_store::test_utils::create_test_store; -use rand::rngs::StdRng; -use rand::{Rng, SeedableRng}; -use std::collections::HashSet; - -use near_chain::{Chain, Provenance}; -use near_chain_configs::{Genesis, GenesisConfig, GenesisRecords}; -use near_client::test_utils::{create_chunk_with_transactions, TestEnv}; -use near_crypto::{InMemorySigner, KeyType, SecretKey}; -use near_o11y::testonly::init_integration_logger; use near_primitives::epoch_manager::AllEpochConfigTestOverrides; use near_primitives::num_rational::Rational32; use near_primitives::shard_layout::ShardLayout; use near_primitives::state_record::StateRecord; +use near_primitives::stateless_validation::state_witness::ChunkStateWitness; use near_primitives::test_utils::{create_test_signer, create_user_test_signer}; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{AccountInfo, EpochId, ShardId}; use near_primitives::utils::derive_eth_implicit_account_id; +use near_primitives::version::ProtocolFeature; use near_primitives::version::{ProtocolVersion, PROTOCOL_VERSION}; use near_primitives::views::FinalExecutionStatus; use near_primitives_core::account::{AccessKey, Account}; use near_primitives_core::hash::CryptoHash; use near_primitives_core::types::{AccountId, NumSeats}; +use near_store::test_utils::create_test_store; use nearcore::test_utils::TestEnvNightshadeSetupExt; +use rand::rngs::StdRng; +use rand::{Rng, SeedableRng}; +use std::collections::HashSet; const ONE_NEAR: u128 = 1_000_000_000_000_000_000_000_000; @@ -392,138 +391,6 @@ fn test_chunk_state_witness_bad_shard_id() { assert_matches!(error, near_chain::Error::InvalidShardId(_)); } -/// Test that processing chunks with invalid transactions does not lead to panics -#[test] -fn test_invalid_transactions() { - let accounts = - vec!["test0".parse().unwrap(), "test1".parse().unwrap(), "test2".parse().unwrap()]; - let signers: Vec<_> = accounts - .iter() - .map(|account_id: &AccountId| { - create_user_test_signer(AccountIdRef::new(account_id.as_str()).unwrap()) - }) - .collect(); - let genesis = Genesis::test(accounts.clone(), 2); - let mut env = TestEnv::builder(&genesis.config) - .validators(accounts.clone()) - .clients(accounts.clone()) - .nightshade_runtimes(&genesis) - .build(); - let new_signer = create_user_test_signer(AccountIdRef::new("test3").unwrap()); - - let tip = env.clients[0].chain.head().unwrap(); - let sender_account = accounts[0].clone(); - let receiver_account = accounts[1].clone(); - let invalid_transactions = vec![ - // transaction with invalid balance - SignedTransaction::send_money( - 1, - sender_account.clone(), - receiver_account.clone(), - &signers[0].clone().into(), - u128::MAX, - tip.last_block_hash, - ), - // transaction with invalid nonce - SignedTransaction::send_money( - 0, - sender_account.clone(), - receiver_account.clone(), - &signers[0].clone().into(), - ONE_NEAR, - tip.last_block_hash, - ), - // transaction with invalid sender account - SignedTransaction::send_money( - 2, - "test3".parse().unwrap(), - receiver_account.clone(), - &new_signer.into(), - ONE_NEAR, - tip.last_block_hash, - ), - ]; - // Need to create a valid transaction with the same accounts touched in order to have some state witness generated - let valid_tx = SignedTransaction::send_money( - 1, - sender_account, - receiver_account, - &signers[0].clone().into(), - ONE_NEAR, - tip.last_block_hash, - ); - let mut start_height = 1; - for tx in invalid_transactions { - for height in start_height..start_height + 3 { - let tip = env.clients[0].chain.head().unwrap(); - let chunk_producer = env.get_chunk_producer_at_offset(&tip, 1, ShardId::new(0)); - let block_producer = env.get_block_producer_at_offset(&tip, 1); - - let client = env.client(&chunk_producer); - let transactions = if height == start_height { vec![tx.clone()] } else { vec![] }; - if height == start_height { - let res = client.process_tx(valid_tx.clone(), false, false); - assert!(matches!(res, ProcessTxResponse::ValidTx)) - } - - let ( - ProduceChunkResult { - chunk, - encoded_chunk_parts_paths, - receipts, - transactions_storage_proof, - }, - _, - ) = create_chunk_with_transactions(client, transactions); - - let shard_chunk = client - .persist_and_distribute_encoded_chunk( - chunk, - encoded_chunk_parts_paths, - receipts, - client.validator_signer.get().unwrap().validator_id().clone(), - ) - .unwrap(); - let prev_block = client.chain.get_block(shard_chunk.prev_block()).unwrap(); - let prev_chunk_header = Chain::get_prev_chunk_header( - client.epoch_manager.as_ref(), - &prev_block, - shard_chunk.shard_id(), - ) - .unwrap(); - client - .send_chunk_state_witness_to_chunk_validators( - &client - .epoch_manager - .get_epoch_id_from_prev_block(shard_chunk.prev_block()) - .unwrap(), - prev_block.header(), - &prev_chunk_header, - &shard_chunk, - transactions_storage_proof, - &client.validator_signer.get(), - ) - .unwrap(); - - env.process_partial_encoded_chunks(); - for i in 0..env.clients.len() { - env.process_shards_manager_responses(i); - } - env.propagate_chunk_state_witnesses_and_endorsements(true); - let block = env.client(&block_producer).produce_block(height).unwrap().unwrap(); - for client in env.clients.iter_mut() { - client - .process_block_test_no_produce_chunk_allow_errors( - block.clone().into(), - Provenance::NONE, - ) - .unwrap(); - } - } - start_height += 3; - } -} - /// Tests that eth-implicit accounts still work with stateless validation. #[test] fn test_eth_implicit_accounts() { diff --git a/integration-tests/src/tests/client/invalid_txs.rs b/integration-tests/src/tests/client/invalid_txs.rs new file mode 100644 index 00000000000..8193b58adc9 --- /dev/null +++ b/integration-tests/src/tests/client/invalid_txs.rs @@ -0,0 +1,291 @@ +use near_chain::{Chain, Provenance}; +use near_chain_configs::Genesis; +use near_client::test_utils::{create_chunk_with_transactions, TestEnv}; +use near_client::{ProcessTxResponse, ProduceChunkResult}; +use near_primitives::account::id::AccountIdRef; +use near_primitives::test_utils::create_user_test_signer; +use near_primitives::transaction::SignedTransaction; +use near_primitives::types::{AccountId, ShardId}; +use nearcore::test_utils::TestEnvNightshadeSetupExt; + +const ONE_NEAR: u128 = 1_000_000_000_000_000_000_000_000; + +/// Test that processing chunks with invalid transactions does not lead to panics +#[test] +fn test_invalid_transactions_no_panic() { + let accounts = + vec!["test0".parse().unwrap(), "test1".parse().unwrap(), "test2".parse().unwrap()]; + let signers: Vec<_> = accounts + .iter() + .map(|account_id: &AccountId| { + create_user_test_signer(AccountIdRef::new(account_id.as_str()).unwrap()) + }) + .collect(); + let genesis = Genesis::test(accounts.clone(), 2); + let mut env = TestEnv::builder(&genesis.config) + .validators(accounts.clone()) + .clients(accounts.clone()) + .nightshade_runtimes(&genesis) + .build(); + let new_signer = create_user_test_signer(AccountIdRef::new("test3").unwrap()); + + let tip = env.clients[0].chain.head().unwrap(); + let sender_account = accounts[0].clone(); + let receiver_account = accounts[1].clone(); + let invalid_transactions = vec![ + // transaction with invalid balance + SignedTransaction::send_money( + 1, + sender_account.clone(), + receiver_account.clone(), + &signers[0].clone().into(), + u128::MAX, + tip.last_block_hash, + ), + // transaction with invalid nonce + SignedTransaction::send_money( + 0, + sender_account.clone(), + receiver_account.clone(), + &signers[0].clone().into(), + ONE_NEAR, + tip.last_block_hash, + ), + // transaction with invalid sender account + SignedTransaction::send_money( + 2, + "test3".parse().unwrap(), + receiver_account.clone(), + &new_signer.into(), + ONE_NEAR, + tip.last_block_hash, + ), + ]; + // Need to create a valid transaction with the same accounts touched in order to have some state witness generated + let valid_tx = SignedTransaction::send_money( + 1, + sender_account, + receiver_account, + &signers[0].clone().into(), + ONE_NEAR, + tip.last_block_hash, + ); + let mut start_height = 1; + for tx in invalid_transactions { + for height in start_height..start_height + 3 { + let tip = env.clients[0].chain.head().unwrap(); + let chunk_producer = env.get_chunk_producer_at_offset(&tip, 1, ShardId::new(0)); + let block_producer = env.get_block_producer_at_offset(&tip, 1); + + let client = env.client(&chunk_producer); + let transactions = if height == start_height { vec![tx.clone()] } else { vec![] }; + if height == start_height { + let res = client.process_tx(valid_tx.clone(), false, false); + assert!(matches!(res, ProcessTxResponse::ValidTx)) + } + + let ( + ProduceChunkResult { + chunk, + encoded_chunk_parts_paths, + receipts, + transactions_storage_proof, + }, + _, + ) = create_chunk_with_transactions(client, transactions); + + let shard_chunk = client + .persist_and_distribute_encoded_chunk( + chunk, + encoded_chunk_parts_paths, + receipts, + client.validator_signer.get().unwrap().validator_id().clone(), + ) + .unwrap(); + let prev_block = client.chain.get_block(shard_chunk.prev_block()).unwrap(); + let prev_chunk_header = Chain::get_prev_chunk_header( + client.epoch_manager.as_ref(), + &prev_block, + shard_chunk.shard_id(), + ) + .unwrap(); + client + .send_chunk_state_witness_to_chunk_validators( + &client + .epoch_manager + .get_epoch_id_from_prev_block(shard_chunk.prev_block()) + .unwrap(), + prev_block.header(), + &prev_chunk_header, + &shard_chunk, + transactions_storage_proof, + &client.validator_signer.get(), + ) + .unwrap(); + + env.process_partial_encoded_chunks(); + for i in 0..env.clients.len() { + env.process_shards_manager_responses(i); + } + env.propagate_chunk_state_witnesses_and_endorsements(true); + let block = env.client(&block_producer).produce_block(height).unwrap().unwrap(); + for client in env.clients.iter_mut() { + client + .process_block_test_no_produce_chunk_allow_errors( + block.clone().into(), + Provenance::NONE, + ) + .unwrap(); + } + } + start_height += 3; + } +} + +/// Test that processing a chunk with invalid transactions within it does not invalidate the entire +/// chunk and discards just the invalid transactions within. +/// +/// Tests the `RelaxedChunkValidation` feature. +#[test] +#[cfg(feature = "nightly")] +fn test_invalid_transactions_dont_invalidate_chunk() { + use near_chain::ChainStoreAccess as _; + near_o11y::testonly::init_test_logger(); + let accounts = + vec!["test0".parse().unwrap(), "test1".parse().unwrap(), "test2".parse().unwrap()]; + let signers: Vec<_> = accounts + .iter() + .map(|account_id: &AccountId| { + create_user_test_signer(AccountIdRef::new(account_id.as_str()).unwrap()) + }) + .collect(); + let genesis = Genesis::test(accounts.clone(), 2); + let mut env = TestEnv::builder(&genesis.config) + .validators(accounts.clone()) + .clients(accounts.clone()) + .nightshade_runtimes(&genesis) + .build(); + let new_signer = create_user_test_signer(AccountIdRef::new("test3").unwrap()); + + let tip = env.clients[0].chain.head().unwrap(); + let sender_account = accounts[0].clone(); + let receiver_account = accounts[1].clone(); + let chunk_transactions = vec![ + SignedTransaction::send_money( + 1, + sender_account.clone(), + receiver_account.clone(), + &signers[0].clone().into(), + ONE_NEAR, + tip.last_block_hash, + ), + // transaction with invalid balance + SignedTransaction::send_money( + 2, + sender_account.clone(), + receiver_account.clone(), + &signers[0].clone().into(), + u128::MAX, + tip.last_block_hash, + ), + // transaction with invalid nonce + SignedTransaction::send_money( + 0, + sender_account, + receiver_account.clone(), + &signers[0].clone().into(), + ONE_NEAR, + tip.last_block_hash, + ), + // transaction with invalid sender account + SignedTransaction::send_money( + 3, + "test3".parse().unwrap(), + receiver_account, + &new_signer.into(), + ONE_NEAR, + tip.last_block_hash, + ), + ]; + + let tip = env.clients[0].chain.head().unwrap(); + let chunk_producer = env.get_chunk_producer_at_offset(&tip, 1, ShardId::new(0)); + let block_producer = env.get_block_producer_at_offset(&tip, 1); + let client = env.client(&chunk_producer); + let ( + ProduceChunkResult { + chunk, + encoded_chunk_parts_paths, + receipts, + transactions_storage_proof, + }, + _, + ) = create_chunk_with_transactions(client, chunk_transactions); + + let shard_chunk = client + .persist_and_distribute_encoded_chunk( + chunk, + encoded_chunk_parts_paths, + receipts, + client.validator_signer.get().unwrap().validator_id().clone(), + ) + .unwrap(); + + let prev_block = client.chain.get_block(shard_chunk.prev_block()).unwrap(); + let prev_chunk_header = Chain::get_prev_chunk_header( + client.epoch_manager.as_ref(), + &prev_block, + shard_chunk.shard_id(), + ) + .unwrap(); + client + .send_chunk_state_witness_to_chunk_validators( + &client.epoch_manager.get_epoch_id_from_prev_block(shard_chunk.prev_block()).unwrap(), + prev_block.header(), + &prev_chunk_header, + &shard_chunk, + transactions_storage_proof, + &client.validator_signer.get(), + ) + .unwrap(); + + env.process_partial_encoded_chunks(); + for i in 0..env.clients.len() { + env.process_shards_manager_responses(i); + } + env.propagate_chunk_state_witnesses_and_endorsements(true); + let block = env.client(&block_producer).produce_block(1).unwrap().unwrap(); + for client in env.clients.iter_mut() { + let signer = client.validator_signer.get(); + client.start_process_block(block.clone().into(), Provenance::NONE, None, &signer).unwrap(); + near_chain::test_utils::wait_for_all_blocks_in_processing(&mut client.chain); + let (accepted_blocks, _errors) = client.postprocess_ready_blocks(None, true, &signer); + assert_eq!(accepted_blocks.len(), 1); + } + + env.process_partial_encoded_chunks(); + for i in 0..env.clients.len() { + env.process_shards_manager_responses(i); + } + env.propagate_chunk_state_witnesses_and_endorsements(true); + let block = env.client(&block_producer).produce_block(2).unwrap().unwrap(); + for client in env.clients.iter_mut() { + let signer = client.validator_signer.get(); + client.start_process_block(block.clone().into(), Provenance::NONE, None, &signer).unwrap(); + near_chain::test_utils::wait_for_all_blocks_in_processing(&mut client.chain); + let (accepted_blocks, _errors) = client.postprocess_ready_blocks(None, true, &signer); + assert_eq!(accepted_blocks.len(), 1); + } + env.propagate_chunk_state_witnesses_and_endorsements(true); + + let mut receipts = std::collections::BTreeSet::::new(); + for client in env.clients.iter_mut() { + let head = client.chain.get_head_block().unwrap(); + let chunk_hash = head.chunks().iter_raw().next().unwrap().chunk_hash(); + let Ok(chunk) = client.chain.mut_chain_store().get_chunk(&chunk_hash) else { + continue; + }; + receipts.extend(chunk.prev_outgoing_receipts().into_iter().map(|r| *r.receipt_id())); + } + assert_eq!(receipts.len(), 1, "only one receipt for the only valid transaction is expected"); +} diff --git a/integration-tests/src/tests/client/mod.rs b/integration-tests/src/tests/client/mod.rs index 9889f8afde7..10cf7816e78 100644 --- a/integration-tests/src/tests/client/mod.rs +++ b/integration-tests/src/tests/client/mod.rs @@ -5,6 +5,7 @@ mod chunks_management; mod cold_storage; mod features; mod flat_storage; +mod invalid_txs; mod process_blocks; mod resharding_v2; mod runtimes; diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index 7cc6e18b705..9615e7817d8 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -25,7 +25,7 @@ use near_store::trie::TrieNodesCount; use std::sync::Arc; use crate::node::Node; -use crate::user::User; +use crate::user::{CommitError, User}; use near_parameters::{RuntimeConfig, RuntimeConfigStore}; use near_primitives::receipt::{ActionReceipt, Receipt, ReceiptEnum, ReceiptV0}; use near_primitives::test_utils; @@ -841,13 +841,13 @@ pub fn test_delete_key_last(node: impl Node) { // forget the nonce when we delete a key! assert_eq!( err, - ServerError::TxExecutionError(TxExecutionError::InvalidTxError( - InvalidTxError::InvalidAccessKeyError( + CommitError::Server(ServerError::TxExecutionError( + TxExecutionError::InvalidTxError(InvalidTxError::InvalidAccessKeyError( InvalidAccessKeyError::AccessKeyNotFound { account_id: account_id.clone(), public_key: node.signer().public_key().into(), }, - ) + )) )) ) } @@ -1058,14 +1058,18 @@ pub fn test_access_key_smart_contract_reject_method_name(node: impl Node) { let transaction_result = node_user .function_call(account_id.clone(), bob_account(), "run_test", vec![], 10u64.pow(14), 0) .unwrap_err(); - assert_eq!( - transaction_result, - ServerError::TxExecutionError(TxExecutionError::InvalidTxError( - InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::MethodNameMismatch { - method_name: "run_test".to_string() - }) - )) - ); + if cfg!(feature = "protocol_feature_relaxed_chunk_validation") { + assert_eq!(transaction_result, CommitError::OutcomeNotFound); + } else { + assert_eq!( + transaction_result, + CommitError::Server(ServerError::TxExecutionError(TxExecutionError::InvalidTxError( + InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::MethodNameMismatch { + method_name: "run_test".to_string() + }) + ))) + ); + } } pub fn test_access_key_smart_contract_reject_contract_id(node: impl Node) { @@ -1093,15 +1097,19 @@ pub fn test_access_key_smart_contract_reject_contract_id(node: impl Node) { 0, ) .unwrap_err(); - assert_eq!( - transaction_result, - ServerError::TxExecutionError(TxExecutionError::InvalidTxError( - InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::ReceiverMismatch { - tx_receiver: eve_dot_alice_account(), - ak_receiver: bob_account().into() - }) - )) - ); + if cfg!(feature = "protocol_feature_relaxed_chunk_validation") { + assert_eq!(transaction_result, CommitError::OutcomeNotFound); + } else { + assert_eq!( + transaction_result, + CommitError::Server(ServerError::TxExecutionError(TxExecutionError::InvalidTxError( + InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::ReceiverMismatch { + tx_receiver: eve_dot_alice_account(), + ak_receiver: bob_account().into() + }) + ))) + ); + } } pub fn test_access_key_reject_non_function_call(node: impl Node) { @@ -1121,12 +1129,16 @@ pub fn test_access_key_reject_non_function_call(node: impl Node) { let transaction_result = node_user.delete_key(account_id.clone(), node.signer().public_key()).unwrap_err(); - assert_eq!( - transaction_result, - ServerError::TxExecutionError(TxExecutionError::InvalidTxError( - InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::RequiresFullAccess) - )) - ); + if cfg!(feature = "protocol_feature_relaxed_chunk_validation") { + assert_eq!(transaction_result, CommitError::OutcomeNotFound); + } else { + assert_eq!( + transaction_result, + CommitError::Server(ServerError::TxExecutionError(TxExecutionError::InvalidTxError( + InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::RequiresFullAccess) + ))) + ); + } } pub fn test_increase_stake(node: impl Node) { diff --git a/integration-tests/src/tests/test_errors.rs b/integration-tests/src/tests/test_errors.rs index c3dfda51e63..443392ecb4d 100644 --- a/integration-tests/src/tests/test_errors.rs +++ b/integration-tests/src/tests/test_errors.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use crate::node::{Node, ThreadNode}; +use crate::user::CommitError; use near_chain_configs::test_utils::{TESTING_INIT_BALANCE, TESTING_INIT_STAKE}; use near_chain_configs::Genesis; use near_crypto::{InMemorySigner, KeyType}; @@ -55,11 +56,13 @@ fn test_check_tx_error_log() { let tx_result = node.user().commit_transaction(tx).unwrap_err(); assert_eq!( tx_result, - InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::AccessKeyNotFound { - account_id: bob_account(), - public_key: Box::new(signer.public_key()), - }) - .rpc_into() + CommitError::Server( + InvalidTxError::InvalidAccessKeyError(InvalidAccessKeyError::AccessKeyNotFound { + account_id: bob_account(), + public_key: Box::new(signer.public_key()), + }) + .rpc_into() + ) ); } @@ -96,11 +99,13 @@ fn test_deliver_tx_error_log() { let tx_result = node.user().commit_transaction(tx).unwrap_err(); assert_eq!( tx_result, - InvalidTxError::NotEnoughBalance { - signer_id: alice_account(), - balance: TESTING_INIT_BALANCE - TESTING_INIT_STAKE, - cost: TESTING_INIT_BALANCE + 1 + cost - } - .rpc_into() + CommitError::Server( + InvalidTxError::NotEnoughBalance { + signer_id: alice_account(), + balance: TESTING_INIT_BALANCE - TESTING_INIT_STAKE, + cost: TESTING_INIT_BALANCE + 1 + cost + } + .rpc_into() + ) ); } diff --git a/integration-tests/src/user/mod.rs b/integration-tests/src/user/mod.rs index e0b65ca549d..fd5f75fec36 100644 --- a/integration-tests/src/user/mod.rs +++ b/integration-tests/src/user/mod.rs @@ -27,6 +27,27 @@ pub mod runtime_user; const POISONED_LOCK_ERR: &str = "The lock was poisoned."; +#[derive(Debug, PartialEq, Eq)] +pub enum CommitError { + Server(ServerError), + OutcomeNotFound, +} + +impl std::error::Error for CommitError {} +impl std::fmt::Display for CommitError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + CommitError::Server(s) => f.write_fmt(format_args!( + "server error occurred while committing a transaction: {}", + s + )), + CommitError::OutcomeNotFound => { + f.write_str("transaction outcome not found while committing it (tx invalid...)") + } + } + } +} + pub trait User { fn view_account(&self, account_id: &AccountId) -> Result; @@ -53,7 +74,7 @@ pub trait User { fn commit_transaction( &self, signed_transaction: SignedTransaction, - ) -> Result; + ) -> Result; fn add_receipts( &self, @@ -78,7 +99,7 @@ pub trait User { fn get_transaction_result(&self, hash: &CryptoHash) -> Option; - fn get_transaction_final_result(&self, hash: &CryptoHash) -> FinalExecutionOutcomeView; + fn get_transaction_final_result(&self, hash: &CryptoHash) -> Option; fn get_state_root(&self) -> CryptoHash; @@ -97,7 +118,7 @@ pub trait User { signer_id: AccountId, receiver_id: AccountId, actions: Vec, - ) -> Result { + ) -> Result { let block_hash = self.get_best_block_hash().unwrap_or_default(); let signed_transaction = SignedTransaction::from_actions( self.get_access_key_nonce_for_signer(&signer_id).unwrap_or_default() + 1, @@ -116,7 +137,7 @@ pub trait User { signer_id: AccountId, receiver_id: AccountId, amount: Balance, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id, receiver_id, @@ -128,7 +149,7 @@ pub trait User { &self, signer_id: AccountId, code: Vec, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id.clone(), signer_id, @@ -144,7 +165,7 @@ pub trait User { args: Vec, gas: Gas, deposit: Balance, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id, contract_id, @@ -163,7 +184,7 @@ pub trait User { new_account_id: AccountId, public_key: PublicKey, amount: Balance, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id, new_account_id, @@ -183,7 +204,7 @@ pub trait User { signer_id: AccountId, public_key: PublicKey, access_key: AccessKey, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id.clone(), signer_id, @@ -195,7 +216,7 @@ pub trait User { &self, signer_id: AccountId, public_key: PublicKey, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id.clone(), signer_id, @@ -209,7 +230,7 @@ pub trait User { old_public_key: PublicKey, new_public_key: PublicKey, access_key: AccessKey, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id.clone(), signer_id, @@ -225,7 +246,7 @@ pub trait User { signer_id: AccountId, receiver_id: AccountId, beneficiary_id: AccountId, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id, receiver_id, @@ -237,7 +258,7 @@ pub trait User { &self, signer_id: AccountId, receiver_id: AccountId, - ) -> Result { + ) -> Result { self.delete_account_with_beneficiary_set(signer_id.clone(), receiver_id, signer_id) } @@ -246,7 +267,7 @@ pub trait User { signer_id: AccountId, public_key: PublicKey, stake: Balance, - ) -> Result { + ) -> Result { self.sign_and_commit_actions( signer_id.clone(), signer_id, @@ -264,7 +285,7 @@ pub trait User { receiver_id: AccountId, relayer_id: AccountId, actions: Vec, - ) -> Result { + ) -> Result { let inner_signer = create_user_test_signer(&signer_id); let user_nonce = self .get_access_key(&signer_id, &inner_signer.public_key) diff --git a/integration-tests/src/user/rpc_user.rs b/integration-tests/src/user/rpc_user.rs index 6b0a445976c..58f5f90dfaa 100644 --- a/integration-tests/src/user/rpc_user.rs +++ b/integration-tests/src/user/rpc_user.rs @@ -26,6 +26,8 @@ use near_primitives::views::{ use crate::user::User; +use super::CommitError; + pub struct RpcUser { account_id: AccountId, signer: Arc, @@ -136,7 +138,7 @@ impl User for RpcUser { fn commit_transaction( &self, transaction: SignedTransaction, - ) -> Result { + ) -> Result { let bytes = borsh::to_vec(&transaction).unwrap(); let result = self.actix(move |client| client.broadcast_tx_commit(to_base64(&bytes))); // Wait for one more block, to make sure all nodes actually apply the state transition. @@ -146,7 +148,9 @@ impl User for RpcUser { } match result { Ok(outcome) => Ok(outcome.final_execution_outcome.unwrap().into_outcome()), - Err(err) => Err(serde_json::from_value::(err.data.unwrap()).unwrap()), + Err(err) => Err(CommitError::Server( + serde_json::from_value::(err.data.unwrap()).unwrap(), + )), } } @@ -188,7 +192,7 @@ impl User for RpcUser { unimplemented!() } - fn get_transaction_final_result(&self, hash: &CryptoHash) -> FinalExecutionOutcomeView { + fn get_transaction_final_result(&self, hash: &CryptoHash) -> Option { let request = RpcTransactionStatusRequest { transaction_info: TransactionInfo::TransactionId { tx_hash: *hash, @@ -199,8 +203,7 @@ impl User for RpcUser { self.actix(move |client| client.tx(request)) .unwrap() .final_execution_outcome - .unwrap() - .into_outcome() + .map(|o| o.into_outcome()) } fn get_state_root(&self) -> CryptoHash { diff --git a/integration-tests/src/user/runtime_user.rs b/integration-tests/src/user/runtime_user.rs index 97c86fba0c6..a7c65c07d36 100644 --- a/integration-tests/src/user/runtime_user.rs +++ b/integration-tests/src/user/runtime_user.rs @@ -215,43 +215,48 @@ impl RuntimeUser { } // TODO(#10942) get rid of copy pasted code, it's outdated comparing to the original - fn get_final_transaction_result(&self, hash: &CryptoHash) -> FinalExecutionOutcomeView { + fn get_final_transaction_result(&self, hash: &CryptoHash) -> Option { let mut outcomes = self.get_recursive_transaction_results(hash); let mut looking_for_id = *hash; let num_outcomes = outcomes.len(); - let status = outcomes - .iter() - .find_map(|outcome_with_id| { - if outcome_with_id.id == looking_for_id { - match &outcome_with_id.outcome.status { - ExecutionStatusView::Unknown if num_outcomes == 1 => { - Some(FinalExecutionStatus::NotStarted) - } - ExecutionStatusView::Unknown => Some(FinalExecutionStatus::Started), - ExecutionStatusView::Failure(e) => { - Some(FinalExecutionStatus::Failure(e.clone())) - } - ExecutionStatusView::SuccessValue(v) => { - Some(FinalExecutionStatus::SuccessValue(v.clone())) - } - ExecutionStatusView::SuccessReceiptId(id) => { - looking_for_id = *id; - None - } + let status = outcomes.iter().find_map(|outcome_with_id| { + if outcome_with_id.id == looking_for_id { + match &outcome_with_id.outcome.status { + ExecutionStatusView::Unknown if num_outcomes == 1 => { + Some(FinalExecutionStatus::NotStarted) + } + ExecutionStatusView::Unknown => Some(FinalExecutionStatus::Started), + ExecutionStatusView::Failure(e) => { + Some(FinalExecutionStatus::Failure(e.clone())) + } + ExecutionStatusView::SuccessValue(v) => { + Some(FinalExecutionStatus::SuccessValue(v.clone())) + } + ExecutionStatusView::SuccessReceiptId(id) => { + looking_for_id = *id; + None } - } else { - None } - }) - .expect("results should resolve to a final outcome"); + } else { + None + } + }); + let status = if cfg!(feature = "protocol_feature_relaxed_chunk_validation") { + // If we don't find the transaction at all, it must have been ignored due to having + // been an invalid transaction (but due to relaxed validation we do not invalidate the + // entire chunk.) + status? + } else { + status.expect("results should resolve to a final outcome") + }; let receipts = outcomes.split_off(1); let transaction = self.transactions.borrow().get(hash).unwrap().clone().into(); - FinalExecutionOutcomeView { + Some(FinalExecutionOutcomeView { status, transaction, transaction_outcome: outcomes.pop().unwrap(), receipts_outcome: receipts, - } + }) } } @@ -307,7 +312,7 @@ impl User for RuntimeUser { block_height: apply_state.block_height, prev_block_hash: apply_state.prev_block_hash, block_hash: apply_state.block_hash, - shard_id: shard_id, + shard_id, epoch_id: apply_state.epoch_id, epoch_height: apply_state.epoch_height, block_timestamp: apply_state.block_timestamp, @@ -337,9 +342,11 @@ impl User for RuntimeUser { fn commit_transaction( &self, transaction: SignedTransaction, - ) -> Result { - self.apply_all(self.apply_state(), vec![], vec![transaction.clone()], true)?; - Ok(self.get_transaction_final_result(&transaction.get_hash())) + ) -> Result { + self.apply_all(self.apply_state(), vec![], vec![transaction.clone()], true) + .map_err(super::CommitError::Server)?; + self.get_transaction_final_result(&transaction.get_hash()) + .ok_or(super::CommitError::OutcomeNotFound) } fn add_receipts( @@ -376,7 +383,7 @@ impl User for RuntimeUser { self.transaction_results.borrow().get(hash).cloned() } - fn get_transaction_final_result(&self, hash: &CryptoHash) -> FinalExecutionOutcomeView { + fn get_transaction_final_result(&self, hash: &CryptoHash) -> Option { self.get_final_transaction_result(hash) } diff --git a/runtime/runtime/Cargo.toml b/runtime/runtime/Cargo.toml index 528c5641af0..4f1f5bc7e28 100644 --- a/runtime/runtime/Cargo.toml +++ b/runtime/runtime/Cargo.toml @@ -44,6 +44,7 @@ nightly = [ "near-wallet-contract/nightly", "nightly_protocol", "protocol_feature_nonrefundable_transfer_nep491", + "protocol_feature_relaxed_chunk_validation", "testlib/nightly", ] nightly_protocol = [ @@ -58,6 +59,10 @@ nightly_protocol = [ "testlib/nightly_protocol", ] protocol_feature_nonrefundable_transfer_nep491 = [] +protocol_feature_relaxed_chunk_validation = [ + "near-primitives/protocol_feature_relaxed_chunk_validation", + "near-primitives-core/protocol_feature_relaxed_chunk_validation", +] no_cpu_compatibility_checks = ["near-vm-runner/no_cpu_compatibility_checks"] no_cache = [ diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 21219bd26ba..348eb0a05e6 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -289,13 +289,17 @@ impl Runtime { debug!(target: "runtime", "{}", log_str); } - /// Takes one signed transaction, verifies it and converts it to a receipt. Add this receipt - /// either to the new local receipts if the signer is the same as receiver or to the new - /// outgoing receipts. + /// Takes one signed transaction, verifies it and converts it to a receipt. + /// + /// Add the produced receipt receipt either to the new local receipts if the signer is the same + /// as receiver or to the new outgoing receipts. + /// /// When transaction is converted to a receipt, the account is charged for the full value of /// the generated receipt. - /// In case of successful verification (expected for valid chunks), returns the receipt and - /// `ExecutionOutcomeWithId` for the transaction. + /// + /// In case of successful verification, returns the receipt and `ExecutionOutcomeWithId` for + /// the transaction. + /// /// In case of an error, returns either `InvalidTxError` if the transaction verification failed /// or a `StorageError` wrapped into `RuntimeError`. #[instrument(target = "runtime", level = "debug", "process_transaction", skip_all, fields( @@ -1388,13 +1392,17 @@ impl Runtime { /// Applies new signed transactions and incoming receipts for some chunk/shard on top of /// given trie and the given state root. + /// /// If the validator accounts update is provided, updates validators accounts. - /// All new signed transactions should be valid and already verified by the chunk producer. - /// If any transaction is invalid, it would return an `InvalidTxError`. + /// /// Returns an `ApplyResult` that contains the new state root, trie changes, - /// new outgoing receipts, execution outcomes for - /// all transactions, local action receipts (generated from transactions with signer == - /// receivers) and incoming action receipts. + /// new outgoing receipts, execution outcomes for all transactions, local action receipts + /// (generated from transactions with signer == receivers) and incoming action receipts. + /// + /// Invalid transactions should have been filtered out by the chunk producer, but if a chunk + /// containing invalid transactions does make it to here, these transactions are skipped. This + /// does pollute the chain with junk data, but it also allows the protocol to make progress, as + /// the only alternative way to handle these transactions is to make the entire chunk invalid. #[instrument(target = "runtime", level = "debug", "apply", skip_all, fields( protocol_version = apply_state.current_protocol_version, num_transactions = transactions.len(), @@ -1556,6 +1564,9 @@ impl Runtime { /// /// Fills the `processing_state` with local receipts generated during processing of the /// transactions. + /// + /// Any transactions that fail to validate (e.g. invalid nonces, unknown signing keys, + /// insufficient NEAR balance, etc.) will be skipped, producing no receipts. fn process_transactions<'a>( &self, processing_state: &mut ApplyProcessingReceiptState<'a>, @@ -1565,12 +1576,32 @@ impl Runtime { let apply_state = &mut processing_state.apply_state; let state_update = &mut processing_state.state_update; for signed_transaction in processing_state.transactions { - let (receipt, outcome_with_id) = self.process_transaction( + let tx_result = self.process_transaction( state_update, apply_state, signed_transaction, &mut processing_state.stats, - )?; + ); + let (receipt, outcome_with_id) = match tx_result { + Ok(v) => v, + Err(e) => { + if checked_feature!( + "protocol_feature_relaxed_chunk_validation", + RelaxedChunkValidation, + processing_state.protocol_version + ) { + // NB: number of invalid transactions are noted in metrics. + tracing::debug!( + target: "runtime", + message="invalid transaction ignored", + tx_hash=%signed_transaction.get_hash() + ); + continue; + } else { + return Err(e.into()); + } + } + }; if receipt.receiver_id() == signed_transaction.transaction.signer_id() { processing_state.local_receipts.push_back(receipt); } else {