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

P2P sync update for 0.13.2 hashing scheme #2033

Merged
merged 21 commits into from
Jan 18, 2025
Merged

Conversation

kirugan
Copy link
Contributor

@kirugan kirugan commented Aug 7, 2024

No description provided.

@kirugan kirugan force-pushed the kirugan/proto-upgrade branch from 7d7d3f1 to 9365b80 Compare August 13, 2024 19:33
@AnkushinDaniil AnkushinDaniil force-pushed the kirugan/proto-upgrade branch 2 times, most recently from 029ef1b to 4ea9127 Compare November 7, 2024 09:04
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.70%. Comparing base (0c9ee5a) to head (1800c6f).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2033      +/-   ##
==========================================
- Coverage   74.71%   74.70%   -0.01%     
==========================================
  Files         110      111       +1     
  Lines       12105    12123      +18     
==========================================
+ Hits         9044     9057      +13     
- Misses       2364     2367       +3     
- Partials      697      699       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AnkushinDaniil AnkushinDaniil self-assigned this Nov 13, 2024
@AnkushinDaniil AnkushinDaniil force-pushed the kirugan/proto-upgrade branch 2 times, most recently from 706f1ba to 9b2feef Compare November 19, 2024 14:51
@AnkushinDaniil AnkushinDaniil force-pushed the kirugan/proto-upgrade branch 2 times, most recently from ed21813 to 71bd8c4 Compare November 27, 2024 08:50
db/buckets.go Outdated Show resolved Hide resolved
@AnkushinDaniil AnkushinDaniil force-pushed the kirugan/proto-upgrade branch 3 times, most recently from a170498 to 9d11219 Compare November 29, 2024 10:53
@AnkushinDaniil AnkushinDaniil force-pushed the kirugan/proto-upgrade branch 2 times, most recently from 7cd0479 to 4ae013b Compare December 3, 2024 05:19
Copy link
Contributor

@IronGauntlets IronGauntlets left a comment

Choose a reason for hiding this comment

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

I don''t think I fully understand what is happening in this PR. We have a migration which is only responsible for storing p2p hashes but i don't fully understand what would happen if someone where to sync from an empty DB.

Can you please explain the work flow of this PR?

adapters/core2p2p/receipt_pkg_test.go Show resolved Hide resolved
blockchain/blockchain.go Outdated Show resolved Hide resolved
blockchain/blockchain.go Outdated Show resolved Hide resolved
blockchain/blockchain.go Outdated Show resolved Hide resolved
p2p/p2p.go Outdated Show resolved Hide resolved
migration/sepolia_block_hashes.bin Outdated Show resolved Hide resolved
@AnkushinDaniil
Copy link
Contributor

AnkushinDaniil commented Dec 9, 2024

I don''t think I fully understand what is happening in this PR. We have a migration which is only responsible for storing p2p hashes but i don't fully understand what would happen if someone where to sync from an empty DB.

Can you please explain the work flow of this PR?

Migration

This migration consists of 2 steps:

  1. Save post-0.13.2 hashes for pre-0.13.2 blocks. This is done in any case, even for empty db, because we need these hashes to validate blocks using post-0.13.2 algorithms.
  2. Update block commitments for pre-0.13.2 blocks we have in db.

Hash calculation

The blockHash function now returns a hash and commitments. The version of these hashes matches the version of the block. Commitments must be post-0.13.2 if possible (there are some exceptions for deprecated networks and tests for them)


Upd.
Now we don't store post-0132 hashes in db and commitments and don't have migration. Instead, we store post-0132 hashes using embed only and compute post-0132 commitments before sending.

@AnkushinDaniil AnkushinDaniil force-pushed the kirugan/proto-upgrade branch 3 times, most recently from 01aed07 to dfd37f9 Compare December 25, 2024 10:47
@AnkushinDaniil AnkushinDaniil force-pushed the kirugan/proto-upgrade branch 2 times, most recently from 472d5c0 to 19295e2 Compare January 10, 2025 09:18
Copy link
Contributor

@rodrigo-pino rodrigo-pino 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

adapters/core2p2p/receipt_pkg_test.go Show resolved Hide resolved
adapters/p2p2core/receipt.go Show resolved Hide resolved
adapters/core2p2p/block.go Outdated Show resolved Hide resolved
blockchain/blockchain_test.go Outdated Show resolved Hide resolved
@AnkushinDaniil AnkushinDaniil merged commit 05b20ac into main Jan 18, 2025
13 checks passed
@AnkushinDaniil AnkushinDaniil deleted the kirugan/proto-upgrade branch January 18, 2025 09:41
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.

4 participants