Added delayed finalization

[esuwu]

No description provided.

[Copilot]

Pull request overview

This PR introduces "delayed finalization" — a 2-block visibility delay on finalized heights. A newly finalized height (PendingBlockHeight) is stored in the finalization record but only becomes externally visible when currentHeight >= PendingBlockHeight + 2. Internal finalization processing continues to use the pending height immediately. The PR also removes the redundant lastFinalizedHeight parameter from FormFinalization(), instead taking the height directly from endorsements in the pool.

Changes:

• Extends finalizationRecord with a PendingBlockHeight field, adds newestVisible() for delayed external visibility and newestForProcessing() for immediate internal use, and updates store() to handle the pending/visible promotion logic
• Updates FormFinalization() in EndorsementPool to read FinalizedBlockHeight directly from the endorsements (removing the lastFinalizedHeight parameter), and removes the lastFinalizedHeight fetch from mineMicro
• Adds a new state test verifying the N+2 visibility behavior, and updates existing tests to pass the required currentHeight argument to store()

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/state/finalization.go	Core change: adds PendingBlockHeight to finalizationRecord, implements delayed visibility via newestVisibleHeight/newestVisible, and updates store() with promotion logic
pkg/state/state.go	Switches LastFinalizedHeight() to use newestVisible() and updates softRollback to pass height to the new store() signature
pkg/state/appender.go	Updates loadLastFinalizedHeight to use newestForProcessing(), finalizeParent to pass height to store(), and expands the finalization-failure debug log
pkg/state/keys.go	Removes the now-unused finalizationKey struct in favor of directly using []byte{finalizationKeyPrefix}
pkg/state/state_test.go	Updates existing tests for the new store() signature and adds TestLastFinalizedHeight_ExposesPreFinalizationOnlyAtNPlus2
pkg/miner/endorsementpool/endorsement_pool.go	Changes FormFinalization() to drop the lastFinalizedHeight parameter, reading height from the endorsements directly
pkg/node/fsm/ng_state.go	Removes lastFinalizedHeight fetch from mineMicro and simplifies getCurrentFinalizationVoting signature
pkg/types/types.go	Updates EndorsementPool interface to reflect the new FormFinalization() signature

Comments suppressed due to low confidence (1)

pkg/miner/endorsementpool/endorsement_pool.go:233

• The FormFinalization function has been significantly changed: instead of accepting lastFinalizedHeight as a parameter, it now reads FinalizedBlockHeight from the first endorsement in the pool (p.h[0].eb.FinalizedBlockHeight). There are no unit tests for FormFinalization in endorsementpool_test.go to validate that the returned FinalizationVoting.FinalizedBlockHeight matches the expected endorsement's height. Given that other pool behaviors are well-tested in this file, a test covering the FormFinalization behavior (including the correct height extraction) would be appropriate.

func (p *EndorsementPool) FormFinalization() (proto.FinalizationVoting, error) {
	p.mu.Lock()
	defer p.mu.Unlock()

	if len(p.h) == 0 {
		return proto.FinalizationVoting{}, errors.New("failed to form finalization: pool is empty")
	}

	signatures := make([]bls.Signature, 0, len(p.h))
	endorsersIndexes := make([]int32, 0, len(p.h))
	var aggregatedSignature bls.Signature

	for _, it := range p.h {
		signatures = append(signatures, it.eb.Signature)
		endorsersIndexes = append(endorsersIndexes, it.eb.EndorserIndex)
	}
	if len(signatures) != 0 {
		aggregatedSignatureBytes, err := bls.AggregateSignatures(signatures)
		if err != nil {
			return proto.FinalizationVoting{}, err
		}
		var errCnvrt error
		aggregatedSignature, errCnvrt = bls.NewSignatureFromBytes(aggregatedSignatureBytes)
		if errCnvrt != nil {
			return proto.FinalizationVoting{}, errCnvrt
		}
	}

	return proto.FinalizationVoting{
		AggregatedEndorsementSignature: aggregatedSignature,
		FinalizedBlockHeight:           proto.Height(p.h[0].eb.FinalizedBlockHeight),
		EndorserIndexes:                endorsersIndexes,
		ConflictEndorsements:           p.conflicts,
	}, nil
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The second ApplyFromFile call imports importHeight (15) blocks starting at startHeight=importHeight (15), advancing the chain from height 15 to approximately height 30. However, only a single additional block (reaching height 16) is necessary for the condition currentHeight >= pendingHeight+2 = 14+2 = 16 to be satisfied and the pending finalization to become visible. The comment "After one more block (N+2)" is misleading since 15 blocks are imported rather than one. Consider using startHeight=importHeight and nBlocks=1 instead to precisely test the N+2 threshold.

Suggested change

	manager, importHeight, importHeight,
	manager, importHeight, 1,

[Copilot]

In softRollback, newest() (now an alias for newestForProcessing()) returns the maximum of FinalizedBlockHeight and PendingBlockHeight. When that value is the PendingBlockHeight, the subsequent call to store(finalizationHeight, height, blockID) can lose the original FinalizedBlockHeight.

Concrete scenario: if the stored record is {FinalizedBlockHeight: 5, PendingBlockHeight: 10} and we roll back to height 8:

1. newest() returns 10 (pending)
2. store(10, 8, blockID) is called; since 8 < 12 (= 10+2), FinalizedBlockHeight is not promoted and stays 0
3. Only PendingBlockHeight=10 is persisted; the original FinalizedBlockHeight=5 is lost
4. After rollback, LastFinalizedHeight() at height 8 would return CalculateLastFinalizedHeight(8) = 1 instead of the correct 5

The fix is to have softRollback read the full finalizationRecord (via newestRecord()) before the rollback and write it back directly via writeRecord() after the rollback, rather than going through the store() path which performs logic that can drop FinalizedBlockHeight.