-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[pallet-revive] evm remove contract storage slot when writing all zero bytes #10309
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
base: master
Are you sure you want to change the base?
[pallet-revive] evm remove contract storage slot when writing all zero bytes #10309
Conversation
This reverts commit 2e73763.
|
/cmd prdoc --audience runtime_dev --bump patch |
…time_dev --bump patch'
| interpreter.ext.gas_meter_mut().adjust_gas( | ||
| charged_amount, | ||
| adjust_cost(if value_to_store.is_some() { 32 } else { 0 }, write_outcome.old_len()), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it OK to do this for transient storage too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I guess it should be consistent
nit
- if value_to_store.is_some() { 32 } else { 0 },
+ value_to_store.unwrap_or_default().len()
Differential Tests Results (PolkaVM)Specified Tests
Counts
FailuresThe test specifiers seen in this section have the format 'path::case_idx::compilation_mode' and they're compatible with the revive differential tests framework and can be specified to it directly in the same way that they're provided through the The failures are provided in an expandable section to ensure that the PR does not get polluted with information. Please click on the section below for more information Detailed Differential Tests Failure Information
|
Differential Tests Results (REVM)Specified Tests
Counts
FailuresThe test specifiers seen in this section have the format 'path::case_idx::compilation_mode' and they're compatible with the revive differential tests framework and can be specified to it directly in the same way that they're provided through the The failures are provided in an expandable section to ensure that the PR does not get polluted with information. Please click on the section below for more information Detailed Differential Tests Failure Information
|
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
| #[test_case(FixtureType::Solc, FixtureType::Resolc; "solc->resolc")] | ||
| #[test_case(FixtureType::Resolc, FixtureType::Solc; "resolc->solc")] | ||
| #[test_case(FixtureType::Resolc, FixtureType::Resolc; "resolc->resolc")] | ||
| fn storage_item_zero_shall_refund_deposit(caller_type: FixtureType, callee_type: FixtureType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again do we need to to a nested call for testing this scenario here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it a nested call to make sure the caller_type does not matter. Maybe I was being overzealous, tell me what you think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah let's keep it simple please, we can have it aon both Solc and Resolc but we dont need a caller & callee
fixes paritytech/contract-issues#216
When an EVM contract writes an all-zero 32-byte storage word, remove the corresponding child‑trie entry so storage-deposit accounting sees bytes/items removed and a refund can be applied.