Skip to content

Conversation

@grarco
Copy link
Collaborator

@grarco grarco commented Nov 5, 2024

Describe your changes

Closes #3961.
Partially addresses #3964.

Improves the error logs of masp fee payment.

try_masp_fee_payment now short circuits and returns error on all failures (not only gas ones) avoiding the need for an extra Option in the returned type. This allows for a better logging of the failures to the user and makes the code simpler to follow.

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:

@grarco grarco marked this pull request as ready for review November 5, 2024 14:19
@grarco grarco closed this Nov 5, 2024
@grarco grarco reopened this Nov 5, 2024
@grarco grarco requested a review from sug0 November 5, 2024 14:26
@codecov
Copy link

codecov bot commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 65.90909% with 30 lines in your changes missing coverage. Please review.

Project coverage is 73.87%. Comparing base (a871c33) to head (836095a).
Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
crates/node/src/protocol.rs 60.52% 30 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3983      +/-   ##
==========================================
- Coverage   73.88%   73.87%   -0.01%     
==========================================
  Files         341      341              
  Lines      106444   106444              
==========================================
- Hits        78647    78640       -7     
- Misses      27797    27804       +7     

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

@brentstone brentstone added the merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass label Nov 5, 2024
mergify bot added a commit that referenced this pull request Nov 5, 2024
mergify bot added a commit that referenced this pull request Nov 5, 2024
@mergify mergify bot merged commit ccbd651 into main Nov 5, 2024
28 of 38 checks passed
@mergify mergify bot deleted the grarco/improve-masp-fee-messages branch November 5, 2024 17:22
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 non-breaking-change refactor / code quality UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Amount to transfer is 1000000 when unshielding with disposable-gas-payer but without gas-spending-key

4 participants