Skip to content

Fix Type Annotation after deranged Update #2261

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

Conversation

wulfraem
Copy link

@wulfraem wulfraem commented Mar 26, 2025

Description of change

Actual change intended here

The deranged crate, that can be included in the project, e.g. via stronghold feature, has added implementations for usize: PartialOrd<_> in its latest updates. This can lead to conflicts with implementations from core, e.g. in code related to the client feature, producing build errors like these:

error[E0283]: type annotations needed
   --> sdk/src/wallet/account/operations/output_claiming.rs:365:59
    |
365 | ...                   > NativeTokens::COUNT_MAX.into()
    |                       -                         ^^^^
    |                       |
    |                       type must be known at this point
    |
    = note: multiple `impl`s satisfying `usize: PartialOrd<_>` found in the following crates: `core`, `deranged`:
            - impl PartialOrd for usize;
            - impl<MIN, MAX> PartialOrd<deranged::RangedUsize<MIN, MAX>> for usize
              where the constant `MIN` has type `usize`, the constant `MAX` has type `usize`;
help: try using a fully qualified path to specify the expected types
    |
365 |                                 > <u8 as Into<T>>::into(NativeTokens::COUNT_MAX)

This can be reproduced by creating a new project, that uses iota-sdk with features client and stronghold enabled:

cargo init build-test
cd build-test
cargo add iota-sdk --features=client,stronghold
cargo build

This PR updates the affected calls to use fully qualified paths in the first commit.

Fixes for findings in CI

Linting rule changes

  • result_large_err = "allow" due to errors related to the large error objects
  • clippy::empty_docs as there were a lot of errors (~100) due to the empty doc blocks, that had been inserted
  • disable clippy::needless_lifetimes in bindings/core due to linting errors from derivative::Derivative
    This could be done differently by wrapping the 16 cases where the error was thrown into an inner module with this rule disabled, and then pub useing the structs back into the parent scope. Another possible solution would be to replace the derivative crate, but decided to suggest disabling the rule for now and postponing the decision how to refactor this to a later point in time.

The two muts

The commits afterward fix findings from the CI. Tried to split the changes into separate commits for the respective steps to not get mixed up in one large commit.

Also, a bit unsure about the updates:

clippy complained about &self being mutable even it isn't required here, so removed the mut. Theoretically, this might allow someone modifying the related instances while the two update async functions are running, which might lead to an unexpected result. So, not sure if we should remove the mut (as currently done in this PR) or just ignore the warning in these places.

cargo audit

Bumped two dependencies where fix versions, where available, not sure about the remaining 5, maybe we have to add them to the ignore list for now, as refactoring the usage of them might require more work.

Python bindings checks

Not really an idea, why this fails with ImportError: No module named iota_sdk.client.client.Client to be honest. :/

Type of change

  • Bug fix (a non-breaking change which fixes an issue)

How the change has been tested

Removed deranged and time dependencies from root Cargo.lock and ran cargo build to force updating to the newest version. Confirmed, that sdk still builds and tests run successfully locally.

Also did the same update to the Cargo.lock in identity.rs and confirmed, that got the same errors with the current state, and that I could build the project again after switching iota-sdk dependency to this PR's branch and applying a similar fix as done for this PR to the identity.rs code.

Change checklist

Tick the boxes that are relevant to your changes, and delete any items that are not.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that new and existing unit tests pass locally with my changes

@wulfraem wulfraem requested a review from a team March 31, 2025 09:32
@wulfraem
Copy link
Author

Closing the PR as with [email protected] being yanked in the meantime, the issue does not persist anymore. The steps to reproduce the error, described above, can now be performed without any problem. ^^

@wulfraem wulfraem closed this Apr 16, 2025
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