-
Notifications
You must be signed in to change notification settings - Fork 474
Missing Limitation for Storage Size on Integration Testing for set_contract_storage() #1961
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
Missing Limitation for Storage Size on Integration Testing for set_contract_storage() #1961
Conversation
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.
|
@ramirez7358 Could you merge |
|
Thanks for the work on this PR. It took a long while, but we now came to a decision that results in the closing of this PR. Some words on the reasoning and why it took so long: The project moved out of Parity and with that came a period in which we didn't have funding and things were unclear. We resumed work a while ago to work on ink! v6; this is a migration away from WebAssembly and While migrating to The reason for this decision is that the off-chain testing environment comes with a lot of maintenance costs and never fully reflected the on-chain behavior. |
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