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

internal/ethapi: use same state for each invocation within EstimateGas #27505

Merged
merged 4 commits into from Jun 20, 2023

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Jun 19, 2023

EstimateGas repeatedly executes a transaction, performing a binary search with multiple gas prices to determine proper pricing. Each call retrieves a new copy of the state https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1017 . Because the pending/latest state can change during the execution of EstimateGas, this can potentially cause strange behavior (as noted here).

This PR modifies EstimateGas to retrieve the state once and use a copy of it for every call invocation it does.

@jwasinger jwasinger marked this pull request as ready for review June 19, 2023 03:13
@fjl fjl changed the title inernal/ethapi: use a copy of the same state for each call invocation within EstimateGas internal/ethapi: use a copy of the same state for each call invocation within EstimateGas Jun 19, 2023
@fjl
Copy link
Contributor

fjl commented Jun 19, 2023

Please explain this change in the PR description.

@jwasinger
Copy link
Contributor Author

@fjl done.

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.

Looks good to me!

@holiman holiman changed the title internal/ethapi: use a copy of the same state for each call invocation within EstimateGas internal/ethapi: use same state for each invocation within EstimateGas Jun 20, 2023
@holiman holiman added this to the 1.12.1 milestone Jun 20, 2023
@holiman holiman merged commit 8c288b5 into ethereum:master Jun 20, 2023
2 checks passed
@jwasinger jwasinger deleted the estimate-gas-state-copy branch June 20, 2023 12:41
spencer-tb pushed a commit to spencer-tb/go-ethereum that referenced this pull request Jul 7, 2023
ethereum#27505)

EstimateGas repeatedly executes a transaction, performing a binary search with multiple gas prices to determine proper pricing. Each call retrieves a new copy of the state (https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1017) . Because the pending/latest state can change during the execution of EstimateGas, this can potentially cause strange behavior (as noted here: ethereum#27502 (comment)).

This PR modifies EstimateGas to retrieve the state once and use a copy of it for every call invocation it does.
Tristan-Wilson added a commit to OffchainLabs/go-ethereum that referenced this pull request Aug 3, 2023
….12.1-pre

internal/ethapi/api.go: Upstream, the body of ethapi.DoCall was mostly
moved into ethapi.doCall, leaving DoCall as a wrapper to get the state
only once, fixing the issue described in
ethereum/go-ethereum#27505, and this conflicts
with our addition of the core.MessageRunMode parameter to DoCall.
Resolved by adding that parameter to the wrapper and the wrapped
functions.
MoonShiesty pushed a commit to MoonShiesty/go-ethereum that referenced this pull request Aug 30, 2023
ethereum#27505)

EstimateGas repeatedly executes a transaction, performing a binary search with multiple gas prices to determine proper pricing. Each call retrieves a new copy of the state (https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1017) . Because the pending/latest state can change during the execution of EstimateGas, this can potentially cause strange behavior (as noted here: ethereum#27502 (comment)).

This PR modifies EstimateGas to retrieve the state once and use a copy of it for every call invocation it does.
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
ethereum#27505)

EstimateGas repeatedly executes a transaction, performing a binary search with multiple gas prices to determine proper pricing. Each call retrieves a new copy of the state (https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1017) . Because the pending/latest state can change during the execution of EstimateGas, this can potentially cause strange behavior (as noted here: ethereum#27502 (comment)).

This PR modifies EstimateGas to retrieve the state once and use a copy of it for every call invocation it does.
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants