fix: use adjusted time on estimate gas when latest block is being used #4287
Conversation
luzzif
changed the title
| | LegacyTransaction | ||
| | EIP2930AccessListTransaction | ||
| | EIP1559FeeMarketTransaction, | ||
| blockNumber: QUANTITY | Ethereum.Tag = Tag.latest |
There was a problem hiding this comment.
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.
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.
| @@ -577,6 +583,40 @@ export default class Blockchain extends Emittery<BlockchainTypedEvents> { | |||
|
|
|||
| coinbase: Address; | |||
|
|
|||
| gasEstimateBlock = async ( | |||
There was a problem hiding this comment.
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.