Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dynamic Protocol State] InstanceParams refactoring #5113

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8bf27ca
Extended InstanceParams interface to return more data
durkmurder Dec 4, 2023
775ac28
Extracted cached values from State
durkmurder Dec 4, 2023
965fb69
Implemented caching of static values in InstanceParams
durkmurder Dec 4, 2023
c9eb3e8
Removed errors in InstanceParams
durkmurder Dec 4, 2023
4021eca
WIP refactoring interface and implementation
durkmurder Dec 4, 2023
f955f8a
Refactored state bootstraping so it's more modular and doesn't depend…
durkmurder Dec 5, 2023
23113e0
Fixed compilation errors due to InstanceParams interface changes
durkmurder Dec 5, 2023
5ecddb5
Updated godoc
durkmurder Dec 6, 2023
73ae8ee
Updated godoc
durkmurder Dec 6, 2023
b831282
Apply suggestions from code review
durkmurder Dec 8, 2023
4099164
Merge branch 'feature/dynamic-protocol-state' of https://github.com/o…
durkmurder Dec 8, 2023
0f06b68
extended and polished documentation of `EpochCommitSafetyThreshold()`…
Dec 9, 2023
2acd236
removed `transaction.WithTx` and consolidated APIs of service methods…
Dec 9, 2023
4bc3a93
Update state/protocol/badger/state.go
durkmurder Dec 11, 2023
3c2d158
Consolidated usage of transaction.WithTx in bootstrapping logic
durkmurder Dec 11, 2023
14b4689
Added TODO for leaving EFM
durkmurder Dec 11, 2023
8098495
Merge branch 'feature/dynamic-protocol-state' of https://github.com/o…
durkmurder Dec 11, 2023
7db2243
Added logic which was deleted during auto-merge
durkmurder Dec 11, 2023
28793bf
removed some white spaces
Dec 12, 2023
9636507
Merge branch 'yurii/instance-params-refactoring' into alex/PR-5113_-_…
Dec 12, 2023
12c8fd2
polished and consolidated revisions after merge
Dec 12, 2023
4a1d387
renamed protocol state's internal `cachedFinal` and `cachedSealed` to…
Dec 12, 2023
0366719
minor updates and polishing of goDoc
Dec 12, 2023
a4e5fba
removed outdated error checks -- functions now don't return any error…
Dec 12, 2023
412c815
Merge pull request #5132 from onflow/alex/PR-5113_-_suggestions
durkmurder Dec 12, 2023
e7b2f94
Apply suggestions from code review
durkmurder Dec 12, 2023
6cf2e9f
Linted
durkmurder Dec 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/scaffold.go
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,7 @@ func (fnb *FlowNodeBuilder) initState() error {
fnb.State = state

// set root snapshot field
rootBlock, err := state.Params().FinalizedRoot()
rootBlock := state.Params().FinalizedRoot()
if err != nil {
return fmt.Errorf("could not get root block from protocol state: %w", err)
}
durkmurder marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -1252,7 +1252,7 @@ func (fnb *FlowNodeBuilder) initLocal() error {
// We enforce this strictly for MainNet. For other networks (e.g. TestNet or BenchNet), we
// are lenient, to allow ghost node to run as any role.
if self.Role.String() != fnb.BaseConfig.NodeRole {
rootBlockHeader, err := fnb.State.Params().FinalizedRoot()
rootBlockHeader := fnb.State.Params().FinalizedRoot()
if err != nil {
return fmt.Errorf("could not get root block from protocol state: %w", err)
}
durkmurder marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion cmd/util/cmd/read-hotstuff/cmd/get_liveness.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func runGetLivenessData(*cobra.Command, []string) {
log.Fatal().Err(err).Msg("could not init protocol state")
}

rootBlock, err := state.Params().FinalizedRoot()
rootBlock := state.Params().FinalizedRoot()
if err != nil {
log.Fatal().Err(err).Msgf("could not get root block")
durkmurder marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down
5 changes: 1 addition & 4 deletions cmd/util/cmd/read-hotstuff/cmd/get_safety.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ func runGetSafetyData(*cobra.Command, []string) {
log.Fatal().Err(err).Msg("could not init protocol state")
}

rootBlock, err := state.Params().FinalizedRoot()
if err != nil {
log.Fatal().Err(err).Msgf("could not get root block")
}
rootBlock := state.Params().FinalizedRoot()

reader := NewHotstuffReader(db, rootBlock.ChainID)

Expand Down
2 changes: 1 addition & 1 deletion cmd/util/cmd/reindex/cmd/results.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var resultsCmd = &cobra.Command{
results := storages.Results
blocks := storages.Blocks

root, err := state.Params().FinalizedRoot()
root := state.Params().FinalizedRoot()
if err != nil {
log.Fatal().Err(err).Msg("could not get root header from protocol state")
}
durkmurder marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,7 @@ func removeExecutionResultsFromHeight(
fromHeight uint64) error {
log.Info().Msgf("removing results for blocks from height: %v", fromHeight)

root, err := protoState.Params().FinalizedRoot()
if err != nil {
return fmt.Errorf("could not get root: %w", err)
}
root := protoState.Params().FinalizedRoot()

if fromHeight <= root.Height {
return fmt.Errorf("can only remove results for block above root block. fromHeight: %v, rootHeight: %v", fromHeight, root.Height)
Expand Down
15 changes: 3 additions & 12 deletions engine/access/ingestion/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,8 @@ func (e *Engine) Start(parent irrecoverable.SignalerContext) {
// If the index has already been initialized, this is a no-op.
// No errors are expected during normal operation.
func (e *Engine) initLastFullBlockHeightIndex() error {
rootBlock, err := e.state.Params().FinalizedRoot()
if err != nil {
return fmt.Errorf("failed to get root block: %w", err)
}
err = e.blocks.InsertLastFullBlockHeightIfNotExists(rootBlock.Height)
rootBlock := e.state.Params().FinalizedRoot()
err := e.blocks.InsertLastFullBlockHeightIfNotExists(rootBlock.Height)
if err != nil {
return fmt.Errorf("failed to update last full block height during ingestion engine startup: %w", err)
}
Expand Down Expand Up @@ -691,13 +688,7 @@ func (e *Engine) updateLastFullBlockReceivedIndex() {
logError(err)
return
}
// use the root height as the last full height
header, err := e.state.Params().FinalizedRoot()
if err != nil {
logError(err)
return
}
lastFullHeight = header.Height
lastFullHeight = e.state.Params().FinalizedRoot().Height
durkmurder marked this conversation as resolved.
Show resolved Hide resolved
}

e.log.Debug().Uint64("last_full_block_height", lastFullHeight).Msg("updating LastFullBlockReceived index...")
Expand Down
24 changes: 9 additions & 15 deletions engine/access/rpc/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,7 @@ func New(params Params) (*Backend, error) {
}

// initialize node version info
nodeInfo, err := getNodeVersionInfo(params.State.Params())
if err != nil {
return nil, fmt.Errorf("failed to initialize node version info: %w", err)
}
nodeInfo := getNodeVersionInfo(params.State.Params())

b := &Backend{
state: params.State,
Expand Down Expand Up @@ -305,15 +302,12 @@ func (b *Backend) GetNodeVersionInfo(_ context.Context) (*access.NodeVersionInfo

// getNodeVersionInfo returns the NodeVersionInfo for the node.
// Since these values are static while the node is running, it is safe to cache.
func getNodeVersionInfo(stateParams protocol.Params) (*access.NodeVersionInfo, error) {
func getNodeVersionInfo(stateParams protocol.Params) *access.NodeVersionInfo {
sporkID := stateParams.SporkID()
protocolVersion := stateParams.ProtocolVersion()
sporkRootBlockHeight := stateParams.SporkRootBlockHeight()

nodeRootBlockHeader, err := stateParams.SealedRoot()
if err != nil {
return nil, fmt.Errorf("failed to read node root block: %w", err)
}
nodeRootBlockHeader := stateParams.SealedRoot()

nodeInfo := &access.NodeVersionInfo{
Semver: build.Version(),
Expand All @@ -324,7 +318,7 @@ func getNodeVersionInfo(stateParams protocol.Params) (*access.NodeVersionInfo, e
NodeRootBlockHeight: nodeRootBlockHeader.Height,
}

return nodeInfo, nil
return nodeInfo
}

func (b *Backend) GetCollectionByID(_ context.Context, colID flow.Identifier) (*flow.LightCollection, error) {
Expand Down Expand Up @@ -371,14 +365,14 @@ func executionNodesForBlockID(
log zerolog.Logger,
) (flow.IdentitySkeletonList, error) {

var executorIDs flow.IdentifierList
var (
executorIDs flow.IdentifierList
err error
)

// check if the block ID is of the root block. If it is then don't look for execution receipts since they
// will not be present for the root block.
rootBlock, err := state.Params().FinalizedRoot()
if err != nil {
return nil, fmt.Errorf("failed to retreive execution IDs for block ID %v: %w", blockID, err)
}
rootBlock := state.Params().FinalizedRoot()

if rootBlock.ID() == blockID {
executorIdentities, err := state.Final().Identities(filter.HasRole[flow.Identity](flow.RoleExecution))
Expand Down
18 changes: 0 additions & 18 deletions engine/access/rpc/backend/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1662,24 +1662,6 @@ func (suite *Suite) TestGetNodeVersionInfo() {

suite.Require().Equal(expected, actual)
})

suite.Run("backend construct fails when SealedRoot lookup fails", func() {
stateParams := protocol.NewParams(suite.T())
stateParams.On("SporkID").Return(sporkID, nil)
stateParams.On("ProtocolVersion").Return(protocolVersion, nil)
stateParams.On("SporkRootBlockHeight").Return(sporkRootBlock.Height, nil)
stateParams.On("SealedRoot").Return(nil, fmt.Errorf("fail"))

state := protocol.NewState(suite.T())
state.On("Params").Return(stateParams, nil).Maybe()

params := suite.defaultBackendParams()
params.State = state

backend, err := New(params)
suite.Require().Error(err)
suite.Require().Nil(backend)
})
}

func (suite *Suite) TestGetNetworkParameters() {
Expand Down
4 changes: 1 addition & 3 deletions engine/consensus/sealing/core.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// (c) 2021 Dapper Labs - ALL RIGHTS RESERVED

package sealing

import (
Expand Down Expand Up @@ -137,7 +135,7 @@ func (c *Core) RepopulateAssignmentCollectorTree(payloads storage.Payloads) erro

// Get the root block of our local state - we allow references to unknown
// blocks below the root height
rootHeader, err := c.state.Params().FinalizedRoot()
rootHeader := c.state.Params().FinalizedRoot()
if err != nil {
return fmt.Errorf("could not retrieve root header: %w", err)
}
Expand Down
7 changes: 2 additions & 5 deletions engine/consensus/sealing/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,7 @@ func NewEngine(log zerolog.Logger,
sealsMempool mempool.IncorporatedResultSeals,
requiredApprovalsForSealConstructionGetter module.SealingConfigsGetter,
) (*Engine, error) {
rootHeader, err := state.Params().FinalizedRoot()
if err != nil {
return nil, fmt.Errorf("could not retrieve root block: %w", err)
}
rootHeader := state.Params().FinalizedRoot()

unit := engine.NewUnit()
e := &Engine{
Expand All @@ -124,7 +121,7 @@ func NewEngine(log zerolog.Logger,
rootHeader: rootHeader,
}

err = e.setupTrustedInboundQueues()
err := e.setupTrustedInboundQueues()
if err != nil {
return nil, fmt.Errorf("initialization of inbound queues for trusted inputs failed: %w", err)
}
Expand Down
2 changes: 0 additions & 2 deletions engine/consensus/sealing/engine_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// (c) 2021 Dapper Labs - ALL RIGHTS RESERVED

durkmurder marked this conversation as resolved.
Show resolved Hide resolved
package sealing

import (
Expand Down
10 changes: 2 additions & 8 deletions engine/execution/ingestion/loader/unexecuted_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,7 @@ func (e *UnexecutedLoader) LoadUnexecuted(ctx context.Context) ([]flow.Identifie
}

// don't reload root block
rootBlock, err := e.state.Params().SealedRoot()
if err != nil {
return nil, fmt.Errorf("failed to retrieve root block: %w", err)
}
rootBlock := e.state.Params().SealedRoot()

blockIDs := make([]flow.Identifier, 0)
isRoot := rootBlock.ID() == last.ID()
Expand Down Expand Up @@ -151,10 +148,7 @@ func (e *UnexecutedLoader) finalizedUnexecutedBlocks(ctx context.Context, finali

// dynamically bootstrapped execution node will reload blocks from
// [sealedRoot.Height + 1, finalizedRoot.Height] and execute them on startup.
rootBlock, err := e.state.Params().SealedRoot()
if err != nil {
return nil, fmt.Errorf("failed to retrieve root block: %w", err)
}
rootBlock := e.state.Params().SealedRoot()

for ; lastExecuted > rootBlock.Height; lastExecuted-- {
header, err := e.getHeaderByHeight(lastExecuted)
Expand Down
3 changes: 1 addition & 2 deletions engine/testutil/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -824,8 +824,7 @@ func ExecutionNode(t *testing.T, hub *stub.Hub, identity bootstrap.NodeInfo, ide
}

func getRoot(t *testing.T, node *testmock.GenericNode) (*flow.Header, *flow.QuorumCertificate) {
rootHead, err := node.State.Params().FinalizedRoot()
require.NoError(t, err)
rootHead := node.State.Params().FinalizedRoot()

signers, err := node.State.AtHeight(0).Identities(filter.HasRole[flow.Identity](flow.RoleConsensus))
require.NoError(t, err)
Expand Down
1 change: 0 additions & 1 deletion model/flow/filter/id/identifier.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// (c) 2021 Dapper Labs - ALL RIGHTS RESERVED
package id

import "github.com/onflow/flow-go/model/flow"
Expand Down
4 changes: 1 addition & 3 deletions module/builder/collection/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,7 @@ func (suite *BuilderSuite) Payload(transactions ...*flow.TransactionBody) model.

// ProtoStateRoot returns the root block of the protocol state.
func (suite *BuilderSuite) ProtoStateRoot() *flow.Header {
root, err := suite.protoState.Params().FinalizedRoot()
suite.Require().NoError(err)
return root
return suite.protoState.Params().FinalizedRoot()
}

// ClearPool removes all items from the pool
Expand Down
Loading
Loading