Add genesis in BlockDAG - #4780
Conversation
📝 WalkthroughWalkthroughThreaded an explicit genesis hash through BlockDAG: added a genesis field, updated constructors and testing helpers to accept and propagate the genesis hash, and updated call sites across crates to compute and pass the genesis id during DAG creation and initialization. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
a4c8251 to
57f2548
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
flexidag/tests/tests.rs (1)
280-281: Minor: Extra blank line after constructor.The test is functionally correct, but there's an extra blank line at line 281 that could be removed for consistency.
♻️ Optional cleanup
- let mut dag = BlockDAG::create_for_testing(genesis.id()).unwrap(); - + let mut dag = BlockDAG::create_for_testing(genesis.id()).unwrap(); dag.init_with_genesis(genesis.clone()).unwrap();
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
chain/mock/src/mock_chain.rschain/tests/block_test_utils.rscmd/generator/src/lib.rsflexidag/src/blockdag.rsflexidag/tests/test_commit_atomicity.rsflexidag/tests/tests.rsgenesis/src/lib.rsnode/src/node.rssimnet/src/scene/mod.rssync/src/tasks/test_tools.rstest-helper/src/chain.rs
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 4722
File: chain/src/chain.rs:2151-2155
Timestamp: 2025-10-20T09:58:31.897Z
Learning: In starcoin/chain/src/chain.rs, for DAG block execution: the BlockChain instance's state (self.statedb.0 and self.statedb.1) must be initialized/positioned at the selected parent's state roots BEFORE execute_dag_block is called. Execution should validate and fail if state is not correctly positioned, rather than forking state mid-execution. This design avoids the performance overhead of repeated state forking during execution.
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 4698
File: cmd/tx-factory/src/main.rs:165-165
Timestamp: 2025-09-28T08:35:37.355Z
Learning: In the tx-factory cmd tool, jackzhhuang prefers long unlock durations (multiple hours) for account unlocking rather than the typical short durations like 1 minute.
📚 Learning: 2025-08-08T10:20:45.797Z
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 4605
File: chain/src/chain.rs:1104-1106
Timestamp: 2025-08-08T10:20:45.797Z
Learning: In starcoin/chain/src/chain.rs, ChainReader::get_block(hash) is intentionally implemented to return any block from storage without verifying membership in the current main chain/DAG view (the previous exist_block_filter was removed). Callers that require main-chain-only results should perform their own existence/membership checks (e.g., exist_block/check_exist_block) as needed.
Applied to files:
cmd/generator/src/lib.rschain/tests/block_test_utils.rssync/src/tasks/test_tools.rsnode/src/node.rsflexidag/src/blockdag.rstest-helper/src/chain.rsgenesis/src/lib.rsflexidag/tests/tests.rschain/mock/src/mock_chain.rs
📚 Learning: 2025-10-20T09:58:31.897Z
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 4722
File: chain/src/chain.rs:2151-2155
Timestamp: 2025-10-20T09:58:31.897Z
Learning: In starcoin/chain/src/chain.rs, for DAG block execution: the BlockChain instance's state (self.statedb.0 and self.statedb.1) must be initialized/positioned at the selected parent's state roots BEFORE execute_dag_block is called. Execution should validate and fail if state is not correctly positioned, rather than forking state mid-execution. This design avoids the performance overhead of repeated state forking during execution.
Applied to files:
cmd/generator/src/lib.rschain/tests/block_test_utils.rssync/src/tasks/test_tools.rssimnet/src/scene/mod.rsnode/src/node.rsflexidag/tests/test_commit_atomicity.rsflexidag/src/blockdag.rstest-helper/src/chain.rsgenesis/src/lib.rsflexidag/tests/tests.rschain/mock/src/mock_chain.rs
📚 Learning: 2025-08-08T10:16:46.394Z
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 4605
File: chain/src/chain.rs:185-187
Timestamp: 2025-08-08T10:16:46.394Z
Learning: In starcoin/chain/src/chain.rs, BlockChain::statedb currently consumes self to move out ChainStateDB during block building to avoid cloning or recreating the state DB for transaction filtering. This is intentional for performance; a rename to into_statedb is acceptable but behavior should remain a consuming getter.
Applied to files:
cmd/generator/src/lib.rssync/src/tasks/test_tools.rsnode/src/node.rstest-helper/src/chain.rschain/mock/src/mock_chain.rs
📚 Learning: 2025-07-03T03:21:32.104Z
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 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.
Applied to files:
cmd/generator/src/lib.rs
📚 Learning: 2025-09-01T03:56:58.362Z
Learnt from: welbon
Repo: starcoinorg/starcoin PR: 4633
File: vm/vm-runtime/Cargo.toml:48-48
Timestamp: 2025-09-01T03:56:58.362Z
Learning: In the Starcoin codebase, vm1 (starcoin-vm1-vm-runtime) may need to expose the same feature flags as vm2 to satisfy cross-version compatibility requirements when downstream projects like genesis depend on features from both versions. Empty feature declarations like `move-unit-test = []` may be intentionally added for compilation compatibility rather than to activate specific functionality.
Applied to files:
cmd/generator/src/lib.rsgenesis/src/lib.rs
📚 Learning: 2025-07-03T03:25:16.732Z
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 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.
Applied to files:
cmd/generator/src/lib.rsnode/src/node.rs
📚 Learning: 2024-09-30T09:31:42.793Z
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 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`.
Applied to files:
test-helper/src/chain.rsflexidag/tests/tests.rs
📚 Learning: 2025-05-28T10:21:10.718Z
Learnt from: jackzhhuang
Repo: starcoinorg/starcoin PR: 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.
Applied to files:
flexidag/tests/tests.rs
🧬 Code graph analysis (9)
cmd/generator/src/lib.rs (2)
types/src/startup_info.rs (1)
genesis_hash(37-39)flexidag/src/blockdag.rs (1)
new(77-121)
chain/tests/block_test_utils.rs (1)
flexidag/src/blockdag.rs (2)
genesis(561-563)create_for_testing(124-131)
sync/src/tasks/test_tools.rs (1)
flexidag/src/blockdag.rs (2)
genesis(561-563)create_blockdag(72-75)
simnet/src/scene/mod.rs (1)
flexidag/src/blockdag.rs (2)
genesis(561-563)new(77-121)
node/src/node.rs (3)
types/src/startup_info.rs (7)
genesis_hash(37-39)new(25-31)new(100-102)new(176-178)new(220-222)new(262-264)new(299-301)chain/src/chain.rs (3)
dag(254-256)dag(1721-1723)new(88-100)flexidag/src/blockdag.rs (1)
new(77-121)
flexidag/tests/test_commit_atomicity.rs (1)
flexidag/src/blockdag.rs (2)
new(77-121)genesis(561-563)
genesis/src/lib.rs (1)
flexidag/src/blockdag.rs (3)
genesis(561-563)create_for_testing(124-131)create_for_testing_with_parameters(134-138)
flexidag/tests/tests.rs (1)
flexidag/src/blockdag.rs (3)
create_for_testing(124-131)genesis(561-563)create_for_testing_with_parameters(134-138)
chain/mock/src/mock_chain.rs (1)
flexidag/src/blockdag.rs (2)
genesis(561-563)create_for_testing_with_parameters(134-138)
🔇 Additional comments (21)
test-helper/src/chain.rs (1)
56-59: LGTM!The genesis hash is correctly computed from
genesis.block().id()and passed toBlockDAG::create_for_testing. This aligns with the PR's goal of threading genesis through DAG construction paths.chain/mock/src/mock_chain.rs (1)
92-93: LGTM!The genesis hash is correctly derived from the genesis block and passed to
BlockDAG::create_for_testing_with_parameters(k, genesis_hash), consistent with the updated constructor signature inflexidag/src/blockdag.rs.cmd/generator/src/lib.rs (1)
53-69: LGTM!The initialization logic correctly handles both scenarios:
- When storage already has genesis: uses the stored genesis hash directly.
- Fresh initialization: uses
HashValue::zero()as placeholder, then updates DAG with actual genesis hash afterGenesis::init_and_check_storagecompletes.This pattern aligns with the equivalent logic in
node/src/node.rs.sync/src/tasks/test_tools.rs (1)
58-65: LGTM!The genesis hash is correctly computed from the genesis block and passed to
BlockDAG::create_blockdag. This matches the updated function signature shown inflexidag/src/blockdag.rs.chain/tests/block_test_utils.rs (1)
42-44: LGTM!The genesis hash is correctly extracted and passed to
BlockDAG::create_for_testing, aligning with the updated constructor signature that now requires a genesis hash parameter.node/src/node.rs (1)
303-322: LGTM!The node initialization correctly:
- Retrieves genesis hash from storage (or uses zero as sentinel for fresh init).
- Creates BlockDAG with the genesis hash parameter.
- After
Genesis::init_and_check_storage, updates the DAG with the actual genesis if it was a fresh initialization.- Registers the DAG in the registry only after genesis is properly set.
This ensures the DAG always has a valid genesis reference before being shared across services.
simnet/src/scene/mod.rs (1)
43-53: LGTM!The refactor correctly:
- Computes
genesis_idearly from the genesis block header.- Passes it to
BlockDAG::newas the required genesis parameter.- Uses the same
genesis_idto initializepruning_point.This is cleaner than computing the genesis ID after DAG initialization and ensures consistency between the DAG's genesis and the adapter's pruning point.
genesis/src/lib.rs (4)
455-456: LGTM! Genesis hash correctly derived and passed to BlockDAG constructor.The changes correctly compute the genesis hash from the genesis block ID and pass it to the updated BlockDAG constructor signature.
470-471: LGTM! Consistent genesis hash handling.The pattern of deriving genesis_hash and passing it to the parameterized constructor is correctly applied.
488-489: LGTM! Test helper updated correctly.The cache storage test helper consistently applies the genesis hash initialization pattern.
564-571: LGTM! Test correctly uses shared genesis hash.The test properly computes genesis_hash once and reuses it for both DAG instances, ensuring they're initialized with the same genesis for valid comparison testing.
flexidag/tests/test_commit_atomicity.rs (2)
19-19: LGTM! Test updated for new BlockDAG constructor.The test correctly passes genesis.id() to the BlockDAG::new constructor, aligning with the new signature.
77-77: LGTM! Consistent test update.The second test function correctly applies the same pattern of passing genesis.id() to BlockDAG::new.
flexidag/tests/tests.rs (3)
41-41: LGTM! Tests consistently updated for genesis-aware DAG construction.Multiple test functions correctly updated to pass genesis.id() to BlockDAG::create_for_testing(), maintaining test semantics while adapting to the new API.
Also applies to: 97-97, 150-150, 207-207
351-351: LGTM! Test uses random hash for minimal DAG state testing.This test uses
Hash::random()instead of an actual genesis block ID, which is acceptable since test_dag_tips_store focuses on DAG state storage mechanics rather than genesis semantics.
376-376: LGTM! Comprehensive test suite updated correctly.All remaining tests consistently apply the genesis hash parameter to BlockDAG constructors, maintaining test functionality while adapting to the new API.
Also applies to: 407-407, 464-464, 590-590, 657-657, 713-713, 890-890, 927-927, 981-981, 1144-1144, 1568-1568, 1629-1629
flexidag/src/blockdag.rs (5)
68-68: LGTM! Genesis field added to BlockDAG struct.The genesis field is appropriately typed and placed, maintaining proper encapsulation as a private field.
72-74: LGTM! Constructor signatures correctly updated.The BlockDAG constructors properly accept and store the genesis hash. The assertion at lines 84-90 ensures k >= max_parents_count, preventing protocol violations.
Also applies to: 77-83, 119-119
124-130: LGTM! Test factory methods updated consistently.All test factory methods correctly accept genesis as a parameter and pass it through to the main constructor.
Also applies to: 134-137, 141-149
561-567: Theset_genesis()method is actively used in the codebase to initialize the genesis hash from chain information during setup (e.g., incmd/generator/src/lib.rsandnode/src/node.rs). The method is necessary and not dead code.
549-559: This aliasing behavior is intentional and safe—no action required, but documentation would be helpful.The
Hash::zero()toself.genesismapping is a defensive design pattern used consistently throughout the codebase. Hash::zero() serves as a sentinel value for "not set," and the blockdag layer transparently converts it to genesis for robustness. This prevents callers from needing to handle the conversion themselves. Other parts of the codebase (e.g.,check_ancestor_of, pruning point logic) use the same pattern. Consider adding a brief doc comment explaining this behavior for future maintainers.
57f2548 to
eafc968
Compare
1 participant
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
Tests
✏️ Tip: You can customize this high-level summary in your review settings.