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.

Sign up for GitHub

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

Jump to bottom

feat(consensus)!: implement preshards and shard groups #1092

Merged
merged 2 commits into from
Aug 5, 2024
Merged

feat(consensus)!: implement preshards and shard groups #1092

merged 2 commits into from
Aug 5, 2024

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Jul 25, 2024 •
edited
Loading

Description

feat!(consensus): implement preshards and shard groups
Implement JMT for each shard.

Motivation and Context

Divide the shard space into a number of preshards. Each VN committee is responsible for a group (1 or more) preshards.

How Has This Been Tested?

Existing tests, updated Substate address unit tests, manually

What process can a PR reviewer use to test or verify this change?

Look at the shard group in the database, and check the shared state_tree table

Breaking Changes

  • None
  • Requires data directory to be deleted
  • Other - Please specify

BREAKING CHANGE: sharding model changed and not backward compatible

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 25, 2024 •
edited
Loading

Test Results (CI)

555 tests  +8   555 ✅ +8   1h 30m 19s ⏱️ - 29m 22s
 65 suites ±0     0 💤 ±0 
  2 files   ±0     0 ❌ ±0 

Results for commit 42029e7. ± Comparison against base commit 0ff6cbf.

This pull request removes 1 and adds 9 tests. Note that renamed tests count towards both. 
tari_dan_common_types ‑ substate_address::tests::shard_range

tari_dan_common_types ‑ num_preshards::export_bindings_numpreshards
tari_dan_common_types ‑ shard_group::export_bindings_shardgroup
tari_dan_common_types ‑ shard_group::tests::encode_decode
tari_dan_common_types ‑ shard_group::tests::to_substate_address_range
tari_dan_common_types ‑ substate_address::tests::shard_group_to_substate_address_range::it_works
tari_dan_common_types ‑ substate_address::tests::to_shard_group::it_returns_the_correct_shard_group
tari_dan_common_types ‑ substate_address::tests::to_shard_group::it_returns_the_correct_shard_group_for_odd_num_committees
tari_dan_common_types ‑ substate_address::tests::to_shard_group::it_returns_the_correct_shard_group_generic
tari_state_tree::test ‑ hash_allows_putting_in_same_version

♻️ This comment has been updated with latest results.

@sdbondi sdbondi force-pushed the presharding branch 3 times, most recently from dbae726 to 6ff275c Compare July 30, 2024 13:50
@sdbondi sdbondi changed the title wip: preshard feat!(consensus): implement preshards and shard groups Jul 30, 2024
@sdbondi sdbondi marked this pull request as ready for review July 30, 2024 13:51
@sdbondi sdbondi changed the title feat!(consensus): implement preshards and shard groups feat(consensus)!: implement preshards and shard groups Jul 30, 2024
@sdbondi sdbondi marked this pull request as draft July 30, 2024 13:52
@sdbondi sdbondi force-pushed the presharding branch 2 times, most recently from 171adb3 to b494bdb Compare July 30, 2024 14:50
@sdbondi sdbondi marked this pull request as ready for review July 30, 2024 14:50
@therealdannzor therealdannzor self-requested a review July 30, 2024 15:15
therealdannzor
therealdannzor reviewed Jul 31, 2024
View reviewed changes
Copy link
Contributor

@therealdannzor therealdannzor 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, just a couple of questions to better understand pre-sharding

applications/tari_indexer/src/substate_storage_sqlite/schema.rs Show resolved Hide resolved
dan_layer/common_types/src/substate_address.rs Show resolved Hide resolved
dan_layer/consensus/src/hotstuff/on_ready_to_vote_on_local_block.rs Outdated Show resolved Hide resolved
dan_layer/consensus_tests/src/support/helpers.rs Show resolved Hide resolved
dan_layer/rpc_state_sync/src/manager.rs Show resolved Hide resolved
dan_layer/storage/src/consensus_models/block.rs Show resolved Hide resolved
@therealdannzor therealdannzor mentioned this pull request Jul 31, 2024
Open
@sdbondi sdbondi added this pull request to the merge queue Aug 5, 2024
Merged via the queue into tari-project:development with commit e1c94f1 Aug 5, 2024
12 checks passed
@sdbondi sdbondi deleted the presharding branch August 5, 2024 08:40
@sdbondi sdbondi mentioned this pull request Aug 5, 2024
Merged
3 tasks
github-merge-queue bot pushed a commit that referenced this pull request Aug 6, 2024
@sdbondi
fix(sync)!: validate checkpoint merkle root (#1108)
6c1dbc6 
Description
---
fix(sync): validate checkpoint merkle root
Rename NumPreshard variants
(#1092 (comment))

Motivation and Context
---
Validates the checkpoint blocks merkle root against the synced state so
that the validator can be sure they have the correct state. Note that
block and QC signatures are not currently validated pending a refactor
in the validations.

How Has This Been Tested?
---
Manually, submitting transactions, mining to next epoch, submitting more
transactions, deleting data of one VN and resyncing.

What process can a PR reviewer use to test or verify this change?
---
As above

Breaking Changes
---

- [ ] None
- [ ] Requires data directory to be deleted
- [x] Other - Please specify

BREAKING CHANGE: the merkle root construction differs slightly from
current development branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@therealdannzor therealdannzor therealdannzor approved these changes

@mrnaveira mrnaveira mrnaveira approved these changes

Assignees
No one assigned
Labels
CR-too_long P-acks_required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sdbondi @therealdannzor @mrnaveira @ghpbot-tari-project