chore: remove faucet sysdata storage slot

[PhilippGackstatter]

Removes the protocol-reserved faucet slot (miden::protocol::faucet::sysdata).

This slot was used for tracking the amount of issued fungible assets or the non-fungible assets that were already minted.

The logic for fungible assets is moved to miden::standards::faucets::{distribute, burn}. The non-fungible logic has no replacement since we don't currently have a standard component for this.

Changes

• Removes building faucet accounts with reserved slots and the corresponding checks in the tx kernel prologue.
• Removes kernel procedures FAUCET_GET_TOTAL_FUNGIBLE_ASSET_ISSUANCE_OFFSET and FAUCET_IS_NON_FUNGIBLE_ASSET_ISSUED_OFFSET that allowed accessing the reserved slot.
• Replaces asset::get_fungible_asset_max_amount with asset::FUNGIBLE_ASSET_MAX_AMOUNT.
• burn_non_fungible_asset seems to have only checked that the asset to burn is from a non-fungible faucet without checking its origin and so that was changed to use validate_non_fungible_asset_origin instead.
• FungibleFaucetExt was removed. Instead, we should use let faucet = BasicFungibleFaucet::try_from(&account) instead and access faucet.token_supply(). A better way to abstract over this would be Implement standard TokenMetadata storage slot #2344.

Follow-Up

NetworkFungibleFaucet wraps more functionality of BasicFungibleFaucet to deduplicate code. The general reason we do this is for reusing the fungible faucet metadata logic, but I think at this point we're using it independent of BasicFungibleFaucet (e.g. also in create_agglayer_faucet_component) which suggests this should be its own standard, e.g. TokenMetadata. This would cleanly allow reusing it in all those faucets without depending on the basic fungible faucet. Will create an issue for this.

closes #2318

[mmagician]

Do we still want to keep around mint_non_fungible_asset?

[PhilippGackstatter]

I think I'm missing context. Did you expect it to be removed?

[mmagician]

I think I'm getting ahead of myself here, thinking about programmable assets already. NVM

[bobbinth]

Looks good! Thank you! I left some comments inline.

[bobbinth]

Looks good! Thank you!

[mmagician]

I think I'm getting ahead of myself here, thinking about programmable assets already. NVM

[mmagician]

I think get_total_issuance or equivalent would be helpful to have in standards/faucets/mod.masm as a convenience wrapper, instead of accessing the metadata slot elements individually which is error prone. Can be done in follow-up work if you agree that it's useful.

[PhilippGackstatter]

Generally agreed, though since this is very likely to change in the short-term, I'd like to avoid adding code that needs to be refactored as part of programmable assets. After we've determined how "built-in" fungible assets work and where they live (protocol, standards) we can for sure re-add these convenience wrappers. But let me know if you need something like this sooner than after that refactor!

[mmagician]

is this check needed? IIUC, if:

• token_supply <= max_supply at construction, and
• token_supply <= max_supply as of last call to distribute

then we don't need to check again

[PhilippGackstatter]

I think you're right, under the assumption that the checks from BasicFungibleFaucet have been done - which is what you mean by construction, I assume. I'm being very defensive here and assume these checks have not been done.

Since this code is deployed by users into their accounts, they'd be responsible for not writing an EvilBasicFungibleFaucet that bypasses those checks.

But, this also protects against other faulty code that updates this slot in invalid ways, in which case distribute might exhibit unintended behavior. So, really depends on what we want to assume.

My thinking now is that, since it's already implemented, the checks are relatively cheap in terms of cycles and making the code somewhat less safe would require additional work, I'm inclined to keep it as-is. What do you think?

[bobbinth]

My thinking now is that, since it's already implemented, the checks are relatively cheap in terms of cycles and making the code somewhat less safe would require additional work, I'm inclined to keep it as-is. What do you think?

That was my thinking as well: the checks are cheap and it's probably fine to have them, even if the edge case they are trying to catch is rather unlikely.

[mmagician]

same here - can't we assert this at construction time?

[mmagician]

While correct, I find this logic a little more complex than expected.
The (subjectively) more intuitive check would be:

1. Compute new_token_supply
2. Assert new_token_supply < max_supply
3. Assert amount < FUNGIBLE_ASSET_MAX_AMOUNT

AFAICS, this will be safe under the following assumptions:

1. max_supply < FUNGIBLE_ASSET_MAX_AMOUNT (by construction? see my above comments)
2. current_supply < FUNGIBLE_ASSET_MAX_AMOUNT (this is true as a result of 1. + the invariant that current_supply < max_supply)

then, new_token_supply might overflow FUNGIBLE_ASSET_MAX_AMOUNT, but will not overflow the Felt, and so checking new_token_supply < max_supply works.

[PhilippGackstatter]

I agree this would also work totally fine and is easier to understand, so may be worth doing (but see other comment).

[mmagician]

this is nice and contained. I'd love to see a similar pattern done for create_agglayer_faucet_component, where we have a struct and a way to convert it into an AccountComponent easily. And then we could avoid hardcoding the metadata layout and use sth like to_metadata_word here.

pub struct AggLayerFaucet {
   // ...
}

impl From<AggLayerFaucet> for AccountComponent {
    // ...
}

I'll open an issue to track this.

(and same for AggLayerBridge)

[PhilippGackstatter]

I think the ideal point to do this would be after #2344, since you can then reuse the TokenMetadata slot easily.

[mmagician]

perfect, even better!