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

poh_verify => run_verification: Rename to be more accurate #30811

Merged
merged 1 commit into from
Mar 22, 2023
Merged

poh_verify => run_verification: Rename to be more accurate #30811

merged 1 commit into from
Mar 22, 2023

Conversation

ilya-bobyr
Copy link
Contributor

Problem

poh_verify actually disables transaction signature, tick count and built in program argument verifications as well. It is somewhat confusing to call it poh_verify.

Summary of Changes

Renamed it to run_verification, along with the CLI argument.
Old argument is still accepted, but produces a deprecation message, suggesting a new name.

@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Merging #30811 (ee1fd37) into master (f4cde7a) will increase coverage by 0.0%.
The diff coverage is 55.5%.

@@           Coverage Diff           @@
##           master   #30811   +/-   ##
=======================================
  Coverage    81.4%    81.4%           
=======================================
  Files         725      725           
  Lines      203465   203465           
=======================================
+ Hits       165740   165748    +8     
+ Misses      37725    37717    -8     

KirillLykov
KirillLykov previously approved these changes Mar 21, 2023
ledger-tool/src/main.rs Outdated Show resolved Hide resolved
KirillLykov
KirillLykov previously approved these changes Mar 22, 2023
`poh_verify` actually disables transaction signature, tick count and
built in program argument verifications as well.  It is somewhat
confusing to call it `poh_verify`.
@ilya-bobyr
Copy link
Contributor Author

I just realized that there is also a skip-poh-verify among the validator arguments.
Added a deprecation warning there and provided skip-verification as an alternative.

Copy link
Contributor

@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.

LGTM

@ilya-bobyr ilya-bobyr merged commit 809041b into solana-labs:master Mar 22, 2023
@ilya-bobyr ilya-bobyr deleted the pr/rename-poh_verify branch March 22, 2023 18:03
ilya-bobyr added a commit that referenced this pull request Mar 27, 2023
Follow-up for

  commit 809041b
  Author: Illia Bobyr <[email protected]>
  Date:   Wed Mar 22 11:03:30 2023 -0700

      poh_verify => run_verification: Rename to be more accurate (#30811)

      `poh_verify` actually disables transaction signature, tick count and
      built in program argument verifications as well.  It is somewhat
      confusing to call it `poh_verify`.
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.

4 participants