0xMiden / 0xMiden/protocol

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

未關閉
#3,554 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
主要語言
Rust
星號
132
分支
167
平均合併
1 天 23 小時
30 天內合併 PR
110

描述

### 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.

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。