Config Validation #7341
Open
Config Validation #7341
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chong-he
added
enhancement
macladson
requested changes
Apr 21, 2025
macladson
added
work-in-progress
|
I'll do that, as well as take a look at the tests. Appreciate the review! |
Comment on lines
+1725
to
+1733
| pub fn validate_extra_fields(&self) -> Result<(), String> { | ||
| for key in self.future_fields.keys() { | ||
| if !FUTURE_FIELDS.contains(&key.as_str()) { | ||
| return Err(format!("Unknown or disallowed field found: {}", key)); | ||
| } | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
I like this approach. Simple and flexible.
Let me know if you get stuck fixing any of the tests
|
Alright, I fixed up the tests. Most of the issues were simply because the validation and removal of defaults meant I had to add the missing fields in the existing configs. For this, I ended up using the original default values to maintain the same functionality as before. |
|
Some required checks have failed. Could you please take a look @Patchoulis? 🙏 |
mergify
bot
added
the
waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Addressed
#6208
Proposed Changes
A few changes. First, as discussed in the discord, the defaults have been removed to ensure that all expected fields are provided.
Second, I do validate that no additional values are present. However, as discussed with sproul, there are some legitimate reasons to add some for future forks. I ended up solving this by adding a small modifiable constant which allows specified optional fields to exist.
I'm doing some tests to ensure presets like SLOTS_PER_EPOCH are also disallowed, but this should also solve that problem.
Additional Info
Please provide any additional information. For example, future considerations
or information useful for reviewers.