0xMiden / 0xMiden/protocol

RoleSymbol's encoded-value constants are not covered by tests, unlike TokenSymbol's

Đang mở
#3,549 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
tests
Ngôn ngữ chính
Rust
Star
132
Fork
167
Merge trung bình
1 ngày 23 giờ
Pull request đã merge (30 ngày)
110

Mô tả

### Summary

`RoleSymbol` and `TokenSymbol` have the same shape: both wrap `ShortCapitalString`, both declare hand-written `MIN_ENCODED_VALUE`/`MAX_ENCODED_VALUE` constants, and both pass those constants straight into `ShortCapitalString::try_from_encoded_felt` as the accepted decode range.

`TokenSymbol` has tests that pin both constants to the encoding they claim to describe (`test_token_symbol_max_value`, `test_token_symbol_min_value`) plus one for the lower range check (`test_token_symbol_underflow`).

`RoleSymbol` has none of them. Its test module covers round-tripping, length and character validation, and `Ord`, but never checks that either constant matches what the encoder produces.

### Why it matters

The two constants are the decode range, so their value is load-bearing rather than informational:

https://github.com/0xMiden/protocol/blob/134cc1fd305943ac6d9bda591dc4b87bf8046d25/crates/miden-protocol/src/account/access.rs#L104-L118

https://github.com/0xMiden/protocol/blob/134cc1fd305943ac6d9bda591dc4b87bf8046d25/crates/miden-protocol/src/account/access.rs#L27-L35

`MAX_ENCODED_VALUE` is a function of the alphabet size and `ShortCapitalString::MAX_LENGTH`. `RoleSymbol::ALPHABET` holds 27 symbols (`A-Z` plus `_`) against `TokenSymbol`'s 26, so the two constants are genuinely different 19-digit numbers and neither can be sanity-checked by eye. If the alphabet or the maximum length is ever changed, the constant silently stops matching the encoder, and the failure mode is either rejecting valid role symbols or accepting felts that cannot decode - on the authorization path in both directions.

To be clear this is not a bug report: the current values are correct. I checked them by reimplementing `as_element` in isolation and comparing against the declared constants:

```
RoleSymbol "A" -> 1 == MIN_ENCODED_VALUE
RoleSymbol "____________" -> 4052555153018976252 == MAX_ENCODED_VALUE
TokenSymbol "A" -> 1 == MIN_ENCODED_VALUE
TokenSymbol "ZZZZZZZZZZZZ" -> 2481152873203736562 == MAX_ENCODED_VALUE
```

What is missing is the regression guard that `TokenSymbol` already carries.

### Proposal

Add the `RoleSymbol` counterparts of the three `TokenSymbol` tests, and additionally cover the upper range check, which neither type tests today.

A change is open as #3550. 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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.