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

add cli to get denom hash from trace path and base denom #679

Merged
merged 17 commits into from
Jan 10, 2022
+772 −35
Merged

add cli to get denom hash from trace path and base denom #679

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
eec0b8f
get denom hash from denom trace
nir1218 Jan 3, 2022
e917922
revert .gitignore
nir1218 Jan 3, 2022
3584e0c
final revert
nir1218 Jan 3, 2022
f9a4363
ignore history files
nir1218 Jan 3, 2022
3d926c4
Merge branch 'main' into main
nir1218 Jan 5, 2022
e349d05
Update modules/apps/transfer/client/cli/query.go
nir1218 Jan 5, 2022
b44c150
Update modules/apps/transfer/keeper/grpc_query.go
nir1218 Jan 5, 2022
9d482f2
Update modules/apps/transfer/keeper/keeper.go
nir1218 Jan 5, 2022
c78e7ee
Update modules/apps/transfer/keeper/grpc_query.go
nir1218 Jan 5, 2022
dbd8286
code review feedback integration
nir1218 Jan 6, 2022
61bc8b8
moved new feature to unreleased section
nir1218 Jan 6, 2022
d0ffb51
fix duplicate block in success test case
nir1218 Jan 6, 2022
4ef0da1
fix CHANGELOG format
nir1218 Jan 7, 2022
c5e6d3b
Merge branch 'cosmos:main' into main
nir1218 Jan 7, 2022
0376c4e
Merge branch 'cosmos:main' into main
nir1218 Jan 8, 2022
5c96dd8
fix invalid trace path argument error handling
nir1218 Jan 8, 2022
135650e
refactor positive test case code
nir1218 Jan 8, 2022
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
Prev Previous commit
Next Next commit
fix duplicate block in success test case
@nir1218
nir1218 committed Jan 6, 2022
commit d0ffb51f5fbca143334119dc0396a47eef9a87fc
3 changes: 0 additions & 3 deletions modules/apps/transfer/keeper/grpc_query_test.go
View file
Original file line number Diff line number Diff line change
@@ -179,9 +179,6 @@ func (suite *KeeperTestSuite) TestQueryDenomHash() {
{
"success",
func() {
req = &types.QueryDenomHashRequest{
Trace: reqTrace.GetFullDenomPath(),
}
suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace(suite.chainA.GetContext(), reqTrace)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally the success case doesn't have to perform any functionality. This allows you to be certain the failing cases would normally succeed except in the scenario specified.

I would set the req to a default before calling tc.malleate in the for loop, I'd also set the denom trace in state. Then the case "not found denom trace" would delete the denom trace in state (or set it to a different trace that doesn't exist in state)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@colin-axner @codecov-commenter I need further clarification on this point. I mistakenly put the request block before and after setting the state. If I removed the duplicate before setting the state would that be good; there is no significant difference between the test method I wrote and the one for GetDemonTrace.

Copy link
Contributor

Choose a reason for hiding this comment

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

Checkout the suggestion I put below. Don't commit that with the github UX because the formatting is probably wrong. But the main difference is adding:

                       req = &types.QueryDenomHashRequest{
					Trace: reqTrace.GetFullDenomPath(),
			}
			suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace(suite.chainA.GetContext(), reqTrace)

in the for loop before tc.malleate()


req = &types.QueryDenomHashRequest{