Reconstruct the chain, flexdag and verification module #4584
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…et the latest pruning point
…10 tip stored in the dag state
jackzhhuang
requested review from
nkysg,
sanlee42 and
yourmoonlight
as code owners
## Walkthrough
This update refactors the handling of ghostdag data and block verification across the chain and DAG modules. It makes ghostdag data mandatory in verified blocks, removes legacy methods for uncle and DAG verification, consolidates verifier usage to `FullVerifier`, and simplifies the DAG commit and ghostdag calculation logic. Related tests and service logic are updated accordingly.
## Changes
| File(s) / Path(s) | Change Summary |
|----------------------------------------------------|------------------------------------------------------------------------------------------------------------------|
| chain/api/src/chain.rs | Made `ghostdata` mandatory in `VerifiedBlock`; renamed and simplified `verify_and_ghostdata` method. |
| chain/src/chain.rs | Removed `can_be_uncle`; unified block verification to `FullVerifier`; simplified ghostdag handling and method signatures. |
| chain/src/verifier/mod.rs | Removed legacy uncle/DAG verification; consolidated logic into `FullVerifier`; removed unused methods and structs.|
| flexidag/src/blockdag.rs | Removed `commit`, historical block checks, and `verify_and_ghostdata`; added `calc_ghostdata`. |
| flexidag/src/ghostdag/protocol.rs | Removed `build_ghostdata` method from `GhostdagManager`. |
| flexidag/tests/tests.rs | Updated tests to use `commit_trusted_block` with explicit ghostdag data instead of `commit`. |
| kube/manifest/starcoin-halley.yaml | Updated container image; removed explicit `command` field. |
| miner/src/create_block_template/new_header_service.rs| Removed extraneous blank line after cloning ghostdag data in `resolve_header`. |
| miner/src/generate_block_event_pacemaker.rs | Changed argument to `send_event` from `false` to `true` in two event handlers. |
| sync/src/block_connector/block_connector_service.rs | Switched verifier from `DagVerifier` to `FullVerifier` for block verification. |
| sync/src/block_connector/write_block_chain.rs | Changed test helper to use `FullVerifier` instead of `DagVerifier`. |
| sync/src/parallel/executor.rs | Updated block execution to use `FullVerifier` instead of `DagVerifierWithGhostData`. |
| sync/src/sync.rs | Changed block verifier import and usage from `DagVerifier` to `FullVerifier`. |
| config/src/genesis_config.rs | Increased default and minimum block time targets and difficulty window constants. |
| sync/src/tasks/tests.rs | Renamed test function and increased block count in test. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Miner
participant Chain
participant DAG
participant Verifier (FullVerifier)
participant Ghostdag
Miner->>Chain: submit Block
Chain->>Verifier (FullVerifier): verify_block(Block)
Verifier (FullVerifier)->>DAG: calc_ghostdata(BlockHeader)
DAG-->>Verifier (FullVerifier): GhostdagData
Verifier (FullVerifier)-->>Chain: VerifiedBlock(with GhostdagData)
Chain->>DAG: commit_trusted_block(BlockHeader, GhostdagData)
DAG-->>Chain: Commit ResultPossibly related PRs
Suggested reviewers
Poem
|
Benchmark for 354b2ffClick to view benchmark
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
config/src/genesis_config.rs(1 hunks)miner/src/create_block_template/new_header_service.rs(0 hunks)sync/src/tasks/tests.rs(1 hunks)
💤 Files with no reviewable changes (1)
- miner/src/create_block_template/new_header_service.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: jackzhhuang
PR: starcoinorg/starcoin#4572
File: vm/types/src/block_metadata/mod.rs:47-48
Timestamp: 2025-07-03T03:21:32.104Z
Learning: In the starcoin repository, the BlockMetadata structure changes are part of a clean slate implementation with no legacy data that needs to be deserialized, so backward compatibility concerns for field type changes are not applicable.
Learnt from: jackzhhuang
PR: starcoinorg/starcoin#4194
File: flexidag/src/blockdag.rs:444-446
Timestamp: 2024-09-30T09:31:42.793Z
Learning: In the service, `get_dag_state` is used to get the current state of the chain and it passes the main header ID to `BlockDAG`.
config/src/genesis_config.rs (2)
Learnt from: jackzhhuang
PR: starcoinorg/starcoin#4540
File: chain/src/chain.rs:1816-1824
Timestamp: 2025-05-28T10:21:10.718Z
Learning: In chain/src/chain.rs, the total_blocks calculation for epoch statistics always results in at least 1 block because total_selectd_chain_blocks = (current_block_number - epoch_start_block_number) + 1, which is always >= 1, making division by zero impossible in the avg_total_difficulty calculation.
Learnt from: jackzhhuang
PR: starcoinorg/starcoin#4572
File: miner/src/create_block_template/new_header_service.rs:0-0
Timestamp: 2025-07-03T03:25:16.732Z
Learning: In Starcoin's miner/src/create_block_template/new_header_service.rs, panic! is intentionally used in impossible code branches (like equal block ID comparison after early equality check) to detect logical errors early and ensure immediate restart rather than allowing potentially corrupted state to continue.
🔇 Additional comments (2)
config/src/genesis_config.rs (1)
730-733: Consensus parameter updates are localized and consistent in genesis_config.rsAll three constants—
G_DEFAULT_BASE_BLOCK_TIME_TARGET,G_DEFAULT_BASE_BLOCK_DIFF_WINDOW, andG_MIN_BLOCK_TIME_TARGET—are defined and used exclusively inconfig/src/genesis_config.rs. Each network’sConsensusConfig(Test, Dev, Halley, etc.) references the updated values uniformly, and no other files depend on these constants.• Defined at lines 730–733 in
config/src/genesis_config.rs
• Used in everyConsensusConfigblock in the same file
• No external references found elsewhere in the codebaseIf this consensus change is intentional, please coordinate with all network participants to ensure they upgrade to the new timing parameters.
sync/src/tasks/tests.rs (1)
341-341: Verify the increased block count aligns with consensus parameter changes.The block production count has been increased from 10 to 50. This change may be related to the consensus parameter modifications where block time targets increased from 100ms to 500ms.
Please confirm that this increase is intentional and necessary for the test to properly exercise sync cancellation under the new consensus parameters.
|
|
||
| #[stest::test] | ||
| pub async fn test_full_sync_cancel() -> Result<()> { | ||
| pub async fn test_full_syn_cancel() -> Result<()> { |
There was a problem hiding this comment.
Fix the typo in function name.
The function name has a typo: test_full_syn_cancel should be test_full_sync_cancel.
-pub async fn test_full_syn_cancel() -> Result<()> {
+pub async fn test_full_sync_cancel() -> Result<()> {🤖 Prompt for AI Agents
In sync/src/tasks/tests.rs at line 338, the function name has a typo: it is
currently named test_full_syn_cancel but should be corrected to
test_full_sync_cancel. Rename the function to fix the typo by adding the missing
'c' in 'sync'.
sanlee42
approved these changes
Jul 16, 2025
7 tasks
jackzhhuang
added a commit
that referenced
this pull request
Jul 31, 2025
* make creating block and executing block parallized * force to mint if the dag block comes from peers * add new header serivice * save code temporarily * reverse flexidag/src/blockdag.rs * monitor the new dag and peer new dag instead of new head message to get the latest pruning point * sort the tips by the work type and blue score and fetch the previous 10 tip stored in the dag state * receive the block header before creating new block * no mining until synced * add dynamic block time * the number of an end block of an epoch should subtract 1 before comparing * use system time as end time * use time plan as default block time target * use difficulty window to calcutlate the next difficulty * fix fmt * use k ratio to calculate the next block time target * use ratio to control the block time target * fix contain checking * use expected_blue_uncles_count to control the block time target * add selected count in next_block_time_target * fix div ratio * fix div ratio not mul * add red blocks in block meta struct * add blue ratio in genesis config * add latest mv * fix fmt * push the red blocks in latest version branch * min and max block time target in halley is 100 and 2000 * no force to mint when new dag block message coming * refactor to avoid duplicate codes * fix fmt * generate the new genesis * update epoch mv and halley genesis * no cache db * fix fmt * update red blocks * update genesis * change the epoch if the header changes * print some info * add some debug info * add more info * remove debug info * no force to mint if new peer blocks come * resolve the tips from the rabbit bot * fix test_example_config_compact * fix test_that_generated_file_are_up_to_date_in_git * fix config for test case in genesis and config * generated genesis for proxima and vega * add upgrade version 13 stlib * Remove blue ratio in config * Update stdlib * Fix genesis * Fix config example * update genesis * force to mint if the dag block comes from peers * switch main in build block service * add new header serivice * save code temporarily * monitor the new dag and peer new dag instead of new head message to get the latest pruning point * sort the tips by the work type and blue score and fetch the previous 10 tip stored in the dag state * receive the block header before creating new block * renew the header in block builder service asynchronically * the new process header will be triggered 1s internally * no mining until synced * reconstruct verify * reconstruct the code for verification and chain * comment the the black list blocks * remove the black list verification * fix test_dag_multiple_commits * Set the block time target from 500ms to 2000ms * update the json and genesis binary files * fix json config --------- Co-authored-by: sanlee42 <[email protected]>
7 tasks
7 tasks
This was referenced Sep 1, 2025
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Other information
Summary by CodeRabbit
Refactor
Bug Fixes
Chores