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

Cleanup - Various feature gates #31750

Closed

Conversation

Lichtso
Copy link
Contributor

@Lichtso Lichtso commented May 22, 2023

Problem

Since epoch 321 (almost a year ago) various features were activated (on all three clusters) but their feature gating logic was never cleaned up.

Summary of Changes

Removes feature gating logic of the following features:

  1. vote_withdraw_authority_may_change_authorized_voter
  2. limit_secp256k1_recovery_id
  3. spl_token_v3_4_0
  4. spl_associated_token_account_v1_1_0
  5. disable_fee_calculator
  6. vote_authorize_with_seed
  7. merge_nonce_error_into_system_error
  8. instructions_sysvar_owned_by_sysvar
  9. require_static_program_ids_in_transaction
  10. include_account_index_in_rent_error
  11. filter_votes_outside_slot_hashes
  12. reject_vote_account_close_unless_zero_credit_epoch
  13. bank_transaction_count_fix
  14. check_syscall_outputs_do_not_overlap

@Lichtso Lichtso force-pushed the cleanup/various_feature_gates branch from 14f4280 to e9d8885 Compare May 22, 2023 18:06
@CriesofCarrots
Copy link
Contributor

Doing this all in one PR seems like a bad idea to me

@Lichtso
Copy link
Contributor Author

Lichtso commented May 22, 2023

I can spit it up if you like, but opening 14 PRs did not seem so good either.

@Lichtso Lichtso force-pushed the cleanup/various_feature_gates branch 4 times, most recently from f3ba505 to 62158ad Compare May 22, 2023 19:12
@mvines
Copy link
Member

mvines commented May 22, 2023

I feel separate PRs is the way to go as well. I understand the pain, I know it'll take longer, etc. Reviews for each 14 PR should be pretty obvious though, certainly should tag the original feature-gate author for that review too.
The effort here is very much appreciated though, it's absolutely a net good!

@Lichtso Lichtso force-pushed the cleanup/various_feature_gates branch from 62158ad to 8c3ed89 Compare May 22, 2023 19:19
@CriesofCarrots
Copy link
Contributor

I can spit it up if you like, but opening 14 PRs did not seem so good either.

Yeah, what Michael said. Unless any of them are too intertwined with each other, I am in favor of 14 different PRs

@Lichtso Lichtso force-pushed the cleanup/various_feature_gates branch from 8c3ed89 to 21e185e Compare May 22, 2023 19:27
@Lichtso
Copy link
Contributor Author

Lichtso commented May 22, 2023

Ok, let me just get this PR through CI, then I will close it and reopen as split PRs.

@Lichtso Lichtso force-pushed the cleanup/various_feature_gates branch from 21e185e to dc05854 Compare May 22, 2023 19:32
@Lichtso Lichtso force-pushed the cleanup/various_feature_gates branch 2 times, most recently from 548539f to da22ebe Compare May 22, 2023 20:14
@Lichtso Lichtso force-pushed the cleanup/various_feature_gates branch from da22ebe to d687bb8 Compare May 22, 2023 21:11
@Lichtso Lichtso force-pushed the cleanup/various_feature_gates branch from d687bb8 to e95f530 Compare May 22, 2023 21:23
@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Merging #31750 (e95f530) into master (bc4d53c) will increase coverage by 0.0%.
The diff coverage is 85.8%.

@@           Coverage Diff            @@
##           master   #31750    +/-   ##
========================================
  Coverage    81.8%    81.8%            
========================================
  Files         736      736            
  Lines      205955   205733   -222     
========================================
- Hits       168631   168495   -136     
+ Misses      37324    37238    -86     

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