Skip to content
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Features

- Added `active_note::get_storage_info` and `active_note::get_bounded_storage`, and used the latter in the `OwnerConfig` and `FaucetPolicyConfig` ([#3563](https://github.com/0xMiden/protocol/pull/3563)).

### Changes

- [BREAKING] Changed asset callbacks into validation-only interfaces that return no asset value; the transaction kernel retains and uses the original value, preventing callbacks from modifying it. The kernel commitment changes ([#3505](https://github.com/0xMiden/protocol/issues/3505), [#3513](https://github.com/0xMiden/protocol/pull/3513)).
Expand Down
71 changes: 71 additions & 0 deletions crates/miden-protocol/asm/protocol/src/active_note.masm
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ const ERR_NOTE_DATA_DOES_NOT_MATCH_COMMITMENT = "note data does not match the co
const ERR_NOTE_INVALID_NUMBER_OF_STORAGE_ITEMS =
"the specified number of note storage items does not match the actual number"

const ERR_NOTE_TOO_MANY_STORAGE_ITEMS =
"the number of note storage items exceeds the maximum accepted by the note script"

# ACTIVE NOTE PROCEDURES
# =================================================================================================
#
Expand Down Expand Up @@ -275,6 +278,74 @@ pub proc get_storage(dest_ptr: MemoryAddress) -> u16
# => [num_storage_items]
end

#! Returns the storage commitment and the number of storage items of the active note.
#!
#! Inputs: []
#! Outputs: [NOTE_STORAGE_COMMITMENT, num_storage_items]
#!
#! Where:
#! - NOTE_STORAGE_COMMITMENT is the commitment to the note's storage.
#! - num_storage_items is the number of storage items the active note was created with.
#!
#! Panics if:
#! - no note is currently active.
#!
#! Invocation: exec
pub proc get_storage_info() -> (word, u16)
Comment thread
bobbinth marked this conversation as resolved.
Outdated
# push a placeholder note_index (ignored when is_active_note = 1) and the active note flag
push.0.1
# => [is_active_note = 1, note_index = 0]

exec.input_note_internal::get_storage_info_raw
# => [NOTE_STORAGE_COMMITMENT, num_storage_items]
end

#! Writes the active note's storage to memory starting at the specified address, provided the note
#! carries at most `max_num_storage_items` items.
#!
#! Inputs:
#! Stack: [dest_ptr, max_num_storage_items]
#! Advice Map: { NOTE_STORAGE_COMMITMENT: [STORAGE] }
#! Outputs:
#! Stack: [num_storage_items]
#!
#! Where:
#! - dest_ptr is the memory address to write the note storage.
#! - max_num_storage_items is the largest number of storage items the caller accepts.
#! - NOTE_STORAGE_COMMITMENT is the commitment to the note's storage.
#! - STORAGE is the data corresponding to the note's storage.
#! - num_storage_items is the number of storage items the active note was created with.
#!
#! Panics if:
#! - no note is currently active.
#! - num_storage_items is greater than max_num_storage_items.
#!
#! Invocation: exec
pub proc get_bounded_storage(dest_ptr: MemoryAddress, max_num_storage_items: u16) -> u16

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably not a good idea, but I wonder if we should just change the get_storage procedure to work like this. The biggest issue is that that would be a pretty bad breaking change (not easy to detect downstream) - but if we were designing the protocol from scratch, that's how I'd probably make get_storage work.

# push a placeholder note_index (ignored when is_active_note = 1) and the active note flag
push.0.1
# => [is_active_note = 1, note_index = 0, dest_ptr, max_num_storage_items]

exec.input_note_internal::get_storage_info_raw
# => [NOTE_STORAGE_COMMITMENT, num_storage_items, dest_ptr, max_num_storage_items]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is the same as exec.get_storage_info right? I'd replace that.


# reject an oversized note before its storage preimage is loaded and hashed
dup.4 dup.7 lte assert.err=ERR_NOTE_TOO_MANY_STORAGE_ITEMS
# => [NOTE_STORAGE_COMMITMENT, num_storage_items, dest_ptr, max_num_storage_items]
Comment thread
bobbinth marked this conversation as resolved.
Outdated

# the bound is no longer needed
movup.6 drop
# => [NOTE_STORAGE_COMMITMENT, num_storage_items, dest_ptr]

# save num_storage_items for the return value
dup.4 movdn.6
# => [NOTE_STORAGE_COMMITMENT, num_storage_items, dest_ptr, num_storage_items]

# write the inputs to the provided destination pointer
exec.write_storage_to_memory
# => [num_storage_items]
end

#! Returns the metadata of the active note.
#!
#! Inputs: []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const ERR_FAUCET_POLICY_CONFIG_UNEXPECTED_NUMBER_OF_STORAGE_ITEMS = "faucet poli
#!
#! Panics if:
#! - the consuming account is not the note's target account, or the target attachment is missing.
#! - the number of storage items exceeds NUM_ITEMS_SET_POLICY.
#! - the selector is not a known action.
#! - the number of storage items does not match the selected action.
#! - the note sender is not authorized, or the policy root is not an allowed alternative (per the
Expand All @@ -77,7 +78,7 @@ pub proc main
# => [pad(16)]

# write the note storage to memory starting at STORAGE_PTR
push.STORAGE_PTR exec.active_note::get_storage
push.NUM_ITEMS_SET_POLICY push.STORAGE_PTR exec.active_note::get_bounded_storage
# => [num_storage_items]

mem_load.SELECTOR_PTR
Expand Down
6 changes: 5 additions & 1 deletion crates/miden-standards/asm/standards/notes/owner_config.masm
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ const NUM_ITEMS_TRANSFER_OWNERSHIP = 3
const NUM_ITEMS_ACCEPT_OWNERSHIP = 1
const NUM_ITEMS_RENOUNCE_OWNERSHIP = 1

# Largest storage item count any action uses
const MAX_NUM_ITEMS = NUM_ITEMS_TRANSFER_OWNERSHIP

# ERRORS
# =================================================================================================

Expand Down Expand Up @@ -58,6 +61,7 @@ const ERR_OWNER_CONFIG_UNEXPECTED_NUMBER_OF_STORAGE_ITEMS = "owner config note s
#!
#! Panics if:
#! - the consuming account is not the note's target account, or the target attachment is missing.
#! - the number of storage items exceeds MAX_NUM_ITEMS.
#! - the selector is not a known action.
#! - the number of storage items does not match the selected action.
#! - the note sender is not authorized for the selected action (per the `Ownable2Step` procedures).
Expand All @@ -73,7 +77,7 @@ pub proc main
# => [pad(16)]

# write the note storage to memory starting at STORAGE_PTR
push.STORAGE_PTR exec.active_note::get_storage
push.MAX_NUM_ITEMS push.STORAGE_PTR exec.active_note::get_bounded_storage

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we propagate this everywhere (maybe in a different PR)? Basically, I'm not sure there should be much usage of get_storage any more.

# => [num_storage_items]

mem_load.SELECTOR_PTR
Expand Down
27 changes: 26 additions & 1 deletion crates/miden-testing/tests/scripts/faucet_policy_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ use alloc::vec::Vec;
use miden_processor::crypto::random::RandomCoin;
use miden_protocol::account::{Account, AccountBuilder, AccountId, AccountType, AssetCallbackFlag};
use miden_protocol::asset::AssetAmount;
use miden_protocol::errors::protocol::ERR_NOTE_TOO_MANY_STORAGE_ITEMS;
use miden_protocol::note::Note;
use miden_protocol::testing::account_id::AccountIdBuilder;
use miden_protocol::{Felt, Word};
use miden_protocol::{Felt, MAX_NOTE_STORAGE_ITEMS, Word};
use miden_standards::account::access::{Authority, Ownable2Step};
use miden_standards::account::faucets::{FungibleFaucet, TokenName};
use miden_standards::account::policies::{
Expand Down Expand Up @@ -248,6 +249,30 @@ async fn wrong_storage_item_count_fails() -> anyhow::Result<()> {
Ok(())
}

/// A note carrying more storage items than any action accepts is rejected before its storage is
/// loaded, so the work an oversized note can impose on whoever attempts to consume it is bounded by
/// the layout the script accepts rather than by `MAX_NOTE_STORAGE_ITEMS`.
#[tokio::test]
async fn oversized_storage_is_rejected_before_the_storage_is_loaded() -> anyhow::Result<()> {
let owner = AccountIdBuilder::new().build_with_seed([1; 32]);

let mut builder = MockChain::builder();
let faucet = create_faucet_with_policies(&mut builder, owner)?;
let mock_chain = builder.build()?;
let mut rng = RandomCoin::new([Felt::from(100u32); 4].into());

let storage = vec![Felt::from(0u32); MAX_NOTE_STORAGE_ITEMS];
let note = malformed_faucet_policy_config_note(owner, faucet.id(), storage, &mut rng)?;
let tx = mock_chain
.build_transaction(faucet.clone())
.unauthenticated_input_note(note)
.build()?;
let result = tx.execute().await;

assert_transaction_executor_error!(result, ERR_NOTE_TOO_MANY_STORAGE_ITEMS);
Ok(())
}

/// The note is bound to its target faucet, so a decoy faucet cannot consume a note meant for
/// another one. The decoy carries the same `TokenPolicyManager` setup with the same owner, so the
/// sender-based authorization would pass; consuming a note targeted at a different faucet aborts at
Expand Down
28 changes: 27 additions & 1 deletion crates/miden-testing/tests/scripts/ownable2step/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ extern crate alloc;
use alloc::vec::Vec;

use miden_processor::crypto::random::RandomCoin;
use miden_protocol::Felt;
use miden_protocol::account::{Account, AccountId, AccountType};
use miden_protocol::errors::protocol::ERR_NOTE_TOO_MANY_STORAGE_ITEMS;
use miden_protocol::note::Note;
use miden_protocol::testing::account_id::AccountIdBuilder;
use miden_protocol::{Felt, MAX_NOTE_STORAGE_ITEMS};
use miden_standards::errors::standards::{
ERR_OWNER_CONFIG_TARGET_ACCOUNT_MISMATCH,
ERR_OWNER_CONFIG_UNEXPECTED_NUMBER_OF_STORAGE_ITEMS,
Expand Down Expand Up @@ -182,6 +183,31 @@ async fn wrong_storage_item_count_fails() -> anyhow::Result<()> {
Ok(())
}

/// A note carrying more storage items than any action accepts is rejected before its storage is
/// loaded, so the work an oversized note can impose on whoever attempts to consume it is bounded by
/// the longest layout the script accepts rather than by `MAX_NOTE_STORAGE_ITEMS`.
#[tokio::test]
async fn oversized_storage_is_rejected_before_the_storage_is_loaded() -> anyhow::Result<()> {
let owner = AccountIdBuilder::new().build_with_seed([1; 32]);

let account = create_ownable_account(owner)?;
let mut builder = MockChain::builder();
builder.add_account(account.clone())?;
let mock_chain = builder.build()?;
let mut rng = RandomCoin::new([Felt::from(100u32); 4].into());

let storage = vec![Felt::from(0u32); MAX_NOTE_STORAGE_ITEMS];
let note = malformed_owner_config_note(owner, account.id(), storage, &mut rng)?;
let tx = mock_chain
.build_transaction(account.clone())
.unauthenticated_input_note(note)
.build()?;
let result = tx.execute().await;

assert_transaction_executor_error!(result, ERR_NOTE_TOO_MANY_STORAGE_ITEMS);
Ok(())
}

/// The note is bound to its target account, so a decoy account cannot consume a note meant for
/// another account. The decoy carries the same `Ownable2Step` setup with the same owner, so the
/// sender-based authorization would pass; consuming a note targeted at a different account aborts
Expand Down
2 changes: 2 additions & 0 deletions docs/src/protocol_library.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ Active note procedures can be used to fetch data from the note that is currently
| `remove_all_assets` | Removes all remaining [assets](note.md#assets) from the active note and writes them into memory starting at the specified address.<br/><br/>**Inputs:** `[dest_ptr]`<br/>**Outputs:** `[num_assets]` | Note |
| `get_recipient` | Returns the [recipient](note.md#note-recipient-restricting-consumption) of the active note.<br/><br/>**Inputs:** `[]`<br/>**Outputs:** `[RECIPIENT]` | Note |
| `get_storage` | Writes the note's [inputs](note.md#storage) to the specified memory address.<br/><br/>**Inputs:** `[dest_ptr]`<br/>**Outputs:** `[num_storage_items, dest_ptr]` | Note |
| `get_storage_info` | Returns the [inputs](note.md#storage) commitment and length of the active note without loading the storage itself.<br/><br/>**Inputs:** `[]`<br/>**Outputs:** `[NOTE_STORAGE_COMMITMENT, num_storage_items]` | Note |
Comment thread
bobbinth marked this conversation as resolved.
| `get_bounded_storage` | Writes the note's [inputs](note.md#storage) to the specified memory address, provided the note carries at most the specified number of items. Rejects a longer note before its storage is loaded.<br/><br/>**Inputs:** `[dest_ptr, max_num_storage_items]`<br/>**Outputs:** `[num_storage_items]` | Note |
| `get_metadata` | Returns the [metadata](note.md#metadata) of the active note.<br/><br/>**Inputs:** `[]`<br/>**Outputs:** `[METADATA]` | Note |
| `get_sender` | Returns the sender of the active note.<br/><br/>**Inputs:** `[]`<br/>**Outputs:** `[sender_id_suffix, sender_id_prefix]` | Note |
| `get_serial_number` | Returns the [serial number](note.md#serial-number) of the active note.<br/><br/>**Inputs:** `[]`<br/>**Outputs:** `[SERIAL_NUMBER]` | Note |
Expand Down
Loading