Skip to content

Add timestamp to simulate logs & blob gas & remove total difficulty#9406

Merged
deffrian merged 21 commits intomasterfrom
fix/simulate-difficulty
Oct 7, 2025
Merged

Add timestamp to simulate logs & blob gas & remove total difficulty#9406
deffrian merged 21 commits intomasterfrom
fix/simulate-difficulty

Conversation

@deffrian
Copy link
Copy Markdown
Contributor

@deffrian deffrian commented Oct 3, 2025

Fixes Closes Resolves #

eth_simulateV1/ethSimulate-simple-send-from-contract (nethermind)
eth_simulateV1/ethSimulate-override-address-twice-in-separate-BlockStateCalls (nethermind)
eth_simulateV1/ethSimulate-eth-send-should-produce-logs (nethermind)
eth_simulateV1/ethSimulate-eth-send-should-produce-more-logs-on-forward (nethermind)
eth_simulateV1/ethSimulate-empty-with-block-num-set-firstblock (nethermind)
eth_getBlockByNumber/get-genesis (nethermind)
eth_getBlockByHash/get-block-by-hash (nethermind)
eth_getBlockByNumber/get-block-london-fork (nethermind)

There is a bug in geth that sets timstamps to zero in eth simulate
For context: ethereum/go-ethereum#32831

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

@deffrian deffrian requested a review from rubo as a code owner October 3, 2025 13:52
@deffrian deffrian requested a review from a team October 3, 2025 13:52
Comment on lines -124 to -125
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public UInt256? TotalDifficulty { get; set; }
Copy link
Copy Markdown
Member

@LukaszRozmej LukaszRozmej Oct 3, 2025

Choose a reason for hiding this comment

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

I say leave it for AuRa and Clique
If we really want we can remove (by setting null) in past blocks if we are post-merge. But that probably will still fail the test.

But the test is shit as TD is valid property for pre-merge blocks and Geth just yolo removed it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would remove it as well. Rn now we store it in the db for no apparent reason
Do we use clique anywhere? I think at this point we can remove it with ethash. And we probably should

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Japan Open Chain still uses Clique and is supported.
  2. We still have some Clique and AuRa uses, but they are not officially supported networks.
  3. We still need AuRa and Ethash for full syncing networks that were merged later (mainnet, gnosis). We could probably rip out large part from them, but can't remove them completely. But it is an effort.

Thus lets not do sudden moves without proper plan and scope.

@deffrian deffrian merged commit c9138c7 into master Oct 7, 2025
79 checks passed
@deffrian deffrian deleted the fix/simulate-difficulty branch October 7, 2025 16:24
@deffrian deffrian mentioned this pull request Oct 8, 2025
8 tasks
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.

2 participants