Address review suggestions

chain/client/src/view_client.rs

@@ -44,14 +44,14 @@ use near_primitives::syncing::{
 };
 use near_primitives::types::{
     AccountId, BlockHeight, BlockId, BlockReference, EpochId, EpochReference, Finality,
-    MaybeBlockId, ShardId, StateChangeValue, TransactionOrReceiptId,
+    MaybeBlockId, ShardId, TransactionOrReceiptId,
 };
 use near_primitives::views::validator_stake_view::ValidatorStakeView;
 use near_primitives::views::{
     BlockView, ChunkView, EpochValidatorInfo, ExecutionOutcomeWithIdView,
     FinalExecutionOutcomeView, FinalExecutionOutcomeViewEnum, FinalExecutionStatus, GasPriceView,
-    LightClientBlockView, QueryRequest, QueryResponse, ReceiptView, StateChangeWithCauseView,
-    StateChangesKindsView, StateChangesView,
+    LightClientBlockView, QueryRequest, QueryResponse, ReceiptView, StateChangesKindsView,
+    StateChangesView,
 };
 
 use crate::{
@@ -775,35 +775,17 @@ impl Handler<GetStateChangesWithCauseInBlockForTrackedShards> for ViewClientActo
         let mut state_changes_with_cause_split_by_shard_id: HashMap<ShardId, StateChangesView> =
             HashMap::new();
         for state_change_with_cause in state_changes_with_cause_in_block {
-            let account_id = match &state_change_with_cause.value {
-                StateChangeValue::AccountUpdate { account_id, .. } => account_id.clone(),
-                StateChangeValue::AccountDeletion { account_id } => account_id.clone(),
-                StateChangeValue::AccessKeyUpdate { account_id, .. } => account_id.clone(),
-                StateChangeValue::AccessKeyDeletion { account_id, .. } => account_id.clone(),
-                StateChangeValue::DataUpdate { account_id, .. } => account_id.clone(),
-                StateChangeValue::DataDeletion { account_id, .. } => account_id.clone(),
-                StateChangeValue::ContractCodeUpdate { account_id, .. } => account_id.clone(),
-                StateChangeValue::ContractCodeDeletion { account_id } => account_id.clone(),
-            };
-            let shard_id = if let Ok(shard_id) =
-                self.runtime_adapter.account_id_to_shard_id(&account_id, &msg.epoch_id)
-            {
-                shard_id
-            } else {
-                return Err(GetStateChangesError::IOError {
-                    error_message: format!("Failed to get ShardID from AccountID {}", account_id),
-                });
+            let account_id = &state_change_with_cause.value.get_affected_account_id();
+            let shard_id = match self.runtime_adapter.account_id_to_shard_id(&account_id, &msg.epoch_id) {
+                Ok(shard_id) => shard_id,
+                Err(err) => return Err(GetStateChangesError::IOError {
+                    error_message: format!("{}", err),
+                })
             };
 
-            let mut state_changes = if let Some(changes_with_cause) =
-                state_changes_with_cause_split_by_shard_id.remove(&shard_id)
-            {
-                changes_with_cause
-            } else {
-                Vec::<StateChangeWithCauseView>::new()
-            };
+            let state_changes =
+                state_changes_with_cause_split_by_shard_id.entry(shard_id).or_default();
             state_changes.push(state_change_with_cause.into());
-            state_changes_with_cause_split_by_shard_id.insert(shard_id, state_changes);
         }
 
         Ok(state_changes_with_cause_split_by_shard_id)

chain/indexer-primitives/Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "indexer-primitives"
+name = "near-indexer-primitives"
 version = "0.0.0"
 authors = ["Near Inc <[email protected]>"]
 publish = true

chain/indexer/CHANGELOG.md

@@ -2,9 +2,9 @@
 
 ## 1.25.x (UNRELEASED)
 
-- `state_changes` is moved to `IndexerShard` struct and represent only
+* `state_changes` is moved to `IndexerShard` struct and represent only
   the state changes happened on the specific shard
-- All the NEAR Indexer Framework types was extracted to a separate crate `indexer-primitives`
+* All the NEAR Indexer Framework types was extracted to a separate crate `indexer-primitives`
 
 ## Breaking changes
 
@@ -14,38 +14,38 @@ to the specific shard.
 
 ## 0.10.1
 
-- (mainnet only) Add additional handler to inject restored receipts to the block #47317863. See [PR 4248](https://github.com/near/nearcore/pull/4248) for reference
+* (mainnet only) Add additional handler to inject restored receipts to the block #47317863. See [PR 4248](https://github.com/near/nearcore/pull/4248) for reference
 
 ## 0.10.0
 
-- Add additional logs on Indexer Framework start
-- Avoid double genesis validation by removing the explicit validation on Indexer instantiation
-- Replaced the method how genesis is being read to optimize memory usage
+* Add additional logs on Indexer Framework start
+* Avoid double genesis validation by removing the explicit validation on Indexer instantiation
+* Replaced the method how genesis is being read to optimize memory usage
 
 ## Breaking changes
 
 Since the change of reading genesis method to optimize memory usage. You'd be able to iterate over genesis records with `near_config.genesis.for_each_record(|record| {...})`. Nothing is changed for you your indexer does nothing about genesis records.
 
 ## 0.9.2
 
-- Optimize the delayed receipts tracking process introduced in previous version to avoid indexer stuck.
+* Optimize the delayed receipts tracking process introduced in previous version to avoid indexer stuck.
 
 ## 0.9.1
 
-- Introduce a hot-fix. Execution outcome for local receipt might appear not in the same block as the receipt. Local receipts are not saved in database and unable to be fetched. To include a receipt in `IndexerExecutionOutcomeWithReceipt` and prevent NEAR Indexer Framework from panic we fetch previous blocks to find corresponding local receipt to include.
+* Introduce a hot-fix. Execution outcome for local receipt might appear not in the same block as the receipt. Local receipts are not saved in database and unable to be fetched. To include a receipt in `IndexerExecutionOutcomeWithReceipt` and prevent NEAR Indexer Framework from panic we fetch previous blocks to find corresponding local receipt to include.
 
 ## 0.9.0 (do not use this version, it contains a bug)
 
-- Introduce `IndexerShard` structure which contains corresponding chunks and `IndexerExecutionOutcomeWithReceipt`
-- `receipt` field in `IndexerExecutionOutcomeWithReceipt` is no longer optional as it used to be always set anyway,
+* Introduce `IndexerShard` structure which contains corresponding chunks and `IndexerExecutionOutcomeWithReceipt`
+* `receipt` field in `IndexerExecutionOutcomeWithReceipt` is no longer optional as it used to be always set anyway,
   so now we explicitly communicate this relation ("every outcome has a corresponding receipt") through the type system
-- Introduce `IndexerExecutionOutcomeWithOptionalReceipt` which is the same as `IndexerExecutionOutcomeWithReceipt`
+* Introduce `IndexerExecutionOutcomeWithOptionalReceipt` which is the same as `IndexerExecutionOutcomeWithReceipt`
   but with optional `receipt` field.
 
 ## Breaking changes
 
-- `IndexerChunkView` doesn't contain field `receipt_execution_outcomes` anymore, this field has been moved to `IndexerShard`
-- `StreamerMessage` structure was aligned more with NEAR Protocol specification and now looks like:
+* `IndexerChunkView` doesn't contain field `receipt_execution_outcomes` anymore, this field has been moved to `IndexerShard`
+* `StreamerMessage` structure was aligned more with NEAR Protocol specification and now looks like:
   ```
   StreamerMessage {
     block: BlockView,
@@ -56,13 +56,13 @@ Since the change of reading genesis method to optimize memory usage. You'd be ab
 
 ## 0.8.1
 
-- Add `InitConfigArgs` and `indexer_init_configs`
+* Add `InitConfigArgs` and `indexer_init_configs`
 
 As current `neard::init_configs()` signature is a bit hard to read and use we introduce `InitConfigArgs` struct to make a process of passing arguments more explicit. That's why we introduce `indexer_init_configs` which is just a wrapper on `neard::init_configs()` but takes `dir` and `InitConfigArgs` as an input.
 
 ## 0.8.0
 
-- Upgrade dependencies
+* Upgrade dependencies
 
 ## Breaking change
 
@@ -72,57 +72,57 @@ created and started on the Indexer implementation, not on the Indexer Framework
 
 ## 0.7.0
 
-- State changes return changes with cause instead of kinds
+* State changes return changes with cause instead of kinds
 
 ## Breaking changes
 
-- `StreamerMessage` now contains `StateChangesView` which is an alias for `Vec<StateChangesWithCauseView>`, previously it contained `StateChangesKindsView`
+* `StreamerMessage` now contains `StateChangesView` which is an alias for `Vec<StateChangesWithCauseView>`, previously it contained `StateChangesKindsView`
 
 ## 0.6.0
 
-- Add a way to turn off the requirement to wait for the node to be fully synced before starting streaming.
+* Add a way to turn off the requirement to wait for the node to be fully synced before starting streaming.
 
 ## Breaking changes
 
-- `IndexerConfig` was extended with another field `await_for_node_synced`. Corresponding enum is `AwaitForNodeSyncedEnum` with variants:
+* `IndexerConfig` was extended with another field `await_for_node_synced`. Corresponding enum is `AwaitForNodeSyncedEnum` with variants:
   - `WaitForFullSync` - await for node to be fully synced (previous default behaviour)
   - `StreamWhileSyncing`- start streaming right away while node is syncing (it's useful in case of Indexing from genesis)
 
 ## 0.5.0
 
-- Attach receipt execution outcomes to a relevant chunk and preserve their execution order (!)
+* Attach receipt execution outcomes to a relevant chunk and preserve their execution order (!)
 
 ## Breaking changes
 
 Since #3529 nearcore stores `ExecutionOutcome`s in their execution order, and we can also attribute outcomes to specific chunk. That's why:
 
-- `receipt_execution_outcomes` was moved from `StreamerMessage` to a relevant `IndexerChunkView`
-- `ExecutionOutcomesWithReceipts` type alias was removed (just use `Vec<IndexerExecutionOutcomeWithReceipt>` instead)
+* `receipt_execution_outcomes` was moved from `StreamerMessage` to a relevant `IndexerChunkView`
+* `ExecutionOutcomesWithReceipts` type alias was removed (just use `Vec<IndexerExecutionOutcomeWithReceipt>` instead)
 
 ## 0.4.0
 
-- Prepend chunk's receipts with local receipts to attach latter to specific chunk
+* Prepend chunk's receipts with local receipts to attach latter to specific chunk
 
 ## Breaking changes
 
-- For local receipt to have a relation to specific chunk we have prepended them to original receipts in particular chunk
+* For local receipt to have a relation to specific chunk we have prepended them to original receipts in particular chunk
   as in the most cases local receipts are executed before normal receipts. That's why there is no reason to have `local_receipts`
   field in `StreamerMessage` struct anymore. `local_receipts` field was removed.
 
 ## 0.3.1
 
-- Add local receipt to `receipt_execution_outcomes` if possible
+* Add local receipt to `receipt_execution_outcomes` if possible
 
 ## 0.3.0
 
 ### Breaking changes
 
-- To extended the `receipt_execution_outcomes` with information about the corresponding receipt we had to break the API
+* To extended the `receipt_execution_outcomes` with information about the corresponding receipt we had to break the API
   (the old outcome structure is just one layer deeper now [under `execution_outcome` field])
 
 ## 0.2.0
 
-- Refactor the way of fetching `ExecutionOutcome`s (use the new way to get all of them for specific block)
-- Rename `StreamerMessage.outcomes` field to `receipt_execution_outcomes` and change type to `HashMap<CryptoHash, ExecutionOutcomeWithId>` and now it includes only `ExecutionOutcome`s for receipts (no transactions)
-- Introduce `IndexerTransactionWithOutcome` struct to contain `SignedTransactionView` and `ExecutionOutcomeWithId` for the transaction
-- Introduce `IndexerChunkView` to replace `StreamerMessage.chunks` to include `IndexerTransactionWithOutcome` vec in `transactions`
+* Refactor the way of fetching `ExecutionOutcome`s (use the new way to get all of them for specific block)
+* Rename `StreamerMessage.outcomes` field to `receipt_execution_outcomes` and change type to `HashMap<CryptoHash, ExecutionOutcomeWithId>` and now it includes only `ExecutionOutcome`s for receipts (no transactions)
+* Introduce `IndexerTransactionWithOutcome` struct to contain `SignedTransactionView` and `ExecutionOutcomeWithId` for the transaction
+* Introduce `IndexerChunkView` to replace `StreamerMessage.chunks` to include `IndexerTransactionWithOutcome` vec in `transactions`

chain/indexer/Cargo.toml

@@ -18,7 +18,7 @@ serde = { version = "1", features = [ "derive" ] }
 serde_json = "1.0.55"
 tokio = { version = "1.1", features = ["time", "sync"] }
 
-indexer-primitives = { path = "../indexer-primitives" }
+near-indexer-primitives = { path = "../indexer-primitives" }
 nearcore = { path = "../../nearcore" }
 near-client = { path = "../client" }
 near-chain-configs = { path = "../../core/chain-configs" }

chain/indexer/src/lib.rs

@@ -3,6 +3,7 @@
 use anyhow::Context;
 use tokio::sync::mpsc;
 
+use near_chain_configs::GenesisValidationMode;
 pub use near_primitives;
 use near_primitives::types::Gas;
 pub use nearcore::{get_default_home, init_configs, NearConfig};
@@ -12,7 +13,6 @@ pub use self::streamer::{
     IndexerExecutionOutcomeWithReceipt, IndexerShard, IndexerTransactionWithOutcome,
     StreamerMessage,
 };
-use near_chain_configs::GenesisValidationMode;
 
 mod streamer;
 

chain/indexer/src/streamer/fetchers.rs

@@ -6,12 +6,12 @@ use actix::Addr;
 use futures::stream::StreamExt;
 use tracing::warn;
 
+use near_indexer_primitives::IndexerExecutionOutcomeWithOptionalReceipt;
 pub use near_primitives::hash::CryptoHash;
 pub use near_primitives::{types, views};
 
 use super::errors::FailedToFetchData;
 use super::INDEXER;
-use indexer_primitives::IndexerExecutionOutcomeWithOptionalReceipt;
 
 pub(crate) async fn fetch_status(
     client: &Addr<near_client::ClientActor>,

chain/indexer/src/streamer/mod.rs

@@ -7,6 +7,11 @@ use tokio::sync::mpsc;
 use tokio::time;
 use tracing::{debug, info};
 
+pub use near_indexer_primitives::{
+    IndexerChunkView, IndexerExecutionOutcomeWithOptionalReceipt,
+    IndexerExecutionOutcomeWithReceipt, IndexerShard, IndexerTransactionWithOutcome,
+    StreamerMessage,
+};
 use near_primitives::hash::CryptoHash;
 pub use near_primitives::views;
 
@@ -20,11 +25,6 @@ use self::fetchers::{
 use self::utils::convert_transactions_sir_into_local_receipts;
 use crate::streamer::fetchers::fetch_protocol_config;
 use crate::INDEXER;
-pub use indexer_primitives::{
-    IndexerChunkView, IndexerExecutionOutcomeWithOptionalReceipt,
-    IndexerExecutionOutcomeWithReceipt, IndexerShard, IndexerTransactionWithOutcome,
-    StreamerMessage,
-};
 
 mod errors;
 mod fetchers;
@@ -87,7 +87,9 @@ async fn build_streamer_message(
             shard_id,
             chunk: None,
             receipt_execution_outcomes: vec![],
-            state_changes: vec![],
+            state_changes: state_changes
+                .remove(&shard_id)
+                .expect("StateChanges for given shard should be present"),
         })
         .collect::<Vec<_>>();
 
@@ -97,10 +99,6 @@ async fn build_streamer_message(
 
         let shard_id = header.shard_id.clone() as usize;
 
-        indexer_shards[shard_id].state_changes = state_changes
-            .remove(&header.shard_id)
-            .expect("StateChanges for given shard should be present");
-
         let mut outcomes = shards_outcomes
             .remove(&header.shard_id)
             .expect("Execution outcomes for given shard should be present");

chain/indexer/src/streamer/utils.rs

@@ -1,11 +1,11 @@
 use actix::Addr;
 
+use near_indexer_primitives::IndexerTransactionWithOutcome;
 use near_primitives::views;
 use node_runtime::config::tx_cost;
 
 use super::errors::FailedToFetchData;
 use super::fetchers::fetch_block_by_hash;
-use indexer_primitives::IndexerTransactionWithOutcome;
 
 pub(crate) async fn convert_transactions_sir_into_local_receipts(
     client: &Addr<near_client::ViewClientActor>,

core/primitives/src/types.rs

@@ -233,6 +233,21 @@ pub enum StateChangeValue {
     ContractCodeDeletion { account_id: AccountId },
 }
 
+impl StateChangeValue {
+    pub fn get_affected_account_id(&self) -> &AccountId {
+        match &self {
+            StateChangeValue::AccountUpdate { account_id, .. } => account_id,
+            StateChangeValue::AccountDeletion { account_id } => account_id,
+            StateChangeValue::AccessKeyUpdate { account_id, .. } => account_id,
+            StateChangeValue::AccessKeyDeletion { account_id, .. } => account_id,
+            StateChangeValue::DataUpdate { account_id, .. } => account_id,
+            StateChangeValue::DataDeletion { account_id, .. } => account_id,
+            StateChangeValue::ContractCodeUpdate { account_id, .. } => account_id,
+            StateChangeValue::ContractCodeDeletion { account_id } => account_id,
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct StateChangeWithCause {
     pub cause: StateChangeCause,