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

refactor: remove params dependency on modules #1261

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

ARR4N
Copy link
Contributor

@ARR4N ARR4N commented Jul 31, 2024

Why this should be merged

Note

This will probably be replaced by #1271

Importing core/types from precompile/{modules,contract} results in a circular dependency because types -> params -> precompile/modules -> precompile/contract -> types.

How this works

As you can see from the commit history, this took a lot of experimentation! The final result:

params only depended on modules for unmarshalling JSON, so all such functionality is abstracted into a new paramsjson package. To avoid having to import paramsjson, the params package instead exposes params.RegisterJSONUnmarshalers(), which allows paramsjson to inject its functionality in an init() function.

The paramsjson package doesn't export anything, and should instead be blank _ imported. This is similar to registering SQL drivers. The _ import has been added where necessary.

The alternative of using paramsjson.Unmarshal() in lieu of json.Unmarshal() proved problematic because of the number of other types with params.ChainConfig and other problematic types as fields.

Methods on *params.ChainConfig that relied on the modules package were only using it to access all precompile addresses. precompileconfig.Config now exposes the precompile's address, and the ChainConfig methods only know of the addresses that they receive as arguments or carry in GenesisPrecompiles and UpgradeConfig fields.

How this was tested

All existing CI tests.

How is this documented

Standard Go comments.

ARR4N added 27 commits July 29, 2024 17:45
* Stop exporting any identifiers, relying solely on `_` importing
* Split out `Unmarshal()` type switch into specific functions
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.

1 participant