Skip to content

[blockchain] proposal pool#4582

Closed
envestcc wants to merge 1 commit intoiotexproject:masterfrom
envestcc:proposalpool
Closed

[blockchain] proposal pool#4582
envestcc wants to merge 1 commit intoiotexproject:masterfrom
envestcc:proposalpool

Conversation

@envestcc
Copy link
Member

Description

The blockchain introduces a proposal pool to store blocks that are being processed but have not yet been confirmed.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • make test
  • [] fullsync
  • [] Other test (please specify)

Test Configuration:

  • Firmware version:
  • Hardware:
  • Toolchain:
  • SDK:

Checklist:

  • [] My code follows the style guidelines of this project
  • [] I have performed a self-review of my code
  • [] I have commented my code, particularly in hard-to-understand areas
  • [] I have made corresponding changes to the documentation
  • [] My changes generate no new warnings
  • [] I have added tests that prove my fix is effective or that my feature works
  • [] New and existing unit tests pass locally with my changes
  • [] Any dependent changes have been merged and published in downstream modules

@envestcc envestcc requested review from a team, CoderZhi, Liuhaai and dustinxie as code owners March 14, 2025 14:39
@envestcc envestcc requested a review from Copilot March 14, 2025 14:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a proposal pool for handling not-yet-confirmed blocks and integrates it with the blockchain, along with a block preparer to asynchronously mint new blocks. Key changes include:

  • Adding new modules for block preparation (blockpreparer.go and blockpreparer_test.go).
  • Implementing a proposal pool to manage draft blocks and forks (proposalpool.go and proposalpool_test.go).
  • Enhancing the blockchain integration to support asynchronous block preparation and a pending block mechanism.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
blockchain/blockpreparer_test.go Adds tests for asynchronous block minting.
blockchain/blockpreparer.go Implements async block minting using a channel and mutex.
blockchain/proposalpool_test.go Covers proposal pool behaviors for adding, retrieving, and fork management.
blockchain/proposalpool.go Implements logic for managing draft blocks and forks.
blockchain/blockchain.go Integrates block preparer and proposal pool into blockchain operations and block production.
test/mock/mock_blockchain/mock_blockchain.go Updates mock definitions to include new methods.
Comments suppressed due to low confidence (2)

blockchain/blockchain.go:519

  • Comparing the prepared block's timestamp for equality with the provided timestamp may be brittle due to possible precision or slight delay differences. Consider using additional fields (like block height or hash) or a tolerance range when checking if the draft block corresponds to the intended block.
if pblk != nil && pblk.Timestamp() == timestamp {

blockchain/blockchain.go:602

  • In tipInfo, when tipHeight exceeds daoHeight the function attempts to fetch a draft header from the proposal pool and wraps db.ErrNotExist if not found. This logic might hide the root issue; consider clarifying the error handling to better distinguish between a missing confirmed block and an unprepared draft.
if tipHeight > daoHeight {

@CoderZhi
Copy link
Collaborator

if we change the implementation of blockchain.Blockchain directly, we may need to review all the places using blockchain.Blockchain.
The base idea is to store proposal for the usage of new proposal. Similar to WorkingSet as StateManager, we may need to construct a new blockchain.Blockchain.

@envestcc envestcc force-pushed the proposalpool branch 6 times, most recently from f64c120 to fa2f44b Compare March 19, 2025 01:42
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
3.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

consensusCfg := consensusfsm.NewConsensusConfig(builder.cfg.Consensus.RollDPoS.FSM, builder.cfg.DardanellesUpgrade, builder.cfg.Genesis, builder.cfg.Consensus.RollDPoS.Delay)
dao := builder.cs.BlockDAO()
builder.cs.blockTimeCalculator, err = blockutil.NewBlockTimeCalculator(consensusCfg.BlockInterval, builder.cs.Blockchain().TipHeight, func(height uint64) (time.Time, error) {
builder.cs.blockTimeCalculator, err = blockutil.NewBlockTimeCalculator(consensusCfg.BlockInterval, func() uint64 {
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix the bug: the tipHeight() will be called during check indexers at startup, but at that time the blockchain has not yet been fully initialized

@envestcc envestcc mentioned this pull request Mar 19, 2025
@envestcc envestcc closed this Mar 28, 2025
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.

3 participants