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
nits
  • Loading branch information
abi87 committed May 4, 2023
commit ee25500b954366128e2b842d7ededf78102a89a8
2 changes: 1 addition & 1 deletion vms/platformvm/blocks/executor/acceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,6 @@ func (a *acceptor) commonAccept(b blocks.Block) error {
a.state.SetLastAccepted(blkID)
a.state.SetHeight(b.Height())
a.state.AddStatelessBlock(b, choices.Accepted)
a.validators.Track(blkID)
a.validators.OnAcceptedBlockID(blkID)
return nil
}
22 changes: 12 additions & 10 deletions vms/platformvm/validators/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,22 @@ var (
ErrMissingValidatorSet = errors.New("missing validator set")
)

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

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

// Manager interface adds to QueribleSet the ability to blocks IDs
// Manager interface adds to QueryManager the ability to blocks IDs
// to serve GetMinimumHeight
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)

// OnAcceptedBlockID register the ID of the latest accepted block.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// OnAcceptedBlockID register the ID of the latest accepted block.
// OnAcceptedBlockID registers the ID of the latest accepted block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

// It is used to update [recentlyAccepted] sliding window.
OnAcceptedBlockID(blkID ids.ID)
}

func NewManager(
Expand Down Expand Up @@ -102,9 +104,9 @@ type manager struct {
// height which is likely (but not guaranteed) to also be older than the
// window's configured TTL.
//
// 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].
// If [UseCurrentHeight] is true, we override the block selection policy
// described above and we will always return the last accepted block height
// as the minimum.
func (m *manager) GetMinimumHeight(ctx context.Context) (uint64, error) {
if m.cfg.UseCurrentHeight {
return m.GetCurrentHeight(ctx)
Expand Down Expand Up @@ -262,7 +264,7 @@ func (m *manager) GetValidatorSet(ctx context.Context, height uint64, subnetID i
return vdrSet, nil
}

// GetCurrentHeight returns the height of the last accepted block
// GetSubnetID returns subnetID of the specified chainID
func (m *manager) GetSubnetID(_ context.Context, chainID ids.ID) (ids.ID, error) {
if chainID == constants.PlatformChainID {
return constants.PrimaryNetworkID, nil
Expand Down Expand Up @@ -298,6 +300,6 @@ func (m *manager) GetValidatorIDs(subnetID ids.ID) ([]ids.NodeID, bool) {
return validatorIDs, true
}

func (m *manager) Track(blkID ids.ID) {
func (m *manager) OnAcceptedBlockID(blkID ids.ID) {
m.recentlyAccepted.Add(blkID)
}
9 changes: 4 additions & 5 deletions vms/platformvm/validators/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ var defaultRewardConfig = reward.Config{
}

func TestVM_GetValidatorSet(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

// Populate the validator set to use below
var (
numVdrs = 4
Expand Down Expand Up @@ -71,10 +68,10 @@ func TestVM_GetValidatorSet(t *testing.T) {
currentPrimaryNetworkValidators []*validators.Validator
currentSubnetValidators []*validators.Validator
// Diff at tip, block before tip, etc.
// This must have [height] - [lastAcceptedHeight] elements
// This must have [lastAcceptedHeight] - [height] elements
weightDiffs []map[ids.NodeID]*state.ValidatorWeightDiff
// Diff at tip, block before tip, etc.
// This must have [height] - [lastAcceptedHeight] elements
// This must have [lastAcceptedHeight] - [height] elements
pkDiffs []map[ids.NodeID]*bls.PublicKey
expectedVdrSet map[ids.NodeID]*validators.GetValidatorOutput
expectedErr error
Expand Down Expand Up @@ -368,6 +365,8 @@ func TestVM_GetValidatorSet(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := require.New(t)
ctrl := gomock.NewController(t)
defer ctrl.Finish()

// setup validators set
vdrs := validators.NewMockManager(ctrl)
Expand Down
2 changes: 1 addition & 1 deletion vms/platformvm/validators/test_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ func (testManager) GetValidatorSet(context.Context, uint64, ids.ID) (map[ids.Nod
func (testManager) GetValidatorIDs(ids.ID) ([]ids.NodeID, bool) {
return nil, false
}
func (testManager) Track(ids.ID) {}
func (testManager) OnAcceptedBlockID(ids.ID) {}
6 changes: 3 additions & 3 deletions vms/platformvm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ func (vm *VM) Initialize(
return err
}

validatorsSet := pvalidators.NewManager(vm.Config, vm.state, vm.metrics, &vm.clock)
vm.QueryManager = validatorsSet
queryManager := pvalidators.NewManager(vm.Config, vm.state, vm.metrics, &vm.clock)
vm.QueryManager = queryManager
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 Expand Up @@ -179,7 +179,7 @@ func (vm *VM) Initialize(
vm.metrics,
vm.state,
txExecutorBackend,
validatorsSet,
queryManager,
)
vm.Builder = blockbuilder.New(
mempool,
Expand Down