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.

Sign up for GitHub

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

Jump to bottom

Implement TryFrom<Bytes> for b256 #6994

Open
ironcev opened this issue Mar 5, 2025 · 1 comment · Fixed by #6958
Open

Implement TryFrom<Bytes> for b256 #6994

ironcev opened this issue Mar 5, 2025 · 1 comment · Fixed by #6958
Assignees
@ironcev
Labels
breaking May cause existing user code to break. Requires a minor or major release. lib: std Standard library tracking-issue Tracking issue for experimental Sway features

Comments

@ironcev
Copy link
Member

ironcev commented Mar 5, 2025 •
edited
Loading

This is a tracking issue for Implement TryFrom<Bytes> for b256.
The experimental feature flag for the issue is try_from_bytes_for_b256.

Description

Currently, b256 implements From<Bytes> which can cause data losses. We want to remove that implementation and replace it with TryFrom<Bytes> which will return None if the provided Bytes cannot be safely converted to b256.

Breaking Changes

Replace calls to from(bytes)/bytes.into() with try_from(bytes)/bytes.try_into()

E.g., calls to from:

let result = b256::from(some_bytes);

will become:

let result = b256::try_from(some_bytes).unwrap();

E.g., calls to into:

let result: b256 = some_bytes.into();

will become:

let result: b256 = some_bytes.try_into().unwrap();

Calling unwrap() assumes that the previous usage was always semantically valid. It is recommended to inspect all the previous usages of from/into to ensure unwraping is always safe, or to check for the Option result of try_from/try_into.
@ironcev ironcev self-assigned this Mar 5, 2025
@ironcev ironcev added lib: std Standard library tracking-issue Tracking issue for experimental Sway features labels Mar 5, 2025
@ironcev ironcev mentioned this issue Mar 5, 2025
Merged
8 tasks
ironcev added a commit that referenced this issue Mar 5, 2025
@bitzoic @ironcev
Implement TryFrom<Bytes> for b256 (#6958)
be5ecbd 
## Description

`impl From<Bytes> for b256` has been removed and replaced with `impl
TryFrom<Bytes> for b256` increasing saftey when converting between the
two types. `impl Into<Bytes> for b256` and `impl TryInto<b256> for
Bytes` have also been added.

This PR breaks the following implementation:

```sway
let mut my_bytes = Bytes::new();
my_bytes.push(1u8);

let result: b256 = b256::from(my_bytes);
```

The implementation should now be:

```sway
let mut my_bytes = Bytes::new();
my_bytes.push(1u8);

// Option 1
let result_1: Option<b256> = b256::try_from(my_bytes);
// Option 2
let result_2 = my_bytes.try_into();
```

Closes #6994.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Igor Rončević <[email protected]>
@ironcev
Copy link
Member Author

ironcev commented Mar 5, 2025

Closed by my mistake in #6958. It must remains open until we fully integrate the feature and remove the experimental flag.

@ironcev ironcev reopened this Mar 5, 2025
@ironcev ironcev mentioned this issue Mar 9, 2025
Merged
8 tasks
@ironcev ironcev added the breaking May cause existing user code to break. Requires a minor or major release. label Mar 10, 2025
ironcev added a commit that referenced this issue Mar 11, 2025
@ironcev
Implement trees visitors and migrations for TryFrom<Bytes> for `b25…
8b3d822 
…6` (#7005)

## Description

This PR:
- extends infrastructure for writing migrations that modify individual
expressions and do not need access to a broader scope. It does that by
providing lexed and typed tree visitors. Visiting lexed and typed tree
is implemented to the level needed to write the migration for `impl
TryFrom<Bytes> for b256`.
- implements the migration for `impl TryFrom<Bytes> for b256` as
explained in #6994.

Adding `#[cfg(experimental_try_from_bytes_for_b256 = true)]` to the
`impl TryFrom<Bytes> for b256` revealed that there existed already an
`impl TryFrom<Bytes> for b256` in `std::primitive_conversions::b256`.
Due the lack of trait coherence there was no any errors when it was
re-implemented in `std::bytes`.

This PR marks the existing impl in `std::primitive_conversions::b256`
with `#[cfg(experimental_try_from_bytes_for_b256 = false)]` so that it
will be removed in the next breaking release, in favor of the new impl
in `std::bytes`.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
@ironcev ironcev mentioned this issue Mar 11, 2025
Merged
8 tasks
ironcev added a commit that referenced this issue Mar 11, 2025
@ironcev
Add migration for Bytes::into(self) -> b256 to Bytes::try_into (#…
1f407fa 
…7009)

## Description

This PR adds migration step for migrating `<bytes>.into()` calls to
`<bytes>.try_into().unwrap`, as explained in the Breaking Changes
chapter of #6994.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ironcev ironcev

Labels
breaking May cause existing user code to break. Requires a minor or major release. lib: std Standard library tracking-issue Tracking issue for experimental Sway features
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement TryFrom<Bytes> for b256 FuelLabs/sway
2 participants
@ironcev @bitzoic