Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Protocol Changes

### Non-protocol Changes
* Transaction-status timeouts (`tx`, `EXPERIMENTAL_tx_status`, and `send_tx`/`broadcast_tx_commit` with `wait_until`) now carry a `reason` in the `TIMEOUT_ERROR`: `pending` (with the last-known status), `not_observed`, or `error`. Previously the timeout gave no detail.

## [2.13.0]

Expand Down
45 changes: 36 additions & 9 deletions chain/chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3096,12 +3096,37 @@ impl Chain {
Ok(FinalExecutionOutcomeView { status, transaction, transaction_outcome, receipts_outcome })
}

/// Returns FinalExecutionOutcomeView for the given transaction.
/// Does not wait for the end of the execution of all corresponding receipts
/// Returns the `FinalExecutionOutcomeView` for the given transaction,
/// assembled from whatever execution outcomes exist so far (it does not
/// wait for all receipts to finish).
///
/// Errors with `DBNotFoundErr` if the transaction is unknown to this node
/// *or* has no execution outcome yet. Use
/// [`Self::get_partial_transaction_result_option`] when those two cases
/// need to be told apart.
pub fn get_partial_transaction_result(
&self,
transaction_hash: &CryptoHash,
) -> Result<FinalExecutionOutcomeView, Error> {
self.get_partial_transaction_result_option(transaction_hash)?.ok_or_else(|| {
Error::DBNotFoundErr(format!(
"Transaction {} has no execution outcome",
transaction_hash
))
})
}

/// Returns the `Ok(Some(FinalExecutionOutcomeView))` for the given
/// transaction, assembled from whatever execution outcomes exist so far (it
/// does not wait for all receipts to finish).
///
/// Returns `Ok(None)` if the transaction is recorded on chain but has no
/// execution outcome yet (included, not executed), and `Err(DBNotFoundErr)`
/// if the transaction is not in the store at all.
pub fn get_partial_transaction_result_option(
&self,
transaction_hash: &CryptoHash,
) -> Result<Option<FinalExecutionOutcomeView>, Error> {
let transaction = self.chain_store.get_transaction(transaction_hash).ok_or_else(|| {
Error::DBNotFoundErr(format!("Transaction {} is not found", transaction_hash))
})?;
Expand All @@ -3110,18 +3135,20 @@ impl Chain {
let mut outcomes = Vec::new();
self.get_recursive_transaction_results(&mut outcomes, transaction_hash, false)?;
if outcomes.is_empty() {
// It can't be, we would fail with tx not found error earlier in this case
// But if so, let's return meaningful error instead of panic on split_off
return Err(Error::DBNotFoundErr(format!(
"Transaction {} is not found",
transaction_hash
)));
// The transaction is in the store (included in a chunk) but its execution outcome has
// not been recorded yet, so there is no result to assemble.
return Ok(None);
}

let status = self.get_execution_status(&outcomes, transaction_hash);
let receipts_outcome = outcomes.split_off(1);
let transaction_outcome = outcomes.pop().unwrap();
Ok(FinalExecutionOutcomeView { status, transaction, transaction_outcome, receipts_outcome })
Ok(Some(FinalExecutionOutcomeView {
status,
transaction,
transaction_outcome,
receipts_outcome,
}))
}

/// Returns corresponding receipts for provided outcome
Expand Down
15 changes: 14 additions & 1 deletion chain/client-primitives/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use near_primitives::types::{
};
use near_primitives::views::{
EpochSyncStatusView, ExecutionOutcomeWithIdView, LightClientBlockLiteView, QueryRequest,
StateChangesRequestView, StateSyncStatusView, SyncStatusView,
StateChangesRequestView, StateSyncStatusView, SyncStatusView, TxStatusView,
};
pub use near_primitives::views::{StatusResponse, StatusSyncInfo};
use near_time::Duration;
Expand Down Expand Up @@ -626,6 +626,19 @@ pub struct TxStatus {
pub fetch_receipt: bool,
}

/// Outcome of the transaction status lookup, including either the status or
/// full context on why the status is unavailable.
#[derive(Debug)]
pub enum TxStatusOutcome {
/// The node tracks the transaction's shard and observed it.
Observed(TxStatusView),
/// The node tracks the shard but has not seen the transaction on chain.
NotObserved,
/// The node does not track the transaction's shard; the query was forwarded
/// to a chunk producer that does, and no answer is available yet.
DoesNotTrackShard { shard_id: ShardId },
}

#[derive(Debug)]
pub enum TxStatusError {
ChainError(near_chain_primitives::Error),
Expand Down
2 changes: 1 addition & 1 deletion chain/client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub use near_client_primitives::types::{
GetReceiptToTxResponse, GetShardChunk, GetSplitStorageInfo, GetStateChanges,
GetStateChangesInBlock, GetStateChangesWithCauseInBlock,
GetStateChangesWithCauseInBlockForTrackedShards, GetValidatorInfo, GetValidatorOrdered, Query,
QueryError, Status, StatusResponse, SyncStatus, TxStatus, TxStatusError,
QueryError, Status, StatusResponse, SyncStatus, TxStatus, TxStatusError, TxStatusOutcome,
};
pub use near_network::client::{
BlockApproval, BlockResponse, ProcessTxRequest, ProcessTxResponse, SetNetworkInfo,
Expand Down
85 changes: 33 additions & 52 deletions chain/client/src/view_client_actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use near_client_primitives::types::{
GetReceipt, GetReceiptError, GetReceiptToTx, GetReceiptToTxError, GetReceiptToTxResponse,
GetSplitStorageInfo, GetSplitStorageInfoError, GetStateChangesError,
GetStateChangesWithCauseInBlock, GetStateChangesWithCauseInBlockForTrackedShards,
GetValidatorInfoError, Query, QueryError, TxStatus, TxStatusError,
GetValidatorInfoError, Query, QueryError, TxStatus, TxStatusError, TxStatusOutcome,
};
use near_epoch_manager::EpochManagerAdapter;
use near_epoch_manager::shard_assignment::{account_id_to_shard_id, shard_id_to_uid};
Expand Down Expand Up @@ -59,10 +59,9 @@ use near_primitives::version::{PROTOCOL_VERSION, ProtocolFeature};
use near_primitives::views::validator_stake_view::ValidatorStakeView;
use near_primitives::views::{
BlockView, ChunkView, EpochValidatorInfo, ExecutionOutcomeWithIdView, ExecutionStatusView,
FinalExecutionOutcomeView, FinalExecutionOutcomeViewEnum, FinalExecutionStatus, GasPriceView,
LightClientBlockView, MaintenanceWindowsView, QueryRequest, QueryResponse, ReceiptView,
SignedTransactionView, SplitStorageInfoView, StateChangesKindsView, StateChangesView,
TxExecutionStatus, TxStatusView,
FinalExecutionOutcomeView, FinalExecutionOutcomeViewEnum, GasPriceView, LightClientBlockView,
MaintenanceWindowsView, QueryRequest, QueryResponse, ReceiptView, SplitStorageInfoView,
StateChangesKindsView, StateChangesView, TxExecutionStatus, TxStatusView,
};
use near_store::adapter::StoreAdapter as _;
use near_store::merkle_proof::MerkleProofAccess;
Expand Down Expand Up @@ -637,20 +636,17 @@ impl ViewClientActor {
tx_hash: CryptoHash,
signer_account_id: AccountId,
fetch_receipt: bool,
) -> Result<TxStatusView, TxStatusError> {
) -> Result<TxStatusOutcome, TxStatusError> {
{
// TODO(telezhnaya): take into account `fetch_receipt()`
// https://github.com/near/nearcore/issues/9545
let mut request_manager = self.request_manager.write();
if let Some(res) = request_manager.tx_status_response.pop(&tx_hash) {
request_manager.tx_status_requests.pop(&tx_hash);
let status = self.get_tx_execution_status(&res)?;
return Ok(TxStatusView {
execution_outcome: Some(FinalExecutionOutcomeViewEnum::FinalExecutionOutcome(
res,
)),
status,
});
let execution_outcome =
Some(FinalExecutionOutcomeViewEnum::FinalExecutionOutcome(res));
return Ok(TxStatusOutcome::Observed(TxStatusView { execution_outcome, status }));
}
}

Expand All @@ -660,8 +656,8 @@ impl ViewClientActor {
.map_err(|err| TxStatusError::InternalError(err.to_string()))?;
// Check if we are tracking this shard.
if self.shard_tracker.cares_about_shard(&head.prev_block_hash, target_shard_id) {
match self.chain.get_partial_transaction_result(&tx_hash) {
Ok(tx_result) => {
match self.chain.get_partial_transaction_result_option(&tx_hash) {
Ok(Some(tx_result)) => {
let status = self.get_tx_execution_status(&tx_result)?;
let res = if fetch_receipt {
let final_result =
Expand All @@ -672,35 +668,18 @@ impl ViewClientActor {
} else {
FinalExecutionOutcomeViewEnum::FinalExecutionOutcome(tx_result)
};
Ok(TxStatusView { execution_outcome: Some(res), status })
}
Err(near_chain::Error::DBNotFoundErr(_)) => {
if let Some(transaction) = self.chain.chain_store.get_transaction(&tx_hash) {
let transaction =
SignedTransactionView::from(Arc::unwrap_or_clone(transaction));
if let Ok(tx_outcome) = self.chain.get_execution_outcome(&tx_hash) {
let outcome = FinalExecutionOutcomeViewEnum::FinalExecutionOutcome(
FinalExecutionOutcomeView {
status: FinalExecutionStatus::Started,
transaction,
transaction_outcome: tx_outcome.into(),
receipts_outcome: vec![],
},
);
Ok(TxStatusView {
execution_outcome: Some(outcome),
status: TxExecutionStatus::Included,
})
} else {
Ok(TxStatusView {
execution_outcome: None,
status: TxExecutionStatus::Included,
})
}
} else {
Err(TxStatusError::MissingTransaction(tx_hash))
}
Ok(TxStatusOutcome::Observed(TxStatusView {
execution_outcome: Some(res),
status,
}))
}
// The transaction is in the store (included) but has no execution outcome yet.
Ok(None) => Ok(TxStatusOutcome::Observed(TxStatusView {
execution_outcome: None,
status: TxExecutionStatus::Included,
})),
// The transaction is not in this node's store at all.
Err(near_chain::Error::DBNotFoundErr(_)) => Ok(TxStatusOutcome::NotObserved),
Err(err) => {
tracing::warn!(target: "client", ?err, "error trying to get transaction result");
Err(err.into())
Expand Down Expand Up @@ -729,7 +708,7 @@ impl ViewClientActor {
NetworkRequests::TxStatus(validator, signer_account_id, tx_hash),
));
}
Ok(TxStatusView { execution_outcome: None, status: TxExecutionStatus::None })
Ok(TxStatusOutcome::DoesNotTrackShard { shard_id: target_shard_id })
}
}

Expand Down Expand Up @@ -876,8 +855,8 @@ impl Handler<GetChunk, Result<ChunkView, GetChunkError>> for ViewClientActor {
}
}

impl Handler<TxStatus, Result<TxStatusView, TxStatusError>> for ViewClientActor {
fn handle(&mut self, msg: TxStatus) -> Result<TxStatusView, TxStatusError> {
impl Handler<TxStatus, Result<TxStatusOutcome, TxStatusError>> for ViewClientActor {
fn handle(&mut self, msg: TxStatus) -> Result<TxStatusOutcome, TxStatusError> {
tracing::debug!(target: "client", ?msg);
let _timer =
metrics::VIEW_CLIENT_MESSAGE_TIME.with_label_values(&["TxStatus"]).start_timer();
Expand Down Expand Up @@ -1697,13 +1676,15 @@ impl Handler<TxStatusRequest, Option<Box<FinalExecutionOutcomeView>>> for ViewCl
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).map(|s| s.execution_outcome)
{
Some(Box::new(result.into_outcome()))
} else {
None
}
let tx_status = self.get_tx_status(tx_hash, signer_account_id, false);
// TODO: the forwarded `TxStatusResponse` only carries the outcome, so the rest of
// `tx_status` (status, `NotObserved`) is dropped. Enrich the response so a forwarded
// query can relay a definitive "not observed" instead of going silent on the RPC node.
let outcome = match tx_status {
Ok(TxStatusOutcome::Observed(view)) => view.execution_outcome,
_ => None,
};
outcome.map(|result| Box::new(result.into_outcome()))
}
}

Expand Down
80 changes: 77 additions & 3 deletions chain/jsonrpc-primitives/src/types/transactions.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use near_primitives::hash::CryptoHash;
use near_primitives::types::AccountId;
use near_primitives::types::{AccountId, ShardId};
use serde_json::Value;

#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
Expand Down Expand Up @@ -54,17 +54,37 @@ pub enum RpcTransactionError {
#[error("The node reached its limits. Try again later. More details: {debug_info}")]
InternalError { debug_info: String },
#[error("Timeout")]
TimeoutError,
TimeoutError(TimeoutErrorCause),
}

#[derive(serde::Serialize, serde::Deserialize, Debug)]
#[derive(Clone, serde::Serialize, serde::Deserialize, Debug)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct RpcTransactionResponse {
#[serde(flatten)]
pub final_execution_outcome: Option<near_primitives::views::FinalExecutionOutcomeViewEnum>,
pub final_execution_status: near_primitives::views::TxExecutionStatus,
}

/// Explains why a transaction-status request returned a `RpcTransactionError::TimeoutError`:
/// it did not reach the requested `wait_until` finality within the node's polling timeout.
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
#[serde(tag = "cause", rename_all = "SCREAMING_SNAKE_CASE")]
pub enum TimeoutErrorCause {
/// The node never observed the transaction on chain.
NotObserved,
Comment thread
wacban marked this conversation as resolved.
/// The transaction was observed but is still pending the requested finality. The
/// last-known status is included so the caller can re-poll for a higher finality.
/// Boxed to keep `RpcTransactionError` small (it is the `Err` type of many RPC results).
Pending { status: Box<RpcTransactionResponse> },
/// The node does not track the transaction's shard and could not get an answer from a
/// chunk producer that does before the timeout.
DoesNotTrackShard { shard_id: ShardId },
/// The node could not produce a usable transaction status before the timeout (for
/// example a repeated internal error, or no response at all).
Error { debug_info: String },
}

#[derive(serde::Serialize, serde::Deserialize, Debug)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct RpcBroadcastTxSyncResponse {
Expand Down Expand Up @@ -140,3 +160,57 @@ impl From<RpcTransactionError> for crate::errors::RpcError {
Self::new_internal_or_handler_error(Some(error_data), error_data_value)
}
}

#[cfg(test)]
mod tests {
use super::*;
use near_primitives::views::TxExecutionStatus;

/// On timeout the RPC returns a `TimeoutError` whose `reason` says how far the
/// transaction got. The `Pending` reason carries the last-known status so callers
/// retain full information and can re-poll for a higher finality.
#[test]
fn timeout_error_reports_pending() {
let error = RpcTransactionError::TimeoutError(TimeoutErrorCause::Pending {
status: Box::new(RpcTransactionResponse {
final_execution_outcome: None,
final_execution_status: TxExecutionStatus::Included,
}),
});

// The cause and last-known status survive the conversion to the wire `RpcError`.
let rpc_error: crate::errors::RpcError = error.into();
let wire = serde_json::to_value(&rpc_error).unwrap();
assert_eq!(wire["cause"]["name"], "TIMEOUT_ERROR");
let info = &wire["cause"]["info"];
assert_eq!(info["cause"], "PENDING");
assert_eq!(info["status"]["final_execution_status"], "INCLUDED");
}

/// The `NotObserved` reason serializes as a bare tag with no payload.
#[test]
fn timeout_error_reports_not_observed() {
let error = RpcTransactionError::TimeoutError(TimeoutErrorCause::NotObserved);
let value = serde_json::to_value(&error).unwrap();
assert_eq!(value["name"], "TIMEOUT_ERROR");
assert_eq!(value["info"]["cause"], "NOT_OBSERVED");
}

/// The error round-trips through serde, including the flattened status carried by
/// `Pending`.
#[test]
fn timeout_error_round_trips() {
let error = RpcTransactionError::TimeoutError(TimeoutErrorCause::Pending {
status: Box::new(RpcTransactionResponse {
final_execution_outcome: None,
final_execution_status: TxExecutionStatus::Included,
}),
});
let json = serde_json::to_string(&error).unwrap();
let decoded: RpcTransactionError = serde_json::from_str(&json).unwrap();
assert!(matches!(
decoded,
RpcTransactionError::TimeoutError(TimeoutErrorCause::Pending { .. })
));
}
}
Loading
Loading