diff --git a/snow/consensus/snowman/oracle_block.go b/snow/consensus/snowman/oracle_block.go index 4688927e566e..2ca81680ae78 100644 --- a/snow/consensus/snowman/oracle_block.go +++ b/snow/consensus/snowman/oracle_block.go @@ -18,6 +18,5 @@ var ErrNotOracle = errors.New("block isn't an oracle") type OracleBlock interface { // Options returns the possible children of this block in the order this // validator prefers the blocks. - // Options is guaranteed to only be called on a verified block. Options(context.Context) ([2]Block, error) } diff --git a/vms/platformvm/block/executor/acceptor.go b/vms/platformvm/block/executor/acceptor.go index 8ee4f6e4a04f..cc2bcef0521f 100644 --- a/vms/platformvm/block/executor/acceptor.go +++ b/vms/platformvm/block/executor/acceptor.go @@ -33,11 +33,11 @@ type acceptor struct { } func (a *acceptor) BanffAbortBlock(b *block.BanffAbortBlock) error { - return a.abortBlock(b, "banff abort") + return a.optionBlock(b, "banff abort") } func (a *acceptor) BanffCommitBlock(b *block.BanffCommitBlock) error { - return a.commitBlock(b, "apricot commit") + return a.optionBlock(b, "banff commit") } func (a *acceptor) BanffProposalBlock(b *block.BanffProposalBlock) error { @@ -50,11 +50,11 @@ func (a *acceptor) BanffStandardBlock(b *block.BanffStandardBlock) error { } func (a *acceptor) ApricotAbortBlock(b *block.ApricotAbortBlock) error { - return a.abortBlock(b, "apricot abort") + return a.optionBlock(b, "apricot abort") } func (a *acceptor) ApricotCommitBlock(b *block.ApricotCommitBlock) error { - return a.commitBlock(b, "apricot commit") + return a.optionBlock(b, "apricot commit") } func (a *acceptor) ApricotProposalBlock(b *block.ApricotProposalBlock) error { @@ -116,46 +116,14 @@ func (a *acceptor) ApricotAtomicBlock(b *block.ApricotAtomicBlock) error { return nil } -func (a *acceptor) abortBlock(b block.Block, blockType string) error { +func (a *acceptor) optionBlock(b block.Block, blockType string) error { parentID := b.Parent() parentState, ok := a.blkIDToState[parentID] if !ok { return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID) } - if a.bootstrapped.Get() { - if parentState.initiallyPreferCommit { - a.metrics.MarkOptionVoteLost() - } else { - a.metrics.MarkOptionVoteWon() - } - } - - return a.optionBlock(b, parentState.statelessBlock, blockType) -} - -func (a *acceptor) commitBlock(b block.Block, blockType string) error { - parentID := b.Parent() - parentState, ok := a.blkIDToState[parentID] - if !ok { - return fmt.Errorf("%w: %s", state.ErrMissingParentState, parentID) - } - - if a.bootstrapped.Get() { - if parentState.initiallyPreferCommit { - a.metrics.MarkOptionVoteWon() - } else { - a.metrics.MarkOptionVoteLost() - } - } - - return a.optionBlock(b, parentState.statelessBlock, blockType) -} - -func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error { blkID := b.ID() - parentID := parent.ID() - defer func() { // Note: we assume this block's sibling doesn't // need the parent's state when it's rejected. @@ -164,7 +132,7 @@ func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error { }() // Note that the parent must be accepted first. - if err := a.commonAccept(parent); err != nil { + if err := a.commonAccept(parentState.statelessBlock); err != nil { return err } @@ -172,10 +140,6 @@ func (a *acceptor) optionBlock(b, parent block.Block, blockType string) error { return err } - parentState, ok := a.blkIDToState[parentID] - if !ok { - return fmt.Errorf("%w %s", errMissingBlockState, parentID) - } if parentState.onDecisionState != nil { if err := parentState.onDecisionState.Apply(a.state); err != nil { return err diff --git a/vms/platformvm/block/executor/acceptor_test.go b/vms/platformvm/block/executor/acceptor_test.go index fb5fcd5f88af..45fd1d54d189 100644 --- a/vms/platformvm/block/executor/acceptor_test.go +++ b/vms/platformvm/block/executor/acceptor_test.go @@ -303,7 +303,7 @@ func TestAcceptorVisitCommitBlock(t *testing.T) { // Set expected calls on dependencies. // Make sure the parent is accepted first. gomock.InOrder( - parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2), + parentStatelessBlk.EXPECT().ID().Return(parentID).Times(1), s.EXPECT().SetLastAccepted(parentID).Times(1), parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1), s.EXPECT().SetHeight(blk.Height()-1).Times(1), @@ -335,7 +335,7 @@ func TestAcceptorVisitCommitBlock(t *testing.T) { // Set expected calls on dependencies. // Make sure the parent is accepted first. gomock.InOrder( - parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2), + parentStatelessBlk.EXPECT().ID().Return(parentID).Times(1), s.EXPECT().SetLastAccepted(parentID).Times(1), parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1), s.EXPECT().SetHeight(blk.Height()-1).Times(1), @@ -413,7 +413,7 @@ func TestAcceptorVisitAbortBlock(t *testing.T) { // Set expected calls on dependencies. // Make sure the parent is accepted first. gomock.InOrder( - parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2), + parentStatelessBlk.EXPECT().ID().Return(parentID).Times(1), s.EXPECT().SetLastAccepted(parentID).Times(1), parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1), s.EXPECT().SetHeight(blk.Height()-1).Times(1), @@ -445,7 +445,7 @@ func TestAcceptorVisitAbortBlock(t *testing.T) { // Set expected calls on dependencies. // Make sure the parent is accepted first. gomock.InOrder( - parentStatelessBlk.EXPECT().ID().Return(parentID).Times(2), + parentStatelessBlk.EXPECT().ID().Return(parentID).Times(1), s.EXPECT().SetLastAccepted(parentID).Times(1), parentStatelessBlk.EXPECT().Height().Return(blk.Height()-1).Times(1), s.EXPECT().SetHeight(blk.Height()-1).Times(1), diff --git a/vms/platformvm/block/executor/block.go b/vms/platformvm/block/executor/block.go index ef05f38442c5..5cd5a02f709c 100644 --- a/vms/platformvm/block/executor/block.go +++ b/vms/platformvm/block/executor/block.go @@ -5,7 +5,6 @@ package executor import ( "context" - "fmt" "time" "go.uber.org/zap" @@ -83,22 +82,18 @@ func (b *Block) Timestamp() time.Time { } func (b *Block) Options(context.Context) ([2]snowman.Block, error) { - options := options{} + options := options{ + log: b.manager.ctx.Log, + primaryUptimePercentage: b.manager.txExecutorBackend.Config.UptimePercentage, + uptimes: b.manager.txExecutorBackend.Uptimes, + state: b.manager.backend.state, + } if err := b.Block.Visit(&options); err != nil { return [2]snowman.Block{}, err } - commitBlock := b.manager.NewBlock(options.commitBlock) - abortBlock := b.manager.NewBlock(options.abortBlock) - - blkID := b.ID() - blkState, ok := b.manager.blkIDToState[blkID] - if !ok { - return [2]snowman.Block{}, fmt.Errorf("block %s state not found", blkID) - } - - if blkState.initiallyPreferCommit { - return [2]snowman.Block{commitBlock, abortBlock}, nil - } - return [2]snowman.Block{abortBlock, commitBlock}, nil + return [2]snowman.Block{ + b.manager.NewBlock(options.preferredBlock), + b.manager.NewBlock(options.alternateBlock), + }, nil } diff --git a/vms/platformvm/block/executor/block_state.go b/vms/platformvm/block/executor/block_state.go index ffc6f679b8f6..9d6b377c2644 100644 --- a/vms/platformvm/block/executor/block_state.go +++ b/vms/platformvm/block/executor/block_state.go @@ -14,10 +14,9 @@ import ( ) type proposalBlockState struct { - initiallyPreferCommit bool - onDecisionState state.Diff - onCommitState state.Diff - onAbortState state.Diff + onDecisionState state.Diff + onCommitState state.Diff + onAbortState state.Diff } // The state of a block. diff --git a/vms/platformvm/block/executor/block_test.go b/vms/platformvm/block/executor/block_test.go index b1d73cefb7f1..c26d71705857 100644 --- a/vms/platformvm/block/executor/block_test.go +++ b/vms/platformvm/block/executor/block_test.go @@ -6,6 +6,7 @@ package executor import ( "context" "testing" + "time" "github.com/stretchr/testify/require" @@ -14,9 +15,16 @@ import ( "github.com/ava-labs/avalanchego/database" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/choices" - "github.com/ava-labs/avalanchego/snow/consensus/snowman" + "github.com/ava-labs/avalanchego/snow/snowtest" + "github.com/ava-labs/avalanchego/snow/uptime" + "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/vms/platformvm/block" + "github.com/ava-labs/avalanchego/vms/platformvm/config" + "github.com/ava-labs/avalanchego/vms/platformvm/reward" "github.com/ava-labs/avalanchego/vms/platformvm/state" + "github.com/ava-labs/avalanchego/vms/platformvm/status" + "github.com/ava-labs/avalanchego/vms/platformvm/txs" + "github.com/ava-labs/avalanchego/vms/platformvm/txs/executor" ) func TestStatus(t *testing.T) { @@ -127,126 +135,485 @@ func TestStatus(t *testing.T) { func TestBlockOptions(t *testing.T) { type test struct { name string - blkF func() *Block + blkF func(*gomock.Controller) *Block expectedPreferenceType block.Block - expectedErr error } tests := []test{ { name: "apricot proposal block; commit preferred", - blkF: func() *Block { - innerBlk := &block.ApricotProposalBlock{} - blkID := innerBlk.ID() + blkF: func(ctrl *gomock.Controller) *Block { + state := state.NewMockState(ctrl) + + uptimes := uptime.NewMockCalculator(ctrl) manager := &manager{ backend: &backend{ - blkIDToState: map[ids.ID]*blockState{ - blkID: { - proposalBlockState: proposalBlockState{ - initiallyPreferCommit: true, + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: 0, + }, + Uptimes: uptimes, + }, + } + + return &Block{ + Block: &block.ApricotProposalBlock{}, + manager: manager, + } + }, + expectedPreferenceType: &block.ApricotCommitBlock{}, + }, + { + name: "banff proposal block; invalid proposal tx", + blkF: func(ctrl *gomock.Controller) *Block { + state := state.NewMockState(ctrl) + + uptimes := uptime.NewMockCalculator(ctrl) + + manager := &manager{ + backend: &backend{ + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: 0, + }, + Uptimes: uptimes, + }, + } + + return &Block{ + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.CreateChainTx{}, + }, + }, + }, + manager: manager, + } + }, + expectedPreferenceType: &block.BanffCommitBlock{}, + }, + { + name: "banff proposal block; missing tx", + blkF: func(ctrl *gomock.Controller) *Block { + stakerTxID := ids.GenerateTestID() + + state := state.NewMockState(ctrl) + state.EXPECT().GetTx(stakerTxID).Return(nil, status.Unknown, database.ErrNotFound) + + uptimes := uptime.NewMockCalculator(ctrl) + + manager := &manager{ + backend: &backend{ + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: 0, + }, + Uptimes: uptimes, + }, + } + + return &Block{ + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.RewardValidatorTx{ + TxID: stakerTxID, }, }, }, }, + manager: manager, + } + }, + expectedPreferenceType: &block.BanffCommitBlock{}, + }, + { + name: "banff proposal block; error fetching staker tx", + blkF: func(ctrl *gomock.Controller) *Block { + stakerTxID := ids.GenerateTestID() + + state := state.NewMockState(ctrl) + state.EXPECT().GetTx(stakerTxID).Return(nil, status.Unknown, database.ErrClosed) + + uptimes := uptime.NewMockCalculator(ctrl) + + manager := &manager{ + backend: &backend{ + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: 0, + }, + Uptimes: uptimes, + }, } return &Block{ - Block: innerBlk, + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.RewardValidatorTx{ + TxID: stakerTxID, + }, + }, + }, + }, manager: manager, } }, - expectedPreferenceType: &block.ApricotCommitBlock{}, + expectedPreferenceType: &block.BanffCommitBlock{}, }, { - name: "apricot proposal block; abort preferred", - blkF: func() *Block { - innerBlk := &block.ApricotProposalBlock{} - blkID := innerBlk.ID() + name: "banff proposal block; unexpected staker tx type", + blkF: func(ctrl *gomock.Controller) *Block { + stakerTxID := ids.GenerateTestID() + stakerTx := &txs.Tx{ + Unsigned: &txs.CreateChainTx{}, + } + + state := state.NewMockState(ctrl) + state.EXPECT().GetTx(stakerTxID).Return(stakerTx, status.Committed, nil) + + uptimes := uptime.NewMockCalculator(ctrl) manager := &manager{ backend: &backend{ - blkIDToState: map[ids.ID]*blockState{ - blkID: {}, + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: 0, }, + Uptimes: uptimes, }, } return &Block{ - Block: innerBlk, + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.RewardValidatorTx{ + TxID: stakerTxID, + }, + }, + }, + }, manager: manager, } }, - expectedPreferenceType: &block.ApricotAbortBlock{}, + expectedPreferenceType: &block.BanffCommitBlock{}, }, { - name: "banff proposal block; commit preferred", - blkF: func() *Block { - innerBlk := &block.BanffProposalBlock{} - blkID := innerBlk.ID() + name: "banff proposal block; missing primary network validator", + blkF: func(ctrl *gomock.Controller) *Block { + var ( + stakerTxID = ids.GenerateTestID() + nodeID = ids.GenerateTestNodeID() + subnetID = ids.GenerateTestID() + stakerTx = &txs.Tx{ + Unsigned: &txs.AddPermissionlessValidatorTx{ + Validator: txs.Validator{ + NodeID: nodeID, + }, + Subnet: subnetID, + }, + } + ) + + state := state.NewMockState(ctrl) + state.EXPECT().GetTx(stakerTxID).Return(stakerTx, status.Committed, nil) + state.EXPECT().GetCurrentValidator(constants.PrimaryNetworkID, nodeID).Return(nil, database.ErrNotFound) + + uptimes := uptime.NewMockCalculator(ctrl) manager := &manager{ backend: &backend{ - blkIDToState: map[ids.ID]*blockState{ - blkID: { - proposalBlockState: proposalBlockState{ - initiallyPreferCommit: true, + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: 0, + }, + Uptimes: uptimes, + }, + } + + return &Block{ + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.RewardValidatorTx{ + TxID: stakerTxID, }, }, }, }, + manager: manager, + } + }, + expectedPreferenceType: &block.BanffCommitBlock{}, + }, + { + name: "banff proposal block; failed calculating primary network uptime", + blkF: func(ctrl *gomock.Controller) *Block { + var ( + stakerTxID = ids.GenerateTestID() + nodeID = ids.GenerateTestNodeID() + subnetID = constants.PrimaryNetworkID + stakerTx = &txs.Tx{ + Unsigned: &txs.AddPermissionlessValidatorTx{ + Validator: txs.Validator{ + NodeID: nodeID, + }, + Subnet: subnetID, + }, + } + primaryNetworkValidatorStartTime = time.Now() + staker = &state.Staker{ + StartTime: primaryNetworkValidatorStartTime, + } + ) + + state := state.NewMockState(ctrl) + state.EXPECT().GetTx(stakerTxID).Return(stakerTx, status.Committed, nil) + state.EXPECT().GetCurrentValidator(constants.PrimaryNetworkID, nodeID).Return(staker, nil) + + uptimes := uptime.NewMockCalculator(ctrl) + uptimes.EXPECT().CalculateUptimePercentFrom(nodeID, constants.PrimaryNetworkID, primaryNetworkValidatorStartTime).Return(0.0, database.ErrNotFound) + + manager := &manager{ + backend: &backend{ + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: 0, + }, + Uptimes: uptimes, + }, } return &Block{ - Block: innerBlk, + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.RewardValidatorTx{ + TxID: stakerTxID, + }, + }, + }, + }, manager: manager, } }, expectedPreferenceType: &block.BanffCommitBlock{}, }, { - name: "banff proposal block; abort preferred", - blkF: func() *Block { - innerBlk := &block.BanffProposalBlock{} - blkID := innerBlk.ID() + name: "banff proposal block; failed fetching subnet transformation", + blkF: func(ctrl *gomock.Controller) *Block { + var ( + stakerTxID = ids.GenerateTestID() + nodeID = ids.GenerateTestNodeID() + subnetID = ids.GenerateTestID() + stakerTx = &txs.Tx{ + Unsigned: &txs.AddPermissionlessValidatorTx{ + Validator: txs.Validator{ + NodeID: nodeID, + }, + Subnet: subnetID, + }, + } + primaryNetworkValidatorStartTime = time.Now() + staker = &state.Staker{ + StartTime: primaryNetworkValidatorStartTime, + } + ) + + state := state.NewMockState(ctrl) + state.EXPECT().GetTx(stakerTxID).Return(stakerTx, status.Committed, nil) + state.EXPECT().GetCurrentValidator(constants.PrimaryNetworkID, nodeID).Return(staker, nil) + state.EXPECT().GetSubnetTransformation(subnetID).Return(nil, database.ErrNotFound) + + uptimes := uptime.NewMockCalculator(ctrl) manager := &manager{ backend: &backend{ - blkIDToState: map[ids.ID]*blockState{ - blkID: {}, + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: 0, }, + Uptimes: uptimes, }, } return &Block{ - Block: innerBlk, + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.RewardValidatorTx{ + TxID: stakerTxID, + }, + }, + }, + }, manager: manager, } }, - expectedPreferenceType: &block.BanffAbortBlock{}, + expectedPreferenceType: &block.BanffCommitBlock{}, }, { - name: "non oracle block", - blkF: func() *Block { + name: "banff proposal block; prefers commit", + blkF: func(ctrl *gomock.Controller) *Block { + var ( + stakerTxID = ids.GenerateTestID() + nodeID = ids.GenerateTestNodeID() + subnetID = ids.GenerateTestID() + stakerTx = &txs.Tx{ + Unsigned: &txs.AddPermissionlessValidatorTx{ + Validator: txs.Validator{ + NodeID: nodeID, + }, + Subnet: subnetID, + }, + } + primaryNetworkValidatorStartTime = time.Now() + staker = &state.Staker{ + StartTime: primaryNetworkValidatorStartTime, + } + transformSubnetTx = &txs.Tx{ + Unsigned: &txs.TransformSubnetTx{ + UptimeRequirement: .2 * reward.PercentDenominator, + }, + } + ) + + state := state.NewMockState(ctrl) + state.EXPECT().GetTx(stakerTxID).Return(stakerTx, status.Committed, nil) + state.EXPECT().GetCurrentValidator(constants.PrimaryNetworkID, nodeID).Return(staker, nil) + state.EXPECT().GetSubnetTransformation(subnetID).Return(transformSubnetTx, nil) + + uptimes := uptime.NewMockCalculator(ctrl) + uptimes.EXPECT().CalculateUptimePercentFrom(nodeID, constants.PrimaryNetworkID, primaryNetworkValidatorStartTime).Return(.5, nil) + + manager := &manager{ + backend: &backend{ + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: .8, + }, + Uptimes: uptimes, + }, + } + return &Block{ - Block: &block.BanffStandardBlock{}, - manager: &manager{}, + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.RewardValidatorTx{ + TxID: stakerTxID, + }, + }, + }, + }, + manager: manager, } }, - expectedErr: snowman.ErrNotOracle, + expectedPreferenceType: &block.BanffCommitBlock{}, + }, + { + name: "banff proposal block; prefers abort", + blkF: func(ctrl *gomock.Controller) *Block { + var ( + stakerTxID = ids.GenerateTestID() + nodeID = ids.GenerateTestNodeID() + subnetID = ids.GenerateTestID() + stakerTx = &txs.Tx{ + Unsigned: &txs.AddPermissionlessValidatorTx{ + Validator: txs.Validator{ + NodeID: nodeID, + }, + Subnet: subnetID, + }, + } + primaryNetworkValidatorStartTime = time.Now() + staker = &state.Staker{ + StartTime: primaryNetworkValidatorStartTime, + } + transformSubnetTx = &txs.Tx{ + Unsigned: &txs.TransformSubnetTx{ + UptimeRequirement: .6 * reward.PercentDenominator, + }, + } + ) + + state := state.NewMockState(ctrl) + state.EXPECT().GetTx(stakerTxID).Return(stakerTx, status.Committed, nil) + state.EXPECT().GetCurrentValidator(constants.PrimaryNetworkID, nodeID).Return(staker, nil) + state.EXPECT().GetSubnetTransformation(subnetID).Return(transformSubnetTx, nil) + + uptimes := uptime.NewMockCalculator(ctrl) + uptimes.EXPECT().CalculateUptimePercentFrom(nodeID, constants.PrimaryNetworkID, primaryNetworkValidatorStartTime).Return(.5, nil) + + manager := &manager{ + backend: &backend{ + state: state, + ctx: snowtest.Context(t, snowtest.PChainID), + }, + txExecutorBackend: &executor.Backend{ + Config: &config.Config{ + UptimePercentage: .8, + }, + Uptimes: uptimes, + }, + } + + return &Block{ + Block: &block.BanffProposalBlock{ + ApricotProposalBlock: block.ApricotProposalBlock{ + Tx: &txs.Tx{ + Unsigned: &txs.RewardValidatorTx{ + TxID: stakerTxID, + }, + }, + }, + }, + manager: manager, + } + }, + expectedPreferenceType: &block.BanffAbortBlock{}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) require := require.New(t) - blk := tt.blkF() + blk := tt.blkF(ctrl) options, err := blk.Options(context.Background()) - require.ErrorIs(err, tt.expectedErr) - if tt.expectedErr != nil { - return - } + require.NoError(err) require.IsType(tt.expectedPreferenceType, options[0].(*Block).Block) }) } diff --git a/vms/platformvm/block/executor/options.go b/vms/platformvm/block/executor/options.go index a8ccea315b0f..f2071c8e13cb 100644 --- a/vms/platformvm/block/executor/options.go +++ b/vms/platformvm/block/executor/options.go @@ -4,19 +4,44 @@ package executor import ( + "errors" "fmt" + "go.uber.org/zap" + "github.com/ava-labs/avalanchego/snow/consensus/snowman" + "github.com/ava-labs/avalanchego/snow/uptime" + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/vms/platformvm/block" + "github.com/ava-labs/avalanchego/vms/platformvm/reward" + "github.com/ava-labs/avalanchego/vms/platformvm/state" + "github.com/ava-labs/avalanchego/vms/platformvm/txs" + "github.com/ava-labs/avalanchego/vms/platformvm/txs/executor" ) -var _ block.Visitor = (*verifier)(nil) +var ( + _ block.Visitor = (*options)(nil) + + errUnexpectedProposalTxType = errors.New("unexpected proposal transaction type") + errFailedFetchingStakerTx = errors.New("failed fetching staker transaction") + errUnexpectedStakerTxType = errors.New("unexpected staker transaction type") + errFailedFetchingPrimaryStaker = errors.New("failed fetching primary staker") + errFailedFetchingSubnetTransformation = errors.New("failed fetching subnet transformation") + errFailedCalculatingUptime = errors.New("failed calculating uptime") +) // options supports build new option blocks type options struct { + // inputs populated before calling this struct's methods: + log logging.Logger + primaryUptimePercentage float64 + uptimes uptime.Calculator + state state.Chain + // outputs populated by this struct's methods: - commitBlock block.Block - abortBlock block.Block + preferredBlock block.Block + alternateBlock block.Block } func (*options) BanffAbortBlock(*block.BanffAbortBlock) error { @@ -32,8 +57,7 @@ func (o *options) BanffProposalBlock(b *block.BanffProposalBlock) error { blkID := b.ID() nextHeight := b.Height() + 1 - var err error - o.commitBlock, err = block.NewBanffCommitBlock(timestamp, blkID, nextHeight) + commitBlock, err := block.NewBanffCommitBlock(timestamp, blkID, nextHeight) if err != nil { return fmt.Errorf( "failed to create commit block: %w", @@ -41,13 +65,35 @@ func (o *options) BanffProposalBlock(b *block.BanffProposalBlock) error { ) } - o.abortBlock, err = block.NewBanffAbortBlock(timestamp, blkID, nextHeight) + abortBlock, err := block.NewBanffAbortBlock(timestamp, blkID, nextHeight) if err != nil { return fmt.Errorf( "failed to create abort block: %w", err, ) } + + prefersCommit, err := o.prefersCommit(b.Tx) + if err != nil { + o.log.Debug("falling back to prefer commit", + zap.Error(err), + ) + // We fall back to commit here to err on the side of over-rewarding + // rather than under-rewarding. + // + // Invariant: We must not return the error here, because the error would + // be treated as fatal. Errors can occur here due to a malicious block + // proposer or even in unusual virtuous cases. + prefersCommit = true + } + + if prefersCommit { + o.preferredBlock = commitBlock + o.alternateBlock = abortBlock + } else { + o.preferredBlock = abortBlock + o.alternateBlock = commitBlock + } return nil } @@ -68,7 +114,7 @@ func (o *options) ApricotProposalBlock(b *block.ApricotProposalBlock) error { nextHeight := b.Height() + 1 var err error - o.commitBlock, err = block.NewApricotCommitBlock(blkID, nextHeight) + o.preferredBlock, err = block.NewApricotCommitBlock(blkID, nextHeight) if err != nil { return fmt.Errorf( "failed to create commit block: %w", @@ -76,7 +122,7 @@ func (o *options) ApricotProposalBlock(b *block.ApricotProposalBlock) error { ) } - o.abortBlock, err = block.NewApricotAbortBlock(blkID, nextHeight) + o.alternateBlock, err = block.NewApricotAbortBlock(blkID, nextHeight) if err != nil { return fmt.Errorf( "failed to create abort block: %w", @@ -93,3 +139,51 @@ func (*options) ApricotStandardBlock(*block.ApricotStandardBlock) error { func (*options) ApricotAtomicBlock(*block.ApricotAtomicBlock) error { return snowman.ErrNotOracle } + +func (o *options) prefersCommit(tx *txs.Tx) (bool, error) { + unsignedTx, ok := tx.Unsigned.(*txs.RewardValidatorTx) + if !ok { + return false, fmt.Errorf("%w: %T", errUnexpectedProposalTxType, tx.Unsigned) + } + + stakerTx, _, err := o.state.GetTx(unsignedTx.TxID) + if err != nil { + return false, fmt.Errorf("%w: %w", errFailedFetchingStakerTx, err) + } + + staker, ok := stakerTx.Unsigned.(txs.Staker) + if !ok { + return false, fmt.Errorf("%w: %T", errUnexpectedStakerTxType, stakerTx.Unsigned) + } + + nodeID := staker.NodeID() + primaryNetworkValidator, err := o.state.GetCurrentValidator( + constants.PrimaryNetworkID, + nodeID, + ) + if err != nil { + return false, fmt.Errorf("%w: %w", errFailedFetchingPrimaryStaker, err) + } + + expectedUptimePercentage := o.primaryUptimePercentage + if subnetID := staker.SubnetID(); subnetID != constants.PrimaryNetworkID { + transformSubnet, err := executor.GetTransformSubnetTx(o.state, subnetID) + if err != nil { + return false, fmt.Errorf("%w: %w", errFailedFetchingSubnetTransformation, err) + } + + expectedUptimePercentage = float64(transformSubnet.UptimeRequirement) / reward.PercentDenominator + } + + // TODO: calculate subnet uptimes + uptime, err := o.uptimes.CalculateUptimePercentFrom( + nodeID, + constants.PrimaryNetworkID, + primaryNetworkValidator.StartTime, + ) + if err != nil { + return false, fmt.Errorf("%w: %w", errFailedCalculatingUptime, err) + } + + return uptime >= expectedUptimePercentage, nil +} diff --git a/vms/platformvm/block/executor/proposal_block_test.go b/vms/platformvm/block/executor/proposal_block_test.go index 08d1ee8939e9..6205dacfa29c 100644 --- a/vms/platformvm/block/executor/proposal_block_test.go +++ b/vms/platformvm/block/executor/proposal_block_test.go @@ -97,14 +97,6 @@ func TestApricotProposalBlockTimeVerification(t *testing.T) { }).Times(2) currentStakersIt.EXPECT().Release() onParentAccept.EXPECT().GetCurrentStakerIterator().Return(currentStakersIt, nil) - onParentAccept.EXPECT().GetCurrentValidator(utx.SubnetID(), utx.NodeID()).Return(&state.Staker{ - TxID: addValTx.ID(), - NodeID: utx.NodeID(), - SubnetID: utx.SubnetID(), - StartTime: utx.StartTime(), - NextTime: chainTime, - EndTime: chainTime, - }, nil) onParentAccept.EXPECT().GetTx(addValTx.ID()).Return(addValTx, status.Committed, nil) onParentAccept.EXPECT().GetCurrentSupply(constants.PrimaryNetworkID).Return(uint64(1000), nil).AnyTimes() onParentAccept.EXPECT().GetDelegateeReward(constants.PrimaryNetworkID, utx.NodeID()).Return(uint64(0), nil).AnyTimes() @@ -206,13 +198,6 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) { require.NoError(nextStakerTx.Initialize(txs.Codec)) nextStakerTxID := nextStakerTx.ID() - onParentAccept.EXPECT().GetCurrentValidator(unsignedNextStakerTx.SubnetID(), unsignedNextStakerTx.NodeID()).Return(&state.Staker{ - TxID: nextStakerTxID, - NodeID: unsignedNextStakerTx.NodeID(), - SubnetID: unsignedNextStakerTx.SubnetID(), - StartTime: unsignedNextStakerTx.StartTime(), - EndTime: chainTime, - }, nil) onParentAccept.EXPECT().GetTx(nextStakerTxID).Return(nextStakerTx, status.Processing, nil) currentStakersIt := state.NewMockStakerIterator(ctrl) diff --git a/vms/platformvm/block/executor/verifier.go b/vms/platformvm/block/executor/verifier.go index 3b43e435c781..b35d2ecdd55c 100644 --- a/vms/platformvm/block/executor/verifier.go +++ b/vms/platformvm/block/executor/verifier.go @@ -386,10 +386,9 @@ func (v *verifier) proposalBlock( blkID := b.ID() v.blkIDToState[blkID] = &blockState{ proposalBlockState: proposalBlockState{ - onDecisionState: onDecisionState, - onCommitState: onCommitState, - onAbortState: onAbortState, - initiallyPreferCommit: txExecutor.PrefersCommit, + onDecisionState: onDecisionState, + onCommitState: onCommitState, + onAbortState: onAbortState, }, statelessBlock: b, diff --git a/vms/platformvm/metrics/metrics.go b/vms/platformvm/metrics/metrics.go index 5357721d9bd6..98b611a017ed 100644 --- a/vms/platformvm/metrics/metrics.go +++ b/vms/platformvm/metrics/metrics.go @@ -19,10 +19,6 @@ var _ Metrics = (*metrics)(nil) type Metrics interface { metric.APIInterceptor - // Mark that an option vote that we initially preferred was accepted. - MarkOptionVoteWon() - // Mark that an option vote that we initially preferred was rejected. - MarkOptionVoteLost() // Mark that the given block was accepted. MarkAccepted(block.Block) error // Mark that a validator set was created. @@ -75,17 +71,6 @@ func New( Help: "Amount (in nAVAX) of AVAX staked on the Primary Network", }), - numVotesWon: prometheus.NewCounter(prometheus.CounterOpts{ - Namespace: namespace, - Name: "votes_won", - Help: "Total number of votes this node has won", - }), - numVotesLost: prometheus.NewCounter(prometheus.CounterOpts{ - Namespace: namespace, - Name: "votes_lost", - Help: "Total number of votes this node has lost", - }), - validatorSetsCached: prometheus.NewCounter(prometheus.CounterOpts{ Namespace: namespace, Name: "validator_sets_cached", @@ -118,9 +103,6 @@ func New( registerer.Register(m.localStake), registerer.Register(m.totalStake), - registerer.Register(m.numVotesWon), - registerer.Register(m.numVotesLost), - registerer.Register(m.validatorSetsCreated), registerer.Register(m.validatorSetsCached), registerer.Register(m.validatorSetsHeightDiff), @@ -140,22 +122,12 @@ type metrics struct { localStake prometheus.Gauge totalStake prometheus.Gauge - numVotesWon, numVotesLost prometheus.Counter - validatorSetsCached prometheus.Counter validatorSetsCreated prometheus.Counter validatorSetsHeightDiff prometheus.Gauge validatorSetsDuration prometheus.Gauge } -func (m *metrics) MarkOptionVoteWon() { - m.numVotesWon.Inc() -} - -func (m *metrics) MarkOptionVoteLost() { - m.numVotesLost.Inc() -} - func (m *metrics) MarkAccepted(b block.Block) error { return b.Visit(m.blockMetrics) } diff --git a/vms/platformvm/txs/executor/advance_time_test.go b/vms/platformvm/txs/executor/advance_time_test.go index 08d5ec6ae078..7c4b4884c904 100644 --- a/vms/platformvm/txs/executor/advance_time_test.go +++ b/vms/platformvm/txs/executor/advance_time_test.go @@ -814,35 +814,6 @@ func TestAdvanceTimeTxDelegatorStakers(t *testing.T) { require.Equal(env.config.MinDelegatorStake+env.config.MinValidatorStake, vdrWeight) } -// Test method InitiallyPrefersCommit -func TestAdvanceTimeTxInitiallyPrefersCommit(t *testing.T) { - require := require.New(t) - env := newEnvironment(t, false /*=postBanff*/, false /*=postCortina*/, false /*=postDurango*/) - env.ctx.Lock.Lock() - defer env.ctx.Lock.Unlock() - now := env.clk.Time() - - // Proposed advancing timestamp to 1 second after sync bound - tx, err := env.txBuilder.NewAdvanceTimeTx(now.Add(SyncBound)) - require.NoError(err) - - onCommitState, err := state.NewDiff(lastAcceptedID, env) - require.NoError(err) - - onAbortState, err := state.NewDiff(lastAcceptedID, env) - require.NoError(err) - - executor := ProposalTxExecutor{ - OnCommitState: onCommitState, - OnAbortState: onAbortState, - Backend: &env.backend, - Tx: tx, - } - require.NoError(tx.Unsigned.Visit(&executor)) - - require.True(executor.PrefersCommit, "should prefer to commit this tx because its proposed timestamp it's within sync bound") -} - func TestAdvanceTimeTxAfterBanff(t *testing.T) { require := require.New(t) env := newEnvironment(t, false /*=postBanff*/, false /*=postCortina*/, false /*=postDurango*/) diff --git a/vms/platformvm/txs/executor/backend.go b/vms/platformvm/txs/executor/backend.go index a5e017090701..847aefc16499 100644 --- a/vms/platformvm/txs/executor/backend.go +++ b/vms/platformvm/txs/executor/backend.go @@ -20,7 +20,7 @@ type Backend struct { Clk *mockable.Clock Fx fx.Fx FlowChecker utxo.Verifier - Uptimes uptime.Manager + Uptimes uptime.Calculator Rewards reward.Calculator Bootstrapped *utils.Atomic[bool] } diff --git a/vms/platformvm/txs/executor/proposal_tx_executor.go b/vms/platformvm/txs/executor/proposal_tx_executor.go index 773eeabad901..0f082cb8b296 100644 --- a/vms/platformvm/txs/executor/proposal_tx_executor.go +++ b/vms/platformvm/txs/executor/proposal_tx_executor.go @@ -10,7 +10,6 @@ import ( "github.com/ava-labs/avalanchego/database" "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/math" "github.com/ava-labs/avalanchego/vms/components/avax" "github.com/ava-labs/avalanchego/vms/components/verify" @@ -57,12 +56,6 @@ type ProposalTxExecutor struct { // [OnAbortState] is modified by this struct's methods to // reflect changes made to the state if the proposal is aborted. OnAbortState state.Diff - - // outputs populated by this struct's methods: - // - // [PrefersCommit] is true iff this node initially prefers to - // commit this block transaction. - PrefersCommit bool } func (*ProposalTxExecutor) CreateChainTx(*txs.CreateChainTx) error { @@ -149,8 +142,6 @@ func (e *ProposalTxExecutor) AddValidatorTx(tx *txs.AddValidatorTx) error { avax.Consume(e.OnAbortState, tx.Ins) // Produce the UTXOs avax.Produce(e.OnAbortState, txID, onAbortOuts) - - e.PrefersCommit = tx.StartTime().After(e.Clk.Time()) return nil } @@ -197,8 +188,6 @@ func (e *ProposalTxExecutor) AddSubnetValidatorTx(tx *txs.AddSubnetValidatorTx) avax.Consume(e.OnAbortState, tx.Ins) // Produce the UTXOs avax.Produce(e.OnAbortState, txID, tx.Outs) - - e.PrefersCommit = tx.StartTime().After(e.Clk.Time()) return nil } @@ -246,8 +235,6 @@ func (e *ProposalTxExecutor) AddDelegatorTx(tx *txs.AddDelegatorTx) error { avax.Consume(e.OnAbortState, tx.Ins) // Produce the UTXOs avax.Produce(e.OnAbortState, txID, onAbortOuts) - - e.PrefersCommit = tx.StartTime().After(e.Clk.Time()) return nil } @@ -296,14 +283,9 @@ func (e *ProposalTxExecutor) AdvanceTimeTx(tx *txs.AdvanceTimeTx) error { return err } - if _, err := AdvanceTimeTo(e.Backend, e.OnCommitState, newChainTime); err != nil { - return err - } - - e.PrefersCommit = !newChainTime.After(now.Add(SyncBound)) - // Note that state doesn't change if this proposal is aborted - return nil + _, err = AdvanceTimeTo(e.Backend, e.OnCommitState, newChainTime) + return err } func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error { @@ -347,17 +329,6 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error ) } - // retrieve primaryNetworkValidator before possibly removing it. - primaryNetworkValidator, err := e.OnCommitState.GetCurrentValidator( - constants.PrimaryNetworkID, - stakerToReward.NodeID, - ) - if err != nil { - // This should never error because the staker set is in memory and - // primary network validators are removed last. - return err - } - stakerTx, _, err := e.OnCommitState.GetTx(stakerToReward.TxID) if err != nil { return fmt.Errorf("failed to get next removed staker tx: %w", err) @@ -400,10 +371,7 @@ func (e *ProposalTxExecutor) RewardValidatorTx(tx *txs.RewardValidatorTx) error return err } e.OnAbortState.SetCurrentSupply(stakerToReward.SubnetID, newSupply) - - // handle option preference - e.PrefersCommit, err = e.shouldBeRewarded(stakerToReward, primaryNetworkValidator) - return err + return nil } func (e *ProposalTxExecutor) rewardValidatorTx(uValidatorTx txs.ValidatorTx, validator *state.Staker) error { @@ -640,26 +608,3 @@ func (e *ProposalTxExecutor) rewardDelegatorTx(uDelegatorTx txs.DelegatorTx, del } return nil } - -func (e *ProposalTxExecutor) shouldBeRewarded(stakerToReward, primaryNetworkValidator *state.Staker) (bool, error) { - expectedUptimePercentage := e.Config.UptimePercentage - if stakerToReward.SubnetID != constants.PrimaryNetworkID { - transformSubnet, err := GetTransformSubnetTx(e.OnCommitState, stakerToReward.SubnetID) - if err != nil { - return false, fmt.Errorf("failed to calculate uptime: %w", err) - } - - expectedUptimePercentage = float64(transformSubnet.UptimeRequirement) / reward.PercentDenominator - } - - // TODO: calculate subnet uptimes - uptime, err := e.Uptimes.CalculateUptimePercentFrom( - primaryNetworkValidator.NodeID, - constants.PrimaryNetworkID, - primaryNetworkValidator.StartTime, - ) - if err != nil { - return false, fmt.Errorf("failed to calculate uptime: %w", err) - } - return uptime >= expectedUptimePercentage, nil -} diff --git a/vms/platformvm/txs/reward_validator_tx.go b/vms/platformvm/txs/reward_validator_tx.go index 01b1e34bde46..85129af4695c 100644 --- a/vms/platformvm/txs/reward_validator_tx.go +++ b/vms/platformvm/txs/reward_validator_tx.go @@ -26,9 +26,6 @@ type RewardValidatorTx struct { // ID of the tx that created the delegator/validator being removed/rewarded TxID ids.ID `serialize:"true" json:"txID"` - // Marks if this validator should be rewarded according to this node. - ShouldPreferCommit bool `json:"-"` - unsignedBytes []byte // Unsigned byte representation of this data }