Skip to content

Implement Asset object#2

Merged
bobbinth merged 1 commit intomainfrom
bobbin-assets
Dec 16, 2022
Merged

Implement Asset object#2
bobbinth merged 1 commit intomainfrom
bobbin-assets

Conversation

@bobbinth
Copy link
Copy Markdown
Contributor

This PR implements Asset and related objects.

One open questions: should we allow non-fungible assets to have amounts set to 0?

Copy link
Copy Markdown
Contributor

@grjte grjte left a comment

Choose a reason for hiding this comment

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

Looks good! Most of my comments are minor but I think there's one issue that should be corrected

Copy link
Copy Markdown

@vlopes11 vlopes11 left a comment

Choose a reason for hiding this comment

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

Looks good! A couple of suggestions. I think the safe arithmetic would be a critical part, specifically for the addition

@bobbinth bobbinth requested review from grjte and vlopes11 December 16, 2022 02:32
Base automatically changed from bobbin-account-id to main December 16, 2022 02:35
Copy link
Copy Markdown

@vlopes11 vlopes11 left a comment

Choose a reason for hiding this comment

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

Looks good! One last point

Copy link
Copy Markdown

@vlopes11 vlopes11 left a comment

Choose a reason for hiding this comment

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

Looks good!

@bobbinth bobbinth merged commit 9b82565 into main Dec 16, 2022
@bobbinth bobbinth deleted the bobbin-assets branch December 16, 2022 22:30
Comment on lines +34 to +36
/// - First the asset data is hashed. This compresses an asset of an arbitrary length to 4 field
/// elements.
/// - The result of step 1 is hashed together with the ID of the faucet which issued the asset.
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.

Is there a reason to not use hash(asset_data || faucet_id)? Do we want to blind the asset_data?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but also I think it will make things a bit easier. i.e., to verify that the asset has been computed correctly, we don't need to hash the full asset data in the VM. Instead, we can hash it outside of the VM and in the VM we just hash data hash and faucet ID.

varun-doshi added a commit to varun-doshi/miden-base that referenced this pull request Nov 4, 2025
varun-doshi added a commit to varun-doshi/miden-base that referenced this pull request Nov 4, 2025
huitseeker added a commit that referenced this pull request Dec 28, 2025
…ree semantics

## Context

The Plonky3 migration includes an updated PartialSmt implementation in miden-crypto
that changes the semantics of "tracked" keys in empty trees.

## Semantic Change

In the new PartialSmt implementation, a key is considered "tracked" if either:
1. Its merkle path was explicitly added via add_path/add_proof, OR
2. The path from the leaf to the root goes through empty subtrees that are
   provably empty (they equal the empty subtree root hash).

Critically, in a COMPLETELY EMPTY tree (root == EMPTY_ROOT), ALL keys satisfy
condition #2 and are therefore implicitly tracked. This is correct cryptographic
behavior: in an empty tree, all leaves are provably empty via zero hash
computations, so there's no need to explicitly track them.

## Test Update

The test `upsert_state_commitments_fails_on_untracked_key` was using
`PartialAccountTree::default()` which creates an empty tree with EMPTY_ROOT.
Under the new semantics, all keys are implicitly tracked in this tree, so
inserting an "untracked" key would succeed rather than fail.

The fix: Initialize the partial tree with a non-empty root so that keys are
NOT implicitly tracked, allowing us to properly test the UntrackedAccountId
error path.

## Related Code

See miden-crypto/src/merkle/smt/partial/mod.rs:368-370:
```rust
// Empty tree - all leaves implicitly trackable
if self.root == Self::EMPTY_ROOT {
    return Some(SmtLeaf::new_empty(leaf_index));
}
```
huitseeker added a commit that referenced this pull request Dec 28, 2025
…ree semantics

## Context

The Plonky3 migration includes an updated PartialSmt implementation in miden-crypto
that changes the semantics of "tracked" keys in empty trees.

## Semantic Change

In the new PartialSmt implementation, a key is considered "tracked" if either:
1. Its merkle path was explicitly added via add_path/add_proof, OR
2. The path from the leaf to the root goes through empty subtrees that are
   provably empty (they equal the empty subtree root hash).

Critically, in a COMPLETELY EMPTY tree (root == EMPTY_ROOT), ALL keys satisfy
condition #2 and are therefore implicitly tracked. This is correct cryptographic
behavior: in an empty tree, all leaves are provably empty via zero hash
computations, so there's no need to explicitly track them.

## Test Update

The test `upsert_state_commitments_fails_on_untracked_key` was using
`PartialAccountTree::default()` which creates an empty tree with EMPTY_ROOT.
Under the new semantics, all keys are implicitly tracked in this tree, so
inserting an "untracked" key would succeed rather than fail.

The fix: Initialize the partial tree with a non-empty root so that keys are
NOT implicitly tracked, allowing us to properly test the UntrackedAccountId
error path.

## Related Code

See miden-crypto/src/merkle/smt/partial/mod.rs:368-370:
```rust
// Empty tree - all leaves implicitly trackable
if self.root == Self::EMPTY_ROOT {
    return Some(SmtLeaf::new_empty(leaf_index));
}
```
huitseeker added a commit that referenced this pull request Dec 28, 2025
…ree semantics

## Context

The Plonky3 migration includes an updated PartialSmt implementation in miden-crypto
that changes the semantics of "tracked" keys in empty trees.

## Semantic Change

In the new PartialSmt implementation, a key is considered "tracked" if either:
1. Its merkle path was explicitly added via add_path/add_proof, OR
2. The path from the leaf to the root goes through empty subtrees that are
   provably empty (they equal the empty subtree root hash).

Critically, in a COMPLETELY EMPTY tree (root == EMPTY_ROOT), ALL keys satisfy
condition #2 and are therefore implicitly tracked. This is correct cryptographic
behavior: in an empty tree, all leaves are provably empty via zero hash
computations, so there's no need to explicitly track them.

## Test Update

The test `upsert_state_commitments_fails_on_untracked_key` was using
`PartialAccountTree::default()` which creates an empty tree with EMPTY_ROOT.
Under the new semantics, all keys are implicitly tracked in this tree, so
inserting an "untracked" key would succeed rather than fail.

The fix: Initialize the partial tree with a non-empty root so that keys are
NOT implicitly tracked, allowing us to properly test the UntrackedAccountId
error path.

## Related Code

See miden-crypto/src/merkle/smt/partial/mod.rs:368-370:
```rust
// Empty tree - all leaves implicitly trackable
if self.root == Self::EMPTY_ROOT {
    return Some(SmtLeaf::new_empty(leaf_index));
}
```
huitseeker added a commit that referenced this pull request Dec 28, 2025
…ree semantics

## Context

The Plonky3 migration includes an updated PartialSmt implementation in miden-crypto
that changes the semantics of "tracked" keys in empty trees.

## Semantic Change

In the new PartialSmt implementation, a key is considered "tracked" if either:
1. Its merkle path was explicitly added via add_path/add_proof, OR
2. The path from the leaf to the root goes through empty subtrees that are
   provably empty (they equal the empty subtree root hash).

Critically, in a COMPLETELY EMPTY tree (root == EMPTY_ROOT), ALL keys satisfy
condition #2 and are therefore implicitly tracked. This is correct cryptographic
behavior: in an empty tree, all leaves are provably empty via zero hash
computations, so there's no need to explicitly track them.

## Test Update

The test `upsert_state_commitments_fails_on_untracked_key` was using
`PartialAccountTree::default()` which creates an empty tree with EMPTY_ROOT.
Under the new semantics, all keys are implicitly tracked in this tree, so
inserting an "untracked" key would succeed rather than fail.

The fix: Initialize the partial tree with a non-empty root so that keys are
NOT implicitly tracked, allowing us to properly test the UntrackedAccountId
error path.

## Related Code

See miden-crypto/src/merkle/smt/partial/mod.rs:368-370:
```rust
// Empty tree - all leaves implicitly trackable
if self.root == Self::EMPTY_ROOT {
    return Some(SmtLeaf::new_empty(leaf_index));
}
```
huitseeker added a commit that referenced this pull request Dec 28, 2025
…ree semantics

## Context

The Plonky3 migration includes an updated PartialSmt implementation in miden-crypto
that changes the semantics of "tracked" keys in empty trees.

## Semantic Change

In the new PartialSmt implementation, a key is considered "tracked" if either:
1. Its merkle path was explicitly added via add_path/add_proof, OR
2. The path from the leaf to the root goes through empty subtrees that are
   provably empty (they equal the empty subtree root hash).

Critically, in a COMPLETELY EMPTY tree (root == EMPTY_ROOT), ALL keys satisfy
condition #2 and are therefore implicitly tracked. This is correct cryptographic
behavior: in an empty tree, all leaves are provably empty via zero hash
computations, so there's no need to explicitly track them.

## Test Update

The test `upsert_state_commitments_fails_on_untracked_key` was using
`PartialAccountTree::default()` which creates an empty tree with EMPTY_ROOT.
Under the new semantics, all keys are implicitly tracked in this tree, so
inserting an "untracked" key would succeed rather than fail.

The fix: Initialize the partial tree with a non-empty root so that keys are
NOT implicitly tracked, allowing us to properly test the UntrackedAccountId
error path.

## Related Code

See miden-crypto/src/merkle/smt/partial/mod.rs:368-370:
```rust
// Empty tree - all leaves implicitly trackable
if self.root == Self::EMPTY_ROOT {
    return Some(SmtLeaf::new_empty(leaf_index));
}
```
huitseeker added a commit that referenced this pull request Dec 28, 2025
…ree semantics

## Context

The Plonky3 migration includes an updated PartialSmt implementation in miden-crypto
that changes the semantics of "tracked" keys in empty trees.

## Semantic Change

In the new PartialSmt implementation, a key is considered "tracked" if either:
1. Its merkle path was explicitly added via add_path/add_proof, OR
2. The path from the leaf to the root goes through empty subtrees that are
   provably empty (they equal the empty subtree root hash).

Critically, in a COMPLETELY EMPTY tree (root == EMPTY_ROOT), ALL keys satisfy
condition #2 and are therefore implicitly tracked. This is correct cryptographic
behavior: in an empty tree, all leaves are provably empty via zero hash
computations, so there's no need to explicitly track them.

## Test Update

The test `upsert_state_commitments_fails_on_untracked_key` was using
`PartialAccountTree::default()` which creates an empty tree with EMPTY_ROOT.
Under the new semantics, all keys are implicitly tracked in this tree, so
inserting an "untracked" key would succeed rather than fail.

The fix: Initialize the partial tree with a non-empty root so that keys are
NOT implicitly tracked, allowing us to properly test the UntrackedAccountId
error path.

## Related Code

See miden-crypto/src/merkle/smt/partial/mod.rs:368-370:
```rust
// Empty tree - all leaves implicitly trackable
if self.root == Self::EMPTY_ROOT {
    return Some(SmtLeaf::new_empty(leaf_index));
}
```
huitseeker added a commit that referenced this pull request Jan 6, 2026
…ree semantics

## Context

The Plonky3 migration includes an updated PartialSmt implementation in miden-crypto
that changes the semantics of "tracked" keys in empty trees.

## Semantic Change

In the new PartialSmt implementation, a key is considered "tracked" if either:
1. Its merkle path was explicitly added via add_path/add_proof, OR
2. The path from the leaf to the root goes through empty subtrees that are
   provably empty (they equal the empty subtree root hash).

Critically, in a COMPLETELY EMPTY tree (root == EMPTY_ROOT), ALL keys satisfy
condition #2 and are therefore implicitly tracked. This is correct cryptographic
behavior: in an empty tree, all leaves are provably empty via zero hash
computations, so there's no need to explicitly track them.

## Test Update

The test `upsert_state_commitments_fails_on_untracked_key` was using
`PartialAccountTree::default()` which creates an empty tree with EMPTY_ROOT.
Under the new semantics, all keys are implicitly tracked in this tree, so
inserting an "untracked" key would succeed rather than fail.

The fix: Initialize the partial tree with a non-empty root so that keys are
NOT implicitly tracked, allowing us to properly test the UntrackedAccountId
error path.

## Related Code

See miden-crypto/src/merkle/smt/partial/mod.rs:368-370:
```rust
// Empty tree - all leaves implicitly trackable
if self.root == Self::EMPTY_ROOT {
    return Some(SmtLeaf::new_empty(leaf_index));
}
```
huitseeker added a commit that referenced this pull request Jan 28, 2026
…ree semantics

## Context

The Plonky3 migration includes an updated PartialSmt implementation in miden-crypto
that changes the semantics of "tracked" keys in empty trees.

## Semantic Change

In the new PartialSmt implementation, a key is considered "tracked" if either:
1. Its merkle path was explicitly added via add_path/add_proof, OR
2. The path from the leaf to the root goes through empty subtrees that are
   provably empty (they equal the empty subtree root hash).

Critically, in a COMPLETELY EMPTY tree (root == EMPTY_ROOT), ALL keys satisfy
condition #2 and are therefore implicitly tracked. This is correct cryptographic
behavior: in an empty tree, all leaves are provably empty via zero hash
computations, so there's no need to explicitly track them.

## Test Update

The test `upsert_state_commitments_fails_on_untracked_key` was using
`PartialAccountTree::default()` which creates an empty tree with EMPTY_ROOT.
Under the new semantics, all keys are implicitly tracked in this tree, so
inserting an "untracked" key would succeed rather than fail.

The fix: Initialize the partial tree with a non-empty root so that keys are
NOT implicitly tracked, allowing us to properly test the UntrackedAccountId
error path.

## Related Code

See miden-crypto/src/merkle/smt/partial/mod.rs:368-370:
```rust
// Empty tree - all leaves implicitly trackable
if self.root == Self::EMPTY_ROOT {
    return Some(SmtLeaf::new_empty(leaf_index));
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants