Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: return 404 on non-existing tx #10992

Merged
merged 6 commits into from
Jan 24, 2022
Merged

fix: return 404 on non-existing tx #10992

merged 6 commits into from
Jan 24, 2022

Conversation

alexanderbez
Copy link
Contributor

Description

Closes: #10985

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@alexanderbez alexanderbez added backport/0.44.x C: gRPC Issues and PRs related to the gRPC service and HTTP gateway. labels Jan 20, 2022
alexanderbez
alexanderbez commented Jan 20, 2022
View reviewed changes
x/auth/tx/service.go
@@ -135,6 +135,10 @@ func (s txServer) GetTx(ctx context.Context, req *txtypes.GetTxRequest) (*txtype
// https://github.com/cosmos/cosmos-sdk/issues/7036.
result, err := QueryTx(s.clientCtx, req.Hash)
if err != nil {
if strings.Contains(err.Error(), "not found") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked and I don't think TM returns a typed error here.

Copy link
Member

Choose a reason for hiding this comment

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

want to open an issue in tendermint for this? or you may have already?

Copy link
Contributor

Choose a reason for hiding this comment

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

If yes, let's link that issue in an inline comment here

tac0turtle
tac0turtle approved these changes Jan 20, 2022
View reviewed changes
Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

utACK

amaury1093
amaury1093 reviewed Jan 21, 2022
View reviewed changes
Copy link
Contributor

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

Can we add a test?

x/auth/tx/service.go
@@ -135,6 +135,10 @@ func (s txServer) GetTx(ctx context.Context, req *txtypes.GetTxRequest) (*txtype
// https://github.com/cosmos/cosmos-sdk/issues/7036.
result, err := QueryTx(s.clientCtx, req.Hash)
if err != nil {
if strings.Contains(err.Error(), "not found") {
Copy link
Contributor

Choose a reason for hiding this comment

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

If yes, let's link that issue in an inline comment here

@alexanderbez
Copy link
Contributor Author

Can we add a test?

We already have tests testing this case...sort-of. I just updated them to check for the correct error messages.

@alexanderbez alexanderbez merged commit 1581289 into master Jan 24, 2022
@alexanderbez alexanderbez deleted the bez/10985-404-tx branch January 24, 2022 15:44
mergify bot pushed a commit that referenced this pull request Jan 24, 2022
@alexanderbez
fix: return 404 on non-existing tx (#10992)
e726c49 
(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md
@mergify mergify bot mentioned this pull request Jan 24, 2022
Merged
mergify bot pushed a commit that referenced this pull request Jan 24, 2022
@alexanderbez
fix: return 404 on non-existing tx (#10992)
27f9691 
(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md
@mergify mergify bot mentioned this pull request Jan 24, 2022
Merged
tac0turtle pushed a commit that referenced this pull request Jan 25, 2022
@mergify @alexanderbez
fix: return 404 on non-existing tx (backport #10992) (#11014)
7ecf4d4 
* fix: return 404 on non-existing tx (#10992)

(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
tac0turtle pushed a commit that referenced this pull request Jan 25, 2022
@mergify @alexanderbez
fix: return 404 on non-existing tx (backport #10992) (#11015)
e9b2db1 
* fix: return 404 on non-existing tx (#10992)

(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
@faddat faddat mentioned this pull request Feb 28, 2022
Closed
8 tasks
Cashmaney pushed a commit to scrtlabs/cosmos-sdk that referenced this pull request Mar 16, 2022
@mergify @alexanderbez @Cashmaney
fix: return 404 on non-existing tx (backport cosmos#10992) (cosmos#11015
72d38f8 
)

* fix: return 404 on non-existing tx (cosmos#10992)

(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
zakir-code pushed a commit to PundiAI/cosmos-sdk that referenced this pull request Apr 8, 2022
@mergify @alexanderbez
fix: return 404 on non-existing tx (backport cosmos#10992) (cosmos#11014
4cb1461 
)

* fix: return 404 on non-existing tx (cosmos#10992)

(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
jiujiteiro pushed a commit to realiotech/cosmos-sdk that referenced this pull request May 6, 2022
@mergify @alexanderbez @jiujiteiro
fix: return 404 on non-existing tx (backport cosmos#10992) (cosmos#11015
749973f 
)

* fix: return 404 on non-existing tx (cosmos#10992)

(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
jiujiteiro pushed a commit to realiotech/cosmos-sdk that referenced this pull request May 10, 2022
@mergify @alexanderbez @jiujiteiro
fix: return 404 on non-existing tx (backport cosmos#10992) (cosmos#11015
dfd3a4d 
)

* fix: return 404 on non-existing tx (cosmos#10992)

(cherry picked from commit 1581289)

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
JimLarson pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Jul 7, 2022
@mergify @alexanderbez @JimLarson
fix: return 404 on non-existing tx (backport cosmos#10992) (cosmos#11014
6fb70ac 
)

* fix: return 404 on non-existing tx (cosmos#10992)

(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Eengineer1 pushed a commit to cheqd/cosmos-sdk that referenced this pull request Aug 26, 2022
@mergify @alexanderbez @Eengineer1
fix: return 404 on non-existing tx (backport cosmos#10992) (cosmos#11014
15e453f 
)

* fix: return 404 on non-existing tx (cosmos#10992)

(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
JeancarloBarrios pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Sep 28, 2024
@mergify @alexanderbez @JeancarloBarrios
fix: return 404 on non-existing tx (backport cosmos#10992) (cosmos#11014
429b7d1 
)

* fix: return 404 on non-existing tx (cosmos#10992)

(cherry picked from commit 1581289)

# Conflicts:
#	CHANGELOG.md

* fix cl

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amaury1093 amaury1093 amaury1093 left review comments

@fedekunze fedekunze fedekunze approved these changes

@tac0turtle tac0turtle tac0turtle approved these changes

@aaronc aaronc Awaiting requested review from aaronc

Assignees
No one assigned
Labels
C: gRPC Issues and PRs related to the gRPC service and HTTP gateway. C:x/auth
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The /cosmos/tx/v1beta1/txs/{hash} endpoint should return a 404 error if no tx is found for the given hash
4 participants
@alexanderbez @amaury1093 @tac0turtle @fedekunze