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

Rename/deprecate num_signatures to num_total_signatures #2200

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

apfitzge
Copy link

Problem

  • The num_signatures function name is confusing since it includes "actual" signatures on the transaction, as well as "pre-compile" signatures that are attached as instructions.

Summary of Changes

  • deprecate SantizedMessage::num_signatures
  • add new function SanitizedMessage::num_total_signatures
  • rename SVMMessage::num_signatures to SVMMessage::num_total_signatures to match

Fixes #

@apfitzge
Copy link
Author

Noticed this odd/misleading naming when working on stuff related to #1918

@apfitzge
Copy link
Author

Have not deprecated anything in SDK before; is this the correct way to do it?

Copy link

@bw-solana bw-solana left a comment

Choose a reason for hiding this comment

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

This looks correct to me. Should wait for @buffalojoec to confirm before merging

@apfitzge apfitzge self-assigned this Jul 19, 2024
@apfitzge
Copy link
Author

Want to use the new name in #2125; because we need a way to distinguish so that we get the right number to inspect the Signatures on transaction.

@apfitzge apfitzge merged commit bd754bb into anza-xyz:master Jul 22, 2024
51 checks passed
@apfitzge apfitzge deleted the num_signatures_rename branch July 22, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants