Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Add custom error for tx-history queries when node does not support #17494

Merged

Conversation

CriesofCarrots
Copy link
Contributor

Problem

Clients making RPC queries for historical data cannot distinguish between a node that has enable_rpc_transaction_history set to false and one that simply no longer has the data retained ledger.

Summary of Changes

Add custom rpc error to return when a historical query is received (getBlock, getTransaction, or getSignatureStatuses with searchTransactionHistory: true) and !config.enable_rpc_transaction_history

Fixes #9316

@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #17494 (67b8878) into master (7e433dd) will increase coverage by 0.0%.
The diff coverage is 88.8%.

@@           Coverage Diff           @@
##           master   #17494   +/-   ##
=======================================
  Coverage    82.6%    82.6%           
=======================================
  Files         425      425           
  Lines      118842   118863   +21     
=======================================
+ Hits        98261    98286   +25     
+ Misses      20581    20577    -4     

@jstarry
Copy link
Contributor

jstarry commented May 26, 2021

looks good, should we add it for get_confirmed_signatures_for_address too or was that intentionally excluded?

@CriesofCarrots
Copy link
Contributor Author

CriesofCarrots commented May 26, 2021

looks good, should we add it for get_confirmed_signatures_for_address too or was that intentionally excluded?

Yes, I did intentionally exclude it since that method is deprecated. Happy to reconsider, if you feel strongly about it @jstarry .
Merging now; can make follow-up if needed.

@CriesofCarrots CriesofCarrots merged commit 6abe089 into solana-labs:master May 26, 2021
mergify bot pushed a commit that referenced this pull request May 26, 2021
…17494)

(cherry picked from commit 6abe089)

# Conflicts:
#	core/src/rpc.rs
@jstarry
Copy link
Contributor

jstarry commented May 26, 2021

looks good, should we add it for get_confirmed_signatures_for_address too or was that intentionally excluded?

Yes, I did intentionally exclude it since that method is deprecated. Happy to reconsider, if you feel strongly about it @jstarry .
Merging now; can make follow-up if needed.

Nope that's fine!

mergify bot added a commit that referenced this pull request May 26, 2021
…ackport #17494) (#17522)

* Add custom error for tx-history queries when node does not support (#17494)

(cherry picked from commit 6abe089)

# Conflicts:
#	core/src/rpc.rs

* Fix conflicts

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
Co-authored-by: Tyera Eulberg <tyera@solana.com>
@CriesofCarrots CriesofCarrots deleted the rpc-tx-hist-config branch July 28, 2021 22:32
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RPC does not expose enable_rpc_transaction_history config setting
3 participants