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

Deprecate confirmTransaction, getSignatureStatus, and getSignatureConfirmation #9298

Merged
merged 2 commits into from
Apr 5, 2020

Conversation

jstarry
Copy link
Contributor

@jstarry jstarry commented Apr 4, 2020

Problem

Various client JSON RPC libraries appear to have problems processing the status: { "Ok": null } transaction status returned by methods such as getSignatureStatuses. The null value causes the removal of Ok entirely and the client ends up with a status object of {}.

We don't know who is using these two endpoints so we'll have to support them for now, but we should actively discourage anyone from using them from this point forward.

Summary of Changes

  • Remove all uses of these endpoints from our clients
  • Remove docs
  • Mark the endpoints as deprecated in the code

Related to: #9297

@jstarry jstarry changed the title Deprecate confirmTransaction and getSignatureStatus Deprecate confirmTransaction, getSignatureStatus, and getSignatureConfirmation Apr 4, 2020
@codecov
Copy link

codecov bot commented Apr 4, 2020

Codecov Report

Merging #9298 into master will decrease coverage by 0.0%.
The diff coverage is 15.0%.

@@           Coverage Diff            @@
##           master   #9298     +/-   ##
========================================
- Coverage    80.9%   80.8%   -0.1%     
========================================
  Files         276     276             
  Lines       61208   61193     -15     
========================================
- Hits        49551   49490     -61     
- Misses      11657   11703     +46     

@jstarry jstarry requested review from mvines and CriesofCarrots April 4, 2020 18:14
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Just one naming nit

)
.map_err(|err| err.into_with_command("ConfirmTransaction"))?;
let Response { context, value } =
self.get_signature_statuses(&[*signature], commitment_config)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Update method name as per suggestion below

@mvines mvines added the v1.1 label Apr 4, 2020
mvines
mvines previously approved these changes Apr 4, 2020
@mergify mergify bot dismissed mvines’s stale review April 5, 2020 04:45

Pull request has been modified.

@mvines mvines added automerge Merge this Pull Request automatically once CI passes v1.0 and removed v1.0 labels Apr 5, 2020
@solana-grimes solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label Apr 5, 2020
@solana-grimes
Copy link
Contributor

💔 Unable to automerge due to CI failure

@jstarry jstarry merged commit b584174 into solana-labs:master Apr 5, 2020
@jstarry jstarry deleted the deprecate-apis branch April 5, 2020 06:31
mergify bot pushed a commit that referenced this pull request Apr 5, 2020
…reConfirmation` (#9298)

* Deprecate `confirmTransaction`, `getSignatureStatus`, etc

* Rename get_signature_statuses to get_signature_statuses_with_commitment

Co-authored-by: Michael Vines <mvines@gmail.com>
(cherry picked from commit b584174)

# Conflicts:
#	client/src/rpc_client.rs
mergify bot pushed a commit that referenced this pull request Apr 5, 2020
…reConfirmation` (#9298)

* Deprecate `confirmTransaction`, `getSignatureStatus`, etc

* Rename get_signature_statuses to get_signature_statuses_with_commitment

Co-authored-by: Michael Vines <mvines@gmail.com>
(cherry picked from commit b584174)
solana-grimes pushed a commit that referenced this pull request Apr 5, 2020
solana-grimes pushed a commit that referenced this pull request Apr 5, 2020
#[rpc(meta, name = "getSignatureStatuses")]
fn get_signature_statuses(
fn get_signature_statuses_with_commitment(
Copy link
Contributor

Choose a reason for hiding this comment

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

I only intended this name change in rpc_client. This is actually not consistent with the rest of the rpc methods. Will revert as part of transaction-history work.

danpaul000 pushed a commit to danpaul000/solana that referenced this pull request Jul 8, 2020
…reConfirmation` (solana-labs#9298)

* Deprecate `confirmTransaction`, `getSignatureStatus`, etc

* Rename get_signature_statuses to get_signature_statuses_with_commitment

Co-authored-by: Michael Vines <mvines@gmail.com>
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.

4 participants