NeoSwap

The `cancel_sol()` and `claim_sol()` functions check that the signer is the initializer of the swap. This means users cannot retrieve their funds themselves and require the admin to cancel/retrieve the deposit for them. This is not the case when cancelling/claiming NFT deposits where users can perform this action by themselves. Additionally, the "NEOSWAP functional graph.png" document shows that users can claim deposits themselves, which means either the code or the documentation is wrong.

The `validate_initialize()` function only checks that the sum of all `swap_data_account.item.amount` entries for non-NFT items is equal to zero. Therefore, it is possible to validate a swap where some NFT entries are still missing.

The `validate_cancel()` function is used to close the swap data account after a swap has been cancelled and all of the items have been returned. Since there are no restrictions on who can call this function, anybody can close these accounts and receive the Sol for the rent exemption. The documentation of this function also states that the signer is supposed to be the initializer.

The functions `claim_nft()` and `cancel_nft()` contain code that checks whether the swap's associated token account no longer contains any tokens and can be closed. Since the transfer happens in the same instruction without reloading the account data in-between, this check will always fail.

The functions `deposit_sol()`, `claim_nft()` and `cancel_nft()` perform checks against the `status` of items using hardcoded values. In case the code gets updated and enum values change, these instances might be missed and lead to incorrect results.

The `TradeStatus` enum is used for both the swap data status and the status of the items contained in the swap data. Additionally, the name of the enumeration does not always match the operation that was performed by an instruction, e.g. the item status is set to `Deposited` when adding a "Sol item" with a negative `amount`.

Many functions iterate over the swap data items and only perform an action if a specific condition is met. These functions use a `transfered` boolean to make sure the action is taken only once per function call. To check whether the function failed and no item met the conditions, the for loop contains the check `item_id == ctx.accounts.swap_data_account.items.len() && !transfered`. Since the for loop iterator `item_id` only goes up to, not including, the `items.len()`, this condition will never be true. This will result in the functions returning `Ok(())` despite failing. Affected functions: - `deposit_nft()` - `deposit_sol()` - `claim_sol()` - `claim_nft()` - `cancel_sol()` - `cancel_nft()` (check is correct but should still be moved to after the loop)

Many functions perform checks at the start that could be performed using anchor annotations instead. Mixing manual checks and anchor annotations makes it harder to reason about the security of the implementation since the checks are done in more than one place.

The size of the object `SwapData` is calculated using the formula `SwapData::LEN + ( nb_items as usize * NftSwapItem::LEN )`. For unusually high values of the variable `nb_items`, an overflow is possible.

The functions `validate_claimed()` and `validate_cancel()` contain code to transfer remaining Sol to the signer. Since the `swap_data_account` member of `ValidateAndClose` is annotated with `close = signer`, anchor already sends any remaining Sol to the signer.