Generators set#2055
Conversation
Generators balances legacy state hash calculation moved to the entity also. Test updated.
…rs set functions. State API updated.
…s storage. Type of endorser index changed to uint32 for convenience. Type EndorseBlock renamed to BlockEndorsement. Unused or used in tests only functions of commitments removed. Mocks regenerated, tests updated.
…of endorsement. Refactored some BLS functions to hide Circl BLS signature.
Removed duplicated code of function LastFinalizedHeight and related. Fixed height issue of generators set initialization. All block finalization checks moved to processBlockFinalization function of finalizer structure. Function calculateLastFinalazedHeight moved to finality structure. Function Validate added to FinalizationVoting structure, it performs basic checks of the structure without accessing the state. Integration test IsolatedFinalitySuite reimplemented to support miner's commitment to generation.
… balance of generators. Checks on sizes of conflicting and normal endorsements added. Duplicated endorsement cryptographic message serialization functions removed. Check on block generator absence in endorsements added. Addition of block generator balance to total endorsements balance added. Interface function CalculateVotingFinalization removed as unused.
There was a problem hiding this comment.
Pull request overview
This PR refactors deterministic finality around a new “generators set” abstraction, updates endorsement/finalization types, and rewires state/node APIs to consume the new generator-set based logic.
Changes:
- Introduces
state.generators+finalizerand integrates generator-set hashing/banning into state processing. - Renames/reshapes endorsement types (
EndorseBlock→BlockEndorsement, endorser indexes touint32) across node/proto/miner/state layers. - Refactors finalization persistence (
finalizations→finality) and updates validation + APIs accordingly.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/wallet/embedded_wallet.go | Changes BLS keypair retrieval to return only secret keys. |
| pkg/types/types.go | Updates EmbeddedWallet and EndorsementPool interfaces for new signatures/types. |
| pkg/state/transaction_performer_test.go | Updates test to use newestGenerators() API. |
| pkg/state/threadsafe_wrapper.go | Replaces several finality/commitment read methods with generator-set queries. |
| pkg/state/state.go | Integrates finality + generators entities; switches miner GB retrieval to generator set; updates state hash inputs. |
| pkg/state/state_test.go | Updates tests to use finality storage. |
| pkg/state/state_hasher.go | Removes unused hasher pop() helper. |
| pkg/state/state_hasher_test.go | Updates state-hash test to use generator-set hashing. |
| pkg/state/mock_state.go | Regenerates mocks for updated State interface (generators/finality). |
| pkg/state/keys.go | Adds key prefix for banned generators tracking. |
| pkg/state/history_storage.go | Adds new blockchain entity type for bannedGenerators. |
| pkg/state/generators.go | New generator-set implementation (balances, bans, lookups, hashing). |
| pkg/state/generators_test.go | Adds placeholder generators test file. |
| pkg/state/finalizer.go | New finalization processor based on generator set + finality store. |
| pkg/state/finalization.go | Renames storage to finality, adds endorsement message builder, consolidates last-finalized-height logic. |
| pkg/state/commitments.go | Adds newestCommitments() and trims legacy commitment helper APIs. |
| pkg/state/commitments_internal_test.go | Updates commitment tests to new APIs and removes removed-API coverage. |
| pkg/state/appender.go | Replaces legacy finalizationProcessor with finalizer usage; adjusts checkerInfo flow and error text. |
| pkg/state/api.go | Updates StateInfo interface to generator-set based APIs. |
| pkg/proto/protobuf_converters.go | Converts endorsements to BlockEndorsement and normalizes index types. |
| pkg/proto/microblock_test.go | Updates finalization voting test data for new types. |
| pkg/proto/finalization.go | Introduces EndorsementCryptoMessage, BlockEndorsement, changes endorser index types, adds validation. |
| pkg/proto/finalization_test.go | Updates tests for new endorsement/voting types and adds validation coverage. |
| pkg/proto/endorse_test.go | Updates endorsement message test to new crypto-message builder. |
| pkg/node/fsm/ng_state.go | Reworks endorsement formation/verification around generator-set lookups and new proto types. |
| pkg/node/fsm/fsm.go | Renames FSM trigger constant and updates endorsement event payload type. |
| pkg/node/fsm/fsm_common.go | Updates FSM event argument type checks for endorsement events. |
| pkg/node/fsm/action.go | Updates action interface + implementation to send BlockEndorsement. |
| pkg/miner/endorsementpool/endorsementpool_test.go | Updates pool tests for BlockEndorsement and uint32 indexes. |
| pkg/miner/endorsementpool/endorsement_pool.go | Updates pool storage/types and endorsement crypto-message usage; aggregates signatures via new API. |
| pkg/crypto/bls/bls.go | Changes AggregateSignatures/VerifyAggregate to use Signature value type end-to-end. |
| pkg/crypto/bls/bls_test.go | Updates aggregated signature compatibility test to new signature parsing type. |
| pkg/consensus/blocks_validation.go | Adds finalization-voting header validation gated by feature activation + de-duplication. |
| pkg/api/node_api.go | Updates generators endpoint to use CommittedGenerators(height) returning GeneratorInfo. |
| pkg/api/node_api_test.go | Updates wallet mock for new KeyPairsBLS() signature. |
| itests/finality_internal_test.go | Updates finality/deposit tests; adds miner commitments and fixes fee typing. |
| itests/config/config.go | Adds GetAccount(address) helper for integration tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Function to clean generators before initialization added. Index used to store the reference to block generator.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (a *NodeApi) GeneratorsAt(w http.ResponseWriter, r *http.Request) error { | ||
| heightStr := chi.URLParam(r, "height") | ||
| height, err := strconv.ParseUint(heightStr, 10, 64) | ||
| if err != nil { | ||
| return errors.Wrap(err, "invalid height") | ||
| } | ||
| isActivated, actErr := a.state.IsActivated(int16(settings.DeterministicFinality)) | ||
| if actErr != nil { | ||
| return errors.Wrap(actErr, "failed to check DeterministicFinality activation") | ||
| } | ||
| if !isActivated { | ||
| return apiErrs.NewUnavailableError(errors.New("deterministic finality is not activated")) | ||
| } | ||
| activationHeight, err := a.state.ActivationHeight(int16(settings.DeterministicFinality)) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to get DeterministicFinality activation height") | ||
| } | ||
|
|
||
| periodStart, err := state.CurrentGenerationPeriodStart(activationHeight, height, | ||
| a.app.settings.GenerationPeriod) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "failed to calculate generationPeriodStart") | ||
| } | ||
| generatorAddresses, err := a.state.CommittedGenerators(periodStart) | ||
| // TODO: This would work only for height still present in history, once the height fell out of history, no | ||
| // result will be returned. | ||
| // Consider to create a separate storage for historical data that will persist data for any block. | ||
| // Consider moving this API under the `-build-extended-api` and `-serve-extended-api` keys. | ||
| gs, err := a.state.CommittedGenerators(height) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
GeneratorsAt now relies on State.CommittedGenerators(height), but the current implementation only supports the current height and returns a generic "not implemented" error for other heights. Since this endpoint accepts an arbitrary height path param, it should either (1) explicitly reject non-current heights with a clear 4xx error, or (2) implement historical generator retrieval (likely from history/commitments) to preserve the endpoint’s semantics.
| // NewestMinerGeneratingBalance returns the generating balance of the miner at the given height. | ||
| // This method includes the challenger bonus if the block has a challenged header. | ||
| // After activation of Deterministic Finality feature, the method also checks presence of the block generator in | ||
| // the generators set and returns 0 together with an error if the block generator is not in the generators set. | ||
| func (s *stateManager) NewestMinerGeneratingBalance(header *proto.BlockHeader, height proto.Height) (uint64, error) { | ||
| minerAddr, err := proto.NewAddressFromPublicKey(s.settings.AddressSchemeCharacter, header.GeneratorPublicKey) | ||
| if err != nil { | ||
| return 0, wrapErr(stateerr.RetrievalError, errors.Wrapf(err, "failed create get miner address from PK %s", | ||
| header.GeneratorPublicKey, | ||
| )) | ||
| } | ||
| minerGB, err := s.stor.balances.newestGeneratingBalance(minerAddr.ID(), height) | ||
| // Get miner's balance from the generators set. | ||
| minerGB, err := s.stor.generators.newestGeneratingBalance(minerAddr.ID(), height) | ||
| if err != nil { |
There was a problem hiding this comment.
The doc comment says this method returns an error when the block generator is not in the generators set after Deterministic Finality activation, but stor.generators.newestGeneratingBalance() falls back to balances.newestGeneratingBalance() when the generators set is empty. Either tighten the logic to always enforce membership after activation, or adjust the comment to match the actual behavior.
| generators, err := a.baseInfo.storage.CommittedGenerators(height) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, fmt.Errorf("failed to get committed generators at height %d: %w", height, err) | ||
| } | ||
| if len(generators) == 0 { | ||
| slog.Debug("No committed generators found, skipping finalization voting") | ||
| } | ||
|
|
||
| commitedGenerators, err := a.baseInfo.storage.CommittedGenerators(periodStart) | ||
| finalization, err := a.baseInfo.endorsements.FormFinalization() | ||
| if err != nil { |
There was a problem hiding this comment.
This block logs "skipping finalization voting" when len(generators) == 0, but then continues and forms/sends a finalization voting anyway. Either return early here (e.g., errNoFinalization) or adjust the log/message and remove the unused CommittedGenerators() call if it’s only informational.
| return a, nil, a.Errorf(errors.Wrapf(err, "failed to get last finalized block header for endorser address")) | ||
| } | ||
| // TODO check if generator is in the generator set. | ||
| endorserPK := gi.BLSPublicKey() |
There was a problem hiding this comment.
What status about this TODO?
There was a problem hiding this comment.
This should be handled in separate PR.
| // TODO: remove height parameter from the following function. Micro-block mining is operates only current | ||
| // generator set. | ||
| blockFinalization, err = a.getCurrentFinalizationVoting(height) | ||
| if err != nil && !errors.Is(err, errNoFinalization) && !errors.Is(err, errNoEndorsements) { |
There was a problem hiding this comment.
No errNoFinalization returns has been found. The error can be safely deleted, I guess?
There was a problem hiding this comment.
The same here, I'll do it in a separate PR
Correct parameters of history storage for banned generators added.
| finalizedHeight, err := s.stor.finality.lastFinalizedHeight(blockHeight) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to build local endorsement message: %w", err) | ||
| } | ||
| slog.Debug("Finalization", slog.Uint64("finalizedHeight", finalizedHeight), | ||
| slog.Uint64("blockHeight", blockHeight), slog.String("blockID", blockID.String())) |
There was a problem hiding this comment.
finalizedHeight is used only for logging. Maybe check log level before calling lastFinalizedHeight?
Though it's not critical, can be left unchanged.
fea0c10
into
determenistic-finality-feature
No description provided.