Conversation
Update all miden-protocol dependencies from 0.14.0-alpha.1 to 0.14.0-beta.1 (crates.io). Update miden-crypto from 0.19.7 to 0.22. Bump node workspace version to 0.14.0-beta.1. Key migration changes: - Serialization traits moved to utils::serde submodule - Felt::as_int() -> Felt::as_canonical_u64() - falcon512_rpo -> falcon512_poseidon2 - ProvenTransactionBuilder removed, use ProvenTransaction::new() - OutputNote variants: Full -> Public, Header -> Private - Asset is now key-value (two words instead of one) - MmrProof fields are now methods - account_id_to_smt_key -> AccountIdKey::from() - miden-air removed (replaced by miden-core 0.21) - SmtStorage trait methods now take &mut self - Various other API changes in miden-crypto 0.22
b785bde to
922cd86
Compare
v0.14.0-beta.1
bb9974c to
5320e57
Compare
|
(force pushed coz I had unsigned commits) |
4c74ff6 to
4d53991
Compare
| #[error("the ntx task panicked")] | ||
| #[expect(dead_code)] | ||
| Panic(#[source] JoinError), |
- Use LargeSmt::load() instead of LargeSmt::new() in load_smt to support loading from non-empty RocksDB storage on restart (new() now errors with StorageNotEmpty in miden-crypto 0.22) - Fix prune logic to remove all versions before cutoff while keeping at least one version per lineage to preserve current state - Only pop roots from SmtForest that are no longer referenced by any lineage (prevents removing shared roots) - Fix test helpers tree_id_for_root/tree_id_for_vault_root to use get_root_at_block instead of latest_root - Update all prune test assertions for the new version-tracking model (our AccountStateForest uses explicit version entries rather than LargeSmtForest's deduplicated tree count)
Replace ~130 lines of hand-rolled DER parsing with proper library calls: - Use spki::SubjectPublicKeyInfoRef::from_der() for SPKI public key parsing - Use k256::ecdsa::Signature::from_der() for DER signature decoding This restores the simplicity of the original code before the migration, where PublicKey::from_der() and Signature::from_der() were available directly on the miden-crypto types.
Resolve conflicts in ntx-builder query tests by keeping test_utils imports and fixing TransactionId::new() signature for the beta API.
| batch.put_cf(metadata_cf, ENTRY_COUNT_KEY, new_entry_count.to_be_bytes()); | ||
| } | ||
|
|
||
| batch.put_cf(metadata_cf, ROOT_KEY, new_root.to_bytes()); |
There was a problem hiding this comment.
why was this removed?
| self.initial_account_commitment, | ||
| self.final_account_commitment, | ||
| Word::empty(), | ||
| AccountUpdateDetails::Private, |
There was a problem hiding this comment.
I was wondering why this is set to Private.
ProvenTransaction::new used to default to Private if no account_update was specified, so this preserves the behavior
Remove stale comment about missing re-exports (fixed in miden-crypto 0.22.5) and import from miden_protocol::crypto::merkle::smt to match the convention used on next.
Public and private accounts were created with overlapping seed index ranges, which in the v0.14 beta can produce accounts with duplicate ID prefixes. Use separate non-overlapping index offsets for each storage mode within a block.
The previous approach used low-entropy seeds (index padded with zeros) which could produce AccountId prefix collisions during the hash grinding process. Use a seeded PRNG to generate full 32-byte random seeds per account while maintaining deterministic behavior.
Resolve conflict in state/loader.rs imports: keep RocksDbOptions from next's rocksdb config PR and AccountIdKey from our beta migration.
| let rebuilt = PublicOutputNote::new(public_note.as_note().clone()) | ||
| .expect("rebuilding an already-valid public output note should not fail"); | ||
| OutputNote::Public(rebuilt) |
There was a problem hiding this comment.
this works but we might want to propagate minify_script to PublicOutputNote to avoid the reconstruction.
There was a problem hiding this comment.
minify_script() is called in PublicOutputNote::new() - so, by construction all PublicOutputNotes have minified scripts. But maybe I didn't understand the comment?
There was a problem hiding this comment.
Not quite: I was thinking of mutating PublicOutputNote, something like:
fn strip_output_note_decorators<'a>(
notes: impl Iterator<Item = &'a OutputNote> + 'a,
) -> impl Iterator<Item = OutputNote> + 'a {
notes.map(|note| match note {
OutputNote::Public(public_note) => {
let minified = public_note.clone().minify_script(); // <------ would need this
OutputNote::Public(public_note)
},
OutputNote::Private(header) => OutputNote::Private(header.clone()),
})
}so rather than fully reconstructing PublicOutputNote::new(), we would avoid some of the other checks that new constructor performs. These checks are not expensive at all, but I think it would also be cleaner to "only do the work necessary" to strip the decorators, using the constructor to do it feels somewhat hacky.
| /// `SmtForest` for efficient account storage reconstruction. | ||
| /// Populated during block import with storage and vault SMTs. | ||
| forest: LargeSmtForest<ForestInMemoryBackend>, | ||
| forest: SmtForest, |
There was a problem hiding this comment.
hmm is this change correct? What's the reason this was changed to not use LargeSmtForest anymore?
There was a problem hiding this comment.
There was a problem hiding this comment.
I'd prefer to not undo the migration to LargeSmtForest in next, it simplified a lot of code.
- Remove dead `Panic(JoinError)` variant from `NtxError` - Remove unused `get_root`/`set_root` and `ROOT_KEY` from `RocksDbStorage` - Replace verbose `std::iter::empty` with `Vec::new()` in rpc tests - Extract inline fee construction to `test_fee()` variable in store tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| // cutoff = 53 - 50 = 3: each of the 3 lineages has one version < 3 that gets removed | ||
| // (account1 v=1, account2 v=2, account3 v=2), since each also has a newer version. |
There was a problem hiding this comment.
also I don't fully understand the lineage changes cc @drahnr
crates/validator/src/signers/kms.rs
Outdated
| .subject_public_key | ||
| .as_bytes() | ||
| .ok_or_else(|| DeserializationError::InvalidValue("Invalid SPKI BIT STRING".into()))?; | ||
| let pub_key = PublicKey::read_from_bytes(sec1_bytes)?; |
There was a problem hiding this comment.
We don't want these diffs. As in we want to keep:
let pub_key = PublicKey::from_der(spki_der)?;There was a problem hiding this comment.
I think the parsing was introduced during the migration because miden-crypto 0.22 does not contain the PublicKey::from_der() and Signature::from_der() methods. Should this also be ported to 0.22.X?
There was a problem hiding this comment.
If from_der() is not in v0.22, @sergerad - could you make a quick PR to cherry-pick relevant commits into v0.22.x branch in miden-crypto and I'll make another patch release.
There was a problem hiding this comment.
Just in case, there's also this that might need porting. You probably saw already but mentioning it in the unlikely case you didn't, so that we don't waste a patch release.
|
I created #1793 using this PR as base. It is a really simple one. If you prefer I can push the commit here, or feel free to just copy the code. |
|
go right ahead @SantiagoPittella! |
|
Pushed a commit to point to 0xMiden/protocol#2598 which fixes the stress test (and the overall functionality of the account tree). Also removed the crypto patch in favor of updating the package version to the recently-released |
Summary
0.14.0-alpha.1to0.14.0-beta.10.19.7to0.220.14.0-beta.1Key API migrations
utils::serdesubmoduleFelt::as_int()->Felt::as_canonical_u64()falcon512_rpo->falcon512_poseidon2ProvenTransactionBuilderremoved, useProvenTransaction::new()+TxAccountUpdate(refactor: removeProvenTransactionBuilderin favor ofProvenTransaction::newprotocol#2567)OutputNotevariants:Full->Public,Header->PrivateAssetis now key-value (two words instead of one)MmrProoffields are now methodsaccount_id_to_smt_key->AccountIdKey::from()SmtStoragetrait methods now take&mut selfAccountStateForestrewritten to replace removedLargeSmtForestTest plan
cargo check --all-targetspassesmake lintpasses (clippy, formatting, machete)🤖 Generated with Claude Code