Web3.js: web3-core-method: If transaction fails, newHeads subscription is not stopped

[sterlu]

TLDR: If a transaction fails (ie. out of gas) sub.unsubscribe() is never called and blockTracker continues to poll for blocks indefinitely

Expected behavior

1. When sending a transaction, startWatching inside web3-core-method is called. It either sends eth_subscribe if it is available on the provider, or starts polling for new blocks.
2. When a receipt is found, the confirmation event is emitted. After all required confirmation events have been emitted (24 by default, changable by web3.eth.transactionConfirmationBlocks), the sub.unsubscribe is called, sending eth_unsubscribe, or stopping the block polling, depending on the underlying provider.
3. If a transaction fails (eg. out of gas), the same would be expected: after emitting a number of confirmation events, the subscription is stopped.

Actual behavior

Steps 1. and 2. work as expected. However, if a tx fails (step 3.), only 1 confirmation is emitted (this may be by design). After that, the defer.eventEmitter.listeners('confirmation').length is equal to 0, never entering the if, and never stopping the subscription.

Suggested solution

1. Add else { sub.unsubscribe(); } here
   OR
2. Call unsubscribe regardless of canUnsubscribe here
   I can create a PR with either one of the suggested fixes if someone with a high-level overview of the packages can confirm they are correct.

Steps to reproduce the behavior

I can set up a mini repo with a test if required.

1. Instantiate the provider
2. Send any valid transaction
3. Observe the network requests being sent until a certain number of confirmations is mined
4. Send any transaction with a lower than required gas limit
5. Observe the network requests being sent infinitely even after the first confirmation is received

Environment

web3 packages, including web3-core-method: 1.2.11
web3-provider-engine: 15.0.12 -> https://github.com/sterlu/web3-provider-engine/tree/disable-polling
In my tests, I've been using web3-provider-engine, but a forked version which disables constant block polling on provider creation. This is how I was able to observe the actual changes in a number of requests. This is a separate issue on web3-provider-engine that I'll try to address in a separate issue/PR.

[sterlu]

Hi, I see a rewrite of mentioned packages is planned. I'd be happy to provide a PR for this in the meantime if someone has the time and will to merge it.

[krychla1]

This same happens when replacing transactions. The replaced ones are still polled for a receipt until max is reached

[sterlu]

@krychla1 while that is a different (and more difficult) problem, it might be useful to have an option to stop polling for receipt on the returned promi-event.

[GregTheGreek]

@sterlu happily accepting PRs!

PS: Love defisaver

[GregTheGreek]

Transaction replacements are actually a much bigger problem that I created an EIP to solve it.