web3 / web3.js

Collection of comprehensive TypeScript libraries for Interaction with the Ethereum JSON RPC API and utility functions.
https://web3js.org/
Other
19.31k stars 4.95k forks source link

calling contract.method.send() only triggers .on('transactionHash') #2680

Closed Jaime-Iglesias closed 5 years ago

Jaime-Iglesias commented 5 years ago

Description

When calling Contract.methods.myMethod.send() the event emitter only triggers .on('transactionHash') never .on('receipt) nor .on('confirmation')

edit: It works as intended in beta.52

Expected behavior

the emitter should be triggered on all 3 events.

Actual behavior

only triggers .on('transactionHash')

Steps to reproduce the behavior

1.setup web3 using Metamask provider + Ganache GUI (localhost)

  const options = {
    transactionConfirmationBlocks: 1,
    transactionBlockTimeout: 5
};
const web3 = new Web3(window.ethereum, null, options);

     async deposit(amount) {
        try{
            const amountWei = this.props.web3Instance.utils.toWei(String(amount), 'ether');
            await this.props.exchangeContract.methods.deposit().send( {
                from: this.props.userAccount,
                value: amountWei
            })
            .on('transactionHash', (hash) => {
                this.setState({
                    message: 'Transaction pending...',
                });
                console.log(this.state.message);
            })
            .on('receipt', (receipt) => {
                this.setState({
                    message: 'Transaction has been mined',
                });
                console.log(this.state.message);
            })
            .on('confirmation', (confirmationNumber, receipt) => {
                this.setState({
                    message: 'Transaction confirmed!',
                });
                console.log(this.state.message);
            })
            .on('error', (err) => {
                this.setState({
                    message: err.message,
                });
            })
            .then( (receipt) => {
                console.log('Never enters here');
                this.props.getUserEthBalance();
                this.props.getUserContractEthBalance();
            });
        } catch (err) {
            console.log(err);
        }
    }

     submitFormEth = (e) => {
        e.preventDefault();

        this.deposit(this.state.ethValue);

        this.setState({
            ethValue: 0,
        });
    }
  1. The console will only print Transaction pending... edit: after updating to beta.52 it prints all events, but doesn't enter the .then() part

Error Logs

logs

None.

Versions

nivida commented 5 years ago

Could you please update to the latest version and adding the console log output to the issue?:)

Jaime-Iglesias commented 5 years ago

Could you please update to the latest version and adding the console log output to the issue?:)

Updated to the latest version, and now it does in fact trigger all 3 events properly, although it doesn't enter the .then() part.

I updated the Issue to reflect this and added the console log ouput.

kvhnuke commented 5 years ago

@nivida this is also due to the fact that https://github.com/ethereum/web3.js/blob/1.0/packages/web3-core-method/src/observers/TransactionObserver.js#L153

this.lastBlock.number is already an integer by passing it to increaseBlockNumber you are parsing the base10 int as a base16 value leading to extremely huge blocknumbers which will never resolve.

in order to test this scenario, use a http provider and do not set transactionConfirmationBlocks: 1 option.

this will cause web3 to never emit on receipt and only one on confirmation.

Also, I noticed the following https://github.com/ethereum/web3.js/blob/1.0/packages/web3-core-method/src/observers/TransactionObserver.js#L151

you only check if(receipt), meaning if the receipt != null it is considered to be included in a block, with parity this isnt the case, you have to check whether receipt != null && receipt.blockNumber != null

levino commented 5 years ago

How hard is it to open a PR with a failing test for this? Is there not setup for this?

nivida commented 5 years ago

This doesn't work because you can't combine the Promise and the EventEmitter in the same call. It isn't possible because of the bug we had with an "Unhandled Promise rejection" error.

@kvhnuke Thanks for the additional information. We have already an open issue related to this. (#2661)