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.

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 confidential transfer queries to seid #1931

Merged
merged 14 commits into from
Nov 15, 2024
Merged

Add confidential transfer queries to seid #1931

merged 14 commits into from
Nov 15, 2024

Conversation

mj850
Copy link
Contributor

@mj850 mj850 commented Nov 14, 2024

Describe your changes and provide context

This PR adds seid queries for the confidential transfers module.

Testing performed to validate your change

@dssei
Copy link
Contributor

dssei commented Nov 14, 2024

lgtm. Similarly (to another cli pr), let's add tests where possible

Short: "Query the account state",
Long: "Queries the account state associated with the address and denom." +
"Pass the --from flag to decrypt the account." +
"Pass the --decryptAvailableBalance flag to attempt to decrypt the available balance.",
Copy link
Contributor

@dssei dssei Nov 14, 2024

Choose a reason for hiding this comment

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

usually multi word flags I think are concatenated with - e.g.
--decrypt-available-balance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to --decrypt-available-balance

"Pass the --from flag to decrypt the account." +
"Pass the --decryptAvailableBalance flag to attempt to decrypt the available balance.",
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think we were moving those functions to separate methods in tx. Might make our code consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to separate functions for both

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 9.25926% with 49 lines in your changes missing coverage. Please review.

Project coverage is 60.12%. Comparing base (32c89cd) to head (6614143).
Report is 36 commits behind head on feature/ct_types.

Files with missing lines Patch % Lines
x/confidentialtransfers/types/account.go 0.00% 29 Missing ⚠️
x/confidentialtransfers/types/transfer.go 0.00% 13 Missing ⚠️
x/confidentialtransfers/types/codec.go 25.00% 6 Missing ⚠️
x/confidentialtransfers/keeper/msg_server.go 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                  @@
##           feature/ct_types    #1931      +/-   ##
====================================================
- Coverage             61.36%   60.12%   -1.24%     
====================================================
  Files                   263      280      +17     
  Lines                 23306    25290    +1984     
====================================================
+ Hits                  14301    15206     +905     
- Misses                 8001     8951     +950     
- Partials               1004     1133     +129     
Files with missing lines Coverage Δ
x/confidentialtransfers/keeper/keeper.go 56.12% <100.00%> (ø)
x/confidentialtransfers/types/keys.go 0.00% <ø> (ø)
x/confidentialtransfers/keeper/msg_server.go 73.23% <50.00%> (ø)
x/confidentialtransfers/types/codec.go 36.36% <25.00%> (ø)
x/confidentialtransfers/types/transfer.go 0.00% <0.00%> (ø)
x/confidentialtransfers/types/account.go 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

@mj850 mj850 merged commit f8e5fac into feature/ct_types Nov 15, 2024
28 of 29 checks passed
@mj850 mj850 deleted the mj/ctMod3 branch November 15, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants