daifoundation / maker-otc

The OasisDEX protocol - Simple on-chain market for ERC20 tokens
GNU Affero General Public License v3.0
101 stars 39 forks source link

simple_market synchronized modifier and affected methods should be updated before a new release #179

Open brianmcmichael opened 5 years ago

brianmcmichael commented 5 years ago

The synchronized pattern in simple_market.sol follows an old reentrancy guard pattern that should be updated prior to another release.

The issue was first described in https://github.com/OpenZeppelin/openzeppelin-contracts/issues/335 where a developer could inadvertently create a situation where the contract enters a locked state.

It seems that this may not be a major issue for 0.4+ versions of solidity, but it looks like an alternate pattern without a locking mechanism is recommended at https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/ReentrancyGuard.sol.

Probably a low-priority issue, but worth adding if a new version is deployed.