Refactor `apply_delta` for Storage Maps

[Dominik1999]

As discussed in #737 (comment), we need a PR that fixes how account storage delta is put together and applied.

Specifically:

• AccountStorageDelta.cleared_items and AccountStorageDelta.updated_items should contain updates only to value slots.
• AccountStorageDelta.updated_maps should contain updates to storage maps.

This should be sufficient to apply the delta because new roots of storage maps can be computed from the data in updated_maps and can then use these roots to update the corresponding storage slots.

To make the above work, we may need to modify MASM - but there may also be a way around it. For example, if the a set_item event is fired on a map storage slot, transaction host can verify that it is correct, but then ignore it.

Separately, we should also implement AccountStorage::set_map_item() and AccountStorage::get_map_item() methods to mimic the ones in the transaction kernel.

Originally posted by @bobbinth in #737 (review)

[Dominik1999]

I am starting this issue today.

I am not sure I understood 100% why this refactoring is needed. In my understanding, for every change in a StorageMap there is always a change, i.e. the root of the map in a StorageSlot.

This is what the MASM code in api.masm does.

miden-base/miden-lib/asm/kernels/transaction/api.masm

Lines 259 to 265 in 6184966

	# set the new map item
	loc_load.0 exec.account::set_map_item swapw
	# => [NEW_MAP_ROOT, OLD_MAP_VALUE, ...]
	# set the root of the map in the respective account storage slot
	loc_load.0 exec.account::set_item
	# => [OLD_MAP_ROOT, OLD_MAP_VALUE, ...]

That means we have two updated entities, AccountStorageDelta.updated_items and AccountStorageDelta.updated_maps, because updated_items tracks changes in StorageSlots, and updated_maps tracks changes in StorageMaps.

Currently, there is also a check that whenever there is a updated_map, there must be a corresponding change in updated_items to ensure consistency.

miden-base/objects/src/accounts/delta/storage.rs

Lines 93 to 106 in 6184966

	// make sure storage map deltas are valid
	for (index, storage_map_delta) in self.updated_maps.iter() {
	if index > &MAX_MUTABLE_STORAGE_SLOT_IDX {
	return Err(AccountDeltaError::ImmutableStorageSlot(*index as usize));
	}
	// for every storage map delta there should be one corresponding storage item update
	if !self.updated_items.iter().any(|(idx, _)| idx == index) {
	return Err(AccountDeltaError::StorageMapDeltaWithoutStorageItemChange(
	*index as usize,
	));
	}
	if !storage_map_delta.is_empty() {
	storage_map_delta.validate()?;
	}

It could be implemented differently, without question, but I don't know what the reason behind it is.

[bobbinth]

The main reasons is that having both storage map updates and storage slot updates (for slots which contain storage maps) is redundant. This means:

• The deltas are slightly bigger (this is very minor).
• It is possible to encode data in the delta inconsistently (e.g., storage slot updates would indicate one root for a storage map, while corresponding storage map updates would result in a different root). It is possible to deal with this, but that means extra code in various places (e.g., during application of the delta, during deserialization of the delta etc.).

[bobbinth]

Closed by #758.