OriginProtocol / origin-js

We've moved to a monorepo: https://github.com/OriginProtocol/origin
MIT License
81 stars 33 forks source link

Add reasons for requires in contracts #519

Closed cuongdo closed 6 years ago

cuongdo commented 6 years ago

Checklist:

Description:

Add reason strings to requires

This makes the cause of an EVM revert much clearer to developers. I've tried to keep the strings short but unambiguous within each function.

I've had to increase the gas limit for contract test deploys, because it now takes ~5 million gas (compared to ~4 million previously) to deploy the marketplace contract. This corresponds to a difference in deployment cost of ~$1.31 USD. The small increase is more than offset by reduction in developer debugging time. The lowest gas limit I've seen from Rinkeby, Ropsten, and mainnet is 7 million, so we're OK there.

I also haven't touched contracts/test-alt/contracts

Takes care of #457 for Origin contract code

OpenZeppelin code remains uninstrumented until we vendor it.

cuongdo commented 6 years ago

cc @wanderingstan is this what you were thinking?

wanderingstan commented 6 years ago

Sorry this is late, but YES! Thank you @cuongdo !