-
Notifications
You must be signed in to change notification settings - Fork 324
automatic gas estimation #855
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
base: master
Are you sure you want to change the base?
Conversation
|
Very nice, been wanting this for some time. I have fixed ci on master, could you rebase so we can run it again? |
consistent gas estimation
update docs
|
This should be good to go now. I've made a couple of changes so that instead of directly calling eth_estimateGas, I make all gas estimations through I also made sure to do this gas estimation in both |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very nice UX upgrade, thanks a lot! We definitely need tests before merging, (you can add tests to this file).
I'm a little worried that there may be some error (either in seth, or the node doing the estimating) that may result in us automatically setting a very high gas value. I think we should at least display the value that we get back from seth estimate and potentially also ask for user confirmation if we're running in an interactive terminal?
| data=$(seth calldata "${@:2}") | ||
| fi | ||
|
|
||
| data=${data:-0x} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we put this in an else for the above if statement?
| data=${data:-0x} | ||
|
|
||
| if [[ -z "$SETH_CREATE" ]]; then | ||
| TO=$(seth --to-address "$1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only for --create?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If SETH_CREATE is unset, then we are doing a normal transaction that has a recipient. If not, we are creating a contract, and TO must be unset since this is a requirement for a contract creation tx.
Description
Adds automatic gas estimation to
seth mktxifETH_GASis not set. Sinceseth mktxis the underlying forseth sendanddapp create, dapptools users will likely never have to input gas limits manually.Checklist