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

Do not execute txs on ZQ1 blocks #2284

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

86667
Copy link
Contributor

@86667 86667 commented Feb 3, 2025

Set ZQ1 block state root hash to zero when converting persistence. For proto networks with incorrect state root hashes currently we set the state root hash to zero upon fetching it.

With there now being blocks without any State in the db we return an error when an attempt to execute transactions on these blocks is made via an API call.

Copy link
Contributor

github-actions bot commented Feb 3, 2025

🐰 Bencher Report

Branch2054-historical-zq1-blocks-should-have-their-state-root-hash-set-to-zero
Testbedself-hosted
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
full-blocks-erc20-transfers/full-blocks-erc20-transfers📈 view plot
🚷 view threshold
1,031.30
(-16.15%)
1,634.95
(63.08%)
full-blocks-evm-transfers/full-blocks-evm-transfers📈 view plot
🚷 view threshold
434.77
(-10.56%)
697.44
(62.34%)
full-blocks-zil-transfers/full-blocks-zil-transfers📈 view plot
🚷 view threshold
4,119.30
(+4.95%)
5,127.80
(80.33%)
process-empty/process-empty📈 view plot
🚷 view threshold
10.32
(+12.89%)
11.51
(89.70%)
🐰 View full continuous benchmarking report in Bencher

@maxconway
Copy link
Contributor

So I think we've still got some functions in the API, e.g. impl TxBlock in api/types/zil.rs, which return state root hashes by calling block.state_root_hash(), which gets a state root hash directly from the block header. Will this address that too or should we also add some calls to is_zq1_block there to zero the hashes if appropriate?

@86667 86667 force-pushed the 2054-historical-zq1-blocks-should-have-their-state-root-hash-set-to-zero branch from e540d50 to 18ce270 Compare February 4, 2025 13:00
@86667
Copy link
Contributor Author

86667 commented Feb 4, 2025

So I think we've still got some functions in the API, e.g. impl TxBlock in api/types/zil.rs, which return state root hashes by calling block.state_root_hash(), which gets a state root hash directly from the block header. Will this address that too or should we also add some calls to is_zq1_block there to zero the hashes if appropriate?

the code has changed as discussed in a meeting since this comment was made but its still worth answering to confirm - yes these changes will change all ZQ1 blocks to have state root hashes of zeros.

@JamesHinshelwood
Copy link
Contributor

@86667 Note that the final ZQ1 block shouldn't have a state root hash of zero since that's the only block where we do actually know the state.

@86667 86667 force-pushed the 2054-historical-zq1-blocks-should-have-their-state-root-hash-set-to-zero branch from 9235578 to 2105b91 Compare February 5, 2025 10:54
@86667
Copy link
Contributor Author

86667 commented Feb 5, 2025

@86667 Note that the final ZQ1 block shouldn't have a state root hash of zero since that's the only block where we do actually know the state.

Ive pushed so that the final ZQ1 state is written to the empty block we write immediately after ZQ1 blocks, the kind-of genesis used as the high_qc. I think its easiest to understand if we say ZQ1 blocks always have no state and the state at block of conversion is at the block where ZQ2 begins. Would you agree?

@86667 86667 force-pushed the 2054-historical-zq1-blocks-should-have-their-state-root-hash-set-to-zero branch from 4940f54 to c9b204f Compare February 6, 2025 16:22
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.

Historical ZQ1 blocks should have their state root hash set to zero
3 participants