Use of nonReentrant in Hub.sol is ineffective

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x573c78274f78b77bd666dc29eb3ad97ff8560d3a7d3b09d32760e5c32b073eb0 Severity: low

Description: Description\

In Hub.sol, the operateFlowMatrix function is decorated with nonReentrant(0).

This makes it impossible to reenter the contract by calling operateFlowMatrix a second time. This is an important safeguard, as operateFlowMatrix can make calls to many external functions - specifically the ERC1155 callbacks

However, the protection is only partial, as the Hub.sol contract can still be re-entered during the execution operateFlowMatrix through all functions that are not decorated with the nonReentrant modifier, such as transfer, mint, burn, etc.

We do not see a particular attack here, but the nonReentrant modifier seems to be used here as a just-in-case safeguard, and is less effective than it could be.

Mitigation\ Mark all public (state-changing) functions nonReentrant

We also note that the single argument in your nonReentrant implementation is ignored, and can be removed for clarity.

[benjaminbollen]

Thanks, this has been remarked but is a clear reformulation. The concern with adding more code is that Hub.sol has been on the compile size limit for a long time, and every opcode added needs to be balanced by other code removed.