From eba365773bcdd4170613ec00edc80d9255cc7c1f Mon Sep 17 00:00:00 2001 From: SwayStar123 <46050679+SwayStar123@users.noreply.github.com> Date: Wed, 7 Aug 2024 10:23:52 +0530 Subject: [PATCH] Revert when loading 0 size typed storagevec (#6358) ## Description Trying to allocate 0 bytes results in a reverts. This pr reverts earlier intentionally and adds documentation about it ## 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) - [ ] 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. - [ ] 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). - [ ] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: K1-R1 <77465250+K1-R1@users.noreply.github.com> Co-authored-by: Cameron Carstens Co-authored-by: IGI-111 --- sway-lib-std/src/storage/storage_vec.sw | 11 +++ .../test_projects/storage_vec_nested/mod.rs | 8 ++ .../storage_vec_nested/src/main.sw | 12 +++ .../test_projects/tx_fields/mod.rs | 80 +++++++++++-------- 4 files changed, 76 insertions(+), 35 deletions(-) diff --git a/sway-lib-std/src/storage/storage_vec.sw b/sway-lib-std/src/storage/storage_vec.sw index 97bdb9f53d0..6e2dffb3d57 100644 --- a/sway-lib-std/src/storage/storage_vec.sw +++ b/sway-lib-std/src/storage/storage_vec.sw @@ -851,10 +851,18 @@ impl StorageKey> { /// Load a `Vec` from the `StorageVec`. /// + /// # Additional Information + /// + /// This method does not work for any `V` type that has a 0 size, such as `StorageVec` itself. Meaning you cannot use this method on a `StorageVec>`. + /// /// # Returns /// /// * [Option>] - The vector constructed from storage or `None`. /// + /// # Reverts + /// + /// * If the size of type `V` is 0. + /// /// # Number of Storage Accesses /// /// * Reads - `2` @@ -888,6 +896,9 @@ impl StorageKey> { len => { // Get the number of storage slots needed based on the size. let size_V_bytes = __size_of::(); + + assert(size_V_bytes != 0); + let bytes = if size_V_bytes < 8 { // Len * size_of_word len * 8 diff --git a/test/src/sdk-harness/test_projects/storage_vec_nested/mod.rs b/test/src/sdk-harness/test_projects/storage_vec_nested/mod.rs index 3bc7ea5f873..4a5a834f81f 100644 --- a/test/src/sdk-harness/test_projects/storage_vec_nested/mod.rs +++ b/test/src/sdk-harness/test_projects/storage_vec_nested/mod.rs @@ -33,3 +33,11 @@ async fn nested_vec_access_insert() { methods.nested_vec_access_insert().call().await.unwrap(); } + +#[tokio::test] +#[should_panic] +async fn revert_on_load_storage_vec() { + let methods = test_storage_vec_nested_instance().await.methods(); + + methods.revert_on_load_storage_vec().call().await.unwrap(); +} diff --git a/test/src/sdk-harness/test_projects/storage_vec_nested/src/main.sw b/test/src/sdk-harness/test_projects/storage_vec_nested/src/main.sw index 7b46cc2e981..122682a6a47 100644 --- a/test/src/sdk-harness/test_projects/storage_vec_nested/src/main.sw +++ b/test/src/sdk-harness/test_projects/storage_vec_nested/src/main.sw @@ -12,6 +12,9 @@ abi ExperimentalStorageTest { #[storage(read, write)] fn nested_vec_access_insert(); + + #[storage(read, write)] + fn revert_on_load_storage_vec(); } impl ExperimentalStorageTest for Contract { @@ -47,6 +50,15 @@ impl ExperimentalStorageTest for Contract { // test_access(inner_vec0, inner_vec1, inner_vec2); } + + #[storage(read, write)] + fn revert_on_load_storage_vec() { + storage.nested_vec.push(StorageVec {}); + storage.nested_vec.push(StorageVec {}); + storage.nested_vec.push(StorageVec {}); + + let _ =storage.nested_vec.load_vec(); + } } #[storage(read, write)] diff --git a/test/src/sdk-harness/test_projects/tx_fields/mod.rs b/test/src/sdk-harness/test_projects/tx_fields/mod.rs index 463c3b861d3..1eb4dda1a61 100644 --- a/test/src/sdk-harness/test_projects/tx_fields/mod.rs +++ b/test/src/sdk-harness/test_projects/tx_fields/mod.rs @@ -4,18 +4,16 @@ use fuels::types::transaction_builders::TransactionBuilder; use fuels::{ accounts::{predicate::Predicate, wallet::WalletUnlocked, Account}, prelude::*, - types::{input::Input as SdkInput, Bits256, output::Output as SdkOutput}, tx::StorageSlot, + types::{input::Input as SdkInput, output::Output as SdkOutput, Bits256}, }; use std::fs; const MESSAGE_DATA: [u8; 3] = [1u8, 2u8, 3u8]; -const TX_CONTRACT_BYTECODE_PATH: &str = - "test_artifacts/tx_contract/out/release/tx_contract.bin"; +const TX_CONTRACT_BYTECODE_PATH: &str = "test_artifacts/tx_contract/out/release/tx_contract.bin"; const TX_OUTPUT_PREDICATE_BYTECODE_PATH: &str = "test_artifacts/tx_output_predicate/out/release/tx_output_predicate.bin"; -const TX_FIELDS_PREDICATE_BYTECODE_PATH: &str = - "test_projects/tx_fields/out/release/tx_fields.bin"; +const TX_FIELDS_PREDICATE_BYTECODE_PATH: &str = "test_projects/tx_fields/out/release/tx_fields.bin"; const TX_CONTRACT_CREATION_PREDICATE_BYTECODE_PATH: &str = "test_artifacts/tx_output_contract_creation_predicate/out/release/tx_output_contract_creation_predicate.bin"; @@ -78,14 +76,11 @@ async fn get_contracts( wallet.set_provider(provider.clone()); deployment_wallet.set_provider(provider); - let contract_id = Contract::load_from( - TX_CONTRACT_BYTECODE_PATH, - LoadConfiguration::default(), - ) - .unwrap() - .deploy(&wallet, TxPolicies::default()) - .await - .unwrap(); + let contract_id = Contract::load_from(TX_CONTRACT_BYTECODE_PATH, LoadConfiguration::default()) + .unwrap() + .deploy(&wallet, TxPolicies::default()) + .await + .unwrap(); let instance = TxContractTest::new(contract_id.clone(), deployment_wallet.clone()); @@ -175,12 +170,10 @@ async fn setup_output_predicate() -> (WalletUnlocked, WalletUnlocked, Predicate, .encode_data(0, Bits256([0u8; 32]), Bits256(*wallet1.address().hash())) .unwrap(); - let predicate = Predicate::load_from( - TX_OUTPUT_PREDICATE_BYTECODE_PATH, - ) - .unwrap() - .with_data(predicate_data) - .with_provider(wallet1.try_provider().unwrap().clone()); + let predicate = Predicate::load_from(TX_OUTPUT_PREDICATE_BYTECODE_PATH) + .unwrap() + .with_data(predicate_data) + .with_provider(wallet1.try_provider().unwrap().clone()); wallet1 .transfer(predicate.address(), 100, asset_id1, TxPolicies::default()) @@ -939,12 +932,17 @@ mod outputs { let provider = wallet.try_provider().unwrap(); // Get the predicate - let predicate: Predicate = Predicate::load_from(TX_CONTRACT_CREATION_PREDICATE_BYTECODE_PATH).unwrap() + let predicate: Predicate = + Predicate::load_from(TX_CONTRACT_CREATION_PREDICATE_BYTECODE_PATH) + .unwrap() .with_provider(provider.clone()); let predicate_coin_amount = 100; - + // Predicate has no funds - let predicate_balance = predicate.get_asset_balance(&provider.base_asset_id()).await.unwrap(); + let predicate_balance = predicate + .get_asset_balance(&provider.base_asset_id()) + .await + .unwrap(); assert_eq!(predicate_balance, 0); // Transfer funds to predicate @@ -955,10 +953,14 @@ mod outputs { *provider.base_asset_id(), TxPolicies::default(), ) - .await.unwrap(); + .await + .unwrap(); // Predicate has funds - let predicate_balance = predicate.get_asset_balance(&provider.base_asset_id()).await.unwrap(); + let predicate_balance = predicate + .get_asset_balance(&provider.base_asset_id()) + .await + .unwrap(); assert_eq!(predicate_balance, predicate_coin_amount); // Get contract ready for deployment @@ -968,19 +970,21 @@ mod outputs { let contract = Contract::new(binary.clone(), salt, storage_slots.clone()); // Start building the transaction - let tb: CreateTransactionBuilder = CreateTransactionBuilder::prepare_contract_deployment( - binary, - contract.contract_id(), - contract.state_root(), - salt, - storage_slots, - TxPolicies::default(), - ); + let tb: CreateTransactionBuilder = + CreateTransactionBuilder::prepare_contract_deployment( + binary, + contract.contract_id(), + contract.state_root(), + salt, + storage_slots, + TxPolicies::default(), + ); // Inputs let inputs = predicate .get_asset_inputs_for_amount(*provider.base_asset_id(), predicate_coin_amount, None) - .await.unwrap(); + .await + .unwrap(); // Outputs let mut outputs = wallet.get_asset_outputs_for_amount( @@ -988,7 +992,10 @@ mod outputs { *provider.base_asset_id(), predicate_coin_amount, ); - outputs.push(SdkOutput::contract_created(contract.contract_id(), contract.state_root())); + outputs.push(SdkOutput::contract_created( + contract.contract_id(), + contract.state_root(), + )); let mut tb = tb.with_inputs(inputs).with_outputs(outputs); @@ -1011,7 +1018,10 @@ mod outputs { assert!(instance.methods().get_output_type(0).call().await.is_ok()); // Verify predicate funds transferred - let predicate_balance = predicate.get_asset_balance(&AssetId::default()).await.unwrap(); + let predicate_balance = predicate + .get_asset_balance(&AssetId::default()) + .await + .unwrap(); assert_eq!(predicate_balance, 0); }