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

fix: Fix FromStr not found error on AccountId20 #1496

Merged
merged 3 commits into from
Aug 24, 2024

Conversation

conr2d
Copy link
Contributor

@conr2d conr2d commented Aug 22, 2024

This PR fixes the following issue.


When building fp-account without default features with the command:

cargo build -p fp-account --no-default-features --target wasm32-unknown-unknown

an error will occur because core::fmt::FromStr is not implemented for H160 without default features.

    Checking fp-account v1.0.0-dev (/Users/conr2d/Projects/frontier/primitives/account)
error[E0599]: no function or associated item named `from_str` found for struct `H160` in the current scope
  --> primitives/account/src/lib.rs:51:9
   |
51 |         H160::from_str(s)
   |               ^^^^^^^^ function or associated item not found in `H160`
   |

@conr2d conr2d requested a review from sorpaas as a code owner August 22, 2024 04:51
@boundless-forest
Copy link
Collaborator

Normally, we don't compile the primitive crate using the command params that you pasted. Is there any specific reason behind this? I personally think this hack is not worth it.

@conr2d
Copy link
Contributor Author

conr2d commented Aug 23, 2024

I am writing a crate to extend Substrate features including fp-account, but this prevents using fp-account in a no_std environment. Is the fp-account crate intended to be used exclusively with Frontier? According to #1218, there are plans to publish Frontier crates to crates.io. In my opinion, a reusable component should build successfully without default features.

I added the --target wasm32-unknown-unknown flag because the underlying Substrate dependency requires a pointer size of 4 bytes in a no_std environment. The use of core::fmt::FromStr instead of std::fmt::FromStr suggests that this implementation is intended for no_std use, but it currently doesn’t support that case correctly. The implementation should either be changed to use std::fmt::FromStr with a feature guard, such as #[cfg(feature = "std")] or corrected to properly handle no_std environments like this PR.

Moreover, I don't believe my change qualifies as a hack since it doesn't introduce any new dependencies or use unsafe or non-idiomatic syntax. :)

conr2d and others added 2 commits August 23, 2024 19:30
Co-authored-by: Bear Wang <boundless.forest@outlook.com>
@boundless-forest boundless-forest merged commit 80c768f into polkadot-evm:master Aug 24, 2024
4 checks passed
ipapandinas pushed a commit to AstarNetwork/frontier that referenced this pull request Sep 4, 2024
* fix: Fix FromStr not found error on AccountId20

* Update primitives/account/src/lib.rs

Co-authored-by: Bear Wang <boundless.forest@outlook.com>

* fix: Fix build error

---------

Co-authored-by: Bear Wang <boundless.forest@outlook.com>
@conr2d conr2d deleted the fix/accountid20-from_str branch October 11, 2024 16:42
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