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

eth: max gas price #1727

Merged
merged 3 commits into from
Mar 10, 2021
Merged

eth: max gas price #1727

merged 3 commits into from
Mar 10, 2021

Conversation

kyriediculous
Copy link
Contributor

@kyriediculous kyriediculous commented Jan 5, 2021

What does this pull request do? Explain your changes. (required)
Changes -gasPrice flag to -maxGasPrice and allows users to set a max gas price to pay for transactions.

Specific updates (required)

  • Changes -gasPrice flag to be -maxGasPrice instead. Removes -gasLimit flag (seems superflious).

  • Implements SuggestGasPrice that checks for the max gas price when set , MaxGasPrice and SetMaxGasPrice on Backend.

  • Changes CLI server API endpoings /gasPrice and /setGasPrice to /maxGasPrice and /setMaxGasPrice

  • Since the gasLimit flag is removed and the gasPrice flag repurposed LivepeerEthClient.GasInfo and LivepeerEthClient.SetGasInfo are no longer needed and removed.

  • Moved unlocking the Eth account into the LivepeerEthClient constructor and removed LivepeerEthClient.Setup as it now only unlocks the account after removing LivepeerEthClient.SetGasInfo

  • Updates the CLI to allow for setting a max gas price

How did you test each of these updates (required)

Does this pull request close any open issues?
Fixes #1671

Checklist:

  • README and other documentation updated
  • Node runs in OSX and devenv
  • All tests in ./test.sh pass

cmd/livepeer/livepeer.go Show resolved Hide resolved
eth/client.go Show resolved Hide resolved
eth/client.go Show resolved Hide resolved
eth/client.go Outdated Show resolved Hide resolved
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

Since we're following the deprecation procedure described in

cmd/livepeer/livepeer.go Show resolved Hide resolved
cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
eth/backend.go Outdated Show resolved Hide resolved
eth/client.go Show resolved Hide resolved
cmd/devtool/devtool.go Outdated Show resolved Hide resolved
cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
eth/backend.go Outdated Show resolved Hide resolved
eth/client.go Outdated Show resolved Hide resolved
@kyriediculous
Copy link
Contributor Author

Should have addressed remaining comments, added max gas price to node stats in CLI, improved error logging.

eth/helpers.go Outdated Show resolved Hide resolved
cmd/livepeer_cli/wizard_stats.go Outdated Show resolved Hide resolved
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

LGTM

@kyriediculous
Copy link
Contributor Author

Rebased and tested on rinkeby, pending green CI

@kyriediculous
Copy link
Contributor Author

Finally got CI to pass , kept running into issues with unrelated tests in the server repo (SubmitSegment).

@kyriediculous kyriediculous merged commit 1832d26 into master Mar 10, 2021
@kyriediculous kyriediculous deleted the nv/max-gasprice branch March 10, 2021 13:18
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.

Transactions can exceed manually set gas price
2 participants