feat(protocol): tie the asset callback flag to the callback slots

CHANGELOG.md

@@ -26,6 +26,7 @@
 - [BREAKING] Refactored `AccountVaultDelta` to track generic assets. `FungibleAssetDelta`, `NonFungibleAssetDelta` and `NonFungibleDeltaAction` were removed ([3485](https://github.com/0xMiden/protocol/pull/3485)).
 - [BREAKING] The transaction kernel no longer requires assets with `AssetComposition::None` to have the non-fungible asset layout ([#3624](https://github.com/0xMiden/protocol/pull/3624)).
 - [BREAKING] Refactored `Asset` into a struct holding `AssetId` and `AssetValue` ([#3625](https://github.com/0xMiden/protocol/pull/3625)).
+- [BREAKING] Refactored the presence of an asset callback slot imply an enabled asset callback flag: the transaction kernel rejects new accounts that violate this and `AccountBuilder` derives the flag from the installed callback slots, replacing `with_asset_callbacks` with `enable_asset_callbacks` ([#3658](https://github.com/0xMiden/protocol/pull/3658)).
 
 ### Fixes
 

bin/bench-transaction/src/context_setups/network_config.rs

@@ -20,7 +20,6 @@ use miden_protocol::account::{
     AccountComponent,
     AccountId,
     AccountType,
-    AssetCallbackFlag,
     RoleSymbol,
 };
 use miden_protocol::asset::AssetAmount;
@@ -101,7 +100,6 @@ pub fn tx_consume_faucet_policy_config_note_network() -> Result<MockTransaction>
         .with_component(faucet)
         .with_component(Ownable2Step::new(owner.id()))
         .with_component(Authority::OwnerControlled)
-        .with_asset_callbacks(AssetCallbackFlag::from(token_policy_manager.has_transfer_policy()))
         .with_components(token_policy_manager)
         .with_assets([super::fee_funding_asset()?]);
     let account = builder.add_account_from_builder(
@@ -221,7 +219,6 @@ pub fn tx_consume_min_burn_amount_config_note_network() -> Result<MockTransactio
         .with_component(faucet)
         .with_component(Ownable2Step::new(owner.id()))
         .with_component(Authority::OwnerControlled)
-        .with_asset_callbacks(AssetCallbackFlag::from(token_policy_manager.has_transfer_policy()))
         .with_components(token_policy_manager)
         .with_assets([super::fee_funding_asset()?]);
     let account = builder.add_account_from_builder(
@@ -362,7 +359,6 @@ fn tx_consume_list_config_note_network(list: ListKind) -> Result<MockTransaction
         .with_component(faucet)
         .with_component(Ownable2Step::new(owner.id()))
         .with_component(Authority::OwnerControlled)
-        .with_asset_callbacks(AssetCallbackFlag::from(token_policy_manager.has_transfer_policy()))
         .with_components(token_policy_manager)
         .with_component(list.manager())
         .with_assets([super::fee_funding_asset()?]);

bin/bench-transaction/src/context_setups/network_faucet.rs

@@ -8,7 +8,7 @@ use std::sync::Arc;
 
 use anyhow::Result;
 use miden_protocol::Felt;
-use miden_protocol::account::{Account, AccountBuilder, AccountId, AccountType, AssetCallbackFlag};
+use miden_protocol::account::{Account, AccountBuilder, AccountId, AccountType};
 use miden_protocol::asset::{AssetId, FungibleAsset, NonFungibleAsset, TokenSymbol};
 use miden_protocol::crypto::merkle::smt::SmtProof;
 use miden_protocol::crypto::rand::FeltRng;
@@ -98,7 +98,6 @@ fn add_fee_funded_network_non_fungible_faucet(
 
     let account_builder = AccountBuilder::new(builder.rng_mut().random())
         .account_type(AccountType::Public)
-        .with_asset_callbacks(AssetCallbackFlag::from(token_policy_manager.has_transfer_policy()))
         .with_component(faucet)
         .with_components(AccessControl::Ownable2Step { owner: owner_account_id })
         .with_components(token_policy_manager)

crates/miden-agglayer/src/lib.rs

@@ -5,7 +5,7 @@ extern crate alloc;
 use alloc::collections::BTreeMap;
 
 use miden_core::{Felt, Word};
-use miden_protocol::account::{AccountBuilder, AccountComponent, AccountId, AssetCallbackFlag};
+use miden_protocol::account::{AccountBuilder, AccountComponent, AccountId};
 use miden_protocol::assembly::Path;
 use miden_protocol::asset::TokenSymbol;
 use miden_protocol::note::NoteScript;
@@ -235,11 +235,8 @@ impl AggLayerFaucet {
             .active_receive_policy(TransferPolicy::allow_all())
             .build();
 
-        let asset_callbacks = AssetCallbackFlag::from(token_policy_manager.has_transfer_policy());
-
         NetworkAccount::builder(seed.into(), AggLayerFaucet::allowed_notes(), fee_policy_manager)
             .expect("faucet note allowlist is non-empty")
-            .with_asset_callbacks(asset_callbacks)
             .with_component(agglayer_component)
             .with_component(Ownable2Step::new(bridge_account_id))
             .with_component(

crates/miden-protocol/asm/kernels/transaction-core/src/prologue.masm

@@ -9,6 +9,8 @@ use miden::protocol_utils::mem as mem_utils
 use miden::tx_kernel_core::asset_vault
 use miden::tx_kernel_core::asset
 use {ASSET_SIZE} from miden::tx_kernel_core::asset
+use {ON_BEFORE_ASSET_ADDED_TO_ACCOUNT_PROC_ROOT_SLOT, ON_BEFORE_ASSET_ADDED_TO_NOTE_PROC_ROOT_SLOT}
+    from miden::tx_kernel_core::callbacks
 use {EMPTY_SMT_ROOT, MAX_ASSETS_PER_NOTE, MAX_INPUT_NOTES_PER_TX, MAX_NOTE_STORAGE_ITEMS, NOTE_TREE_DEPTH}
     from miden::tx_kernel_core::constants
 use miden::tx_kernel_core::memory
@@ -63,6 +65,9 @@ const ERR_PROLOGUE_INPUT_NOTES_COMMITMENT_MISMATCH =
 
 const ERR_PROLOGUE_NEW_ACCOUNT_NONCE_MUST_BE_ZERO = "new account must have a zero nonce"
 
+const ERR_PROLOGUE_CALLBACK_SLOT_REQUIRES_ENABLED_ASSET_CALLBACK_FLAG =
+    "an account whose storage contains an asset callback slot must have the asset callback flag enabled"
+
 const ERR_PROLOGUE_NUMBER_OF_NOTE_STORAGE_ITEMS_EXCEEDED_LIMIT =
     "number of note storage items exceeded the maximum limit of 1024"
 
@@ -303,6 +308,7 @@ end
 #! - assert that the account nonce is set to 0.
 #! - read the account seed from the advice provider and assert it satisfies seed requirements.
 #! - assert that the storage slots and the account procedures are sorted and unique.
+#! - assert that the asset callback flag is enabled if an asset callback slot is present.
 #!
 #! Validating storage and procedures for new accounts is sufficient because the storage and code
 #! commitments of an existing account are bound to its committed state, which was produced by the
@@ -346,6 +352,46 @@ proc validate_new_account
     # ---------------------------------------------------------------------------------------------
     exec.account::validate_procedures
     # => []
+
+    # Assert the asset callback flag is consistent with the installed asset callback slots.
+    # ---------------------------------------------------------------------------------------------
+    exec.validate_asset_callbacks
+    # => []
+end
+
+#! Validates that an account whose storage contains an asset callback slot has the asset callback
+#! flag of its account ID enabled.
+#!
+#! The kernel decides whether to invoke an account's asset callbacks solely from the asset callback
+#! flag encoded in its account ID, and that flag is immutable once the ID is ground. A callback slot
+#! installed on an account whose flag is disabled would therefore look correctly configured while
+#! never being invoked, silently and permanently disabling whatever the callback enforces.
+#!
+#! Inputs:  []
+#! Outputs: []
+#!
+#! Panics if:
+#! - the account's storage contains an asset callback slot but its asset callback flag is disabled.
+proc validate_asset_callbacks
+    push.ON_BEFORE_ASSET_ADDED_TO_ACCOUNT_PROC_ROOT_SLOT[0..2] exec.account::has_storage_slot
+    # => [has_account_callback_slot]
+
+    push.ON_BEFORE_ASSET_ADDED_TO_NOTE_PROC_ROOT_SLOT[0..2] exec.account::has_storage_slot
+    # => [has_note_callback_slot, has_account_callback_slot]
+
+    or
+    # => [has_callback_slot]
+
+    exec.memory::get_native_account_id drop exec.account_id::asset_callback_flag
+    # => [has_callbacks, has_callback_slot]
+
+    # the flag must be enabled if a callback slot is present, so reject the case where a slot is
+    # present but the flag is disabled
+    not and
+    # => [is_inconsistent]
+
+    assertz.err=ERR_PROLOGUE_CALLBACK_SLOT_REQUIRES_ENABLED_ASSET_CALLBACK_FLAG
+    # => []
 end
 
 #! Saves the account data to memory and validates it.

crates/miden-protocol/asm/kernels/transaction/lib/api.masm

@@ -224,6 +224,10 @@ end
 pub proc account_upgrade
     # TODO(code_upgrades): Account upgrades must ensure the same conditions hold for an upgraded
     # account as validated in account::{validate_storage, validate_procedures}.
+    # The same applies to the asset callback rule validated in
+    # prologue::validate_asset_callbacks: an upgrade must not add an asset callback slot to an
+    # account whose asset callback flag is disabled, since the flag is immutable and the callback
+    # could then never be invoked.
     # check that this procedure was executed against the native account
     exec.memory::assert_native_account
     # => [CODE_UPGRADE_COMMITMENT, STORAGE_UPGRADE_COMMITMENT, pad(8)]

crates/miden-protocol/src/account/builder/mod.rs

@@ -101,14 +101,18 @@ impl AccountBuilder {
         self
     }
 
-    /// Sets the immutable [`AssetCallbackFlag`] of the account.
+    /// Enables the immutable [`AssetCallbackFlag`] of the account even if none of its components
+    /// install an asset callback slot.
     ///
-    /// This determines whether assets issued by the account (if any) trigger callbacks. It must be
-    /// set to [`AssetCallbackFlag::Enabled`] for faucets that configure a transfer policy, and
-    /// is encoded into the resulting [`AccountId`] at creation. Defaults to
-    /// [`AssetCallbackFlag::Disabled`].
-    pub fn with_asset_callbacks(mut self, asset_callbacks: AssetCallbackFlag) -> Self {
-        self.asset_callbacks = asset_callbacks;
+    /// The flag determines whether assets issued by the account (if any) trigger callbacks and is
+    /// encoded into the resulting [`AccountId`] at creation. It is normally derived from the
+    /// account's storage: it is [`AssetCallbackFlag::Enabled`] if any component installs one of the
+    /// protocol-reserved asset callback slots (see [`AssetCallbacks::is_installed`]) and
+    /// [`AssetCallbackFlag::Disabled`] otherwise. There is deliberately no way to disable the flag
+    /// for an account that does install such a slot, since the kernel gates callback invocation on
+    /// the flag alone and the flag cannot be changed after the ID is ground.
+    pub fn enable_asset_callbacks(mut self) -> Self {
+        self.asset_callbacks = AssetCallbackFlag::Enabled;
         self
     }
 
@@ -169,57 +173,34 @@ impl AccountBuilder {
             )
         })?;
 
-        self.validate_asset_callbacks(&storage)?;
-
         Ok((vault, code, storage))
     }
 
-    /// Validates that the configured [`AssetCallbackFlag`] is consistent with the asset callback
-    /// slots installed by the builder's components.
-    ///
-    /// The kernel decides whether to invoke a faucet's asset callbacks solely from the
-    /// [`AssetCallbackFlag`] encoded in its [`AccountId`], and that flag is immutable once the ID
-    /// is ground. A component that installs a callback slot while the flag is
-    /// [`AssetCallbackFlag::Disabled`] therefore looks correctly configured but can never have its
-    /// callbacks invoked, silently and permanently disabling whatever the callbacks enforce. This
-    /// is rejected at build time so the misconfiguration cannot reach a deployed account.
+    /// Derives the account's [`AssetCallbackFlag`] from the asset callback slots installed by its
+    /// components.
     ///
-    /// The converse (the flag enabled without callback slots) is valid: the kernel skips the
-    /// callback when the slot is absent or holds the empty word.
-    fn validate_asset_callbacks(&self, storage: &AccountStorage) -> Result<(), AccountError> {
-        if self.asset_callbacks == AssetCallbackFlag::Enabled {
-            return Ok(());
-        }
-
-        for slot_name in [
-            AssetCallbacks::on_before_asset_added_to_account_slot(),
-            AssetCallbacks::on_before_asset_added_to_note_slot(),
-        ] {
-            if storage.get(slot_name).is_some_and(|slot| !slot.value().is_empty()) {
-                return Err(AccountError::BuildError(
-                    format!(
-                        "component installs the asset callback slot `{slot_name}` but the account's asset callback flag is disabled, so the callback would never be invoked"
-                    ),
-                    None,
-                ));
-            }
-        }
-
-        Ok(())
+    /// The flag is [`AssetCallbackFlag::Enabled`] if any component installs one of the
+    /// protocol-reserved asset callback slots, or if [`Self::enable_asset_callbacks`] was called,
+    /// and [`AssetCallbackFlag::Disabled`] otherwise.
+    fn derive_asset_callbacks(&self, storage: &AccountStorage) -> AssetCallbackFlag {
+        AssetCallbackFlag::from(
+            self.asset_callbacks.is_enabled() || AssetCallbacks::is_installed(storage),
+        )
     }
 
     /// Grinds a new [`AccountId`] using the `init_seed` as a starting point.
     fn grind_account_id(
         &self,
         init_seed: [u8; 32],
         version: AccountIdVersion,
+        asset_callbacks: AssetCallbackFlag,
         code_commitment: Word,
         storage_commitment: Word,
     ) -> Result<Word, AccountError> {
         let seed = AccountIdV1::compute_account_seed(
             init_seed,
             self.account_type,
-            self.asset_callbacks,
+            asset_callbacks,
             version,
             code_commitment,
             storage_commitment,
@@ -245,8 +226,6 @@ impl AccountBuilder {
     /// - The number of [`StorageSlot`](crate::account::StorageSlot)s of all components exceeds 255.
     /// - [`MastForest::merge`](miden_processor::mast::MastForest::merge) fails on the given
     ///   components.
-    /// - A component installs an asset callback slot while the configured [`AssetCallbackFlag`] is
-    ///   [`AssetCallbackFlag::Disabled`], since the kernel would never invoke that callback.
     /// - If duplicate assets were added to the builder (only under the `testing` feature).
     /// - If the vault is not empty on new accounts (only under the `testing` feature).
     pub fn build(mut self) -> Result<Account, AccountError> {
@@ -260,9 +239,12 @@ impl AccountBuilder {
             ));
         }
 
+        let asset_callbacks = self.derive_asset_callbacks(&storage);
+
         let seed = self.grind_account_id(
             self.init_seed,
             self.id_version,
+            asset_callbacks,
             code.commitment(),
             storage.to_commitment(),
         )?;
@@ -276,7 +258,7 @@ impl AccountBuilder {
         .expect("get_account_seed should provide a suitable seed");
 
         debug_assert_eq!(account_id.account_type(), self.account_type);
-        debug_assert_eq!(account_id.asset_callback_flag(), self.asset_callbacks);
+        debug_assert_eq!(account_id.asset_callback_flag(), asset_callbacks);
 
         // SAFETY: The account ID was derived from the seed and the seed is provided, so it is safe
         // to bypass the checks of `Account::new`.
@@ -322,7 +304,7 @@ impl AccountBuilder {
                 bytes,
                 AccountIdVersion::Version1,
                 self.account_type,
-                self.asset_callbacks,
+                self.derive_asset_callbacks(&storage),
             )
         };
 
@@ -596,12 +578,12 @@ mod tests {
         assert_matches!(build_error, AccountError::BuildError(msg, _) if msg == "account asset vault must be empty on new accounts")
     }
 
-    /// A component that installs an asset callback slot must not be built into an account whose
-    /// [`AssetCallbackFlag`] is disabled: the kernel gates callback invocation on that flag alone
-    /// and the flag is immutable once the ID is ground, so whatever the callback enforces would
-    /// be silently and permanently bypassed.
+    /// The [`AssetCallbackFlag`] is derived from the installed asset callback slots: the kernel
+    /// gates callback invocation on that flag alone and the flag is immutable once the ID is
+    /// ground, so an account that installs a callback slot must have callbacks enabled or whatever
+    /// the callback enforces would be silently and permanently bypassed.
     #[test]
-    fn account_builder_rejects_callback_slot_with_disabled_flag() {
+    fn account_builder_derives_asset_callback_flag_from_callback_slots() {
         let callback_component = |slots| {
             AccountComponent::new(
                 CUSTOM_PACKAGE1.clone(),
@@ -619,23 +601,69 @@ mod tests {
                 .on_before_asset_added_to_account(Word::from([1u32, 2, 3, 4]))
                 .into_storage_slots(),
         ] {
-            let build_error = Account::builder([7; 32])
-                .with_component(NoopAuthComponent)
-                .with_component(callback_component(slots.clone()))
-                .build()
-                .unwrap_err();
-
-            assert_matches!(build_error, AccountError::BuildError(msg, _) if msg.contains("asset callback flag is disabled"));
-
-            // The same component is accepted once the flag is enabled.
-            Account::builder([7; 32])
-                .with_asset_callbacks(AssetCallbackFlag::Enabled)
+            let account = Account::builder([7; 32])
                 .with_component(NoopAuthComponent)
                 .with_component(callback_component(slots))
                 .build()
                 .unwrap();
+
+            assert_eq!(account.id().asset_callback_flag(), AssetCallbackFlag::Enabled);
         }
     }
 
+    /// Without an installed callback slot the flag is disabled, unless callbacks are explicitly
+    /// enabled to reserve the capability for the account's lifetime.
+    #[test]
+    fn account_builder_derives_disabled_asset_callback_flag_without_callback_slots() {
+        let account = Account::builder([7; 32])
+            .with_component(NoopAuthComponent)
+            .with_component(CustomComponent1 { slot0: 25 })
+            .build()
+            .unwrap();
+
+        assert_eq!(account.id().asset_callback_flag(), AssetCallbackFlag::Disabled);
+
+        let account = Account::builder([7; 32])
+            .enable_asset_callbacks()
+            .with_component(NoopAuthComponent)
+            .with_component(CustomComponent1 { slot0: 25 })
+            .build()
+            .unwrap();
+
+        assert_eq!(account.id().asset_callback_flag(), AssetCallbackFlag::Enabled);
+    }
+
+    /// Accounts constructed outside of the builder are rejected if they install a callback slot
+    /// without having callbacks enabled.
+    #[test]
+    fn account_new_rejects_callback_slot_with_disabled_flag() {
+        let account = Account::builder([7; 32])
+            .with_component(NoopAuthComponent)
+            .with_component(CustomComponent1 { slot0: 25 })
+            .build_existing()
+            .unwrap();
+
+        assert_eq!(account.id().asset_callback_flag(), AssetCallbackFlag::Disabled);
+
+        let mut slots = account.storage().clone().into_slots();
+        slots.push(StorageSlot::with_value(
+            AssetCallbacks::on_before_asset_added_to_account_slot().clone(),
+            Word::from([1u32, 2, 3, 4]),
+        ));
+        let storage = AccountStorage::new(slots).unwrap();
+
+        let error = Account::new(
+            account.id(),
+            account.vault().clone(),
+            storage,
+            account.code().clone(),
+            account.nonce(),
+            None,
+        )
+        .unwrap_err();
+
+        assert_matches!(error, AccountError::AssetCallbackSlotWithDisabledFlag { .. });
+    }
+
     // TODO: Test that a BlockHeader with a number which is not a multiple of 2^16 returns an error.
 }