Rara

In L111 and L140 of `Sigmoid.sol` and in L123-L128 and L189-L194 of `SigmoidCuratorVault.sol` primitive cast operations (`uint256(...)` and `int256(...)`), which are prone for over-/underflows, are used.

Modifying the bonding curve parameters using `ParameterManager.setBondingCurveParams(...)` can be problematic, if a curator id is already set, meaning that users might not get the expected token amount when selling back to the contract if `a`, `b`, and `c` parameters are updated after its creation.

The `pow2` and `pow3` functions from the `ExtendedMath` library implement explicit overflow checks. Since the contract is compiled with solidity version `0.8.9`, these computations are already checked. That means, the require statements will never be reached in case of an overflow but are still executed when there is no overflow, which wastes gas.

An event should always be emitted when a state change is performed in order to facilitate smart contract monitoring by other systems which want to integrate with the smart contract. This is not the case for the functions and the correspondingly modified state variables: 1. `AddressManager.setRoleManager()`, after changing `roleManager`. 1. `AddressManager.setParameterManager()`, after changing `parameterManager`. 1. `AddressManager.setMakerRegistrar()`, after changing `makerRegistrar`. 1. `AddressManager.setReactionNftContract()`, after changing `reactionNftContract`. 1. `AddressManager.setDefaultCuratorVault()`, after changing `defaultCuratorVault`. 1. `AddressManager.setChildRegistrar()`, after changing `childRegistrar`. 1. `ParameterManager.setPaymentToken()`, after changing `paymentToken`. 1. `ParameterManager.setReactionPrice()`, after changing `reactionPrice`. 1. `ParameterManager.setSaleCuratorLiabilityBasisPoints()`, after changing `saleCuratorLiabilityBasisPoints`. 1. `ParameterManager.setSaleReferrerBasisPoints()`, after changing `saleReferrerBasisPoints`. 1. `ParameterManager.setSpendTakerBasisPoints()`, after changing `spendTakerBasisPoints`. 1. `ParameterManager.setSpendReferrerBasisPoints()`, after changing `spendReferrerBasisPoints`. 1. `ParameterManager.setApprovedCuratorVaults()`, after changing `approvedCuratorVaults[]`. 1. `ParameterManager.setBondingCurveParams()`, after changing `bondingCurveParams`.

`fxRootTunnel` is not initialized in the constructor of `ChildRegistrar` and `ChildRegistrar.setFxRootTunnel()` can be front-run since it is not restricted and can be called by anyone. Similarly, `fxChildTunnel` can be set by anyone. Even if there is no direct consequence to this action, it is recommended not to leave any function unrestricted.

It is important to validate inputs, even if they only come from trusted addresses, to avoid human error. The following functions do not have a proper validation of input parameters: 1. `ParameterManager.setSaleCuratorLiabilityBasisPoints()` does not check that parameter `_saleCuratorLiabilityBasisPoints` is less than 10000 basis points (100%). 1. `ParameterManager.setSaleReferrerBasisPoints()` does not check that parameter `_saleReferrerBasisPoints` is less than 10000 basis points (100%). 1. `ParameterManager.setSpendTakerBasisPoints()` does not check that parameter `_spendTakerBasisPoints` is less than 10000 basis points (100%). 1. `ParameterManager.setSpendReferrerBasisPoints()` does not check that parameter `_spendReferrerBasisPoints` is less than 10000 basis points (100%). 1. `ParameterManager.setBondingCurveParams()` does not check that parameters `a`, `b` and `c` are less than `type(int256).max`. 1. `ParameterManager.intialize()` does not check the manager address is different than `0x0`. 1. `RoleManager.initialize()` does not check the protocol admin address is different than `0x0`. 1. `SigmoidCuratorVault.initialize()` does not validate the inputs. 1. `ReactionVault.initialize()` does not validate the inputs.

The `_verifyOwnership` function uses `staticcall` directly by encoding the parameters with `abi.encodeWithSignature` and decoding the results with `abi.decode`. Since this is not type-checked by the compiler, it is easy to introduce errors this way.

The main purpose of the Checks Effects Interactions pattern is to reduce the attack surface for malicious contracts trying to hijack control flow after an external call. Even if it seems that implementing a non-reentrant modifier will actually prevent reentrancy at a contract level, other Dapps or contracts of the same application might rely on a sensitive state that can be manipulated by an attacker. `ReactionVault._buyReaction()` calls `paymentToken.safeTransferFrom` before updating the contract state. Similarly, `SigmoidCuratorVault.buyCuratorTokens()` calls `paymentToken.safeTransferFrom()`.