trufflesuite / drizzle

Reactive Ethereum dapp UI suite
906 stars 238 forks source link

Send from useCacheSend sends trasactions twice. #101

Open SalahAdDin opened 3 years ago

SalahAdDin commented 3 years ago

Hi guys, I'm following the old documentation about hooks and I'm getting a problem at sending transactions: every transaction is sent twice, why?

The next is the current core:

 <form
             onSubmit={React.useCallback(
               ({
                 to = drizzleState.clientAccount,
                 value = toHex(parseAmountToToken(amountToSend, tokenDecimals)),
               }) => send(to, value),
               [amountToSend]
             )}
           >
             <label>Amount:</label>
             <input
               type="number"
               name="value"
               onChange={(e) => setAmountToSend(e.target.value)}
             />
             <button type="submit">Send</button>
           </form>

Also TXObjects is ever undefined: <div>{TXObjects?.map((t) => t.status)}</div>.

I'm seeing the send function is calling the contract method twice for each transaction: Screen Capture_select-area_20200903141705

But I don't understand why it is happening.

Now, if i decide use cacheSend directly, i don't get duplicated transactions:

  const sendTokens = (e) => {
    e.preventDefault();
    contract.cacheSend(
      drizzleState.clientAccount,
      toHex(parseAmountToToken(amountToSend, tokenDecimals))
    );
  };

Can anyone guide me? Is this a bug?

kombos commented 3 years ago

it could probably be because since you are using the dependancy array [amountToSend], which means the callback will be fired whenever the value of amountToSend changes. Now, during the submit action, the amountToSend changes twice. First its value is whatever is set by user, then the next value becomes "" as the submit action erases the value. so the both times the callback is being fired. not sure but this could be the problem.

SalahAdDin commented 3 years ago

@kombos No, even when we use send with memoization i still get the problem:

  const sendTokens = (e) => {
    e.preventDefault();
    send(
      drizzleState.clientAccount,
      toHex(parseAmountToToken(amountToSend, tokenDecimals))
    );
  };

@cds-amal The problem is here: if we remove the input variable and we left just the array, it won't repeat the transaction one more time.

cds-amal commented 3 years ago

Hi @SalahAdDin,

Thanks for the investigation. Can you provide a repo with the problem please? I'm not quite following your explanation.

SalahAdDin commented 3 years ago

Hi @SalahAdDin,

Thanks for the investigation. Can you provide a repo with the problem please? I'm not quite following your explanation.

@cds-amal I can't, but I can explain the problem.

The main goal for us is having a form from you add a token amount and you do click on send to transfer that amount of tokens: image

Following the documentation we tried this first:

      <TransferForm
        onSubmit={
          // Use memoization to avoid unnecessary rerenders.
          useCallback(({ to, value }) => send(to, value), [])
        }
      />

But it has the effect to send transactions twice: Screen Capture_select-area_20200905001225 Screen Capture_select-area_20200905001245

Therefore, we tried another approach using barebone cacheSend:

  const sendTokens = (e) => {
    e.preventDefault();
    contract.cacheSend(
      drizzleState.clientAccount,
      toHex(parseAmountToToken(amountToSend, tokenDecimals))
    );
  };

And we didn't have duplicated transactions.

Later, as we want to track the transactions, we made a copy of the hook functionality and we got the same: duplicated transactions.

  const sendTokens = (e) => {
    e.preventDefault();
    setStackIDs(stackIDs => [
      ...stackIDs,
    contract.cacheSend(
      account,
      toHex(parseAmountToToken(amountToSend, tokenDecimals))
      ),
    ]);
  };

Removing stackIDs => from the set setStackIDs avoids transaction's duplicating, hence we have now the same function without that input:

const sendTokens = (e) => {
    e.preventDefault();
    setStackIDs([
      ...stackIDs,
      contract.cacheSend(
        "0x3cCe2F1442A0E55616B78369F280ae22D6d9a6Fc",
        account,
        toHex(parseAmountToToken(amountToSend, tokenDecimals))
      ),
    ]);
  };