Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@

### Changes

- Added a check that the guardian public key is not one of the approver public keys ([#3764](https://github.com/0xMiden/protocol/pull/3764)).
- [BREAKING] `AccountBuilder` now rejects accounts that enable asset callbacks with a private account type ([#3717](https://github.com/0xMiden/protocol/issues/3717), [#3812](https://github.com/0xMiden/protocol/pull/3812)).
- [BREAKING] Updated the Miden VM and crypto crate family to v0.31.0 and `midenc-hir-type` to v0.13.0. Execution proofs now include a format version and compatible VM and PVM verifier roots, and protocol deserialization rejects unversioned proof bytes from earlier releases. Verifier outcomes now report separate VM and precompile security parameters ([#3806](https://github.com/0xMiden/protocol/pull/3806)).
- [BREAKING] Updated the Miden VM and crypto crate family to v0.30.0 and `midenc-hir-type` to v0.12.0. `LocalTransactionProver::new` now takes `miden_prover::Prover`, `CoreLibrary` exposes one merged package, and `TransactionVerifier::verify` now returns `VerificationOutcome` so callers can handle outstanding precompile work ([#3782](https://github.com/0xMiden/protocol/pull/3782)).
- [BREAKING] Removed the `BlockProof` placeholder in favor of `ExecutionProof` on `ProvenBlock`, matching `ProvenTransaction` and `ProvenBatch`, and `LocalBlockProver::prove` now takes an `ExecutedBlock` ([#3703](https://github.com/0xMiden/protocol/pull/3703)).
- Added the `miden::protocol::tx::before_block_witness_load` kernel event, emitted before a block other than the reference block is read from the partial blockchain ([#3699](https://github.com/0xMiden/protocol/pull/3699)).
Comment on lines +16 to +21

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Added a check that the guardian public key is not one of the approver public keys ([#3764](https://github.com/0xMiden/protocol/pull/3764)).
- [BREAKING] `AccountBuilder` now rejects accounts that enable asset callbacks with a private account type ([#3717](https://github.com/0xMiden/protocol/issues/3717), [#3812](https://github.com/0xMiden/protocol/pull/3812)).
- [BREAKING] Updated the Miden VM and crypto crate family to v0.31.0 and `midenc-hir-type` to v0.13.0. Execution proofs now include a format version and compatible VM and PVM verifier roots, and protocol deserialization rejects unversioned proof bytes from earlier releases. Verifier outcomes now report separate VM and precompile security parameters ([#3806](https://github.com/0xMiden/protocol/pull/3806)).
- [BREAKING] Updated the Miden VM and crypto crate family to v0.30.0 and `midenc-hir-type` to v0.12.0. `LocalTransactionProver::new` now takes `miden_prover::Prover`, `CoreLibrary` exposes one merged package, and `TransactionVerifier::verify` now returns `VerificationOutcome` so callers can handle outstanding precompile work ([#3782](https://github.com/0xMiden/protocol/pull/3782)).
- [BREAKING] Removed the `BlockProof` placeholder in favor of `ExecutionProof` on `ProvenBlock`, matching `ProvenTransaction` and `ProvenBatch`, and `LocalBlockProver::prove` now takes an `ExecutedBlock` ([#3703](https://github.com/0xMiden/protocol/pull/3703)).
- Added the `miden::protocol::tx::before_block_witness_load` kernel event, emitted before a block other than the reference block is read from the partial blockchain ([#3699](https://github.com/0xMiden/protocol/pull/3699)).
- [BREAKING] `AccountBuilder` now rejects accounts that enable asset callbacks with a private account type ([#3812](https://github.com/0xMiden/protocol/pull/3812)).

- Fixed `RoleBasedAccessControl` role administration becoming permanently unmanageable when a role's admin was delegated to a memberless role ([#3476](https://github.com/0xMiden/protocol/pull/3476)).
- [BREAKING] Moved the `note_tag` MASM module from `miden::standards::note_tag` to `miden::standards::note::note_tag` ([#3473](https://github.com/0xMiden/protocol/pull/3473)).
- [BREAKING] Moved the `note_creator` account component MASM namespace from `miden::standards::components::wallets::note_creator` to `miden::standards::components::note::note_creator`, and moved the Rust `NoteCreator` type from `account::wallets` to `account::note_creator` ([#3473](https://github.com/0xMiden/protocol/pull/3473)).
Expand Down
59 changes: 56 additions & 3 deletions crates/miden-protocol/src/account/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ use crate::{Felt, Word};
/// installing a callback slot, so that the account retains the ability to add a callback slot via
/// an account upgrade later. This is particularly useful if new types of callbacks are introduced.
///
/// An account with an enabled flag must be of type [`AccountType::Public`].
///
/// [`AccountBuilder::with_component`] (or [`AccountBuilder::with_components`]) must be called at
/// least once, and exactly one of the added components must be an authentication component (i.e. a
/// component exporting a procedure marked with the `@auth_script` attribute). The auth component is
Expand Down Expand Up @@ -197,6 +199,20 @@ impl AccountBuilder {
AssetCallbackFlag::from(self.asset_callbacks.is_enabled() || storage.has_callback_slots())
}

/// Derives the account's [`AssetCallbackFlag`] and rejects enabling it on a private account.
fn validated_asset_callbacks(
&self,
storage: &AccountStorage,
) -> Result<AssetCallbackFlag, AccountError> {
let asset_callbacks = self.derive_asset_callbacks(storage);

if asset_callbacks.is_enabled() && self.account_type.is_private() {
return Err(AccountError::AssetCallbacksOnPrivateAccount);
}

Ok(asset_callbacks)
}
Comment on lines +203 to +214

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: do we still need to keep the non-validated one, derive_asset_callbacks? I would think that we could just replace its method body with this validated one and inline the derivation logic?


/// Grinds a new [`AccountId`] using the `init_seed` as a starting point.
fn grind_account_id(
&self,
Expand Down Expand Up @@ -235,6 +251,8 @@ 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.
/// - The account's asset callback flag is enabled while its account type is
/// [`AccountType::Private`].
/// - 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> {
Expand All @@ -248,7 +266,7 @@ impl AccountBuilder {
));
}

let asset_callbacks = self.derive_asset_callbacks(&storage);
let asset_callbacks = self.validated_asset_callbacks(&storage)?;

let seed = self.grind_account_id(
self.init_seed,
Expand Down Expand Up @@ -313,7 +331,7 @@ impl AccountBuilder {
bytes,
AccountIdVersion::Version1,
self.account_type,
self.derive_asset_callbacks(&storage),
self.validated_asset_callbacks(&storage)?,
)
};

Expand Down Expand Up @@ -612,6 +630,7 @@ mod tests {
.into_storage_slots(),
] {
let account = Account::builder([7; 32])
.account_type(AccountType::Public)
.with_component(NoopAuthComponent)
.with_component(callback_component(slots))
.build()
Expand All @@ -621,6 +640,36 @@ mod tests {
}
}

/// An enabled [`AssetCallbackFlag`] on a private account is rejected at build time.
#[test]
fn account_builder_rejects_asset_callbacks_on_private_account() {
let callback_component = AccountComponent::new(
CUSTOM_PACKAGE1.clone(),
AssetCallbacks::new()
.on_before_asset_added_to_account(Word::from([1u32, 2, 3, 4]))
.into_storage_slots(),
AccountComponentMetadata::new("test::callback_component"),
)
.expect("component should be valid");

let error = Account::builder([7; 32])
.account_type(AccountType::Private)
.with_component(NoopAuthComponent)
.with_component(callback_component)
.build()
.expect_err("private account with a callback slot should be rejected");
assert_matches!(error, AccountError::AssetCallbacksOnPrivateAccount);

let error = Account::builder([7; 32])
.account_type(AccountType::Private)
.with_component(NoopAuthComponent)
.with_component(CustomComponent1 { slot0: 25 })
.enable_asset_callbacks()
.build()
.expect_err("private account with enabled callbacks should be rejected");
assert_matches!(error, AccountError::AssetCallbacksOnPrivateAccount);
}

/// Without an installed callback slot the flag is disabled, unless callbacks are explicitly
/// enabled to reserve the capability for the account's lifetime.
#[test]
Expand All @@ -632,7 +681,11 @@ mod tests {
let account = builder.clone().build().unwrap();
assert_eq!(account.id().asset_callback_flag(), AssetCallbackFlag::Disabled);

let account = builder.enable_asset_callbacks().build().unwrap();
let account = builder
.account_type(AccountType::Public)
.enable_asset_callbacks()
.build()
.unwrap();
assert_eq!(account.id().asset_callback_flag(), AssetCallbackFlag::Enabled);
}

Expand Down
4 changes: 4 additions & 0 deletions crates/miden-protocol/src/errors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ pub enum AccountError {
"storage of account {0} contains an asset callback slot but its asset callback flag is disabled, so the callback would never be invoked"
)]
AssetCallbackSlotWithDisabledFlag(AccountId),
#[error(
"account has asset callbacks enabled but is of type private, so holders of its assets could not load its state to dispatch the callbacks"
)]
AssetCallbacksOnPrivateAccount,
#[error("failed to update asset vault")]
AssetVaultUpdateError(#[source] AssetVaultError),
#[error("account build error: {0}")]
Expand Down
15 changes: 15 additions & 0 deletions crates/miden-standards/src/account/faucets/fungible/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,11 @@ impl TryFrom<&Account> for FungibleFaucet {
/// Caller passes a fully-configured [`AuthSingleSig`]. Every authority-gated setter on the faucet
/// (`mint_and_send`, the metadata setters, the policy setters, and `pause` / `unpause`) requires a
/// signature.
///
/// # Errors
///
/// Returns an error if `account_type` is [`AccountType::Private`] while `token_policy_manager`
/// registers a transfer policy, since such a policy enables asset callbacks.
pub fn create_singlesig_user_fungible_faucet(
init_seed: [u8; 32],
faucet: FungibleFaucet,
Expand All @@ -589,6 +594,11 @@ pub fn create_singlesig_user_fungible_faucet(
}

/// Creates a new **user-account** fungible faucet authenticated by a multisig approver set.
///
/// # Errors
///
/// Returns an error if `account_type` is [`AccountType::Private`] while `token_policy_manager`
/// registers a transfer policy, since such a policy enables asset callbacks.
pub fn create_multisig_user_fungible_faucet(
init_seed: [u8; 32],
faucet: FungibleFaucet,
Expand All @@ -610,6 +620,11 @@ pub fn create_multisig_user_fungible_faucet(
}

/// Creates a new **user-account** fungible faucet authenticated by a guardian-backed multisig.
///
/// # Errors
///
/// Returns an error if `account_type` is [`AccountType::Private`] while `token_policy_manager`
/// registers a transfer policy, since such a policy enables asset callbacks.
pub fn create_guarded_user_fungible_faucet(
init_seed: [u8; 32],
faucet: FungibleFaucet,
Expand Down
51 changes: 42 additions & 9 deletions crates/miden-standards/src/account/faucets/fungible/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use miden_protocol::account::{
StorageMapKey,
};
use miden_protocol::asset::{AssetAmount, FungibleAsset, TokenSymbol};
use miden_protocol::errors::AccountError;
use miden_protocol::{Felt, Word};

use super::{
Expand Down Expand Up @@ -70,7 +71,7 @@ fn user_fungible_faucet_with_single_sig() {
sample_faucet(),
auth_component,
allow_all_policy_manager(),
AccountType::Private,
AccountType::Public,
)
.unwrap();

Expand Down Expand Up @@ -129,7 +130,7 @@ fn user_fungible_faucet_with_multisig() {
sample_faucet(),
auth_component,
allow_all_policy_manager(),
AccountType::Private,
AccountType::Public,
)
.unwrap();

Expand Down Expand Up @@ -166,7 +167,7 @@ fn user_fungible_faucet_with_guarded_multisig() {
sample_faucet(),
auth_component,
allow_all_policy_manager(),
AccountType::Private,
AccountType::Public,
)
.unwrap();

Expand Down Expand Up @@ -276,12 +277,22 @@ fn faucet_create_from_account() {
}

/// Every fungible faucet factory must grind `AssetCallbackFlag::Enabled` into the account ID when
/// the policy manager registers a transfer policy, and `Disabled` when it does not.
/// the policy manager registers a transfer policy, and `Disabled` when it does not. A faucet with a
/// transfer policy must be public.
#[rstest::rstest]
#[case::with_transfer_policy(allow_all_policy_manager(), AssetCallbackFlag::Enabled)]
#[case::without_transfer_policy(mint_burn_only_policy_manager(), AssetCallbackFlag::Disabled)]
#[case::with_transfer_policy(
allow_all_policy_manager(),
AccountType::Public,
AssetCallbackFlag::Enabled
)]
#[case::without_transfer_policy(
mint_burn_only_policy_manager(),
AccountType::Private,
AssetCallbackFlag::Disabled
)]
fn fungible_faucet_factories_encode_transfer_policy_callback_flag(
#[case] token_policy_manager: TokenPolicyManager,
#[case] account_type: AccountType,
#[case] expected_flag: AssetCallbackFlag,
) {
use miden_protocol::testing::account_id::ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE;
Expand All @@ -296,7 +307,7 @@ fn fungible_faucet_factories_encode_transfer_policy_callback_flag(
sample_faucet(),
AuthSingleSig::new(approver),
token_policy_manager.clone(),
AccountType::Private,
account_type,
)
.unwrap();
assert_eq!(singlesig.id().asset_callback_flag(), expected_flag);
Expand All @@ -306,7 +317,7 @@ fn fungible_faucet_factories_encode_transfer_policy_callback_flag(
sample_faucet(),
user_faucet_multisig(sample_approvers(3), 2).unwrap(),
token_policy_manager.clone(),
AccountType::Private,
account_type,
)
.unwrap();
assert_eq!(multisig.id().asset_callback_flag(), expected_flag);
Expand All @@ -316,7 +327,7 @@ fn fungible_faucet_factories_encode_transfer_policy_callback_flag(
sample_faucet(),
user_faucet_guarded(sample_approvers(3), 2, GuardianConfig::new(approver)).unwrap(),
token_policy_manager.clone(),
AccountType::Private,
account_type,
)
.unwrap();
assert_eq!(guarded.id().asset_callback_flag(), expected_flag);
Expand All @@ -333,6 +344,28 @@ fn fungible_faucet_factories_encode_transfer_policy_callback_flag(
assert_eq!(network.id().asset_callback_flag(), expected_flag);
}

/// The user faucet factories must reject a private faucet with a transfer policy.
#[test]
fn private_fungible_faucet_with_transfer_policy_is_rejected() {
let approver = Approver::new(
PublicKeyCommitment::from(Word::new([Felt::from(11_u32); 4])),
AuthScheme::Falcon512Poseidon2,
);

let err = create_singlesig_user_fungible_faucet(
[21u8; 32],
sample_faucet(),
AuthSingleSig::new(approver),
allow_all_policy_manager(),
AccountType::Private,
)
.expect_err("private faucet with a transfer policy should be rejected");
assert_matches!(
err,
FungibleFaucetError::AccountError(AccountError::AssetCallbacksOnPrivateAccount)
);
}

/// Check that the obtaining of the fungible faucet procedure roots does not panic.
#[test]
fn get_faucet_procedures() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,11 @@ impl TryFrom<&Account> for NonFungibleFaucet {
/// The caller passes a fully-configured [`AuthSingleSig`]. Every authority-gated setter
/// (`mint_and_send`, the metadata setters, the policy setters, and `pause` / `unpause`) requires a
/// signature.
///
/// # Errors
///
/// Returns an error if `account_type` is [`AccountType::Private`] while `token_policy_manager`
/// registers a transfer policy, since such a policy enables asset callbacks.
pub fn create_user_non_fungible_faucet(
init_seed: [u8; 32],
faucet: NonFungibleFaucet,
Expand Down
44 changes: 39 additions & 5 deletions crates/miden-standards/src/account/faucets/non_fungible/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use assert_matches::assert_matches;
use miden_protocol::account::auth::{AuthScheme, PublicKeyCommitment};
use miden_protocol::account::{AccountId, AccountType, AssetCallbackFlag};
use miden_protocol::asset::{FungibleAsset, TokenSymbol};
use miden_protocol::errors::AccountError;
use miden_protocol::{Felt, Word};

use super::{
Expand All @@ -10,11 +12,11 @@ use super::{
};
use crate::account::access::AccessControl;
use crate::account::auth::{Approver, AuthSingleSig};
use crate::account::faucets::TokenName;
use crate::account::faucets::test_utils::{
allow_all_policy_manager,
mint_burn_only_policy_manager,
};
use crate::account::faucets::{NonFungibleFaucetError, TokenName};
use crate::account::fees::FeePolicyManager;
use crate::account::policies::TokenPolicyManager;

Expand All @@ -41,12 +43,22 @@ fn sample_faucet() -> NonFungibleFaucet {
}

/// Every non-fungible faucet factory must grind `AssetCallbackFlag::Enabled` into the account ID
/// when the policy manager registers a transfer policy, and `Disabled` when it does not.
/// when the policy manager registers a transfer policy, and `Disabled` when it does not. A faucet
/// with a transfer policy must be public.
#[rstest::rstest]
#[case::with_transfer_policy(allow_all_policy_manager(), AssetCallbackFlag::Enabled)]
#[case::without_transfer_policy(mint_burn_only_policy_manager(), AssetCallbackFlag::Disabled)]
#[case::with_transfer_policy(
allow_all_policy_manager(),
AccountType::Public,
AssetCallbackFlag::Enabled
)]
#[case::without_transfer_policy(
mint_burn_only_policy_manager(),
AccountType::Private,
AssetCallbackFlag::Disabled
)]
fn non_fungible_faucet_factories_encode_transfer_policy_callback_flag(
#[case] token_policy_manager: TokenPolicyManager,
#[case] account_type: AccountType,
#[case] expected_flag: AssetCallbackFlag,
) {
use miden_protocol::testing::account_id::ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE;
Expand All @@ -61,7 +73,7 @@ fn non_fungible_faucet_factories_encode_transfer_policy_callback_flag(
sample_faucet(),
AuthSingleSig::new(approver),
token_policy_manager.clone(),
AccountType::Private,
account_type,
)
.unwrap();
assert_eq!(user.id().asset_callback_flag(), expected_flag);
Expand All @@ -78,6 +90,28 @@ fn non_fungible_faucet_factories_encode_transfer_policy_callback_flag(
assert_eq!(network.id().asset_callback_flag(), expected_flag);
}

/// The user faucet factory must reject a private faucet with a transfer policy.
#[test]
fn private_non_fungible_faucet_with_transfer_policy_is_rejected() {
let approver = Approver::new(
PublicKeyCommitment::from(Word::new([Felt::from(11_u32); 4])),
AuthScheme::Falcon512Poseidon2,
);

let err = create_user_non_fungible_faucet(
[31u8; 32],
sample_faucet(),
AuthSingleSig::new(approver),
allow_all_policy_manager(),
AccountType::Private,
)
.expect_err("private faucet with a transfer policy should be rejected");
assert_matches!(
err,
NonFungibleFaucetError::AccountCreationFailed(AccountError::AssetCallbacksOnPrivateAccount)
);
}

/// `compute_asset_commitment` is deterministic and salt-sensitive.
#[test]
fn compute_asset_commitment_is_salt_sensitive() {
Expand Down
3 changes: 2 additions & 1 deletion crates/miden-standards/src/account/policies/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ struct PolicyConfig {
/// the flag is an immutable property of the account ID, it applies for the faucet's entire
/// lifetime, so promoting a reserved policy later via `set_send_policy` / `set_receive_policy`
/// enforces it against the whole circulating supply rather than only assets minted after the
/// switch.
/// switch. A faucet that registers a transfer policy must be created as
/// [`AccountType::Public`][miden_protocol::account::AccountType::Public].
///
/// The slots are omitted only when no send or receive policy of any kind is registered, in which
/// case the faucet's account ID is created with
Expand Down
Loading
Loading