canister_state_changes
is going to be Some(changes)
instead of None
if the canister made changes to its state that need to be persisted (perfectly normal for a cleanup callback).
The assert that used to be there was trying to check that there was no case that the canister’s cycle balance was reduced as part of those changes while executing the cleanup callback (note that we are talking during execution, obviously the canister is charged for executing the message but that happens after execution). This was true when the assert was first introduced (that’s about 2 years ago).
In the mean time, things (and some assumptions) have changed. The storage reservation mechanism is one case where if the subnet is above the storage capacity where this mechanism kicks in (450GiB right now), it would result in some cycles being reserved and removed from the cycles balance when the canister would try to allocate some memory in its cleanup callback. The ic0.cycles_burn
API is another case which would result in cycles being removed from the canister’s balance (and it’s allowed to be called in a cleanup). So, the assert was simply checking the wrong invariant given the state of the implementation.
Obviously, we should have caught this when these features were implemented and we have been discussing how to improve this going forward. We will post a post-mortem (as per our usual practice) once we have finalized these discussions.