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

toml config, take 2 #298

Merged
merged 19 commits into from
May 31, 2024
Merged

toml config, take 2 #298

merged 19 commits into from
May 31, 2024

Conversation

karmacoma-eth
Copy link
Collaborator

@karmacoma-eth karmacoma-eth commented May 30, 2024

New design principles

  • names, types and default values defined in a plain Config dataclass
    • easy to parse, pickle and pass around, with well defined property names
    • Config dataclasses are immutable
  • config file loaded separately through basic toml, applies overrides to the default Config
  • the arg parser can be generated from the Config dataclass and has no defaults, it applies overrides to the config object again
  • the arg parser can still be reused for contract level and test level dev docs, and should require no copy because it has no defaults (so it only adds overrides for explicit parameters)

Advantages:

  • no external dependencies
  • the Config dataclass is the source of truth for parameter names, values, types and defaults
    • easy to go to the definition of a setting
  • the override order is very clear and explicit
  • Config objects are aware of their parents, so when you request a value from a child, it checks first in itself and falls back to its parent
    • 👍 no copies
    • 👍 explainable (you can print a test config and see the source for each parameter — default, file, CLI, contract or test devdocs)
  • should be easy to generate a sample config file with up to date defaults too

@karmacoma-eth
Copy link
Collaborator Author

different take on #125

@karmacoma-eth karmacoma-eth requested a review from daejunpark May 30, 2024 16:50
libs: Dict,
) -> Exec:
setup_timer = NamedTimer("setup")
setup_timer.create_subtimer("decode")

sevm = SEVM(mk_options(args))
sevm = SEVM(args)
Copy link
Collaborator

Choose a reason for hiding this comment

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

😍

src/halmos/config.py Outdated Show resolved Hide resolved
src/halmos/config.py Outdated Show resolved Hide resolved
src/halmos/sevm.py Show resolved Hide resolved
src/halmos/config.py Show resolved Hide resolved
src/halmos/config.py Outdated Show resolved Hide resolved
src/halmos/config.py Show resolved Hide resolved
src/halmos/config.py Show resolved Hide resolved
src/halmos/sevm.py Show resolved Hide resolved
@karmacoma-eth
Copy link
Collaborator Author

I'll add a full halmos.toml example in a follow up

@karmacoma-eth karmacoma-eth merged commit c101af3 into main May 31, 2024
53 checks passed
@karmacoma-eth karmacoma-eth deleted the toml-config branch May 31, 2024 20:57
pcaversaccio added a commit to pcaversaccio/snekmate that referenced this pull request Jun 2, 2024
### 🕓 Changelog

The PRs a16z/halmos#296 and
a16z/halmos#298 have added support for
configuration files in `halmos` (https://github.com/a16z/halmos). This
PR refactors the configurations currently used inline via the CLI and
moves them to the new configuration file `halmos.toml`, which is located
in the `test/` subdirectory. I also rename the configuration file
`echidna-config.yaml` to `echidna.yaml` and move it to the subdirectory
`test/` as well. Eventually, I bump the submodules
`FreshCryptoLib` (https://github.com/rdubois-crypto/FreshCryptoLib) and
`properties` (https://github.com/crytic/properties) to the latest
available commit.

---------

Signed-off-by: Pascal Marco Caversaccio <[email protected]>
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.

2 participants