Sequence

The lack of chain ID integration in contract's message hashing could pose risks in scenarios involving multiple blockchain networks. 1. The `ERC1155BaseToken` contract inherits `ERC1155Meta` (out of scope) which includes functions `metaSafeTransferFrom()` and `metaSafeBatchTransferFrom()`. The functions enable assets to be transferred from the owner to a recipient, provided there is a valid signature from the asset owner authorizing the transfer. The transfer functions use the `hashEIP712Message()` for message hashing. However, this function does not include a chain ID in its implementation. The absence of a chain ID in the EIP-712 message structure could lead to a situation where a valid signature on one blockchain might be replayed on another blockchain. 2. Although the `Trust` contract checks the validity of the signed message in `isValidSignature()`, the hashed value excludes a chain ID, and, therefore, the signed message may be reused across different Ethereum chains. Typically, the inclusion of a chain ID in the signed message is a common security measure to prevent such replay attacks, ensuring that signatures are valid only on the intended blockchain.

The `mint()` function in the contracts `ERC721Sale` and `ERC1155Sale`, is vulnerable to front running from the `MINT_ADMIN_ROLE`. If the `MINT_ADMIN_ROLE` finds a mint transaction in the mempool, and the mint caller has given sufficient approval, the `MINT_ADMIN_ROLE` can front-run the mint with a call to `setSaleDetails()` and increase the `cost`. Thus, the mint caller will pay more for the token than expected.

"Dead" code refers to code which is never executed and hence makes no impact on the final result of running a program. Dead code raises a concern, since either the code is unnecessary or the necessary code's results were ignored. 1. `ERC1155Sale._ERC20_TRANSFERFROM_SELECTOR` is never used in `ERC1155Sale`. 2. `ERC721Sale._ERC20_TRANSFERFROM_SELECTOR` is never used in `ERC721Sale`.

Factory contracts call `abi.encodePacked()` to concatenate the parameters of the contract before hashing the result. This is the salt used by `CREATE2` to generate the new contract. However, `abi.encodePacked()` does not pad the dynamic types, potentially leading to hash collisions (e.g. `[token, $token]`, will have the same hash as `[token$t, oken]`).

The orderbook, designed for handling ERC-1155 and ERC-721 tokens, is vulnerable to unintended listings of ERC-20 tokens due to the identical `transferFrom()` function signature shared between ERC-20 and ERC-721 standards. In both standards, the function signature is `transferFrom(address, address, uint256)`. However, in the context of ERC-20, the third parameter (`uint256`) represents the amount of tokens to be transferred, while in ERC-721, it denotes the unique `tokenId` of the NFT. This similarity in function structure allows for the possibility of ERC-20 tokens being listed in the orderbook as if they were ERC-721 tokens, with the `tokenId` inadvertently interpreted as the quantity of ERC-20 tokens. This misalignment can lead to transactional errors and confusion, as the orderbook is not designed to accommodate the fungible nature and transactional semantics of ERC-20 tokens. Although unlikely, if a user were to unintentionally accept what they believe is an ERC-721 order, and they have a pre-existing ERC-20 token allowance set for the orderbook, this could lead to an unintentional transfer of ERC-20 tokens to the creator of the spoofed ERC-721 order. Although there is an approval check in `_hasApprovedTokens` when creating the spoofed ERC-721 order, the ownership validation occurs only when the order is a listing.

Several initializers in the codebase call `_setupRole()` to assign privileged roles to certain addresses, instead of using `_grantRole()`. Both functions have the same effect, as `_setupRole()` just calls `_grantRole()`. `_setupRole()` will be deprecated in `v5.0.0` of OpenZeppelin contracts, and its use is not recommended in the current version used by the codebase (`v4.9.3`). See `_setupRole()` comment: ``` NOTE: This function is deprecated in favor of {_grantRole}. ```

Repeatedly accessing certain variables can be gas-intensive. To optimize gas usage, values frequently accessed should be stored in memory variables. Consider storing the value in a memory variable and reference the memory variable for the following variables: 1. `requests.length` from `Orderbook.createOrderBatch()` 2. `ordersIds.length` from `Orderbook.acceptOrderBatch()` 3. `ordersIds.length` from `Orderbook.getOrderBatch()` 4. `ordersIds.length` from `Orderbook.isOrderValidBatch()`

Solidity's `calldata` is a read-only byte-addressable space where function arguments reside. It is more cost-effective to employ `calldata` over `memory`. This is because `calldata` is not stored in memory but is directly accessed from the function call data, resulting in gas savings. It is recommended to change the `memory` to `calldata` in the mentioned functions: 1. `createOrder()` 2. `createOrderBatch()` 3. `_createOrder()` 4. `acceptOrder()` 5. `_acceptOrder()` 6. `acceptOrderBatch()` 7. `getOrderBatch()` 8. `cancelOrderBatch()` 9. `isOrderValidBatch()`

Sale contracts can be configured to accept payments in ERC-20 or native tokens. However, the functions are `payable`, so a user can send native tokens by mistake, locking them in the contract.

The `hashOrder()` function generates an order ID by hashing order data, including `uint256 quantity`. However, for orders that are partially fulfilled, the `quantity` within an order changes over time with each partial fulfillment. As a result, the `hashOrder()` function cannot consistently calculate the correct `orderId` based on the current `_orders` state variable since varying quantities lead to different hash outputs and, consequently, different IDs. In the protocol, operations such as accepting or cancelling orders use only the `orderId` to reference the order. This issue is specifically relevant for off-chain components or other protocols that use `hashOrder()` to reference or retrieve the order ID based on order data. These systems may end up with mismatched or incorrect order IDs due to the changing quantities in orders over time.