0xMiden / 0xMiden/node

Follow-ups of `protocol` 0.15 integration

Ouverte
#2,135 1 commentaire 1 réaction 0 personnes assignées Voir sur GitHub
Langage dominant
Rust
Étoiles
104
Forks
138
Merge moyen
1 j 13 h
PR mergées (30 j)
56

Description

#2095 had various comments after upgrading the protocol to version 0.15. https://github.com/0xMiden/node/pull/2132 addresses all of them, except for these remaining ones:

- `conv.rs` dedup: de-duplicate the conversion helpers shared between `conv.rs` and `conv.rs`, and rename the module to `conversions.rs`. ([link](https://github.com/0xMiden/node/pull/2095#discussion_r3293878039))
- ~~`details_commitment` design question: do we still need `details_commitment`, or can we drop the column? Investigate how we use it and potentially remove it. ([link](https://github.com/0xMiden/node/pull/2095#discussion_r3292382546))~~ <- ended up being addressed in #2132
- ~~`get_account()` is expensive: `get_account()` deserializes the full `Account` on every partial delta; switch to applying updates instead of loading the whole account each time. ([link](https://github.com/0xMiden/node/pull/2095#discussion_r3293972017))~~ tracked in #2363
- ~~`helpers.rs` explanation: add comments explaining why these helpers live here instead of upstream. ([link](https://github.com/0xMiden/node/pull/2095#discussion_r3293951779))~~ no longer relevant, code has move to crypto.
- `Felt::new_unchecked` safety: `Felt::new_unchecked` / `Word::unchecked` are used with `Rng`. Already tracked upstream in `crypto#1025`. [link](https://github.com/0xMiden/node/pull/2095#discussion_r3288189838)
Related upstream tracking issue: https://github.com/0xMiden/miden-vm/issues/3531

Some of these may require their own issue/discussion, but for now I'm writing them here to have the backlog up to date

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.