Skip to content

Conversation

@karalabe
Copy link
Member

@karalabe karalabe commented Jun 7, 2021

No description provided.

@holiman
Copy link
Contributor

holiman commented Jun 7, 2021

So, this changes feeCap to gasFeeCap, and tip to `gasTipCap.

	GasFeeCap        *hexutil.Big      `json:"maxFeePerGas,omitempty"`
	GasTipCap        *hexutil.Big      `json:"maxPriorityFeePerGas,omitempty"`

I agree it's a step forward, and think it's better than what we have now -- but I would personally prefer MaxFeePerGas and MaxPriorityFeePerGas, and live with the longer names.

Comment on lines 14 to 15
"gasFeeCap" : "0xfa0",
"gasTipCap" : "0x0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess these should really map the actual json names, rather than what we use internally

Copy link
Member Author

Choose a reason for hiding this comment

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

Where are these used?

Copy link
Contributor

Choose a reason for hiding this comment

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

When you run what's in the readme for that folder. But I just checked, it works fine now!

mkDynamicTx(0, common.Address{}, params.TxGas, big.NewInt(0), big.NewInt(0)),
},
want: "could not apply tx 0 [0xc4ab868fef0c82ae0387b742aee87907f2d0fc528fc6ea0a021459fb0fc4a4a8]: fee cap less than block base fee: address 0x71562b71999873DB5b286dF957af199Ec94617F7, feeCap: 0 baseFee: 875000000",
want: "could not apply tx 0 [0xc4ab868fef0c82ae0387b742aee87907f2d0fc528fc6ea0a021459fb0fc4a4a8]: fee cap less than block base fee: address 0x71562b71999873DB5b286dF957af199Ec94617F7, gasFeeCap: 0 baseFee: 875000000",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe these also should use the 'public' names, not the internal

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@karalabe
Copy link
Member Author

karalabe commented Jun 8, 2021

I agree it's a step forward, and think it's better than what we have now -- but I would personally prefer MaxFeePerGas and MaxPriorityFeePerGas, and live with the longer names.

I'm kind of against that. It's important to keep the codebase consistent.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman holiman merged commit c503f98 into ethereum:master Jun 8, 2021
atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
…um#23010)

* all: rename internal 1559 gas fields, add support for graphql

* cmd/evm/testdata, core: use public 1559 gas names on API surfaces
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 1, 2024
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.

3 participants