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

[ISSUE #3241] Do not use hardcoded gas price #3288

Merged
merged 1 commit into from
Feb 14, 2018
Merged

[ISSUE #3241] Do not use hardcoded gas price #3288

merged 1 commit into from
Feb 14, 2018

Conversation

jeluard
Copy link
Contributor

@jeluard jeluard commented Feb 12, 2018

relates to #3241

Summary:

Rely on geth to access current gas price

Steps to test:

  • Open Status
  • Navigate wallet / send
  • Verify it's not using default hardcoded value

status: ready

@jeluard jeluard added the wallet label Feb 12, 2018
@jeluard jeluard self-assigned this Feb 12, 2018
@status-github-bot status-github-bot bot added this to REVIEW in Pipeline for QA Feb 12, 2018
@dshulyak
Copy link
Contributor

Hey, I just want to mention that when we are signing and sending tx in status-go we are always setting suggested (by eth network) gas price and estimated gas limit if they are not provided.

@jeluard
Copy link
Contributor Author

jeluard commented Feb 12, 2018

Thanks @dshulyak ! We want to own those values UI side as we have to display it and allow user to modify this default.

(handlers/register-handler-db
:wallet/update-gas-price
(fn [{:keys [web3] :as db}]
(ethereum/gas-price web3 #(re-frame/dispatch [:wallet/update-gas-price-success %2]))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this code be moved to effects?

@jeluard
Copy link
Contributor Author

jeluard commented Feb 13, 2018

@rasom Updated to a fx

@status-github-bot status-github-bot bot moved this from REVIEW to CONTRIBUTOR in Pipeline for QA Feb 13, 2018
@janherich
Copy link
Contributor

@jeluard please resolve conflicts

@jeluard
Copy link
Contributor Author

jeluard commented Feb 13, 2018

@janherich done

@janherich janherich moved this from CONTRIBUTOR to TO TEST in Pipeline for QA Feb 13, 2018
@annadanchenko annadanchenko moved this from TO TEST to IN TESTING in Pipeline for QA Feb 13, 2018
@annadanchenko annadanchenko self-assigned this Feb 13, 2018
@annadanchenko
Copy link

annadanchenko commented Feb 13, 2018

If use Send Transaction from Wallet: I assume that Gas price adjustment should work for mainnet too. I’ve switched to mainnet with ropsten RPC but see 20 Gwei instead of SafeLow 1 Gwei (at least according to https://ethgasstation.info/index.php ) In the same time 1 Gwei is shown ok for testnets.

@jeluard
Copy link
Contributor Author

jeluard commented Feb 14, 2018

Updated description to make sure related issue is not closed (and bounty not claimed).

@annadanchenko annadanchenko moved this from IN TESTING to MERGE in Pipeline for QA Feb 14, 2018
Signed-off-by: Eric Dvorsak <eric@dvorsak.fr>
@yenda yenda merged commit d74b27f into develop Feb 14, 2018
Pipeline for QA automation moved this from MERGE to DONE Feb 14, 2018
@yenda yenda deleted the bug/#3241 branch April 6, 2018 17:51
@yenda yenda restored the bug/#3241 branch April 6, 2018 17:52
@yenda yenda deleted the bug/#3241 branch April 6, 2018 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants