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

fix: change from transfer to call value #189

Merged
merged 4 commits into from
Jun 30, 2022

Conversation

0xvangrim
Copy link
Contributor

@0xvangrim 0xvangrim commented Jun 27, 2022

Issue: #84

Change from transfer to .call.value

rossneilson
rossneilson previously approved these changes Jun 28, 2022
Copy link
Member

@AugustoL AugustoL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test missing, you can check by sending ETH to a contract that cannot doesnt have the fallback set.

@@ -70,9 +70,10 @@ contract Avatar is Ownable {
* @return bool which represents success
*/
function sendEther(uint256 _amountInWei, address payable _to) public onlyOwner returns (bool) {
_to.transfer(_amountInWei);
(bool sent, bytes memory data) = _to.call.value(_amountInWei)("");
require(sent, "Failed to send ethers");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change menssage to "eth transfer failed"

@0xvangrim
Copy link
Contributor Author

0xvangrim commented Jun 28, 2022

Test missing, you can check by sending ETH to a contract that cannot doesnt have the fallback set.

You've written a test for this one already in Controller.js

var tx = await controller.sendEther(
      web3.utils.toWei("1", "ether"),
      otherAvatar.address,
      avatar.address
    );
    await avatar
      .getPastEvents("SendEther", {
        filter: { _addr: avatar.address }, // Using an array means OR: e.g. 20 or 23
        fromBlock: tx.blockNumber,
        toBlock: "latest",
      })
      .then(function (events) {
        assert.equal(events[0].event, "SendEther");
      });
    var avatarBalance =
      (await web3.eth.getBalance(avatar.address)) /
      web3.utils.toWei("1", "ether");
    assert.equal(avatarBalance, 0);
    var otherAvatarBalance =
      (await web3.eth.getBalance(otherAvatar.address)) /
      web3.utils.toWei("1", "ether");
    assert.equal(otherAvatarBalance, 1);
  });```

@0xvangrim 0xvangrim force-pushed the fix/sendEtherCall branch from 2c7ba87 to e4d0e71 Compare June 29, 2022 13:15
@0xvangrim 0xvangrim force-pushed the fix/sendEtherCall branch from e4d0e71 to 1104139 Compare June 29, 2022 13:23
@0xvangrim 0xvangrim requested a review from rossneilson June 29, 2022 22:08
@0xvangrim 0xvangrim merged commit e9e6e8f into DXgovernance:develop Jun 30, 2022
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.

3 participants