### Potential undefined behaviour in `token_uri._setTokenURI` if `may_use_special_token_mode` not set to either `0` or `1`. **File(s):** `contracts/set_erc721/token_uri.cairo` **Description:** As it stands, `_setTokenURI`, as an internal function, is only called with `may_use_special_token_mode` set to 0 or 1. However, there is currently no assertion that it is set to one of these values in `_setTokenURI`. ```python= # This is 0 or 1 # if uri_len == 1 && may_use == 2 # if uri_len == 2 && may_use == 1 if uri_len * may_use_special_token_mode == 2: let (rem) = bitwise_and(uri[1], 2 ** 59 - 1) if uri[1] == rem: # The rest has already been written in the token-id # Flag it with both special bits for continuation and 'part of token_id'. _token_uri.write(token_id, uri[0] * 4 + 3) return () end # Write the first URI with the special continuation LSB _token_uri.write(token_id, uri[0] * 4 + 1) _setExtraTokenURI(token_id, uri_len - 2, 0, uri + 1) # event_uri.emit(token_id, uri_len, uri) ``` If `may_use_special_token_mode` is at some point in the future determined non-deterministically, and the implementation to calculate its value is incorrect, this would allow the `uri_len * may_use_special_token_mode == 2` condition to pass with an invalid `uri` array. **Recommendation:** To improve the maintainability, there should be a check to assert that `may_use_special_token_mode` is a boolean value. This can be done by either using [the Cairo `bool` library](https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/cairo/common/bool.cairo), or by asserting that `may_use_special_token_mode` is 0 or 1, as below: ```diff assert_not_zero(uri_len) assert_lt_felt(uri[0], 2 ** 249) + # Assert may_use_special_token_mode >= 0 + [range_check_ptr] = may_use_special_token_mode + # Assert may_use_special_token_mode <= 1 + assert [range_check_ptr + 1] = 1 - may_use_special_token_mode + let range_check_ptr = range_check_ptr + 2 # This is 0 or 1 if uri_len * may_use_special_token_mode == 2: ``` **Update from client**: ### Testing function implemented inside core contract **File(s):** `contracts/set_erc721/token_uri.cairo` **Description:** [`token_uri.setTokenURI_`](https://github.com/briqNFT/briq-protocol/blob/next/contracts/set_erc721/token_uri.cairo#L171) is noted to be a function used only in testing. It is generally a good idea to separate smart contract code used exclusively in testing from smart contract code that constitutes core protocol functionality. **Recommendation:** Move `setTokenURI_` to a separate contract which imports `set_impl.cairo` to create a separation of concerns between testing code and core protocol code. **Update from the client:** ### Lacking documentation regarding set token URIs **File(s):** `contracts/set_erc721/token_uri.cairo` **Description:** There are numerous different ways a set token's URI can be set and used. The concept extra URI is explained sufficiently, but inline comments explaining the generation and fetching process, as well as the purpose and functionality of the "special token mode" is lacking. **Recommendation:** Consider improving the inline documentation of the contract, and more detailed documents