Tax calculated wrong

I created this contract trough Remix but whenever I make a transaction on Bsc Testnet with tax instead of give 95% to receiver and 5% to tax address it gives 100 to receiver plus 95% to tax wallet.
If any token was burned it does give it creating more tokens. If supply is full it doesn’t do anything but still receive the successful transaction confirmation.

pragma solidity ^0.8.4;
// SPDX-License-Identifier: UNLICENSED

contract GDC {
    mapping(address => uint) public balances;
    mapping(address => mapping(address => uint)) public allowance;
    mapping (address => bool) private notax;
    uint public totalSupply = 100000000 * 10 ** 9; 
    string public name = "Nameless";
    string public symbol = "NML";
    uint public decimals = 9;
    address public contractowner;
    address private taxacc = address(0x55555555555aaaaaaaaaa);
    
    uint public Tax = 5;
    uint private NewTax = Tax;
    
    // event for EVM logging
    event OwnerSet(address indexed oldOwner, address indexed newOwner);
    event Transfer(address indexed from, address indexed to, uint value);
    event Approval(address indexed owner, address indexed spender, uint value);
    event Settax(uint indexed oldtax, uint indexed newtax);
    event NewTaxAcc(address indexed oldtaxacc, address indexed newtaxacc);
    
    
    constructor() {
        contractowner = msg.sender; // 'msg.sender' is sender of current call, contract deployer for a constructor
        emit OwnerSet(address(0), contractowner);
        balances[msg.sender] = totalSupply;
        Tax = 5;
        uint newtax = Tax;
        emit Settax(uint(newtax), Tax);
        notax[taxacc] = true;
        notax[contractowner] = true;
        notax[address(this)] = true;
    }
    
    //Function to change taxes max to 10%.
    function setTaxFeePercent(uint newTax) external onlyOwner() {
        require (newTax <= 10, "Tax shouldn't exceed 10%");
        Tax = newTax;
    }
    
    function gettax(uint value) internal view returns (uint tax, uint valuein, uint valueout) {
    tax = value*Tax/ 100;
    valuein = value - tax;
    valueout = value;
    return (valuein, valueout, tax);
    }
    
    function excludetax(address account) public onlyOwner {
        notax[account] = true;
    }
    
    function includetax(address account) public onlyOwner {
        notax[account] = false;
    }
    
    function removealltax() private {
        require(Tax > 0, "Tax already 0%");
        if(Tax == 0 ) return;
        
        NewTax = Tax;
        Tax = 0;
    }
    
    function restoreAlltax() private {
        require(Tax == 0, "Tax already greater than 0%");
        Tax = NewTax;
    }
    
    function balanceOf(address owner) public view returns(uint) {
        return balances[owner];
    }
    
    function sendtoken(address to, uint value) public returns (bool) {
        require(balances[msg.sender] >= value, "Amount excedes balance.");
        
        TRansfer(msg.sender, to, value);
        return true;
    }
    
    function sendtokenFrom(address from, address to, uint value) public returns (bool) {
        require(allowance[from][msg.sender] >= value, 'allowance too low');
        require(balances[from] >= value, "Amount excedes balance.");
        
        TRansfer(from, to, value);
        return true;
    }
    
    
    function TRansfer(address from, address to, uint value) private {
        require(from != address(0), "ERC20: transfer from the zero address");
        require(to != address(0), "ERC20: transfer to the zero address");
        require(value > 0, "Transfer amount must be greater than zero");
        
      
        //if any account belongs to notax account then remove the tax
        if (notax[to] || notax[from]){
            transfernotax(from, to, value);
        } else {
            transfer(from, to, value);
		}
    }
    
    
    function transfer(address from, address to, uint value) private returns (bool) {
        require(balances[from] >= value, "Amount excedes balance.");
        
        (uint tax, uint valuein, uint valueout) = gettax(value);
        
        balances[from] -= valueout;
        balances[to] += valuein;
        balances[taxacc] += tax;
        emit Transfer(from, to, valuein);
        emit Transfer(from, taxacc, tax);
        return true;
        
    }
    
    function transfernotax(address from, address to, uint value) private returns(bool) {
        
        balances[from] -= value;
        balances[to] += value;
        emit Transfer(from, to, value);
        return true;
    }
    
   
    //Burns amount from message sender.
     function burn(uint amount) public {
        require(msg.sender != address(0), "ERC20: burn from the zero address");
        beforeTokenTransfer(msg.sender, address(0), amount);

        uint accountBalance = balances[msg.sender];
        require(accountBalance >= amount, "ERC20: burn amount exceeds balance");
        balances[msg.sender] -= amount;
        totalSupply -= amount;

        emit Transfer(msg.sender, address(0), amount);
    }
    
    function beforeTokenTransfer(address from, address to, uint amount) internal { }
    
    
    
}

Can anyone help me to figure out what’s is wrong here? I’m done with it ‘-’

Hi Yuri, welcome to Open Zeppelin!

    function transfer(address from, address to, uint value) private returns (bool) {
        require(balances[from] >= value, "Amount excedes balance.");
        
        (uint tax, uint valuein, uint valueout) = gettax(value);
        
        balances[from] -= valueout;
        balances[to] += valuein;
        balances[taxacc] += tax;
        emit Transfer(from, to, valuein);
        emit Transfer(from, taxacc, tax);
        return true;
        
    }
    
    function transfernotax(address from, address to, uint value) private returns(bool) {
        
        balances[from] -= value;
        balances[to] += value;
        emit Transfer(from, to, value);
        return true;
    }

This is bad design, because on transfer of any kind of a token, you should me manipulating your balances mapping once and only once.

If you get into the situation that you are in, where you have places that are changing balances in multiple places - it can lead to buggy behavior like this has.

You should account for the taxes by manipulating the amount. Not creating a new function for the different value to be handled.

I would take a look at SafeMoon for starters (everyone is doing it!) and look at how they handle taxes.

You should also take a look at Open Zeppelin’s ERC 20 contract https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol

And see how they handle transfers.

Hope this helps!

The issue was that
return (valuein, valueout, tax);
wasn’t matching this
(uint tax, uint valuein, uint valueout) = gettax(value);