0xMiden / 0xMiden/protocol

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

Aberta
#3,549 0 comentários 0 reações 0 responsáveis Ver no GitHub
tests
Linguagem predominante
Rust
Estrelas
132
Forks
167
Merge médio
1d 23h
PRs com merge (30d)
110

Descrição

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

Guia de contribuição

Abrir o guia de contribuição

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.