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

setter and getter method for TIMEOUTBLOCK #1581

Closed
wants to merge 5 commits into from
Closed

setter and getter method for TIMEOUTBLOCK #1581

wants to merge 5 commits into from

Conversation

Harzu
Copy link

@Harzu Harzu commented Apr 24, 2018

Hello everyone!
We've got a problem with your library.
In test network, transactions sometimes do not run in 50 blocks.
And when we run functions on the contract we've received an error.

Suggestion: Please allow users to choose the timeout like 50 seconds of the blocks themselves

I made some changes. Please have a look and if you don't mind send us your feedback how it can be improved

For now, we've just added the getter and setter to variable TIMEOUTBLOCK and made the method web3.setTimeoutBlock (count)

@coveralls
Copy link

coveralls commented Apr 24, 2018

Coverage Status

Coverage increased (+0.05%) to 85.683% when pulling 4581313 on DaoCasino:dc-changes into c0e727e on ethereum:1.0.

@jimmy-collazos
Copy link

@Harzu check your PR and remove unnecesary files (lerna-debug.log, packages/web3-core-method/.gitignore, ....) and don't change de git url of project.

@frozeman
Copy link
Contributor

frozeman commented Aug 8, 2018

This should not be on the web3 object itself, as blocks are only part of web3.eth, better would be as a getter setter using web3.eth.blockTimeout, and people can set it or change it.
Same as here: https://web3js.readthedocs.io/en/1.0/web3-eth.html#defaultblock
Please also add docs to it.

This should also simplify your code a lot

@nivida
Copy link
Contributor

nivida commented Nov 28, 2018

Hay I've already implemented it in the ethereumProvider branch (PR #2000). Because of this, I will close this PR now.

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.

5 participants