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

Enable std #530

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Enable std #530

wants to merge 7 commits into from

Conversation

mrLSD
Copy link
Member

@mrLSD mrLSD commented Jun 8, 2022

Enable std

By introducing this compilation option (feature), we are able to include the standard rust library.

Measured binary size (release):

  • Without std: 1018479
  • With std: 1016468 (-2%)

Exclusions

Not all dependencies turn off as no-defaults. For some reason, it is several crates that compilation success but unit tests failed.

Reason

There are some dependencies we use which we had to fork and make non-std. The problem is that we need to be active and update these libraries with upstream. Critical issues on these upstream libraries may not easily notify us of any issues, and we may be left with a critical bug for some time.

The second issue is that we can be limited in the dependencies that we want to use. At times, a task which should take a day could take a week at times, for example.

Reverting to std will increase gas usage slightly, but will make the engine more maintainable.

@mrLSD mrLSD added the C-dependencies Category: Pull requests that update a dependency label Jun 8, 2022
@mrLSD mrLSD requested a review from joshuajbouw as a code owner June 8, 2022 08:54
@mrLSD mrLSD self-assigned this Jun 8, 2022
@mrLSD mrLSD changed the base branch from master to develop June 8, 2022 08:56
Copy link
Contributor

@joshuajbouw joshuajbouw left a comment

Choose a reason for hiding this comment

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

Theres a bit more to be done. I wouldn't add it as a feature.Just make it only std.

These need to be removed under features in each Cargo.toml:
std = ["borsh/std", "evm/std", "primitive-types/std", "rlp/std", "sha3/std", "ethabi/std", "logos/std", "bn/std", "aurora-engine-types/std", "rjson/std", "aurora-engine-precompiles/std", "aurora-engine-transactions/std"]

Also, any dependency that has default-features = false needs to be removed as we won't be going back to no_std.

@mrLSD mrLSD changed the title Add std wasm32 feature Enable std Jun 8, 2022
@joshuajbouw joshuajbouw added the S-do-not-merge Status: Do not merge label Jun 22, 2022
@Aurora-ClickUp
Copy link

Aurora-ClickUp commented Jun 24, 2022

Task linked: CU-2e6u98n

@joshuajbouw
Copy link
Contributor

Please resolve conflicts and squash.

@joshuajbouw joshuajbouw removed the S-do-not-merge Status: Do not merge label Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-dependencies Category: Pull requests that update a dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants