Release v3.2.0

[rya-sge]

3.2.0

Note: This version has not been audited.

Added

• Support of ERC-7943 (#337).
• New dedicated deployment variant with DebtEngine support (see Removed section for rationale).
• IRuleEngine: ERC-165 support added (#342) to enable interface compliance checks.
• New base contract CMTATBaseAccessControl (#350).

Changed

• Transfer now reverts with specific errors when the contract is paused or deactivated (#338) to improve error clarity.
• The approve function now reverts when the contract is paused for all deployment variants except Light (#335).
• Update Solidity version to 0.8.33 in Hardhat config file.

Removed

• DocumentEngine and SnapshotEngine removed from constructors and initialization (#343) to simplify deployment and reduce bytecode size.
• DebtDeployment: DebtEngine support removed and moved to a dedicated deployment variant (#339) to reduce contract size and enable additional modules in DebtEngine-based deployments.

[Domson97]

Hi @rya-sge
Great work on v3.2.0! I reviewed most of the changes (except automated doc) and added some comments. There could be notes not specifically addressing 3.2.0 changes, but since its my first CMTAT review here on Github I tried to dive a little deeper.

[Domson97]

Is there a specific reason you are differentiating between the fungible keyword and erc20 like in these examples: IERC7943ERC20Enforcement, IERC7943FungibleTransferError. Maybe because it should be also working with erc1155?

[rya-sge]

Thank a lot for all the feedback!

ERC-7943 is an ERC which also works for ERC-721 and ERC-1155 and they use the terms "Fungible" for ERC-20 interface.

Good catch I should use Fungible everywhere for ERC-7943 instead of ERC-20. I will make the change

See also
ERC-7943

[Domson97]

• Is address(0) checked anywhere (inside enforcement circuit) when adding to list? 
Maybe checking it could prevent gas consumption or unexpected behavior like freezing address(0) which doesn’t make sense.
• _addAddressToTheList does not check if status actually changed. Maybe add require($._list[account] != status, "Status unchanged") for saving gas and only emit event if status really changes in enforcement module.

[rya-sge]

Thanks for the feedback. They are indeed relevant improvement. I am just a bit concerned about the increase on the smart contract code size. We are very limited depending of the deployment version.

I don't think I will change it for this release but I have opened an issue to keep the question opened: #353

[Domson97]

• _canTransferGenericByModuleAndRevert() will revert on failure, not return false. The if condition and return false are unreachable code


• setting rule engine to address(0) disables rule checking. Is it documented somewhere?

[rya-sge]

_canTransferGenericByModuleAndRevert()

Right, thanks, I have removed the boolean return

RuleEngine => it is not really documented but for me it seems clear that if you remove the RuleEngine, all check performed through this RuleEngine will be disabled.

[Domson97]

Is there a scenario where debtEngine is not set (address(0))? Functions like creditEvents()
Or debt() would return empty structs if that’s the case.

[rya-sge]

I am not sure to really understand but:

Debt deployment version => credit events and debt are set in the token contract

Debt Engine deployment version => credit events and debt will indeed return empty struct if the debt engine is not set