Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provider.getTransactionReceipt Takes Forever if Transaction was Dropped from the Network #331

Closed
nikitaeverywhere opened this issue Nov 5, 2018 · 7 comments
Labels
discussion Questions, feedback and general information.

Comments

@nikitaeverywhere
Copy link

Hello!

Still, 2 days later, I cannot completely migrate my project to Ethers v4.

Here's another thing I found: after sending a transaction with provider.sendTransaction, in case the transaction is immediately dropped from the network, this condition makes a provider.getTransactionReceipt(txHash) stuck and it never resolves. I see it happens because of this lines being executed during transaction publishing, which doesn't satisfy _this._emitted['t:' + transactionHash] == null.

I am personally impressed by the complexity of the code and logic behind Ethers modules. For instance, by following the single responsibility principle providers must perform a single thing - an RPC call. Then, fallback provider extends basic provider and makes the calls repeat on failures. But in fact, Ethers base provider turned to be a 1000+ lines of code module which tries to cover all possible cases and fill all possible gaps. Software doesn't work like that... Some projects that take years to develop could have been completed in a couple of months by being designed properly...

Getting back to the problem, can we do something that won't break the paradigm? For now I see using provider.perform("getTransactionReceipt", { transactionHash }) instead of provider.getTransactionReceipt as the only solution that works for me.

Thanks!

@nikitaeverywhere nikitaeverywhere changed the title Get Transaction Receipts Takes Forever if Transaction was Dropped from the Network provider.getTransactionReceipt Takes Forever if Transaction was Dropped from the Network Nov 5, 2018
@ricmoo
Copy link
Member

ricmoo commented Nov 5, 2018

Heya! :)

What is the situation in which you want a transaction that is immediately dropped? You can never guarantee this is the case. If you ask for the transaction receipt for a transaction you know (well, think with high confidence) made it to the network, the call should not resolve until the receipt is available, no? Otherwise, sending a transaction and immediately requesting the receipt would often simply return null, and you would need to add some additional code every time to retry.

I will be adding a timeout parameter at some point in the near future, so you can specify how long to possibly wait. Keep in mind, just because the receipt call times out, does not mean the tx won’t be mined.

A timeout would solve your problem, I think?

The complexity of Provider almost entirely revolves around events, which in an eventually consistent environment require lots of extras. The abstract Provider class can be extended directly by anything that does not require the BaseProvider functionality; all the BaseProvider functionality is required for the current concrete classes though. The event stuff will likely be broken out into an intermediate abstract class too, since WebSocketProvider and the new EIP 1193 provider won’t need the events code that is based on polling. This will be transparent though.

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Nov 5, 2018
@nikitaeverywhere
Copy link
Author

nikitaeverywhere commented Nov 5, 2018

Timeout will just make the thing even worse, please don't do this.

If you ask for the transaction receipt for a transaction you know (well, think with high confidence) made it to the network, the call should not resolve until the receipt is available, no? Otherwise, sending a transaction and immediately requesting the receipt would often simply return null, and you would need to add some additional code every time to retry.

Well, when I request a transaction receipt I want to know whether transaction is in the network or not, regardless of what happened before. All side effects are very, very undesirable. Simply put, instead of just using library I constantly dig its code to find out why these two functions doesn't work together while each works separately. Thus, null returned by getTransactionReceipt right after the transaction submission is expected.

The logic you described will work for 99.9% cases indeed. However, there will always be a chance when immediately after propagating your new transaction the node receives a new block with high-gas-price "the next supercoin" minting started and your transaction will be dropped from the network (or because of whatever reason could be). Hence, getTransactionReceipt will result with null. I think if we try this approach on a real network, we will eventually discover that some of our "transaction publishing workers" stuck occasionally...

Instead, think about this. We are developing a reliable back end for transaction publishing which works as following:

  1. We have a separate worker which just publishes transaction.
  2. A separate worker which listens to transactions statuses
  3. A separate worker which tries to publish transactions again and again if they are gone.

This is one of the ways of how we can ensure that nothing will ever stuck nor disappear. But agree, expecting a single async function to handle all edge cases is fine for some kind of PoC project only.

@nikitaeverywhere
Copy link
Author

What is the situation in which you want a transaction that is immediately dropped?

I simply have a test case for this :)

@ricmoo
Copy link
Member

ricmoo commented Nov 5, 2018

You can also just instantiate a second provider, which you do not use for state tracking, and use that one for “uninformed queries”. So, submit transactions on one, and query for them on the other, keeping in mind the second returning null does not mean it was dropped; if could just not have been seen yet. You will still need some sort of timeout, since the problem can return only two answers, “yes” and “not sure”. It can never return “no”, so you need a heuristic to decide your confidence in “no”.

In general, if you send a transaction to the network, you can assume at some point in the future it may be mined. Also, if an event is triggered in a transaction, or a block is mined, you can assume they will return a value (although, possibly not immediately, especially in the case of INFURA where you are communicating with a large cluster of nodes, which at any given point not be the one that last informed you of the change). That is the reason for the events tracking, which before that was added meant the common case (99.9%) case, required crazy extra logic everywhere to handle.

Eventually consistent will always have edge cases, for example orphan blocks, but the tools should make it as easy as possibly to perform the common case, while not precluding the edge cases. I think a second instance of the provider covers your use case.

@nikitaeverywhere
Copy link
Author

nikitaeverywhere commented Nov 5, 2018

You can also just instantiate a second provider, which you do not use for state tracking, and use that one for “uninformed queries”.

Well, it could work...
image

I hear what you try to say. Try to hear me: libraries, especially for Ethereum, can never guarantee everything to function reliably. It's the application's responsibility to handle all edge cases.

As for reliable event tracking (in the context of Ethereum events), which takes into account any possible forks, inconsistencies and so on we've build a special event tracking back end, and API which provides events 100% reliably to other back end services. If we just limit everything to a single library, what would happen if our servers accidentally go down for a while? What would happen if we're disconnected from providers/internet for a while? What would happen if the application unexpectedly stuck at some point? On a memory overflow? We'll miss the event! And no single library can solve this problem.

@nikitaeverywhere
Copy link
Author

Feel free to close this issue, I resolved a problem with provider.perform("getTransactionReceipt", { transactionHash }). All side effects should be documented though.

@ricmoo
Copy link
Member

ricmoo commented Nov 15, 2018

I think you may be able to useTransactionReceipt as of 4.0.12, since I only set the pending emit event if the transaction is actually wait-ed on.

The confirmations in both tx.wait and provider.waitForTransaction may be set to 0, to prevent blocking now too.

let confirmations = 0;
let receipt = await provider.waitForTransaction(hash, confirmations);

// ... OR ...

provider.sendTransaction(signedTx).then((tx) => {
  // The tx
  console.log(tx);
  tx.wait(confirmations).then((receipt) => {
    // confirmations was 0, so if the receipt does not exist, it is null.
  });
});

Hopefully that helps your use case, and generalizes receipt fetching a bit better in general.

If you have any issues, please feel free to re-open. :)

Thanks! :)

@ricmoo ricmoo closed this as completed Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Questions, feedback and general information.
Projects
None yet
Development

No branches or pull requests

2 participants