This repository was archived by the owner on Aug 19, 2024. It is now read-only.
A list of APIs to be changed or added for dynamic gas price #1452
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
June 20, 2022 10:21
hyunsooda
reviewed
Jun 23, 2022
8000
jimni1222
reviewed
Jun 23, 2022
jimni1222
reviewed
Jun 23, 2022
jimni1222
reviewed
Jun 23, 2022
jimni1222
reviewed
Jun 23, 2022
jimni1222
reviewed
Jun 23, 2022
jimni1222
reviewed
Jun 27, 2022
jimni1222
approved these changes
Jun 27, 2022
aeharvlee
reviewed
Jun 27, 2022
governance/api.go
Outdated
| @@ -62,22 +63,33 @@ var ( | |||
| errInvalidUpperBound = errors.New("upperboundbasefee cannot be set lower than lowerboundbasefee") | |||
| ) | |||
|
|
|||
| // TODO-Klaytn-Governance: Refine this API and consider the gas price of txpool | |||
| // GasPriceAt returns the baseFeePerGas of the given block in peb. | |||
There was a problem hiding this comment.
Suggested change
| // GasPriceAt returns the baseFeePerGas of the given block in peb. | |
| // GasPriceAt returns the baseFeePerGas of the given block in peb. | |
| // It returns unit price by using governance if there is no base fee set in header. |
kjhman21
reviewed
Jun 27, 2022
api/api_ethereum.go
Outdated
| @@ -308,7 +311,8 @@ func (api *EthereumAPI) GasPrice(ctx context.Context) (*hexutil.Big, error) { | |||
|
|
|||
| // MaxPriorityFeePerGas returns a suggestion for a gas tip cap for dynamic fee transactions. | |||
| func (api *EthereumAPI) MaxPriorityFeePerGas(ctx context.Context) (*hexutil.Big, error) { | |||
| return api.publicKlayAPI.MaxPriorityFeePerGas(ctx) | |||
| // TODO-Klaytn maybe need to branch kip71 hardfork | |||
There was a problem hiding this comment.
We'd better make sure this. Do we need to do different action based on the hardfork or not?
There was a problem hiding this comment.
I think we can remove this TODO comment.
api.GasPrice will return unit price before hard fork and return next block'sm baseFee after hard fork.
@bobpkr Please let me know if i misunderstood
There was a problem hiding this comment.
We need to discuss this! What to do with maxPriorityFeePerGas...! Please share your opinions.
There was a problem hiding this comment.
First of all, you are right.
I heard that @kjeom will be more organized and will give you some feedback.
jimni1222
reviewed
Jun 28, 2022
jimni1222
reviewed
Jun 28, 2022
jimni1222
reviewed
Jun 29, 2022
jimni1222
reviewed
Jun 29, 2022
jimni1222
reviewed
Jun 29, 2022
added 2 commits
June 29, 2022 12:52
- add a condition for checking maxFeePerGas and maxPriorityFeePerGas
hyunsooda
approved these changes
Jun 30, 2022
jimni1222
approved these changes
Jun 30, 2022
This was referenced Jun 30, 2022
7 tasks
This pull request was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
$ make test)Related issues