Skip to content

dump tx output for reveal_pk transactions when requested#4061

Merged
mergify[bot] merged 2 commits intonamada-net:mainfrom
dan-u410:dan/dump-reveal-pk
Nov 21, 2024
Merged

dump tx output for reveal_pk transactions when requested#4061
mergify[bot] merged 2 commits intonamada-net:mainfrom
dan-u410:dan/dump-reveal-pk

Conversation

@dan-u410
Copy link
Copy Markdown
Contributor

Describe your changes

It is currently not possible to generate the reveal_pk transaction offline. This PR will dump the transaction when requested.

Checklist before merging

  • If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
    • This will require 2 reviewers to approve the changes
  • If this PR requires changes to the docs or specs, a corresponding PR is opened in the namada-docs repo
    • Relevant PR if applies:
  • If this PR affects services such as namada-indexer or namada-masp-indexer, a corresponding PR is opened in that repo
    • Relevant PR if applies:

@dan-u410
Copy link
Copy Markdown
Contributor Author

dan-u410 commented Nov 20, 2024

there are a few ways to implement this so wanted to add color to my choices. happy to change -

  • to construct the reveal_pk transaction body, the code calls out to a helper submit_reveal_aux
  • this method has some internal complexity and is used elsewhere so I dont want to change that function
  • so I chose to isolate the complexity here to just the reveal-pk subcommand and call out to build_reveal_pk directly when users request to get the raw tx json

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.31%. Comparing base (4938fa3) to head (3420f00).
Report is 83 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4061      +/-   ##
==========================================
+ Coverage   73.91%   74.31%   +0.40%     
==========================================
  Files         341      341              
  Lines      106496   107401     +905     
==========================================
+ Hits        78718    79817    +1099     
+ Misses      27778    27584     -194     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

if let Address::Implicit(ImplicitAddress(pkh)) =
&(&args.public_key).into()
{
let public_key = &namada
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we don't need to look-up the PK as we already have it in args.public_key

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fixed in aaf7679

Copy link
Copy Markdown
Collaborator

@tzemanovic tzemanovic left a comment

Choose a reason for hiding this comment

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

thx for the fix!

@tzemanovic tzemanovic added the merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass label Nov 21, 2024
mergify bot added a commit that referenced this pull request Nov 21, 2024
@mergify mergify bot merged commit 48f3ccd into namada-net:main Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants