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

graphql: fee history fields #24452

Merged
merged 7 commits into from
Mar 10, 2022
Merged

graphql: fee history fields #24452

merged 7 commits into from
Mar 10, 2022

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Feb 22, 2022

This PR adds the NextBaseFeePerGas to Block and EffectiveTip to Transaction to make it easier for clients to compute fee history themselves. Note I made the tx reward sorting logic in our fee history algorithm stable so clients can compute consistent results.

Example client script: https://gist.github.com/s1na/851ad57e8fca7c74b2464207b2e99021
Result of a run on a few Goerli blocks:

❯ node index.js 4 6415924 "50, 75"
# JSON-RPC result
{
  oldestBlock: '0x61e631',
  reward: [
    [ '0x9502f900', '0x165a0bbf9' ],
    [ '0x59682f00', '0x59682f00' ],
    [ '0x3b9aca00', '0x9502f900' ],
    [ '0x9502f900', '0x9502f900' ]
  ],
  baseFeePerGas: [ '0x7', '0x7', '0x7', '0x7', '0x7' ],
  gasUsedRatio: [
    0.10796890077097406,
    0.0061631333333333335,
    0.008546466666666667,
    0.13900370378341118
  ]
}
# GraphQL Result
{
  oldestBlock: '0x61e631',
  baseFeePerGas: [ '0x7', '0x7', '0x7', '0x7', '0x7' ],
  gasUsedRatio: [
    0.10796890077097406,
    0.0061631333333333335,
    0.008546466666666667,
    0.13900370378341118
  ],
  rewards: [
    [ '0x9502f900', '0x165a0bbf9' ],
    [ '0x59682f00', '0x59682f00' ],
    [ '0x3b9aca00', '0x9502f900' ],
    [ '0x9502f900', '0x9502f900' ]
  ]
}

Fixes #24386

graphql/graphql.go Outdated Show resolved Hide resolved
graphql/graphql.go Outdated Show resolved Hide resolved
@s1na
Copy link
Contributor Author

s1na commented Feb 28, 2022

I applied the review comments and fixed some minor issues. Ready for review

Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

Left a few comments.

graphql/schema.go Outdated Show resolved Hide resolved
if err != nil || tx == nil {
return nil, err
}
header, err := t.block.resolveHeader(ctx)
Copy link
Member

Choose a reason for hiding this comment

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

for the non-finalized transaction, the t.block can be nil. Please add one more check here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right. I had copied some stuff from EffectiveGasPrice which has the same issue. Added a check in both methods.

Do you think it makes sense to compute and use the latest block's nextBaseFee for pending txes?

Copy link
Member

Choose a reason for hiding this comment

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

maybe we can simply return an error instead? @fjl @karalabe What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's safer to return an error rather than 'speculate' on the tip, for pending transactions.

graphql/graphql.go Show resolved Hide resolved
@ligi ligi removed the status:triage label Mar 1, 2022
Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

We need to decide what's the semantic of EffectiveGasPrice, EffectiveTip and NextBaseFeePerGas in case the basefee is not existent.

Currently it's returned as 0 in this PR, it's good to me though. Otherwise code is good to me.

@holiman
Copy link
Contributor

holiman commented Mar 10, 2022

in case the basefee is not existent.

in which cases is it not existant? Do you mean blocks prior to the fork?

@s1na
Copy link
Contributor Author

s1na commented Mar 10, 2022

Do you mean blocks prior to the fork?

Yes that's what Gary is referring to. Right now these fields (EffectiveTip) simply return nil. I was exploring an alternative to returning an error, which is have different Transaction types, each with fields that makes sense for that type

@holiman
Copy link
Contributor

holiman commented Mar 10, 2022

This pr, as is, what happens if you compare json-rpc versus graphql on blocks prior to the fork?

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 added this to the 1.10.17 milestone Mar 10, 2022
@holiman holiman merged commit 57cec89 into ethereum:master Mar 10, 2022
sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Mar 11, 2022
This PR adds the `NextBaseFeePerGas` to `Block` and `EffectiveTip` to `Transaction` to make it easier for clients to compute fee history themselves via graphql queries.
JacekGlen pushed a commit to JacekGlen/go-ethereum that referenced this pull request May 26, 2022
This PR adds the `NextBaseFeePerGas` to `Block` and `EffectiveTip` to `Transaction` to make it easier for clients to compute fee history themselves via graphql queries.
cp-wjhan pushed a commit to cp-yoonjin/go-wemix that referenced this pull request Nov 16, 2022
This PR adds the `NextBaseFeePerGas` to `Block` and `EffectiveTip` to `Transaction` to make it easier for clients to compute fee history themselves via graphql queries.
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 12, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 12, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 12, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 12, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 19, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 21, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 22, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 23, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 24, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 25, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 25, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 26, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 26, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Aug 26, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 2, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 6, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 9, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 9, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 9, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 10, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 12, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 12, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 19, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 19, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 19, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 21, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 22, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 22, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 23, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Sep 23, 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.

GraphQL API should contain EIP-1559 London Hardfork methods
4 participants