Skip to content

Commit

Permalink
[hardening] updating SUI conservation check to account for dynamic fi…
Browse files Browse the repository at this point in the history
…elds, fixing two SUI conservation bugs

@tnowacki previously pointed out that the previous logic for identifying dynamic field inputs was incorrect + the right way to do this is to look at `writes` and `deletes` to identify objects that are not inputs. Refactored `check_sui_conserved` to use this approach, and turn off the logic that skips any code with dynamic fields.

After doing this, a lot of tests using dynamic fields started failing conservation checks. I investigated this and discovered/fixed two related conservation bugs in `charge_gas_for_storage_changes`:
- If a dynamic field with storage rebate `N` gets mutated, we will burn `N` SUI
- If a dynamic field with storage rebate `N` gets deleted, we will burn `N` SUI

After fixing these bugs, all tests pass the conservation checks. But it would still be good to make sure we have test coverage for all {wrap/unwrap/delete}, {dynamic field/input object} scenarios.
  • Loading branch information
sblackshear committed Mar 22, 2023
1 parent 0a3f4ee commit b0c5d78
Show file tree
Hide file tree
Showing 2 changed files with 268 additions and 195 deletions.
2 changes: 1 addition & 1 deletion crates/sui-adapter/src/execution_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ fn execute_transaction<
#[cfg(debug_assertions)]
{
// ensure that this transaction did not create or destroy SUI
temporary_store.check_sui_conserved();
temporary_store.check_sui_conserved().unwrap();
}
}
let cost_summary = gas_status.summary();
Expand Down
Loading

0 comments on commit b0c5d78

Please sign in to comment.