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

Avoid duplicated signatures when building transactions #4230

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

grarco
Copy link
Collaborator

@grarco grarco commented Jan 10, 2025

Describe your changes

Closes #4203.

This PR modifies adds a new method on the Tx type to remove duplicated sections. This function is then called in the signing function before producing the wrapper signature (which is computed on all the sections) and the dump_tx function (in case the dumped tx needed to be signed offline).

To support this logic the PartialEq implementation for the Authorization type is now manually provided and ignores the way the signer is specified, focusing instead on the targets and the actual signature.

As a side note, also the PartialEq implementation for SigningTxData has been updated to account for the shielded hash.

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 January 10, 2025 19:46
@grarco grarco requested review from tzemanovic and murisi January 10, 2025 19:47
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 97.14286% with 2 lines in your changes missing coverage. Please review.

Project coverage is 74.61%. Comparing base (9527ea9) to head (7430933).

Files with missing lines Patch % Lines
crates/tx/src/section.rs 93.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4230      +/-   ##
==========================================
+ Coverage   74.59%   74.61%   +0.02%     
==========================================
  Files         342      342              
  Lines      108724   108787      +63     
==========================================
+ Hits        81101    81175      +74     
+ Misses      27623    27612      -11     

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

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.

Duplicated signatures in transactions
1 participant