Use `safeTransfer()`/`safeTransferFrom()` consistently instead of `transfer()`/`transferFrom()`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/staking/JPEGStaking.sol#L34 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/staking/JPEGStaking.sol#L52 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L899

Vulnerability details

Impact

It is a good idea to add a require() statement that checks the return value of ERC20 token transfers or to use something like OpenZeppelin’s safeTransfer()/safeTransferFrom() unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

Proof of Concept

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/staking/JPEGStaking.sol#L34
https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/staking/JPEGStaking.sol#L52
https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L899

Tools Used

Manual review/slither

Recommended mitigation steps

Consider using safeTransfer()/safeTransferFrom() consistently instead of transfer()/transferFrom() or use require() to check the return value.

[spaghettieth]

Duplicate of #90

[dmvt]

Invalid. All tokens involved are created by sponsor, included in the repo, and known to revert correctly.