This warden has never seen a Solidity code base like this before. It is almost flawlessly documented, simple and elegant. Simple is not the same as easy so this is a real achievement.
The heavy use of assembly, including tricks such as overwriting the free memory pointer and updating structures in place feel very dangerous even though they have a clear advantage in saving gas. The reference implementation was instructive but there is a worry that if the reference implementation changes it would be so easy to make a small mistake in updating the assembly. What techniques are used to keep the two implementations in line with each other?
Non-critical: Use AccumulatorArmed constant instead of magic number in function Executor._triggerIfArmed
Function _triggerIfArmed compares against magic number 64 on Executor.sol:433 instead of AccumulatorArmed constant from ConsiderationConstants
QA Report
Remarks/Recommendations
This warden has never seen a Solidity code base like this before. It is almost flawlessly documented, simple and elegant. Simple is not the same as easy so this is a real achievement.
The heavy use of assembly, including tricks such as overwriting the free memory pointer and updating structures in place feel very dangerous even though they have a clear advantage in saving gas. The reference implementation was instructive but there is a worry that if the reference implementation changes it would be so easy to make a small mistake in updating the assembly. What techniques are used to keep the two implementations in line with each other?
Non-critical: Use
AccumulatorArmed
constant instead of magic number in functionExecutor._triggerIfArmed
Function
_triggerIfArmed
compares against magic number64
on Executor.sol:433 instead ofAccumulatorArmed
constant fromConsiderationConstants