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

fix issue #1423 #1735

Closed
wants to merge 1 commit into from
Closed

fix issue #1423 #1735

wants to merge 1 commit into from

Conversation

leon740727
Copy link

@leon740727 leon740727 commented Jun 27, 2018

fix issue #1423

I am using web3.eth.sendSignedTransactio on websocket provider, and it won't get receipt in first time, but if wait some seconds, it will get the receipt.

In the program, it will unsubscribe the newBlockHeaders listener immediately when failed to get receipt, that is why I got error, we should let it try until TIMEOUTBLOCK exceed.

@@ -253,16 +253,10 @@ Method.prototype._confirmTransaction = function (defer, result, payload) {
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please do this changes in a new branch an submit this PR against 1.0? Because it has nothing to do with the new ABI lib. Thanks :)

@leon740727
Copy link
Author

Sure! my bad. Should I just merge this commit into 1.0 branch or create a new branch against 1.0 and cherry pick this commit onto it? I am a newbie on github, please help me, thanks :-)

@nivida
Copy link
Contributor

nivida commented Jul 16, 2018

@leon740727 i would create a new branch with 1.0 as base and cherry pick the commits :)

@leon740727
Copy link
Author

@nivida thanks! :)

@frozeman frozeman closed this Aug 8, 2018
@frozeman
Copy link
Contributor

frozeman commented Aug 8, 2018

Can you add tests, i dont see how your fix is related to timeout blocks?

@leon740727
Copy link
Author

sorry, I am very busy recently :-(
and that situation won't happen every time. If need some test cases, it will need mock the block chain.
I haven't mock like that before. So, I will try to add some test case when I get some time...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants