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

Pchain validators repackaging #1284

Merged
merged 35 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
795a10a
repack pchain validators
abi87 Apr 3, 2023
629ffd8
nit
abi87 Apr 3, 2023
61fd676
Merge branch 'dev' into pchain_proposers_repack
StephenButtolph Apr 4, 2023
e725784
nits
abi87 Apr 5, 2023
0c8bc40
nits
abi87 Apr 5, 2023
7bd3093
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 10, 2023
d91c656
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 11, 2023
97bafa4
minor renaming
abi87 Apr 11, 2023
d9b9452
clock cleanup
abi87 Apr 11, 2023
2248f3c
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 13, 2023
bea7c15
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 17, 2023
816dd86
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 20, 2023
da0d822
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 21, 2023
18e9bb8
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 24, 2023
9a978cc
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 26, 2023
db2e956
Merge branch 'dev' into pchain_proposers_repack
abi87 Apr 26, 2023
bcbe002
Merge branch 'dev' into pchain_proposers_repack
abi87 May 1, 2023
d53f5e9
Merge branch 'dev' into pchain_proposers_repack
abi87 May 2, 2023
fa3ee2b
Merge branch 'dev' into pchain_proposers_repack
abi87 May 3, 2023
4f24a50
Merge branch 'dev' into pchain_proposers_repack
abi87 May 4, 2023
ee25500
nits
abi87 May 4, 2023
42b1dd5
cleanup
abi87 May 4, 2023
a736dfc
fixed cleanup
abi87 May 4, 2023
40e6092
Merge branch 'dev' into pchain_proposers_repack
abi87 May 10, 2023
cb1eafb
nits
abi87 May 11, 2023
1e3d889
Merge branch 'dev' into pchain_proposers_repack
abi87 May 16, 2023
c2ccff7
Merge branch 'dev' into pchain_proposers_repack
abi87 May 17, 2023
4855ec9
Merge branch 'dev' into pchain_proposers_repack
abi87 May 18, 2023
330e83a
fix merge
StephenButtolph May 18, 2023
79262a5
nit rename
StephenButtolph May 18, 2023
d55469b
nit rename
StephenButtolph May 18, 2023
cfbd56f
remove duplicate comments
StephenButtolph May 18, 2023
dcc6aa7
minimize interface size
StephenButtolph May 18, 2023
bb3be27
license
StephenButtolph May 18, 2023
12590b2
Merge branch 'dev' into pchain_proposers_repack
StephenButtolph May 18, 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
Prev Previous commit
Next Next commit
minor renaming
  • Loading branch information
abi87 committed Apr 11, 2023
commit 97bafa46f6df52b53b3f66e9eafe487e8604fa03
2 changes: 1 addition & 1 deletion vms/platformvm/blocks/builder/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func newEnvironment(t *testing.T) *environment {
metrics,
res.state,
&res.backend,
pvalidators.TestSet,
pvalidators.TestManager,
)

res.Builder = New(
Expand Down
2 changes: 1 addition & 1 deletion vms/platformvm/blocks/executor/acceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var _ blocks.Visitor = (*acceptor)(nil)
type acceptor struct {
*backend
metrics metrics.Metrics
validators validators.Set
validators validators.Manager
bootstrapped *utils.Atomic[bool]
}

Expand Down
10 changes: 5 additions & 5 deletions vms/platformvm/blocks/executor/acceptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestAcceptorVisitProposalBlock(t *testing.T) {
state: s,
},
metrics: metrics.Noop,
validators: validators.TestSet,
validators: validators.TestManager,
}

err = acceptor.ApricotProposalBlock(blk)
Expand Down Expand Up @@ -98,7 +98,7 @@ func TestAcceptorVisitAtomicBlock(t *testing.T) {
},
},
metrics: metrics.Noop,
validators: validators.TestSet,
validators: validators.TestManager,
}

blk, err := blocks.NewApricotAtomicBlock(
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestAcceptorVisitStandardBlock(t *testing.T) {
},
},
metrics: metrics.Noop,
validators: validators.TestSet,
validators: validators.TestManager,
}

blk, err := blocks.NewBanffStandardBlock(
Expand Down Expand Up @@ -270,7 +270,7 @@ func TestAcceptorVisitCommitBlock(t *testing.T) {
},
},
metrics: metrics.Noop,
validators: validators.TestSet,
validators: validators.TestManager,
bootstrapped: &utils.Atomic[bool]{},
}

Expand Down Expand Up @@ -356,7 +356,7 @@ func TestAcceptorVisitAbortBlock(t *testing.T) {
},
},
metrics: metrics.Noop,
validators: validators.TestSet,
validators: validators.TestManager,
bootstrapped: &utils.Atomic[bool]{},
}

Expand Down
4 changes: 2 additions & 2 deletions vms/platformvm/blocks/executor/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func newEnvironment(t *testing.T, ctrl *gomock.Controller) *environment {
metrics,
res.state,
res.backend,
pvalidators.TestSet,
pvalidators.TestManager,
)
addSubnet(res)
} else {
Expand All @@ -213,7 +213,7 @@ func newEnvironment(t *testing.T, ctrl *gomock.Controller) *environment {
metrics,
res.mockedState,
res.backend,
pvalidators.TestSet,
pvalidators.TestManager,
)
// we do not add any subnet to state, since we can mock
// whatever we need
Expand Down
2 changes: 1 addition & 1 deletion vms/platformvm/blocks/executor/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func NewManager(
metrics metrics.Metrics,
s state.State,
txExecutorBackend *executor.Backend,
validatorsSet validators.Set,
validatorsSet validators.Manager,
) Manager {
backend := &backend{
Mempool: mempool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,34 +30,34 @@ const (
)

var (
_ validators.State = (*set)(nil)
_ validators.State = (*manager)(nil)

ErrMissingValidatorSet = errors.New("missing validator set")
)

// P-chain must be able to provide information about validators active
// at different heights. [QueribleSet] interface encapsulates all the machinery
// at different heights. [QueryManager] interface encapsulates all the machinery
// to achieve this.
type QueribleSet interface {
type QueryManager interface {
validators.State

GetValidatorIDs(subnetID ids.ID) ([]ids.NodeID, bool)
}

// Set interface adds to QueribleSet the ability to blocks IDs
// Manager interface adds to QueribleSet the ability to blocks IDs
// to serve GetMinimumHeight
type Set interface {
QueribleSet
type Manager interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need this interface, we can have an interface with only Track that is implemented by the same struct.
We could also rename Track to OnAccept and name the interface AcceptedBlockTracker or similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think Track sucks and we should find a better name. OnAccept is a reasonble suggestion and I have implemented it.
On the other hand, I don't really like the idea of reducing Manager interface to AcceptedBlockTracker.
This manager is a pretty big class doing a bunch of not-so-related things, which are together only because there are listed under the same validators.State. It seems to me that AcceptedBlockTracker goes in the direction of slicing off one of this things which may be the right thing to do here. Only if we go there, we should probably first slice validators.State and then extract the OnAccept interface.

QueryManager
Track(blkID ids.ID)
}

func NewSet(
func NewManager(
cfg config.Config,
state state.State,
metrics metrics.Metrics,
clk mockable.Clock,
) Set {
return &set{
) Manager {
return &manager{
cfg: cfg,
state: state,
metrics: metrics,
Expand All @@ -73,7 +73,7 @@ func NewSet(
}
}

type set struct {
type manager struct {
cfg config.Config
state state.State
metrics metrics.Metrics
Expand Down Expand Up @@ -105,17 +105,17 @@ type set struct {
// If [UseCurrentHeight] is true, we will always return the last accepted block
// height as the minimum. This is used to trigger the proposervm on recently
// created subnets before [recentlyAcceptedWindowTTL].
func (vs *set) GetMinimumHeight(ctx context.Context) (uint64, error) {
if vs.cfg.UseCurrentHeight {
return vs.GetCurrentHeight(ctx)
func (m *manager) GetMinimumHeight(ctx context.Context) (uint64, error) {
if m.cfg.UseCurrentHeight {
return m.GetCurrentHeight(ctx)
}

oldest, ok := vs.recentlyAccepted.Oldest()
oldest, ok := m.recentlyAccepted.Oldest()
if !ok {
return vs.GetCurrentHeight(ctx)
return m.GetCurrentHeight(ctx)
}

blk, _, err := vs.state.GetStatelessBlock(oldest)
blk, _, err := m.state.GetStatelessBlock(oldest)
if err != nil {
return 0, err
}
Expand All @@ -129,9 +129,9 @@ func (vs *set) GetMinimumHeight(ctx context.Context) (uint64, error) {
}

// GetCurrentHeight returns the height of the last accepted block
func (vs *set) GetCurrentHeight(context.Context) (uint64, error) {
lastAcceptedID := vs.state.GetLastAccepted()
lastAccepted, _, err := vs.state.GetStatelessBlock(lastAcceptedID)
func (m *manager) GetCurrentHeight(context.Context) (uint64, error) {
lastAcceptedID := m.state.GetLastAccepted()
lastAccepted, _, err := m.state.GetStatelessBlock(lastAcceptedID)
if err != nil {
return 0, err
}
Expand All @@ -140,22 +140,22 @@ func (vs *set) GetCurrentHeight(context.Context) (uint64, error) {

// GetValidatorSet returns the validator set at the specified height for the
// provided subnetID.
func (vs *set) GetValidatorSet(ctx context.Context, height uint64, subnetID ids.ID) (map[ids.NodeID]*validators.GetValidatorOutput, error) {
validatorSetsCache, exists := vs.caches[subnetID]
func (m *manager) GetValidatorSet(ctx context.Context, height uint64, subnetID ids.ID) (map[ids.NodeID]*validators.GetValidatorOutput, error) {
validatorSetsCache, exists := m.caches[subnetID]
if !exists {
validatorSetsCache = &cache.LRU[uint64, map[ids.NodeID]*validators.GetValidatorOutput]{Size: validatorSetsCacheSize}
// Only cache tracked subnets
if subnetID == constants.PrimaryNetworkID || vs.cfg.TrackedSubnets.Contains(subnetID) {
vs.caches[subnetID] = validatorSetsCache
if subnetID == constants.PrimaryNetworkID || m.cfg.TrackedSubnets.Contains(subnetID) {
m.caches[subnetID] = validatorSetsCache
}
}

if validatorSet, ok := validatorSetsCache.Get(height); ok {
vs.metrics.IncValidatorSetsCached()
m.metrics.IncValidatorSetsCached()
return validatorSet, nil
}

lastAcceptedHeight, err := vs.GetCurrentHeight(ctx)
lastAcceptedHeight, err := m.GetCurrentHeight(ctx)
if err != nil {
return nil, err
}
Expand All @@ -164,16 +164,16 @@ func (vs *set) GetValidatorSet(ctx context.Context, height uint64, subnetID ids.
}

// get the start time to track metrics
startTime := vs.clk.Time()
startTime := m.clk.Time()

currentSubnetValidators, ok := vs.cfg.Validators.Get(subnetID)
currentSubnetValidators, ok := m.cfg.Validators.Get(subnetID)
if !ok {
currentSubnetValidators = validators.NewSet()
if err := vs.state.ValidatorSet(subnetID, currentSubnetValidators); err != nil {
if err := m.state.ValidatorSet(subnetID, currentSubnetValidators); err != nil {
return nil, err
}
}
currentPrimaryNetworkValidators, ok := vs.cfg.Validators.Get(constants.PrimaryNetworkID)
currentPrimaryNetworkValidators, ok := m.cfg.Validators.Get(constants.PrimaryNetworkID)
if !ok {
// This should never happen
return nil, ErrMissingValidatorSet
Expand All @@ -195,7 +195,7 @@ func (vs *set) GetValidatorSet(ctx context.Context, height uint64, subnetID ids.
}

for i := lastAcceptedHeight; i > height; i-- {
weightDiffs, err := vs.state.GetValidatorWeightDiffs(i, subnetID)
weightDiffs, err := m.state.GetValidatorWeightDiffs(i, subnetID)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -235,7 +235,7 @@ func (vs *set) GetValidatorSet(ctx context.Context, height uint64, subnetID ids.
}
}

pkDiffs, err := vs.state.GetValidatorPublicKeyDiffs(i)
pkDiffs, err := m.state.GetValidatorPublicKeyDiffs(i)
if err != nil {
return nil, err
}
Expand All @@ -255,20 +255,20 @@ func (vs *set) GetValidatorSet(ctx context.Context, height uint64, subnetID ids.
// cache the validator set
validatorSetsCache.Put(height, vdrSet)

endTime := vs.clk.Time()
vs.metrics.IncValidatorSetsCreated()
vs.metrics.AddValidatorSetsDuration(endTime.Sub(startTime))
vs.metrics.AddValidatorSetsHeightDiff(lastAcceptedHeight - height)
endTime := m.clk.Time()
m.metrics.IncValidatorSetsCreated()
m.metrics.AddValidatorSetsDuration(endTime.Sub(startTime))
m.metrics.AddValidatorSetsHeightDiff(lastAcceptedHeight - height)
return vdrSet, nil
}

// GetCurrentHeight returns the height of the last accepted block
func (vs *set) GetSubnetID(_ context.Context, chainID ids.ID) (ids.ID, error) {
func (m *manager) GetSubnetID(_ context.Context, chainID ids.ID) (ids.ID, error) {
if chainID == constants.PlatformChainID {
return constants.PrimaryNetworkID, nil
}

chainTx, _, err := vs.state.GetTx(chainID)
chainTx, _, err := m.state.GetTx(chainID)
if err != nil {
return ids.Empty, fmt.Errorf(
"problem retrieving blockchain %q: %w",
Expand All @@ -283,8 +283,8 @@ func (vs *set) GetSubnetID(_ context.Context, chainID ids.ID) (ids.ID, error) {
return chain.SubnetID, nil
}

func (vs *set) GetValidatorIDs(subnetID ids.ID) ([]ids.NodeID, bool) {
validatorSet, exist := vs.cfg.Validators.Get(subnetID)
func (m *manager) GetValidatorIDs(subnetID ids.ID) ([]ids.NodeID, bool) {
validatorSet, exist := m.cfg.Validators.Get(subnetID)
if !exist {
return nil, false
}
Expand All @@ -298,6 +298,6 @@ func (vs *set) GetValidatorIDs(subnetID ids.ID) ([]ids.NodeID, bool) {
return validatorIDs, true
}

func (vs *set) Track(blkID ids.ID) {
vs.recentlyAccepted.Add(blkID)
func (m *manager) Track(blkID ids.ID) {
m.recentlyAccepted.Add(blkID)
}
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ func TestVM_GetValidatorSet(t *testing.T) {
r.NoError(err)

clk := mockable.Clock{}
validatorssSet := NewSet(cfg, mockState, metrics, clk)
validatorssSet := NewManager(cfg, mockState, metrics, clk)

// Mock the VM's validators
mockSubnetVdrSet := validators.NewMockSet(ctrl)
Expand Down
36 changes: 36 additions & 0 deletions vms/platformvm/validators/test_manager.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (C) 2019-2022, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package validators

import (
"context"

"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/snow/validators"
)

var TestManager Manager = testManager{}

type testManager struct{}

func (testManager) GetMinimumHeight(context.Context) (uint64, error) {
return 0, nil
}

func (testManager) GetCurrentHeight(context.Context) (uint64, error) {
return 0, nil
}

func (testManager) GetSubnetID(context.Context, ids.ID) (ids.ID, error) {
return ids.Empty, nil
}

func (testManager) GetValidatorSet(context.Context, uint64, ids.ID) (map[ids.NodeID]*validators.GetValidatorOutput, error) {
return nil, nil
}

func (testManager) GetValidatorIDs(ids.ID) ([]ids.NodeID, bool) {
return nil, false
}
func (testManager) Track(ids.ID) {}
36 changes: 0 additions & 36 deletions vms/platformvm/validators/test_set.go

This file was deleted.

6 changes: 3 additions & 3 deletions vms/platformvm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ var (
type VM struct {
config.Config
blockbuilder.Builder
pvalidators.QueribleSet
pvalidators.QueryManager

metrics metrics.Metrics
atomicUtxosManager avax.AtomicUTXOManager
Expand Down Expand Up @@ -139,8 +139,8 @@ func (vm *VM) Initialize(
return err
}

validatorsSet := pvalidators.NewSet(vm.Config, vm.state, vm.metrics, vm.clock)
vm.QueribleSet = validatorsSet
validatorsSet := pvalidators.NewManager(vm.Config, vm.state, vm.metrics, vm.clock)
vm.QueryManager = validatorsSet
vm.atomicUtxosManager = avax.NewAtomicUTXOManager(chainCtx.SharedMemory, txs.Codec)
utxoHandler := utxo.NewHandler(vm.ctx, &vm.clock, vm.fx)
vm.uptimeManager = uptime.NewManager(vm.state)
Expand Down