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

Keep Storage external to Bucket #559

Closed
wants to merge 7 commits into from

Conversation

ethanfrey
Copy link
Member

@ethanfrey ethanfrey commented Oct 3, 2020

Inspired by #557 but also a long struggle with the Bucket and ReadOnlyBucket types.

We can make the Bucket always immutable, just by storing the access/mapping info there. And then pass in &Storage or &mut Storage everywhere. We can use Bucket or &Bucket everywhere, also in queries, and the compiler will automatically prevent use from calling the mutable functions if we only have immutable storage.

This also let's us have 2 buckets open at once without worrying about lifetimes.

The downside is one more argument to each call. However, as the typical usage is more or less:

let acct = tokens(&deps.storage).load(&sender_raw)?

we just transmute that to:

let acct = tokens().load(&deps.storage, &sender_raw)?

tokens (the bucket constructor with type and namespace info) need no more arguments, and could theoretically be a const (created compile time) if we migrate from Vec<u8> to &'a [u8] for namespace storage, which I have done in #557, making these even shorter to write and faster to run.

You can look at a202e4d to see what it took to migrate a moderate contract to the new form (not much).

  • move Storage into arg for Bucket
  • also update Singleton likewise.
  • remove unused TypedStorage
  • Update CHANGELOG, MIGRATING
  • Update README

@ethanfrey ethanfrey force-pushed the dont-internalize-storage-bucket branch from e7b71e8 to 12f26dc Compare October 5, 2020 11:51
@ethanfrey ethanfrey changed the base branch from master to add-env-to-query October 5, 2020 11:51
Base automatically changed from add-env-to-query to master October 5, 2020 12:06
@ethanfrey ethanfrey force-pushed the dont-internalize-storage-bucket branch from 12f26dc to 219ebd7 Compare October 5, 2020 12:06
@ethanfrey ethanfrey added the Blocked Blocked on some external dependency label Oct 5, 2020
@ethanfrey
Copy link
Member Author

After discussion, I will not continue this in cosmwasm, and not modify Bucket, but build the new structures with new names in cosmwasm-plus. We can extend cosmwasm-storage later when they are stable, but for now, let's keep it stable.

Leaving this PR until all logic has been moved to cosmwasm-plus

@ethanfrey
Copy link
Member Author

This has been done in https://github.com/CosmWasm/cosmwasm-plus in the cw-storage-plus package (see linked PR)

@ethanfrey ethanfrey closed this Oct 20, 2020
@ethanfrey ethanfrey deleted the dont-internalize-storage-bucket branch October 20, 2020 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Blocked on some external dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant