-
Couldn't load subscription status.
- Fork 686
fix: use adjusted time on estimate gas when latest block is being used #4287
base: develop
Are you sure you want to change the base?
fix: use adjusted time on estimate gas when latest block is being used #4287
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.
Thanks for opening this!
I've added a couple of thoughts - can you please also add tests to cover this behaviour?
| | LegacyTransaction | ||
| | EIP2930AccessListTransaction | ||
| | EIP1559FeeMarketTransaction, | ||
| blockNumber: QUANTITY | Ethereum.Tag = Tag.latest |
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.
QUANTITY type shouldn't be used inside of the application logic - it's an alias type that's used to describe parameters in the API layer. Instead, accept type Quantity here, and do the conversion in the caller.
| let timestamp = previousHeader.timestamp; | ||
| if (blockNumber === "latest") | ||
| timestamp = Quantity.from(this.#adjustedTime(previousHeader.timestamp)); |
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.
Seems to me that if the request is for "latest", this should be incrementing the timestamp, otherwise we use the timestamp of the previous block, rather than the "next" block.
|
|
||
| coinbase: Address; | ||
|
|
||
| gasEstimateBlock = async ( |
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.
it'd be nice if this were a verb, ie createEstimateGasBlock or something.
I haven't the time to look into this further today, but I wonder if there's some value in combining the readyNextBlock and this function, as they are very similar (it might be that it just becomes too complicated to be worthwhile though).
|
A bit of delay on this but... I just implemented the same logic on |
When estimating gas, use the adjusted timestamp in case the
latestblock tag is being used. This helps avoid some errors in time-dependent contracts when using Ganache in fork mode.Should fix #3528.
This is my first PR here, would love some guidance.