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

Cast Vec to StorageVec #2439

Closed
bitzoic opened this issue Aug 1, 2022 · 5 comments · Fixed by #5123
Closed

Cast Vec to StorageVec #2439

bitzoic opened this issue Aug 1, 2022 · 5 comments · Fixed by #5123
Assignees
@bitzoic
Labels
enhancement New feature or request lib: std Standard library

Comments

@bitzoic
Copy link
Member

bitzoic commented Aug 1, 2022

There is currently no relation between the Vec and StorageVec in the standard library. There should be a way to cast a Vec to a StorageVec and vice versa.

An example where this is needed would be passing a Vec to a function and then storing the given Vec. Or returning a stored StorageVec as a Vec to the SDK.

We should also consider the case of utility methods within Vec as described by #2014 and whether these should be duplicated in StorageVec or the end user should cast back and forth to perform certain operations.
@bitzoic bitzoic added enhancement New feature or request lib: std Standard library labels Aug 1, 2022
@mohammadfawaz
Copy link
Contributor

StorageVec is really just a StorageMap with a length and where the keys are 0, 1, 2, .... The only way a cast would work here is trough a method that simply copies the data from a StorageVec to a Vec or vice versa. For example, returning a StorageVec as Vec would first require that we read all the elements of the StorageVec and store them in a local Vec at the same indices. We then return the resulting Vec as we usually do (once this is supported of course). Is this what you have in mind?

@bitzoic
Copy link
Member Author

bitzoic commented Aug 1, 2022

This is what I had in mind initially as I see developers needing to do this often. The only problem being that this could get quite expensive.

I bring this up as I have a need for this in my English Auction application. I have a Vec of token ids that I will need to store. I may also need something similar when implementing a strings library.

@mohammadfawaz
Copy link
Contributor

Yeah this is going to be expensive. Now two things:

  • Storing the elements of Vec in a StorageVec is going to be as expensive as manually copying the elements one by one, and I don't think there's a way around that.
  • Returning a StorageVec to the SDK may be possible without having to go through Vec<T>. We could potentially return the storage keys themselves and have the SDK rebuild the StorageVec once its able to access storage slots. At least this moves the burden of reading from the contract to the SDK.

@adlerjohn
Copy link
Contributor

The fact that it will be very expensive is pointing toward this not being used often.

@jtriley-eth jtriley-eth mentioned this issue Jan 31, 2023
Open
@jtriley-eth
Copy link
Contributor

Bumping this issue, as we have some possible use cases while updating the Vec and StorageVec API's (issue #2014).

While the utility of sorting algorithm implementations for storage vectors are limited, it would be good to match API's between the memory and storage vectors. However, any algorithm that requires more than N reads or writes on a storage vector of length N could benefit from being converted into memory vectors first. So StorageVec::sort would ideally create a memory vector, load each value into the memory vector, sort them, then write them to storage.

This may be useful for the following:

  • sort
  • sort_unstable
  • split_off
  • split_at

(for more on the splits, see discussion here)

@bitzoic bitzoic self-assigned this Jul 5, 2023
@bitzoic bitzoic mentioned this issue Sep 18, 2023
Merged
7 tasks
bitzoic added a commit that referenced this issue Sep 26, 2023
@bitzoic
Enable storing of a Vec with the StorageVec type (#5123)
5392f8b 
## Description

Currently, `StorageVec` and `Vec` do not have any relation with one
another. This PR takes advantage of the recent optimization and
refactoring done to the Storage API `read()` and `write()` functions in
#4795 and enables the storing of a
`Vec` using the `StorageVec` type.

To do this, the `StorageVec` now stores things sequentially rather than
a different key for every element. Due to the optimizations done to the
Storage API, this has become feasible as we can now load a single
element of the sequential `StorageVec`.

The storing of elements sequentially mimics the existing `Vec`, allowing
us to store it as a `raw_slice` with a ***single*** read/write to
storage as opposed to looping over the `Vec` and having ***n***
read/writes.

It should be noted that due to
#409, the storing of a `Vec` is
written in the `StorageVec` file. This is the resulting syntax:
```sway
let my_vec = Vec::new();
storage.storage_vec.store_vec(my_vec); // Store the Vec
let other_vec = storage.storage_vec.load_vec(); // Read the Vec
```
When this issue is resolved, this should be changed to a `From`
implementation changing the syntax to:
```sway
let my_vec = Vec::new();
storage.storage_vec = my_vec.into(); // Store the Vec
let other_vec = Vec::from(storage.storage_vec); // Read the Vec
```

Closes #2439

## 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).
- [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: bitzoic <bitzoic.eth@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bitzoic bitzoic

Labels
enhancement New feature or request lib: std Standard library
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

bitzoic-2439 FuelLabs/sway
Enable storing of a Vec with the StorageVec type FuelLabs/sway
4 participants
@adlerjohn @jtriley-eth @bitzoic @mohammadfawaz