followup safety checks for #23295 #23321
Conversation
runtime/src/builtins.rs
Outdated
| BuiltinFeatureTransition::new_remove_or_retain( | ||
| "secp256k1_program", | ||
| solana_sdk::secp256k1_program::id(), | ||
| dummy_process_instruction, | ||
| feature_set::secp256k1_program_enabled::id(), | ||
| feature_set::prevent_calling_precompiles_as_programs::id(), | ||
| ), |
There was a problem hiding this comment.
Please construct the enum variants directly here without the constructor methods. The constructor methods make it difficult to know what each feature arg is used for.
There was a problem hiding this comment.
hmm I was trying to avoid exposing implementation details. Figured it's fine because all of the behavior is selfcontained
There was a problem hiding this comment.
I think my original version was fine
There was a problem hiding this comment.
Sigh. I can hold my nose for a Rust release cycle
There was a problem hiding this comment.
Ha, sorry man. I just don't trust that things won't get refactored and broken again before it's all removed.
t-nelson
force-pushed
the
autotrait-resovler-break
branch
from
b3b946f to
020dd1c
Compare
t-nelson
force-pushed
the
autotrait-resovler-break
branch
from
020dd1c to
a219b70
Compare
Codecov Report
@@ Coverage Diff @@
## master #23321 +/- ##
=======================================
Coverage 81.3% 81.3%
=======================================
Files 571 572 +1
Lines 155738 155786 +48
=======================================
+ Hits 126682 126757 +75
+ Misses 29056 29029 -27 |
2 participants
Problem
#23295 isn't as safe as it could be
Summary of Changes
add marker trait with trait bounds on our explicit traits to ensure inner type conforms
h/t @jstarry