Skip to content

Conversation

@mjdietzx
Copy link

@mjdietzx mjdietzx commented Mar 3, 2018

Fixes #787

🚀 Description

An interesting erc20 token use-case I keep seeing is a 'taxed' transfer (i.e. a small fee (for example 1%) is diverted to a feeAccount in transfer). This PR implements that functionality and includes corresponding unit-tests.

  • 📘 I've reviewed the OpenZeppelin Contributor Guidelines
  • ✅ I've added tests where applicable to test my new functionality.
  • 📖 I've made sure that my contracts are well-documented.
  • 🎨 I've run the JS/Solidity linters and fixed any issues (npm run lint:all:fix).


balances[msg.sender] = balances[msg.sender].sub(_value);

uint256 fee = Math.min256(_value.mul(transferFeePercentage).div(100), maxTransferFee);
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to use super.transfer() here, to avoid reimplementing lines 23, 24, 27, 32, 33 and 38?

Copy link
Author

Choose a reason for hiding this comment

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

yes, good point! I think this is possible and is the cleaner solution. I will update the PR w/ improvement

@shrugs
Copy link
Contributor

shrugs commented Mar 6, 2018

LGTM, but haven't done a thorough look-through of the code to make sure all the math works, etc. Requesting a review from a fellow maintainer :)

@nventuro
Copy link
Contributor

Closing due to staleness. Also, this would be better implemented in the current version of OpenZeppelin by overriding the _transfer function.

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.

TaxedToken implementation that charges a fee for token transfers

4 participants