Feedback on dai escrow contract that reimburses a relayer using Uniswap

[abcoathup]

@VypoMouse

I was looking at setting up some automated tests, but found truffle to be a bit… heavy for just starting out. I’ll definitely give it another go.

You definitely want to write unit tests (ideally 100% coverage).

I would start following the OpenZeppelin Learn guides on Writing Automated Smart Contract Tests is a great place to start.

Also use Test smart contracts like a rockstar

Feel free to ask questions about writing tests. Let me know if you get stuck.

@VypoMouse

should I be including those contracts in my testing, or would you recommend mocking them?

I would mock them for unit tests.

I would also look at doing some additional testing using ganache --fork of mainnet to test against the real thing.

@VypoMouse

I hadn’t heard of the Gas Station Network, so thanks for the pointer! I did a quick google to see if it ties in with Dai’s permit functionality, but it seems like I would still need to get a signature for that to unlock the user’s Dai balance?

With an ERC20 token you would need the user to approve an allowance (or use Dai's permit).

My thought would be to work on the Escrow functionality before adding meta transactions.

@VypoMouse

This contract is a snippet from a larger repository, that I’m not ready to share yet. It includes a README and all the other good stuff. However, if you have any suggestions for my natspec comment in the contract source itself, I’d very much appreciate it!

Glad to hear that you have appropriate good stuff in your repository.

You may want to look at Solidity Docgen 0.5
Nothing like generating documentation from your code to see how good your natspec is.

@VypoMouse

Would passing them in as a constructor arguments affect the deployment fees significantly? I’m still not entirely clear on how that whole process works.

I assume there would be some smallish difference in deployment costs. (Feel free to try it out and compare).

My main thought is having anything specific for testing in your mainnet contract.