-
Notifications
You must be signed in to change notification settings - Fork 20.5k
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: return decimal for estimateGas
and cumulativeGas
queries
#22126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -811,15 +811,15 @@ type CallData struct { | |||
// CallResult encapsulates the result of an invocation of the `call` accessor. | |||
type CallResult struct { | |||
data hexutil.Bytes // The return data from the call | |||
gasUsed hexutil.Uint64 // The amount of gas used | |||
gasUsed Long // The amount of gas used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either a typo or type alias?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LONG $ETH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
long eth, short tron
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this financial advice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gas is too long to pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please conform to this format in the future.
@@ -301,21 +301,21 @@ func (t *Transaction) Status(ctx context.Context) (*hexutil.Uint64, error) { | |||
return &ret, nil | |||
} | |||
|
|||
func (t *Transaction) GasUsed(ctx context.Context) (*hexutil.Uint64, error) { | |||
func (t *Transaction) GasUsed(ctx context.Context) (*Long, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too long
receipt, err := t.getReceipt(ctx) | ||
if err != nil || receipt == nil { | ||
return nil, err | ||
} | ||
ret := hexutil.Uint64(receipt.GasUsed) | ||
ret := Long(receipt.GasUsed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
way too long
receipt, err := t.getReceipt(ctx) | ||
if err != nil || receipt == nil { | ||
return nil, err | ||
} | ||
ret := hexutil.Uint64(receipt.CumulativeGasUsed) | ||
ret := Long(receipt.CumulativeGasUsed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
man, this is really long
@@ -811,15 +811,15 @@ type CallData struct { | |||
// CallResult encapsulates the result of an invocation of the `call` accessor. | |||
type CallResult struct { | |||
data hexutil.Bytes // The return data from the call | |||
gasUsed hexutil.Uint64 // The amount of gas used | |||
gasUsed Long // The amount of gas used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd like to speak to the manager
@@ -811,15 +811,15 @@ type CallData struct { | |||
// CallResult encapsulates the result of an invocation of the `call` accessor. | |||
type CallResult struct { | |||
data hexutil.Bytes // The return data from the call | |||
gasUsed hexutil.Uint64 // The amount of gas used | |||
gasUsed Long // The amount of gas used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
long eth, short tron
@@ -117,6 +117,12 @@ func TestGraphQLBlockSerialization(t *testing.T) { | |||
want: `{"errors":[{"message":"Cannot query field \"bleh\" on type \"Query\".","locations":[{"line":1,"column":2}]}]}`, | |||
code: 400, | |||
}, | |||
// should return `estimateGas` as decimal | |||
{ | |||
body: `{"query": "{block{ estimateGas(data:{}) }}"}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
peer pressure
Woah, I didn't evne comment and this got merged. Gonna have to revert, sorry. |
This PR updates the return values for
estimateGas
andcumulativeGas
to decimal rather than hexadecimal, as defined by the spec.