0xMiden / 0xMiden/node

Follow-ups of `protocol` 0.15 integration

Aperta
#2,135 1 commento 1 reazione 0 assegnatari Vedi su GitHub
Lingua principale
Rust
Stelle
104
Fork
138
Merge medio
1g 13h
PR unite (30g)
56

Descrizione

#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

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.