William Entriken

# Blockers - [ ] Release Solidity 0.6.2 final # Issue Currently, code is used like this: ```solidity constructor(address instanceRegistry, address templateContract) public { CountdownGriefingEscrow template; // set instance type...

proofHash is underspecified

3

The specification is given: > A proofHash can be generated by prepending the submitted data..… This leaves the prepending of data optional. Instead this should be required: > A proofHash...

Build errors should propagate to the test runner so that errors are more visible. **Recommendation: update build (run-ganache-deploy.sh) and test (run-ganache-tests.sh) steps so that errors result in the test suite...

Including a linter in a test suite is a quality signal for the project. Recommendation: add Solhint or similar to test suite. References: - https://github.com/erasureprotocol/erasure-protocol/blob/4a3d98ce023a264a9f3c7ba62ef77a9207bba5fe/contracts/modules/Griefing.sol#L20

In the implementation, many uses of the type `address` could be made more specific. Example: ```solidity contract CountdownGriefing_Factory is Factory { constructor(address instanceRegistry, address templateContract) public { CountdownGriefing template; //...

Some third-party code is copy-pasted into the project but it is not attributed. Copy-pasting leads to the risk that code will become (or has been) stale. By attributing code, it...

Factory uses bytes4 to track instanceTypes, this is unjustifiably restrictive

1

The `Factory` contract wants to track types of contracts. It does this using a `bytes4` type. It seems this is inspired from the ERC-165 standard. The baseline choice should be...

onlyPayloadSize should not be necessary with Solidity 0.5

1

I learned the hard way that Solidity has stronger type safety. Because it broke the ERC-165 reference implementation. The function `onlyPayloadSize` seems to be testing for something that Solidity now...

Use of “counterparty” is confusing

1

Griefing contracts use the word “punisher” and “counterparty” interchangeably. Also, in the advertised use cases, both the seller and buyer will be a “counterparty” for different agreements related to the...

Contract-level introductions are missing

2

Contract-level documentation is user-visible (because people read Etherscan) and NatSpec specifies that it can be shown to users. Every contract should explain what it does cleary. For contracts that are...