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

precompile-utils refactor #2165

Merged
merged 27 commits into from
Apr 19, 2023
Merged

precompile-utils refactor #2165

merged 27 commits into from
Apr 19, 2023

Conversation

nanocryk
Copy link
Contributor

@nanocryk nanocryk commented Mar 16, 2023

What does it do?

Refactor precompile-utils in preparation of migrating it into frontier.

Crate is re-organized to group features by domain:

  • solidity: Conventions related to the Solidity language or ABI
    • codec: Solidity data encoding, old ::data. EvmData/EvmDataWriter/EvmDataReader are renamed into Codec/Writer/Reader, and are moved inside solidity::codec module. Codec is re-exported inside solidity which itself is included in the prelude, such that using the prelude allows to use solidity::Codec. Codec is not included directly in the prelude to avoid confusion with parity_scale_codec. Codec is both the trait and the derive macro. Reader and Writer are no longer re-exported in the prelude, instead functions like encode_arguments or decode_arguments should be used (with tuples for multiple values).
    • modifier: Checks related to view and payable keywords in Solidity.
    • revert: Allow generating more detailed error message that can include "backtraces" inside struct/array decoding.
  • evm: Utils related more closely to EVM functionality
    • logs: Basic functions to create logs. A derive macro on structs would be better in the future.
    • handle: Contains an extension trait to add helpers on impl PrecompileHandle. Also contain an environmental used mainly to support re-entrancy with local XCM execution.
    • costs: few gas cost computation functions copied from the EVM. Should be made public instead inside the EVM itself.
  • testing: Provide functions and mocks to more easily write precompile tests. As encoding has been largely improved execute_returns_encoded is shortened as execute_returns, while old execute_returns is renamed execute_returns_raw. There shouldn't be a need to use execute_returns_raw but I leave it just in case.
  • substrate: Allows dispatching Substrate call with weight to gas convertion and proper gas checks and recording.
  • precompile_set: Allows constructing a precompile set from other precompiles or precompile sets with opt-out security checks.

Left for future PRs

Implement derive macro from events.

@nanocryk nanocryk added A3-inprogress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C3-medium Elevates a release containing this PR to "medium priority". D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited breaking Needs to be mentioned in breaking changes labels Mar 16, 2023
@nanocryk nanocryk marked this pull request as ready for review March 17, 2023 11:03
@nanocryk nanocryk added A0-pleasereview Pull request needs code review. and removed A3-inprogress Pull request is in progress. No review needed at this stage. labels Mar 17, 2023
@nanocryk nanocryk merged commit 9c23bd3 into master Apr 19, 2023
@nanocryk nanocryk deleted the jeremy-precompile-utils-refactor branch April 19, 2023 08:51
@notlesh notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. B0-silent Changes should not be mentioned in any release notes breaking Needs to be mentioned in breaking changes C3-medium Elevates a release containing this PR to "medium priority". D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants