Darwin-Coin / darwin-token-contracts

Darwin Protocol Smart Contracts
https://darwinprotocol.io
0 stars 0 forks source link

Potential Exploit - and solution! #63

Closed SomeTimesIInvent closed 1 year ago

SomeTimesIInvent commented 1 year ago

I believe it is possible that Tokenomics 2.0 itself could be an exploit (please ignore the sync issue when reading this as it is unrelated - just assume everything syncs 100% in this and compare how what I am describing would act against a token with no tax whatsoever).

Here is what 'could'(?) happen:

Person X buys a load of tokens. We have no tax on buys so the value of the token will rise in line with what it would on any other token.

Person X then sends half of those tokens to Wallet Y.

The remaining tokens Person X sells. Now because we take a 5% tax on them direct from the transaction, while still giving the person 100% of the BNB this means that the token value in the LP would decrease slightly less than it would if no tax was applied because while all the BNB is going out slightly fewer tokens are going in.

Now lets assume Person X uses all the BNB they just got from the sale to buy tokens again.

The BNB inside the pool would rise to the same level it was at before.

However, what happens to the token level? Would it to lower to the level it was before the sale? Or would it decrease slightly further due to the amended token/coin ration caused by our 5% tax?

NOTE: I HONESTLY DO NOT KNOW THE ANSWER TO THIS. IF THE ANSWER IS THAT IT WOULD DECREASE TO THE SAME LEVEL AS IT WAS BEFORE THEN WE ARE ALL GOOD. BUT IF THE ANSWER IS THAT IT WOULD DECREASE TO A SLIGHTLY LOWER LEVEL (THUS CREATING A HIGHER TOKEN VALUE) THEN THE EXPLOIT EXISTS.

WHAT I WOULD SAY IS THAT IN THE TEST TOKEN WHEN WE HAD 90% BURNS I WOULD DO TESTS BY SELLING ALL THE TOKENS I HAD IN A WALLET, AND THEN BUYING TOKENS BACK WITH ALL THE BNB I HAD. I DO BELIEVE EACH TIME I DID IT, I GOT BACK OUT MORE TOKENS THAN THE 10% THAT ENTERED FROM MY SALE WHICH SUGGESTS THE EXPLOIT DOES EXIST - IT COULD BE CHECKED ON THE FIRST FEW TRANSACTIONS OF THE TOKEN ON BSCSCAN I IMAGINE.

Assuming the exploit does exist, and the value of the token is slightly higher than it would have been before, then what happens is Person X could run a script across loads of wallets (to avoid our anti-whale measures) doing lots of buys, sells, buys etc until the value of the tokens in Wallet Y was high enough to sell them and make a significant profit.

I believe it prudent we therefore see if this exploit is possible by running tests against pancakeswap in testnet.

If it does, the solution should be simple:

By doing that the value of the token will rise less on sales discouraging the practice, but should anyone attempt it they will find the 5% sale tax a significant hindrance to trying to create any profit.

What are your thoughts @thesios @SmartGambleDev

SomeTimesIInvent commented 1 year ago

When we do the tests please try 1 test with us having 5% at sales.

And then do another test with us having 5% at buys.

I want to see if there is any difference there. Doing it at buys may actually slightly decrease the value of the token meaning it would be disadvantageous to try and abuse it as you would lose value.

So we need to test both to make sure.

ghost commented 1 year ago

MODIFIES

  1. Commented out the code for variable taxing based on desync (we don't need that anymore).

  2. ON BUYS: pool is taxed 5% of the trade. It is not actually taxed in the exact moment that the trade is made, its taxation is "saved" to be executed when a trade that is not a buy is made after that.

  3. ON SELLS: syncTokens() is called. Meaning that all the unsynced taxations on the pool are now executed. 5% of the trade is taxed from the user.

  4. Added setBuyWhitelist and setSellWhitelist functions to be able to whitelist addresses such as CEXes in the future.

  5. Modified a bit also the ERC20Upgradeable contract because the Tokenomics 1.0 taxation (normal user taxation) is happening in beforeTokenTransfer. (we needed it to return the taxed amount so that _transfer() does not revert)

  6. Other minor modifies like refactoring some variables. (like poolTaxPercentageSell --> userTaxPercentageSell)

TESTS

Tests such as buying and selling repeatedly, or buying repeatedly and then selling, have been conducted. (Looks like) the exploit is no more.

PULL REQUEST

The pull request refering to this issue is #64