0xMiden / 0xMiden/protocol

AddressInterface documents an 11-bit budget but the receiver profile encoding only has 10 bits

Ouverte
#3,554 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
Rust
Étoiles
132
Forks
167
Merge moyen
1 j 23 h
PR mergées (30 j)
110

Description

### Summary

`AddressInterface` documents a bit budget that does not match the encoder, and the mismatch is not caught in release builds.

The documented guarantee says 11 bits:

https://github.com/0xMiden/protocol/blob/134cc1fd305943ac6d9bda591dc4b87bf8046d25/crates/miden-protocol/src/address/interface.rs#L14-L17

The encoder uses 10:

https://github.com/0xMiden/protocol/blob/134cc1fd305943ac6d9bda591dc4b87bf8046d25/crates/miden-protocol/src/address/routing_parameters.rs#L267-L278

and so does the decoder:

https://github.com/0xMiden/protocol/blob/134cc1fd305943ac6d9bda591dc4b87bf8046d25/crates/miden-protocol/src/address/routing_parameters.rs#L299-L302

Four independent places agree on 10 - the inline comment ("the interface takes up 10 bits and the tag length 6 bits"), the `debug_assert`, the `<< 10` shift that places the tag length, and the `& 0b0000_0011_1111_1111` mask that recovers the interface. Only the doc comment says 11.

The documented number is also arithmetically impossible. The receiver profile is a single `u16` shared with the note tag length, and `ABSENT_NOTE_TAG_LEN = 63` shows that field is 6 bits wide. `10 + 6 = 16` fits exactly; `11 + 6 = 17` does not fit in a `u16` at all.

### Why it matters

`AddressInterface` is `#[non_exhaustive]` and the type documentation explicitly invites new variants ("so it can be extended in the future without it being a breaking change"). The bit budget is the contract a future variant is written against.

If someone trusts the documented 11 bits and adds a variant in `1024..=2047`:

- `encode_receiver_profile` guards this with a `debug_assert`, which is compiled out in release builds.
- In release, bit 10 of the interface lands in the tag-length field, so `receiver_profile` carries a corrupted tag length.
- On decode, `& 0b0000_0011_1111_1111` truncates that bit away, so the interface decodes to a different variant than the one encoded, and the tag length decodes to a wrong value or a spurious `invalid note tag length` error.

So the failure mode is silent corruption of two address fields, only in release builds, for a variant the documentation says is allowed.

### Suggested fix

Two small things:

1. Correct the documented budget to 10 bits.
2. Express the budget as a named constant and enforce it with a `const _: () = assert!(...)`, so a future variant that exceeds it fails to compile rather than relying on a `debug_assert`. This repo already uses that pattern in `constants.rs` and `transaction/kernel/memory.rs`.

Deriving the shift and the mask in `routing_parameters.rs` from that same constant would also remove the magic `10` and the hand-written mask, keeping encoder, decoder and documentation on one source of truth.

A fix is open as #3555. I put it up rather than holding it back so the diff is reviewable alongside this report; per CONTRIBUTING I am not asking for it to be merged ahead of this issue being assigned, and I am happy to close it if you would rather handle this internally.

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.