Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes gh-99 PullPaymentMock needed a payable constructor #106

Merged
merged 4 commits into from
Dec 13, 2016
Merged

fixes gh-99 PullPaymentMock needed a payable constructor #106

merged 4 commits into from
Dec 13, 2016

Conversation

Qkyrie
Copy link

@Qkyrie Qkyrie commented Dec 13, 2016

Added payable to the PullPaymentMock constructor, which is necessary to receive the x-amount of ether needed in the tests. This fixes the travis build.

@maraoz
Copy link
Contributor

maraoz commented Dec 13, 2016

Wow, I can't believe this was it... Still don't understand why this was working for most devs locally.
THANKS A LOT @Qkyrie!!! I spent a couple of hours on this yesterday and still hadn't been able to fix travis build.

@maraoz
Copy link
Contributor

maraoz commented Dec 13, 2016

BTW: If you can share some lines on how you found/debugged the problem it would be helpful :)

@maraoz maraoz merged commit e859d53 into OpenZeppelin:master Dec 13, 2016
@Qkyrie
Copy link
Author

Qkyrie commented Dec 13, 2016

basically, because the error in the test was an invalid jump, only a few possibilities arise:

  • array access out of bounds
  • failed sub-call (due to any reason reported by the EVM including invalid jump dest in the sub-call)
  • ether sent to a library fallback function
  • sending ether to a function not defined as payable

because the test already failed at

let ppce = await PullPaymentMock.new({value: AMOUNT})

it was immediately clear that it had to be a constructor without payable. This became necessary since Release 0.4.0.

The reason I probably found it faster is because I had the same issues when I was upgrading the FundRequest Token.

As a sidenote, I'll be using OpenZeppelin in FundRequest's developments, love your project and I'll be writing a blogpost about it soon. I won't refrain from helping out where possible ;)

cheers!

@maraoz
Copy link
Contributor

maraoz commented Dec 13, 2016

@Qkyrie Thanks a lot! Happy to hear that

ProphetDaniel pushed a commit to classicdelta/Smart-Contracts that referenced this pull request Mar 9, 2018
fixes OpenZeppelingh-99 PullPaymentMock needed a payable constructor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants