bonding, rounds: revert reasons

[kyriediculous]

What does this pull request do? Explain your changes. (required)
This PR adds revert reasons to the require statements for the upgradeable contracts.

NOTE: tests that use a GenerickMock.sol contract to execute methods in unit tests don't return revert reasons yet, see #333.

NOTE: public view functions seem to not return revert reasons as they are calls and not transactions.

How did you test each of these updates (required)
Adjusted & ran unit and integation tests

Does this pull request close any open issues?
Fixes #313

Checklist:

• README and other documentation updated
• All unit & integration tests pass

[yondonfu]

Mind switching the target branch for this PR to streamflow?

[kyriediculous]

Done ! Probably overlapped opening this PR with merging expand-active-transcoder-set into streamflow

[coveralls]

Pull Request Test Coverage Report for Build 1128

• 26 of 26 (100.0%) changed or added relevant lines in 4 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 1107:	0.0%
Covered Lines:	683
Relevant Lines:	683

---

💛 - Coveralls

[yondonfu]

Changes look good! I left one comment about omitting the -k petersburg flag when starting testrpc-sc. Let's address the comment and then rebase to resolve the merge conflicts. No need for re-review prior to the rebase.

[kyriediculous]

rebased !

[yondonfu]

Actually, the current block number as returned by web3.eth.getBlockNumber() should be equal to latestBlock here since eth_call by default executes in the context of the latest block and not in the context of the pending block to be mined. So, in this case latestBlock + 1 would be a block number in the future.

[yondonfu]

Based on my previous comment, latestBlock would actually be the current block here.

[kyriediculous]

Fixed it up & rebased

…

On Sun, Sep 29, 2019 at 11:52 PM Yondon Fu ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In test/unit/RoundsManager.js <#335 (comment)>: > @@ -254,19 +253,22 @@ contract("RoundsManager", accounts => { describe("blockHash", () => { it("should fail if block is in the future", async () => { const latestBlock = await web3.eth.getBlockNumber() - await expectThrow(roundsManager.blockHash(latestBlock + 1)) + // Note that current block = latestBlock + 1, so latestBlock + 2 is in the future Actually, the current block number as returned by web3.eth.getBlockNumber() should be equal to latestBlock here since eth_call by default executes in the context of the latest block and not in the context of the pending block to be mined. So, in this case latestBlock + 1 would be a block number in the future. ------------------------------ In test/unit/RoundsManager.js <#335 (comment)>: > }) it("should fail if block is the current block", async () => { const latestBlock = await web3.eth.getBlockNumber() - await expectThrow(roundsManager.blockHash(latestBlock)) + // Note that current block = latestBlock + 1 Based on my previous comment, latestBlock would actually be the current block here. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#335?email_source=notifications&email_token=AFJZZWW55CQ6OFHG3B6CLDLQMEPTNA5CNFSM4IZFLBTKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCGINLAY#pullrequestreview-294704515>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFJZZWWHRODNS3R4Z3I5UIDQMEPTNANCNFSM4IZFLBTA> .

[yondonfu]

LGTM!