{"action":"created","issue":{"url":"https://api.github.com/repos/ethereum/vyper/issues/928","repository_url":"https://api.github.com/repos/ethereum/vyper","labels_url":"https://api.github.com/repos/ethereum/vyper/issues/928/labels{/name}","comments_url":"https://api.github.com/repos/ethereum/vyper/issues/928/comments","events_url":"https://api.github.com/repos/ethereum/vyper/issues/928/events","html_url":"https://github.com/ethereum/vyper/issues/928","id":335207129,"node_id":"MDU6SXNzdWUzMzUyMDcxMjk=","number":928,"title":"How to assert transfer() and transferFrom() are successful for all implementations","user":{"login":"haydenadams","id":9343483,"node_id":"MDQ6VXNlcjkzNDM0ODM=","avatar_url":"https://avatars0.githubusercontent.com/u/9343483?v=4","gravatar_id":"","url":"https://api.github.com/users/haydenadams","html_url":"https://github.com/haydenadams","followers_url":"https://api.github.com/users/haydenadams/followers","following_url":"https://api.github.com/users/haydenadams/following{/other_user}","gists_url":"https://api.github.com/users/haydenadams/gists{/gist_id}","starred_url":"https://api.github.com/users/haydenadams/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/haydenadams/subscriptions","organizations_url":"https://api.github.com/users/haydenadams/orgs","repos_url":"https://api.github.com/users/haydenadams/repos","events_url":"https://api.github.com/users/haydenadams/events{/privacy}","received_events_url":"https://api.github.com/users/haydenadams/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-06-24T20:02:08Z","updated_at":"2018-06-25T04:57:08Z","closed_at":null,"author_association":"NONE","body":"There are several ways I have seen contracts call transfer() and transferFrom() and they all have issues.\r\n\r\n1) \r\n```python\r\ntoken: address(ERC20)\r\ntotal_deposits: uint256\r\n\r\ndef deposit(amount: uint256):\r\n    total_deposits += amount\r\n    self.token.transferFrom(msg.sender, self, amount)\r\n```\r\n\r\nIssue: Although most tokens return an exception if the transaction fails, [BAT](https://etherscan.io/address/0x0d8775f648430679a709e98d2b0cb6250d2887ef#code) and several other tokens return false. For these tokens, total_deposits can be increased without tokens ever being transferred. \r\n\r\n2) \r\n\r\n```python\r\ntoken: address(ERC20)\r\ntotal_deposits: uint256\r\n\r\ndef deposit(amount: uint256):\r\n    total_deposits += amount\r\n    assert self.token.transferFrom(msg.sender, self, amount) == True\r\n```\r\n\r\nIssue: Safer for most ERC20s, but [over 130](https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca) existing ERC20s including OMG do not return bools. These tokens can never be deposited, since all function calls will fail.\r\n\r\n3) `assert self.token.transfer(msg.sender, amount) != False`\r\n\r\nSee above. I have a new solution I've been thinking about:\r\n\r\n4)\r\n```python\r\ntoken: address(ERC20)\r\ntotal_deposits: uint256\r\n\r\ndef deposit(amount: uint256):\r\n    total_deposits += amount\r\n    balance: uint256 = self.token.balanceOf(self)\r\n    self.token.transferFrom(msg.sender, self, amount)\r\n    assert self.token.balanceOf(self) == balance + amount\r\n``` \r\nThis seems to guarantee desired behavior under all implementations differences. Unfortunately, it adds ~4000 gas. Are there any other catch-all methods?"},"comment":{"url":"https://api.github.com/repos/ethereum/vyper/issues/comments/399831087","html_url":"https://github.com/ethereum/vyper/issues/928#issuecomment-399831087","issue_url":"https://api.github.com/repos/ethereum/vyper/issues/928","id":399831087,"node_id":"MDEyOklzc3VlQ29tbWVudDM5OTgzMTA4Nw==","user":{"login":"haydenadams","id":9343483,"node_id":"MDQ6VXNlcjkzNDM0ODM=","avatar_url":"https://avatars0.githubusercontent.com/u/9343483?v=4","gravatar_id":"","url":"https://api.github.com/users/haydenadams","html_url":"https://github.com/haydenadams","followers_url":"https://api.github.com/users/haydenadams/followers","following_url":"https://api.github.com/users/haydenadams/following{/other_user}","gists_url":"https://api.github.com/users/haydenadams/gists{/gist_id}","starred_url":"https://api.github.com/users/haydenadams/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/haydenadams/subscriptions","organizations_url":"https://api.github.com/users/haydenadams/orgs","repos_url":"https://api.github.com/users/haydenadams/repos","events_url":"https://api.github.com/users/haydenadams/events{/privacy}","received_events_url":"https://api.github.com/users/haydenadams/received_events","type":"User","site_admin":false},"created_at":"2018-06-25T04:57:08Z","updated_at":"2018-06-25T04:57:08Z","author_association":"NONE","body":"Honestly don't think its an issue with the ERC20 standard. If every token returned a boolean value, it would be as simple as:\r\n\r\n```python\r\nassert self.token.transferFrom(msg.sender, self, amount) == True\r\n```\r\n\r\nAnd politically I agree that tokens that did not follow the standard should create a simple wrapping contract which could become the \"real\" token, or get left out of decentralized exchanges. \r\n\r\nAs someone currently building an automated market maker its frustrating because I know they prob wont and I have to make the choice between increasing the gas cost for all other tokens, not supporting certain tokens there is demand for, or building multiple versions of my smart contracts and figuring out a way for them to trade between each other. But yeah I agree not a ton that can be done on your end, was just checking if anyone had any ideas or alternate solutions. "}}
{"actor":{"display_login":"haydenadams"}}