CREDITSCOM / node

Credits Node is the main module that provide an opportunity to run a node and participate in CREDITS blockchain network.
https://developers.credits.com/
GNU Affero General Public License v3.0
151 stars 16 forks source link

Token balances not being updated on monitor & web wallet #22

Closed tkoen93 closed 5 years ago

tkoen93 commented 5 years ago

Bug in summary Calling the transfer or transferFrom function indirectly causes token balances not to update properly on both monitor & web wallet. These token transactions are also not visible as transactions on the monitor.

Describe the bug (extended version) Token balances do not update on both monitor and web wallet when the transfer or transferFrom function is not called directly.

When I transfer a token in a regular way (for example via web wallet) the following shows on the monitor: image

As an extension to the basic contract, which still contains some other issues I've mentioned before (#11) I created a function where I can import token balances from a specific token in case I'd like to upgrade my token contract. This function clears the balance hashmap, sets all tokens to the contractAddress and starts transferring them to the wallets I import.

public void importBalances(String importBal) {
 if (!initiator.equals(owner)) {
 throw new RuntimeException("The wallet " + initiator + " is not owner");
 }
 balances.clear(); // Clear old hashmap as we're replacing balances with this import method

 balances.put(contractAddress, totalCoins); // set all tokens to contract address

 importBal = importBal.replaceAll("\\s+","");
 String[] newBalances = importBal.split(",");
 for (int i=0;i<newBalances.length;i++) {
 String newValue = newBalances[i];
 String[] keyValue = newValue.split("=");
 transferFrom(contractAddress, keyValue[0], keyValue[1]);
 }
 }

This function does what it needs to do, but the transaction I create shows up like this: image

The string I imported via this function is as follows and should send tokens to two wallets: 33JWxAMxXzMmuuAoBZwAfc3qHP6xp1WTimoFqkdVFUwk=995, 7tiMoADn9VJfP3xEf58BAUj85mJuGmfqTyN86JeCk9TA=5

The hashmap does get updated in the background, but token balances are not updated on both monitor and web wallet. There are also no visible transactions of these two 'internal' transactions.

The contract address shows only 1 address as holder: https://monitor.credits.com/testnet-r4_2/Token/CcyY5NypSAVMGNoyQMYKeSZzZJP8jqjzwqTPvdByfXHc

image

When I use the balanceOf function for the other wallet, the correct amount is returned. This contract execution is also visible in the url mentioned above.

As far as I know the same issue happens when calling the transfer or transferFrom function from another contract using invokeExternalContract.

IGoryunov commented 5 years ago

In your case “transferFrom” method had to throw exception because owner of transferred tokens didn’t call approve method before. "transfer" and "transferFrom" are methods of smart contract and you can change their logic before deploying

tkoen93 commented 5 years ago

There is no possibility to take a look at the contract I mentioned in the first post due to testnet reset.

The transferFrom function did exactly what it needed to do, it moved balances from the contract address to the addresses I used via my importBalances method. The issue is that the balance did not update on both monitor and web wallet, but both wallets did have balance as I verified using the balanceOf method. Beside the fact that balances did not update on both monitor and web wallet, the transferFrom function did not initiate a visible transaction of these token transfers on the monitor.

I assume that when those transactions are visible on the monitor, the balances will be updated accordingly.

The code I've looked into shows me a function static bool isTransfer(const std::string& method, const std::vector<general::Variant>& params); (line 78: https://github.com/CREDITSCOM/node/blob/dev/api/include/tokens.hpp)

When digging a bit deeper into the code I found some more information about the isTransfer function here: https://github.com/CREDITSCOM/node/blob/dev/api/src/tokens.cpp

To me it looks like this method checks the transaction that was done by someone, in this case by me. I did not call transfer or transferFrom directly, I called importBalances, which is probably causing this boolean to return false. However, I did call transferFrom in my importBalances function,

Quick retry on the new testnet, will try to make it visual: Another example. Deployed a contract (https://monitor.credits.com/testnet-r4_2/contract/6xbvpsja41Y7KKuVdo3b86Dpvot7MpDGEEBoNSPJbRLZ) with an additional transfer function:

public boolean anotherTransferFunction(String to, String amount) {
 contractIsNotFrozen();
 transfer(to, amount);
 return true;
 }

This function is pretty useless, but only to show the issue I'm experiencing.

When calling the transfer function when initiating the transaction: image Results in: image

When I call my custom method, that calls the transfer function from inside the function. image Leads to: image

For whatever reason token balances did update with this contract, might be because the 7tiMo wallet already had some balance.

Deployed the contract again (https://monitor.credits.com/testnet-r4_2/Token/B7sDaTRYd6uPtGurzA9KQ3dHjacKvzxkKChuYGCpfeai) and only used the anotherTransferFunction. As seen on the holders page, there is only 1 holder with a balance of '4999'. Did not update succesfully. Also did a transaction calling the balanceOf method so you can verify the 7tiMo wallet actually got '1' of this custom token.

angelesmanzo commented 5 years ago

Hi Timo!

There are two different ways to transfer tokens depending on is the receiver address a contract or a wallet address. You might call transferfunction to send tokens to a wallet address or call approveon token contract and then transferFromon receiver contract to send tokens to contract. It could be the reason why the balance is not being properly updated/imported on both the monitor and web wallet. Token transactions between wallet address and contract are a couple of two different transactions based on the approach of the current Credits Standard (Basic and Extension). We should call approve on token contract and then call transferFrom on another contract where you want to deposit your tokens into it. As I know, there are some improvements that are being considered in order to fix this kind of issue.

I hope that this information will be useful :)

micmac0 commented 5 years ago

This is a start but it would be better if you can give us some exemples.

For exemple here is my use case. I need to send many bets to my smartcontract address. When all is done, i get result and then dispatch totalBetted between all winners.

First question : Why should I use approve if from value in transferFrom is the initiator i mean the wallet that send bet ? Also how make this initiator = smartcontract ? When smartcontract get all bets i don t know how to payout token as iniator is always an external wallet ?

Here is the workflow : Many betters send bet to smartcontract for each transaction smartcontract collect tokens. (How do that as initiator = better, then in transferFrom method i set From to better address and as result initator = from ???, why use approve ?) When all bet are done smartcontract do not accept more bet (already coded) Smartcontract compute all winners among all better and pay them back . How do that as smartcontract can t be initiator as far as I see ?

To start with I thought use an external wallet but this is not the good way as smartcontract can be trusted and not wallet see #23

angelesmanzo commented 5 years ago

Hello!

Here an example code by using approveand transferFrommethods.

  1. User’s smart contract call the smart contract method where the first parameter is bet and second parameter is the token quantity or amount. Here (initiator) is who transfer the tokens.
public void setBet(int bet, double amount){
    ...
    transfer(contractAddress, amount);
    ...
}
  1. When “N” user has made a bet to the smart-contract owner, could be called this method:
public void determineWinner() {
    if(initiator.equals(owner)){
        String winnerAddress = calculateWinner();
        approveContractTokens(winnerAddress, prize);
    }
}  
  1. Example of a private method for issuing approval for the smart-contract tokens from the side of the wallet. Only the smart-contract owner can call this function.
private void approveContractTokens(Stringspender,Stringamount){
        if(initiator.equals(owner)){
        ...
                ...

       }
}
  1. Then the user get a prize by implementing the method:
public getPrize(){
        transferFrom(contractAddress, inititator, prize);
    }

Currently the team is working in some improvements to be implemented in a new standard where only one method (send or transfer) might be called to transfer tokens. Nowdays, different approaches are being considered. The IEP for ERC-233 standard, for example, proposes a tokenFallbackmethod, which might be called by the receiver contract.

micmac0 commented 5 years ago

I created new issue #31 to let this thread focus on initial topic and focus on new discussion on new thread. Thank you for your fast answearing on previous messages !