add status field to response in RPC methods tx, `EXPERIMENTAL_tx_st…

…atus`, `broadcast_tx_commit`, `broadcast_tx_async` (#9556)

This PR is a part of RPC tx methods redesign
https://docs.google.com/document/d/1jGuXzBlMAGQENsUpy5x5Xd7huCLOd8k6tyMOEE41Rro/edit?usp=sharing
Addressing point 3 in #9542 and
start working on #6837

Changes:
- New field `status` appeared in the response of RPC methods `tx`,
`EXPERIMENTAL_tx_status`, `broadcast_tx_commit`, `broadcast_tx_async`
- Added some comments with the explanations

In #6837, Illia suggested to have
the structure
```rust
enum BroadcastWait {
 /// Returns right away.
 None,
 /// Waits only for inclusion into the block.
 Inclusion,
 /// Waits until block with inclusion is finalized.
 InclusionFinal,
 /// Waits until all non-refund receipts are executed.
 Executed,
 /// Waits until all non-refund receipts are executed and the last of the blocks is final.
 ExecutedFinal,
 /// Waits until everything has executed and is final.
 Final,
 }
```

I've renamed it to `TxExecutionStatus` and simplified it a little by
dropping all refund options:
1. We may add them later without breaking backwards compatibility
2. To support them, we need to have the access to `receipt` (it's the
method `get_tx_execution_status`). We have there only
`execution_outcome` by default. We created special logic to be able not
to retrieve the receipts (it's the only difference between `tx` and
`EXPERIMENTAL_tx_status`). I have a feeling (please correct me here if
I'm wrong) that retrieving corresponding receipt may slow down the
execution that we tried to optimise.
BTW, maybe the data from `execution_outcome` is enough (we have there
`gas_burnt` and `tokens_burnt`). @jakmeier suggested the condition
`outcome.tokens_burnt == 0 && outcome.status == Success`. Unfortunately,
we need to work with any status. But maybe the first part
(`outcome.tokens_burnt == 0`) is enough to say that it could be only
refund receipt. I need more input here.

chain/chain/src/chain.rs

@@ -3750,7 +3750,7 @@ impl Chain {
 
     pub fn check_blocks_final_and_canonical(
         &self,
-        block_headers: &[&BlockHeader],
+        block_headers: &[BlockHeader],
     ) -> Result<(), Error> {
         let last_final_block_hash = *self.head_header()?.last_final_block();
         let last_final_height = self.get_block_header(&last_final_block_hash)?.height();

chain/client-primitives/src/types.rs

@@ -15,10 +15,9 @@ use near_primitives::types::{
 use near_primitives::views::validator_stake_view::ValidatorStakeView;
 use near_primitives::views::{
     BlockView, ChunkView, DownloadStatusView, EpochValidatorInfo, ExecutionOutcomeWithIdView,
-    FinalExecutionOutcomeViewEnum, GasPriceView, LightClientBlockLiteView, LightClientBlockView,
-    MaintenanceWindowsView, QueryRequest, QueryResponse, ReceiptView, ShardSyncDownloadView,
-    SplitStorageInfoView, StateChangesKindsView, StateChangesRequestView, StateChangesView,
-    SyncStatusView,
+    GasPriceView, LightClientBlockLiteView, LightClientBlockView, MaintenanceWindowsView,
+    QueryRequest, QueryResponse, ReceiptView, ShardSyncDownloadView, SplitStorageInfoView,
+    StateChangesKindsView, StateChangesRequestView, StateChangesView, SyncStatusView, TxStatusView,
 };
 pub use near_primitives::views::{StatusResponse, StatusSyncInfo};
 use std::collections::HashMap;
@@ -721,7 +720,7 @@ impl From<near_chain_primitives::Error> for TxStatusError {
 }
 
 impl Message for TxStatus {
-    type Result = Result<Option<FinalExecutionOutcomeViewEnum>, TxStatusError>;
+    type Result = Result<TxStatusView, TxStatusError>;
 }
 
 #[derive(Debug)]

chain/client/src/tests/query_client.rs

@@ -186,8 +186,8 @@ fn test_execution_outcome_for_chunk() {
                 .await
                 .unwrap()
                 .unwrap()
-                .unwrap()
                 .into_outcome()
+                .unwrap()
                 .transaction_outcome
                 .block_hash;
 

chain/client/src/view_client.rs

@@ -53,14 +53,14 @@ use near_primitives::types::{
 };
 use near_primitives::views::validator_stake_view::ValidatorStakeView;
 use near_primitives::views::{
-    BlockView, ChunkView, EpochValidatorInfo, ExecutionOutcomeWithIdView,
+    BlockView, ChunkView, EpochValidatorInfo, ExecutionOutcomeWithIdView, ExecutionStatusView,
     FinalExecutionOutcomeView, FinalExecutionOutcomeViewEnum, GasPriceView, LightClientBlockView,
     MaintenanceWindowsView, QueryRequest, QueryResponse, ReceiptView, SplitStorageInfoView,
-    StateChangesKindsView, StateChangesView,
+    StateChangesKindsView, StateChangesView, TxExecutionStatus, TxStatusView,
 };
 use near_store::{DBCol, COLD_HEAD_KEY, FINAL_HEAD_KEY, HEAD_KEY};
 use std::cmp::Ordering;
-use std::collections::{HashMap, VecDeque};
+use std::collections::{BTreeSet, HashMap, VecDeque};
 use std::hash::Hash;
 use std::sync::{Arc, Mutex, RwLock};
 use std::time::{Duration, Instant};
@@ -417,17 +417,63 @@ impl ViewClientActor {
         }
     }
 
+    fn get_tx_execution_status(
+        &self,
+        execution_outcome: &FinalExecutionOutcomeView,
+    ) -> Result<TxExecutionStatus, TxStatusError> {
+        Ok(match execution_outcome.transaction_outcome.outcome.status {
+            // Return the lowest status the node can proof.
+            ExecutionStatusView::Unknown => TxExecutionStatus::None,
+            _ => {
+                if execution_outcome
+                    .receipts_outcome
+                    .iter()
+                    .all(|e| e.outcome.status != ExecutionStatusView::Unknown)
+                {
+                    let block_hashes: BTreeSet<CryptoHash> =
+                        execution_outcome.receipts_outcome.iter().map(|e| e.block_hash).collect();
+                    let mut headers = vec![];
+                    for block_hash in block_hashes {
+                        headers.push(self.chain.get_block_header(&block_hash)?);
+                    }
+                    // We can't sort and check only the last block;
+                    // previous blocks may be not in the canonical chain
+                    match self.chain.check_blocks_final_and_canonical(&headers) {
+                        Ok(_) => TxExecutionStatus::Final,
+                        Err(_) => TxExecutionStatus::Executed,
+                    }
+                } else {
+                    match self.chain.check_blocks_final_and_canonical(&[self
+                        .chain
+                        .get_block_header(&execution_outcome.transaction_outcome.block_hash)?])
+                    {
+                        Ok(_) => TxExecutionStatus::InclusionFinal,
+                        Err(_) => TxExecutionStatus::Inclusion,
+                    }
+                }
+            }
+        })
+    }
+
     fn get_tx_status(
         &mut self,
         tx_hash: CryptoHash,
         signer_account_id: AccountId,
         fetch_receipt: bool,
-    ) -> Result<Option<FinalExecutionOutcomeViewEnum>, TxStatusError> {
+    ) -> Result<TxStatusView, TxStatusError> {
         {
+            // TODO(telezhnaya): take into account `fetch_receipt()`
+            // https://github.com/near/nearcore/issues/9545
             let mut request_manager = self.request_manager.write().expect(POISONED_LOCK_ERR);
             if let Some(res) = request_manager.tx_status_response.pop(&tx_hash) {
                 request_manager.tx_status_requests.pop(&tx_hash);
-                return Ok(Some(FinalExecutionOutcomeViewEnum::FinalExecutionOutcome(res)));
+                let status = self.get_tx_execution_status(&res)?;
+                return Ok(TxStatusView {
+                    execution_outcome: Some(FinalExecutionOutcomeViewEnum::FinalExecutionOutcome(
+                        res,
+                    )),
+                    status,
+                });
             }
         }
 
@@ -445,6 +491,7 @@ impl ViewClientActor {
         ) {
             match self.chain.get_final_transaction_result(&tx_hash) {
                 Ok(tx_result) => {
+                    let status = self.get_tx_execution_status(&tx_result)?;
                     let res = if fetch_receipt {
                         let final_result =
                             self.chain.get_final_transaction_result_with_receipt(tx_result)?;
@@ -454,11 +501,14 @@ impl ViewClientActor {
                     } else {
                         FinalExecutionOutcomeViewEnum::FinalExecutionOutcome(tx_result)
                     };
-                    Ok(Some(res))
+                    Ok(TxStatusView { execution_outcome: Some(res), status })
                 }
                 Err(near_chain::Error::DBNotFoundErr(_)) => {
                     if self.chain.get_execution_outcome(&tx_hash).is_ok() {
-                        Ok(None)
+                        Ok(TxStatusView {
+                            execution_outcome: None,
+                            status: TxExecutionStatus::None,
+                        })
                     } else {
                         Err(TxStatusError::MissingTransaction(tx_hash))
                     }
@@ -481,7 +531,7 @@ impl ViewClientActor {
                     NetworkRequests::TxStatus(validator, signer_account_id, tx_hash),
                 ));
             }
-            Ok(None)
+            Ok(TxStatusView { execution_outcome: None, status: TxExecutionStatus::None })
         }
     }
 
@@ -647,7 +697,7 @@ impl Handler<WithSpanContext<GetChunk>> for ViewClientActor {
 }
 
 impl Handler<WithSpanContext<TxStatus>> for ViewClientActor {
-    type Result = Result<Option<FinalExecutionOutcomeViewEnum>, TxStatusError>;
+    type Result = Result<TxStatusView, TxStatusError>;
 
     #[perf]
     fn handle(&mut self, msg: WithSpanContext<TxStatus>, _: &mut Self::Context) -> Self::Result {
@@ -1052,7 +1102,7 @@ impl Handler<WithSpanContext<GetBlockProof>> for ViewClientActor {
             metrics::VIEW_CLIENT_MESSAGE_TIME.with_label_values(&["GetBlockProof"]).start_timer();
         let block_header = self.chain.get_block_header(&msg.block_hash)?;
         let head_block_header = self.chain.get_block_header(&msg.head_block_hash)?;
-        self.chain.check_blocks_final_and_canonical(&[&block_header, &head_block_header])?;
+        self.chain.check_blocks_final_and_canonical(&[block_header.clone(), head_block_header])?;
         let block_header_lite = block_header.into();
         let proof = self.chain.get_block_proof(&msg.block_hash, &msg.head_block_hash)?;
         Ok(GetBlockProofResponse { block_header_lite, proof })
@@ -1140,7 +1190,9 @@ impl Handler<WithSpanContext<TxStatusRequest>> for ViewClientActor {
         let _timer =
             metrics::VIEW_CLIENT_MESSAGE_TIME.with_label_values(&["TxStatusRequest"]).start_timer();
         let TxStatusRequest { tx_hash, signer_account_id } = msg;
-        if let Ok(Some(result)) = self.get_tx_status(tx_hash, signer_account_id, false) {
+        if let Ok(Some(result)) =
+            self.get_tx_status(tx_hash, signer_account_id, false).map(|s| s.execution_outcome)
+        {
             Some(Box::new(result.into_outcome()))
         } else {
             None

chain/jsonrpc-primitives/src/types/transactions.rs

@@ -42,7 +42,8 @@ pub enum RpcTransactionError {
 #[derive(serde::Serialize, serde::Deserialize, Debug)]
 pub struct RpcTransactionResponse {
     #[serde(flatten)]
-    pub final_execution_outcome: near_primitives::views::FinalExecutionOutcomeViewEnum,
+    pub final_execution_outcome: Option<near_primitives::views::FinalExecutionOutcomeViewEnum>,
+    pub final_execution_status: near_primitives::views::TxExecutionStatus,
 }
 
 #[derive(serde::Serialize, serde::Deserialize, Debug)]

chain/jsonrpc/CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## 0.2.3
+
+* Added `final_execution_status` field to the response of methods `tx`, `EXPERIMENTAL_tx_status`, `broadcast_tx_commit`
+
+### Breaking changes
+
+Response from `broadcast_tx_async` changed the type from String to Dict.
+The response has the same structure as in `broadcast_tx_commit`.
+It no longer contains transaction hash (which may be retrieved from Signed Transaction passed in request).
+
 ## 0.2.2
 
 * Extended error structures to be more explicit. See [#2976 decision comment for reference](https://github.com/near/nearcore/issues/2976#issuecomment-865834617)

chain/jsonrpc/client/src/lib.rs

@@ -5,13 +5,13 @@ use near_jsonrpc_primitives::message::{from_slice, Message};
 use near_jsonrpc_primitives::types::changes::{
     RpcStateChangesInBlockByTypeRequest, RpcStateChangesInBlockByTypeResponse,
 };
+use near_jsonrpc_primitives::types::transactions::RpcTransactionResponse;
 use near_jsonrpc_primitives::types::validator::RpcValidatorsOrderedRequest;
 use near_primitives::hash::CryptoHash;
 use near_primitives::types::{AccountId, BlockId, BlockReference, MaybeBlockId, ShardId};
 use near_primitives::views::validator_stake_view::ValidatorStakeView;
 use near_primitives::views::{
-    BlockView, ChunkView, EpochValidatorInfo, FinalExecutionOutcomeView, GasPriceView,
-    StatusResponse,
+    BlockView, ChunkView, EpochValidatorInfo, GasPriceView, StatusResponse,
 };
 use std::time::Duration;
 
@@ -177,7 +177,7 @@ macro_rules! jsonrpc_client {
 
 jsonrpc_client!(pub struct JsonRpcClient {
     pub fn broadcast_tx_async(&self, tx: String) -> RpcRequest<String>;
-    pub fn broadcast_tx_commit(&self, tx: String) -> RpcRequest<FinalExecutionOutcomeView>;
+    pub fn broadcast_tx_commit(&self, tx: String) -> RpcRequest<RpcTransactionResponse>;
     pub fn status(&self) -> RpcRequest<StatusResponse>;
     #[allow(non_snake_case)]
     pub fn EXPERIMENTAL_check_tx(&self, tx: String) -> RpcRequest<serde_json::Value>;
@@ -186,9 +186,9 @@ jsonrpc_client!(pub struct JsonRpcClient {
     #[allow(non_snake_case)]
     pub fn EXPERIMENTAL_broadcast_tx_sync(&self, tx: String) -> RpcRequest<serde_json::Value>;
     #[allow(non_snake_case)]
-    pub fn EXPERIMENTAL_tx_status(&self, tx: String) -> RpcRequest<serde_json::Value>;
+    pub fn EXPERIMENTAL_tx_status(&self, tx: String) -> RpcRequest<RpcTransactionResponse>;
     pub fn health(&self) -> RpcRequest<()>;
-    pub fn tx(&self, hash: String, account_id: AccountId) -> RpcRequest<FinalExecutionOutcomeView>;
+    pub fn tx(&self, hash: String, account_id: AccountId) -> RpcRequest<RpcTransactionResponse>;
     pub fn chunk(&self, id: ChunkId) -> RpcRequest<ChunkView>;
     pub fn validators(&self, block_id: MaybeBlockId) -> RpcRequest<EpochValidatorInfo>;
     pub fn gas_price(&self, block_id: MaybeBlockId) -> RpcRequest<GasPriceView>;

chain/jsonrpc/jsonrpc-tests/tests/rpc_transactions.rs

@@ -13,7 +13,7 @@ use near_primitives::hash::{hash, CryptoHash};
 use near_primitives::serialize::to_base64;
 use near_primitives::transaction::SignedTransaction;
 use near_primitives::types::BlockReference;
-use near_primitives::views::FinalExecutionStatus;
+use near_primitives::views::{FinalExecutionStatus, TxExecutionStatus};
 
 use near_jsonrpc_tests::{self as test_utils, test_with_client};
 
@@ -62,7 +62,9 @@ fn test_send_tx_async() {
                             .tx(tx_hash.to_string(), signer_account_id)
                             .map_err(|err| println!("Error: {:?}", err))
                             .map_ok(|result| {
-                                if let FinalExecutionStatus::SuccessValue(_) = result.status {
+                                if let FinalExecutionStatus::SuccessValue(_) =
+                                    result.final_execution_outcome.unwrap().into_outcome().status
+                                {
                                     System::current().stop();
                                 }
                             })
@@ -93,7 +95,15 @@ fn test_send_tx_commit() {
         );
         let bytes = tx.try_to_vec().unwrap();
         let result = client.broadcast_tx_commit(to_base64(&bytes)).await.unwrap();
-        assert_eq!(result.status, FinalExecutionStatus::SuccessValue(Vec::new()));
+        assert_eq!(
+            result.final_execution_outcome.unwrap().into_outcome().status,
+            FinalExecutionStatus::SuccessValue(Vec::new())
+        );
+        assert!(
+            vec![TxExecutionStatus::Executed, TxExecutionStatus::Final]
+                .contains(&result.final_execution_status),
+            "All the receipts should be already executed"
+        );
     });
 }
 

chain/jsonrpc/src/api/transactions.rs

@@ -5,12 +5,11 @@ use serde_with::serde_as;
 use near_client_primitives::types::TxStatusError;
 use near_jsonrpc_primitives::errors::RpcParseError;
 use near_jsonrpc_primitives::types::transactions::{
-    RpcBroadcastTransactionRequest, RpcTransactionError, RpcTransactionResponse,
-    RpcTransactionStatusCommonRequest, TransactionInfo,
+    RpcBroadcastTransactionRequest, RpcTransactionError, RpcTransactionStatusCommonRequest,
+    TransactionInfo,
 };
 use near_primitives::borsh::BorshDeserialize;
 use near_primitives::transaction::SignedTransaction;
-use near_primitives::views::FinalExecutionOutcomeViewEnum;
 
 use super::{Params, RpcFrom, RpcRequest};
 
@@ -53,12 +52,6 @@ impl RpcFrom<TxStatusError> for RpcTransactionError {
     }
 }
 
-impl RpcFrom<FinalExecutionOutcomeViewEnum> for RpcTransactionResponse {
-    fn rpc_from(final_execution_outcome: FinalExecutionOutcomeViewEnum) -> Self {
-        Self { final_execution_outcome }
-    }
-}
-
 fn decode_signed_transaction(value: Value) -> Result<SignedTransaction, RpcParseError> {
     #[serde_as]
     #[derive(serde::Deserialize)]