MyCryptoHQ / MyCrypto

MyCrypto is an open-source tool that allows you to manage your Ethereum accounts privately and securely. Developed by and for the community since 2015, we’re focused on building awesome products that put the power in people’s hands.
https://mycrypto.com
MIT License
1.36k stars 650 forks source link

Automated Balance and Nonce Updates #1030

Closed aitrean closed 6 years ago

aitrean commented 6 years ago

At the moment, we don't automatically refresh the user's balance or nonce after broadcasting a tx, since we need to wait for block confirmation. Given the variable nature of Ethereum block times, and the fact that we don't presently have any websockets that are trustful, I propose a solution as follows:

  1. Create a utility saga that will kick off a fork once called. Ideally, we would call it immediately following a successful broadcast.

  2. In the forked saga, poll the node for the present block number every x number of seconds until the block number updates. On a successful update, check if we can get a transaction receipt. If the transaction doesn't go through in the first block after the update, we repeat this process until we can get a proper txReceipt.

  3. Parse the txReceipt.

If successful:
  1. dispatch currently-existing actions to update:
    1. balance
    2. nonce
  2. Show a green notification indicating the user had a successful transaction.
If unsuccessful:
  1. dispatch action to update balance. Show a red footer notification the user's transaction failed.

The main variable in this is how frequently we poll the blockchain. Poll by average expected blocktime (~15s)? Use an API for blocktimes? I'd prefer to poll as frequently as possible so that we don't have to wait 15s after sending a transaction to see an update.

What are the suggestions on this? I'm assuming that previous transactions aren't really states we need to keep, so I haven't suggested any reducers we don't already have.

dternyak commented 6 years ago

I would like to see an exponential back-off(ish) for polling, and a hard-cap to limit retries (for backend overhead).

The polling cycle can look something like this:

tx broadcast |15s| poll |15s| poll |15s| poll |30s| poll |60s| stop polling

kvhnuke commented 6 years ago

I highly recommend not to do polling you guys will end up hitting high number of network requests as your number of visitors grow. Instead I would use something similar to below

  1. If it is a value transfer (no code) simply verify basic verifications such as it is above gasprice, and valid nonce and user's balance is sufficient. Then simply decrease (value + tx fee) from user's balance and increase the nonce by one.

  2. However if the tx has data, then do an actual eth_call and verify whether the tx will go through, (however, I recommend using the ethereumjs-vm with a backend cache) then increase the nonce and decrease the balance.

$30k mistake made me realize how inefficient is polling

dternyak commented 6 years ago

Thanks for the input.

  1. If it is a value transfer (no code) simply verify basic verifications such as it is above gasprice, and valid nonce and user's balance is sufficient. Then simply decrease (value + tx fee) from user's balance and increase the nonce by one.

A tx could be mined at any time that would change this state, so we shouldn't optimistically decrease balance / update nonce in this way. We have no guarantee the tx will ever be mined, and no guarantee that a different tx won't be generated / mined that will cause a difference in expected balance / nonce state.

  1. However if the tx has data, then do an actual eth_call and verify whether the tx will go through, (however, I recommend using the ethereumjs-vm with a backend cache) then increase the nonce and decrease the balance.

Using eth_call to determine if a transaction with data is valid has some value, but it's more validation related (e.g. we shouldn't even allow the user to generate the tx is invalid). It still doesn't allow us to make guarantees about if we can update the balance / nonce.

By capping the number of retries we will make and employing an exponential back-off, this functionality will not significantly increase the number of JSON-RPC calls the traditional user makes during a session. Long-term, we're planning to move towards websockets to avoid polling altogether.

kvhnuke commented 6 years ago

@dternyak good thoughts Also, think about the following conditions and how you can solve them. Web-sockets definitely way to go

  1. Clients sending back to back txs (if the nonce only update after the tx is included this might be a problem) as the nonce might not update at all if the tx takes more than 120 secs
  2. Balance wont immediately reflect the balance after sending the transaction, instead it might not reflect at all if it takes more than 120 secs to that tx to be included

you can guarantee with a fair certainty (Higher than it will be included within 2 mins ) that the tx will be included if the following conditions are met

  1. Gasprice is >= avg price
  2. nonce is valid
  3. enough balance
  4. valid state for contract execution
  5. txSize < 32*1024
  6. gasLimit < maxLimit
  7. gasLimit > intrinsicGas

you can verify all those without polling only exception to this is a user actually replacing the tx with higherGas price, you can handle this case to if they use MyCrypto to replace the tx as you can keep a table of sentTxs with their corresponding nonces and if a user replace it, you can adjust balance accordingly

and if you really need polling you can do it once every min or so to update to sync the local state with the blockchain

just a quick thought!

SharonManrique commented 6 years ago

Added to Asana

abara-kedavra commented 6 years ago

Hi, Connor! My topic wasn't about nonce updating. I completely change the wallet! It should have new parameters.