From 1a9dcd6244f8b1386d042d5d99536f21aadea200 Mon Sep 17 00:00:00 2001 From: Longarithm Date: Fri, 20 Dec 2024 23:13:45 +0700 Subject: [PATCH 1/3] fix --- core/store/src/trie/mod.rs | 2 +- .../src/test_loop/tests/resharding_v3.rs | 114 ++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/core/store/src/trie/mod.rs b/core/store/src/trie/mod.rs index d998f5129b0..3b7553402aa 100644 --- a/core/store/src/trie/mod.rs +++ b/core/store/src/trie/mod.rs @@ -698,7 +698,7 @@ impl Trie { storage, memtries, root, - charge_gas_for_trie_node_access: flat_storage_chunk_view.is_none(), + charge_gas_for_trie_node_access: false, flat_storage_chunk_view, accounting_cache, recorder: None, diff --git a/integration-tests/src/test_loop/tests/resharding_v3.rs b/integration-tests/src/test_loop/tests/resharding_v3.rs index 2f4e2045c87..775ee440e72 100644 --- a/integration-tests/src/test_loop/tests/resharding_v3.rs +++ b/integration-tests/src/test_loop/tests/resharding_v3.rs @@ -5,6 +5,7 @@ use near_chain_configs::test_genesis::{TestGenesisBuilder, ValidatorsSpec}; use near_chain_configs::DEFAULT_GC_NUM_EPOCHS_TO_KEEP; use near_client::Query; use near_o11y::testonly::init_test_logger; +use near_primitives::action::{Action, FunctionCallAction}; use near_primitives::epoch_manager::EpochConfigStore; use near_primitives::shard_layout::ShardLayout; use near_primitives::types::{ @@ -251,6 +252,8 @@ fn fork_before_resharding_block(double_signing: bool) -> LoopActionFn { ) } +/// Returns a loop action that sends money transfers to random accounts at +/// every block height. fn execute_money_transfers(account_ids: Vec) -> LoopActionFn { const NUM_TRANSFERS_PER_BLOCK: usize = 20; @@ -305,6 +308,90 @@ fn execute_money_transfers(account_ids: Vec) -> LoopActionFn { ) } +/// Returns a loop action that makes storage read and write at every block +/// height. +fn execute_storage_operations(sender_id: AccountId, receiver_id: AccountId) -> LoopActionFn { + const TX_CHECK_DEADLINE: u64 = 5; + let latest_height = Cell::new(0); + let txs = Cell::new(vec![]); + let nonce = Cell::new(102); + + Box::new( + move |node_datas: &[TestData], + test_loop_data: &mut TestLoopData, + client_account_id: AccountId| { + let client_actor = + retrieve_client_actor(node_datas, test_loop_data, &client_account_id); + let tip = client_actor.client.chain.head().unwrap(); + + // Run this action only once at every block height. + if latest_height.get() == tip.height { + return; + } + latest_height.set(tip.height); + + let mut remaining_txs = vec![]; + for (tx, tx_height) in txs.take() { + if tx_height + TX_CHECK_DEADLINE >= tip.height { + remaining_txs.push((tx, tx_height)); + continue; + } + + let tx_outcome = client_actor.client.chain.get_partial_transaction_result(&tx); + let status = tx_outcome.as_ref().map(|o| o.status.clone()); + assert_matches!(status, Ok(FinalExecutionStatus::SuccessValue(_))); + } + txs.set(remaining_txs); + + let clients = node_datas + .iter() + .map(|test_data| { + &test_loop_data.get(&test_data.client_sender.actor_handle()).client + }) + .collect_vec(); + + // Send transaction which reads a key and writes a key-value pair + // to the contract storage. + let anchor_hash = get_anchor_hash(&clients); + let gas = 20 * TGAS; + let salt = 2 * tip.height; + nonce.set(nonce.get() + 1); + let tx = SignedTransaction::from_actions( + nonce.get(), + sender_id.clone(), + receiver_id.clone(), + &create_user_test_signer(&sender_id).into(), + vec![ + Action::FunctionCall(Box::new(FunctionCallAction { + args: near_primitives::test_utils::encode(&[salt]), + method_name: "read_value".to_string(), + gas, + deposit: 0, + })), + Action::FunctionCall(Box::new(FunctionCallAction { + args: near_primitives::test_utils::encode(&[salt + 1, salt * 10]), + method_name: "write_key_value".to_string(), + gas, + deposit: 0, + })), + ], + anchor_hash, + 0, + ); + + store_and_submit_tx( + &node_datas, + &client_account_id, + &txs, + &sender_id, + &receiver_id, + tip.height, + tx, + ); + }, + ) +} + /// Returns a loop action that invokes a costly method from a contract /// `CALLS_PER_BLOCK_HEIGHT` times per block height. /// @@ -754,6 +841,15 @@ fn test_resharding_v3_base(params: TestReshardingParameters) { } trie_sanity_check.assert_state_sanity(&clients, expected_num_shards); latest_block_height.set(tip.height); + let shard_layout = + client.epoch_manager.get_epoch_config(&tip.epoch_id).unwrap().shard_layout; + println!( + "Block: {:?} {} {:?}", + tip.last_block_hash, + tip.height, + block_header.chunk_mask() + ); + println!("Shard IDs: {:?}", shard_layout.shard_ids().collect_vec()); if params.all_chunks_expected && params.chunk_ranges_to_drop.is_empty() { assert!(block_header.chunk_mask().iter().all(|chunk_bit| *chunk_bit)); } @@ -916,6 +1012,24 @@ fn test_resharding_v3_shard_shuffling_intense() { test_resharding_v3_base(params); } +/// Executes storage operations at every block height. +/// In particular, checks that storage gas costs are computed correctly during +/// resharding. Caught a bug with invalid storage costs computed during flat +/// storage resharding. +#[test] +fn test_resharding_v3_storage_operations() { + let sender_account: AccountId = "account1".parse().unwrap(); + let account_in_parent: AccountId = "account4".parse().unwrap(); + let params = TestReshardingParametersBuilder::default() + .deploy_test_contract(account_in_parent.clone()) + .add_loop_action(execute_storage_operations(sender_account, account_in_parent)) + .all_chunks_expected(true) + .delay_flat_state_resharding(2) + .epoch_length(13) + .build(); + test_resharding_v3_base(params); +} + #[test] #[cfg_attr(not(feature = "test_features"), ignore)] fn test_resharding_v3_delayed_receipts_left_child() { From ffa5d620aa27fbbe46432f653a9eb190f144cb3a Mon Sep 17 00:00:00 2001 From: wacban Date: Mon, 6 Jan 2025 17:12:37 +0100 Subject: [PATCH 2/3] fix tests --- core/store/src/trie/trie_recording.rs | 4 +++- core/store/src/trie/trie_tests.rs | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/core/store/src/trie/trie_recording.rs b/core/store/src/trie/trie_recording.rs index c38180cf715..99559fabb24 100644 --- a/core/store/src/trie/trie_recording.rs +++ b/core/store/src/trie/trie_recording.rs @@ -469,7 +469,9 @@ mod trie_recording_tests { false, ) } else { - tries.get_trie_for_shard(shard_uid, state_root) + let mut trie = tries.get_trie_for_shard(shard_uid, state_root); + trie.charge_gas_for_trie_node_access = true; + trie } } diff --git a/core/store/src/trie/trie_tests.rs b/core/store/src/trie/trie_tests.rs index 1cae1d30e96..da916ee4712 100644 --- a/core/store/src/trie/trie_tests.rs +++ b/core/store/src/trie/trie_tests.rs @@ -183,7 +183,8 @@ mod nodes_counter_tests { (create_trie_key(&[0, 1, 1]), Some(vec![1])), (create_trie_key(&[1, 0, 0]), Some(vec![2])), ]; - let trie = create_trie(&trie_items); + let mut trie = create_trie(&trie_items); + trie.charge_gas_for_trie_node_access = true; assert_eq!(get_touched_nodes_numbers(&trie, &trie_items), vec![5, 5, 4]); } @@ -197,7 +198,8 @@ mod nodes_counter_tests { (create_trie_key(&[0, 0]), Some(vec![1])), (create_trie_key(&[1, 1]), Some(vec![1])), ]; - let trie = create_trie(&trie_items); + let mut trie = create_trie(&trie_items); + trie.charge_gas_for_trie_node_access = true; assert_eq!(get_touched_nodes_numbers(&trie, &trie_items), vec![4, 4]); } } From 576d8b768e8afa02bd8bb237a2d8221b1fdc0caa Mon Sep 17 00:00:00 2001 From: wacban Date: Mon, 6 Jan 2025 19:01:49 +0100 Subject: [PATCH 3/3] fix more tests --- chain/chain/src/runtime/mod.rs | 4 ++-- core/store/src/trie/mod.rs | 11 ++++++++--- integration-tests/src/user/runtime_user.rs | 5 ++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/chain/chain/src/runtime/mod.rs b/chain/chain/src/runtime/mod.rs index 671acbdb3fa..5e331cbd0a0 100644 --- a/chain/chain/src/runtime/mod.rs +++ b/chain/chain/src/runtime/mod.rs @@ -635,7 +635,7 @@ impl RuntimeAdapter for NightshadeRuntime { // flat storage by not charging gas for trie nodes. // WARNING: should never be used in production! Consider this option only for debugging or replaying blocks. let mut trie = self.tries.get_trie_for_shard(shard_uid, storage_config.state_root); - trie.dont_charge_gas_for_trie_node_access(); + trie.set_charge_gas_for_trie_node_access(false); trie } StorageDataSource::Recorded(storage) => Trie::from_recorded_storage( @@ -862,7 +862,7 @@ impl RuntimeAdapter for NightshadeRuntime { storage_config.state_root, false, )?; - trie.dont_charge_gas_for_trie_node_access(); + trie.set_charge_gas_for_trie_node_access(false); trie } StorageDataSource::Recorded(storage) => Trie::from_recorded_storage( diff --git a/core/store/src/trie/mod.rs b/core/store/src/trie/mod.rs index 3b7553402aa..c75de1e5cf6 100644 --- a/core/store/src/trie/mod.rs +++ b/core/store/src/trie/mod.rs @@ -694,11 +694,16 @@ impl Trie { )))), None => RefCell::new(TrieAccountingCache::new(None)), }; + // Technically the charge_gas_for_trie_node_access should be set based + // on the flat storage protocol feature. When flat storage is enabled + // the trie node access should be free and the charge flag should be set + // to false. + let charge_gas_for_trie_node_access = false; Trie { storage, memtries, root, - charge_gas_for_trie_node_access: false, + charge_gas_for_trie_node_access, flat_storage_chunk_view, accounting_cache, recorder: None, @@ -711,8 +716,8 @@ impl Trie { } /// Helper to simulate gas costs as if flat storage was present. - pub fn dont_charge_gas_for_trie_node_access(&mut self) { - self.charge_gas_for_trie_node_access = false; + pub fn set_charge_gas_for_trie_node_access(&mut self, value: bool) { + self.charge_gas_for_trie_node_access = value; } /// Makes a new trie that has everything the same except that access diff --git a/integration-tests/src/user/runtime_user.rs b/integration-tests/src/user/runtime_user.rs index e94f6eb7905..3d54b20b8b1 100644 --- a/integration-tests/src/user/runtime_user.rs +++ b/integration-tests/src/user/runtime_user.rs @@ -102,7 +102,10 @@ impl RuntimeUser { false, ) } else { - client.tries.get_trie_for_shard(ShardUId::single_shard(), client.state_root) + let shard_uid = ShardUId::single_shard(); + let mut trie = client.tries.get_trie_for_shard(shard_uid, client.state_root); + trie.set_charge_gas_for_trie_node_access(true); + trie }; let apply_result = client .runtime