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

feat: add manual initial storage slots #441

Merged
merged 14 commits into from
Jul 5, 2022
Merged

Conversation

MujkicA
Copy link
Contributor

@MujkicA MujkicA commented Jun 30, 2022

Closes #86

This PR enables manual initialization of storage slots via the SDK. deploy and deploy_with_salt() now accept a Vec<StorageSlot> which is used to calculate the contract_id on the SDK side and forwarded via the contract deployment transaction.

I have discussed multiple options for the UI with @digorithm. The conclusion is that the above approach creates the least amount of cognitive load for the user, i.e. they don't have to remember another workflow.

let storage_slot = ...

let contract_id = Contract::deploy(
        "tests/test_projects/storage/out/debug/storage.bin",
        &wallet,
        TxParameters::default(),
        vec![storage_slot.clone()], // <----------
    )
    .await?;

Since StorageSlot requires the user to convert key/value to Bytes32 I thought it would be convenient to provide a helper to create a slot from a generic key-value pair.
create_storage_slot uses unsafe Rust and panics if key/value is larger than 32 byte since this is their limit as stated in the specs.

let storage_slot = create_storage_slot("slot", 42);

@MujkicA MujkicA requested review from adlerjohn and a team June 30, 2022 11:40
@MujkicA MujkicA added the enhancement New feature or request label Jun 30, 2022
@MujkicA MujkicA self-assigned this Jun 30, 2022
Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use already existing functions for storage slots, and make the storage slot vec non mandatory for vanilla deployment

examples/contracts/src/lib.rs Outdated Show resolved Hide resolved
examples/contracts/src/lib.rs Outdated Show resolved Hide resolved
packages/fuels-test-helpers/src/lib.rs Outdated Show resolved Hide resolved
packages/fuels-test-helpers/src/lib.rs Outdated Show resolved Hide resolved
@MujkicA MujkicA enabled auto-merge (squash) July 5, 2022 12:56
@MujkicA MujkicA disabled auto-merge July 5, 2022 13:01
@MujkicA MujkicA enabled auto-merge (squash) July 5, 2022 13:01
@MujkicA MujkicA disabled auto-merge July 5, 2022 13:10
@MujkicA MujkicA enabled auto-merge (squash) July 5, 2022 13:10
Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you removed too many things
EDIT: thought I had received a review request, mb sry

#[tokio::test]
// ANCHOR: deploy_with_salt
async fn deploy_with_salt() -> Result<(), Error> {
// ANCHOR: deploy_with_parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you removed a commit by mistake

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some changes the test in examples turned out to be the same as the one in harness. I moved the ANCHOR tags to harness and removed the example.

@MujkicA MujkicA requested a review from iqdecay July 5, 2022 13:48
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice work on the API!

Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@MujkicA MujkicA merged commit 83089a2 into master Jul 5, 2022
@MujkicA MujkicA deleted the ahmedm/manual-storage-init branch July 5, 2022 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add manual initial storage slots in the SDK
5 participants