Skip to content

feat: handle minimal tx response for NONE/INCLUDED wait_until#128

Merged
r-near merged 8 commits intomainfrom
fix/transaction-response-variants
Mar 3, 2026
Merged

feat: handle minimal tx response for NONE/INCLUDED wait_until#128
r-near merged 8 commits intomainfrom
fix/transaction-response-variants

Conversation

@r-near
Copy link
Copy Markdown
Contributor

@r-near r-near commented Feb 28, 2026

Summary

  • Intercepts InvalidResponsePayload errors in send_impl when the RPC returns a minimal response (only final_execution_status) for wait_until set to NONE or INCLUDED
  • Parses the minimal response and returns TransactionResult::Pending { status } instead of propagating a transport error
  • Wraps full execution responses in TransactionResult::Final(...)
  • Adds is_failure(), is_success(), and assert_failure() delegate methods to TransactionResult

Context

When wait_until is NONE or INCLUDED, the NEAR RPC returns:

{"jsonrpc":"2.0","result":{"final_execution_status":"INCLUDED"},"id":"0"}

The openapi client can't deserialize this into RpcTransactionResponse (which expects full execution data like receipts_outcome, status, etc.), so it returns InvalidResponsePayload. Previously this surfaced as a TransportError to callers.

Refs: near/near-openapi-client-rs#40

Test plan

  • Verify cargo check and cargo check --tests pass
  • Test with wait_until(TxExecutionStatus::None) and wait_until(TxExecutionStatus::Included) against testnet
  • Verify existing tests still pass with wait_until(TxExecutionStatus::Final) (default)

When `wait_until` is set to `NONE` or `INCLUDED`, the NEAR RPC returns a
minimal response containing only `final_execution_status` without full
execution data. The openapi client fails to deserialize this into
`RpcTransactionResponse` and returns `InvalidResponsePayload`.

This change intercepts that error in `send_impl`, parses the minimal
response, and returns `TransactionResult::Pending { status }` instead of
propagating a transport error. Full responses are wrapped in
`TransactionResult::Final(...)`.

Also adds `is_failure()`, `is_success()`, and `assert_failure()` delegate
methods to `TransactionResult` for test compatibility.

Refs: near/near-openapi-client-rs#40
Copilot AI review requested due to automatic review settings February 28, 2026 04:04
@r-near r-near requested review from a team and akorchyn as code owners February 28, 2026 04:04
@github-project-automation github-project-automation bot moved this to NEW❗ in DevTools Feb 28, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the transaction sending flow to correctly handle NEAR RPC’s minimal transaction response returned for wait_until = NONE/INCLUDED, avoiding a surfaced transport error and instead returning a structured pending result.

Changes:

  • Introduces TransactionResult (Pending { status } vs Final(ExecutionFinalResult)) plus helper/delegate methods in types.
  • Updates ExecuteSignedTransaction::send_to* to return TransactionResult and parses minimal RPC responses from InvalidResponsePayload.
  • Wraps full RPC execution responses into TransactionResult::Final(...) while returning Pending for minimal responses.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
types/src/transaction/result.rs Adds TransactionResult and TransactionResultError plus helper methods (is_*, assert_*, into_result).
api/src/common/send.rs Changes send APIs to return TransactionResult and adds minimal-response parsing fallback for NONE/INCLUDED.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Fix cargo-fmt formatting issues
- Box large enum variants (TransactionResult::Final, TransactionResultError::Failure) to satisfy clippy::large_enum_variant
- Make pending_status() const per clippy::missing_const_for_fn
- Gate minimal response fallback on wait_until being NONE/INCLUDED per review feedback
- Add transaction() and logs() delegation methods to TransactionResult
@r-near r-near marked this pull request as draft February 28, 2026 04:20
Fixes MSRV CI failures caused by deflate64 v0.1.11 using `unbounded_shr`
(stabilized in Rust 1.87). near-sandbox 0.3.7 pins deflate64 < 0.1.11.
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 1, 2026

Codecov Report

❌ Patch coverage is 23.57724% with 94 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.90%. Comparing base (17f9c3c) to head (2b6b76a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
types/src/transaction/result.rs 16.21% 62 Missing ⚠️
api/src/common/send.rs 34.69% 32 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
- Coverage   51.58%   50.90%   -0.69%     
==========================================
  Files          40       40              
  Lines        5172     5265      +93     
==========================================
+ Hits         2668     2680      +12     
- Misses       2504     2585      +81     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@r-near r-near marked this pull request as ready for review March 2, 2026 11:29
Copy link
Copy Markdown
Contributor

@vsavchyn-dev vsavchyn-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few questions regarding naming, but it works with all TxExecutionStatus-es from what I checked without giving me error on 'None', 'Included', and SemiFinal ones

/// - Higher finality levels (`ExecutedOptimistic`, `Final`, etc.) will return
/// [`TransactionResult::Final`] with the full execution outcome.
#[derive(Clone, Debug)]
#[must_use]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe current message is confusing:

Image

Maybe we should write something similar to this:

#[must_use = "use `into_result()` to handle potential execution errors and cases when transaction is pending"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/// The `status` field indicates how far the transaction has progressed.
Pending { status: TxExecutionStatus },
/// Full execution result is available.
Final(Box<ExecutionFinalResult>),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we call it 'Full'?

It might be only me, but I get confused between 'TxExecutionStatus::Final' and 'TxResult::Final', as when we wait for 'TxExecutionStatus::ExecutedOptimistic', it will provide us with valid result, but it won't be "Final" per say in some cases.

Also, I might be wrong about naming conventions from nearcore RPC types so correct me if I'm wrong

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, renamed it here: e1c727e (this PR)

Comment on lines +421 to +427
/// Returns the pending status, if the transaction is still pending.
pub const fn pending_status(&self) -> Option<&TxExecutionStatus> {
match self {
Self::Pending { status } => Some(status),
Self::Final(_) => None,
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Returns the pending status, if the transaction is still pending.
pub const fn pending_status(&self) -> Option<&TxExecutionStatus> {
match self {
Self::Pending { status } => Some(status),
Self::Final(_) => None,
}
}
/// Returns the pending status, if the transaction is still pending.
pub const fn pending_status(self) -> Option<TxExecutionStatus> {
match self {
Self::Pending { status } => Some(status),
Self::Final(_) => None,
}
}

I think this function should work without ref to TxExecutionStatus? Or maybe there is some other reason to have ref here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Fixed it: 2b6b76a (this PR)

r-near added 3 commits March 2, 2026 17:50
The bare #[must_use] attribute produces a confusing compiler warning.
Add an explicit message guiding users to `into_result()` for handling
both execution errors and pending transaction cases.
Avoids confusion with TxExecutionStatus::Final which refers to NEAR
finality level. TransactionResult::Full means "full execution data is
available" regardless of which finality level was requested.
TxExecutionStatus is Copy, so returning Option<&TxExecutionStatus>
via &self is unnecessary. Take self by value and return
Option<TxExecutionStatus> directly.
Copy link
Copy Markdown
Contributor

@vsavchyn-dev vsavchyn-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@r-near r-near merged commit 95b6b3a into main Mar 3, 2026
14 checks passed
@r-near r-near deleted the fix/transaction-response-variants branch March 3, 2026 16:12
@github-project-automation github-project-automation bot moved this from NEW❗ to Shipped 🚀 in DevTools Mar 3, 2026
@frol frol mentioned this pull request Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Shipped 🚀

Development

Successfully merging this pull request may close these issues.

4 participants