Missing Limitation for Storage Size on Integration Testing for set_contract_storage() #1961
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ramirez7358
requested review from
cmichi,
ascjones and
SkymanOne
as code owners
10 tasks
Changed the limit to encoded_value.len() + encoded_key.len() > 2**14
try_insert() and try_set() were ignoring that insert() and set() increase the length of the key by 4. After making the off-chain set_contract_storage() respect the size limit, this was causing some tests to fail. The tests were updated to test the correct limits.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1960
cargo-contractorpallet-contracts?Description
Issue Description
The storage in the integration environment does not have the same limitation as in the e2e environment. In the e2e environment, the storage size is limited by 16380 bytes.
Issue Documentation & Test Case
https://github.com/CoinFabrik/on-ink-integration-tests/tree/main/test-cases/set-contract-storage
Current Status
Missing Limitation Implemented. Test Cases Passed. Pull Request Performed to Corresponding Repository.
Implementation Notes
The function
set_contract_storage()sets the storage in a contract. There was a missing validation in integration tests that was present in e2e. This validation checked that the size of the storage set did not exceed 16380 bytes.We added a validation to the function
set_contract_storage()in integration tests that checks the size of the input value against the same limit set in e2e test: 16380 bytes. In case this limit is exceeded, a panic is raised with the message: "Value too large to be stored in contract storage, maximum size is {} bytes".Updated Documentation
We updated the original documentation of this issue in our repository, adding the section Update on Correcting this Issue and informing of the correction.
There is no necessary update to the associated documentation: ink_env set_contract_storage.
Testing Guide
In the directory
integration-tests/set_contract_storageof the target directory, we include in our pull request a test case showing that the observed implementation difference has been corrected. Note that this test is different from the original test case in our repository, which showed the difference.In order to run the integration tests run:
cargo testIn order to run the e2e tests run:
cargo test -–features e2e-testsThese tests are run separately because we do not need to compare their results.
Checklist before requesting a review
CHANGELOG.md