Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Update SoV struct to align with latest ACP-77 spec #3492

Merged
merged 5 commits into from
Oct 24, 2024
Merged

Update SoV struct to align with latest ACP-77 spec #3492

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
330d7ca
Update SoV struct to align with latest ACP-77 spec
StephenButtolph Oct 23, 2024
f5a0353
nit
StephenButtolph Oct 23, 2024
d4c1c31
nit
StephenButtolph Oct 23, 2024
033b180
rename
StephenButtolph Oct 24, 2024
3387878
tables tests are so out
StephenButtolph Oct 24, 2024
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
74 changes: 54 additions & 20 deletions vms/platformvm/state/subnet_only_validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,28 @@
package state

import (
"bytes"
"errors"
"fmt"

"github.com/google/btree"

"github.com/ava-labs/avalanchego/database"
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/utils"
"github.com/ava-labs/avalanchego/vms/platformvm/block"
)

var _ btree.LessFunc[*SubnetOnlyValidator] = (*SubnetOnlyValidator).Less
var (
_ btree.LessFunc[SubnetOnlyValidator] = SubnetOnlyValidator.Less
_ utils.Sortable[SubnetOnlyValidator] = SubnetOnlyValidator{}

ErrMutatedSubnetOnlyValidator = errors.New("subnet only validator contains mutated constant fields")
)

// SubnetOnlyValidator defines an ACP-77 validator. For a given ValidationID, it
// is expected for SubnetID, NodeID, PublicKey, RemainingBalanceOwner, and
// StartTime to be constant.
type SubnetOnlyValidator struct {
// ValidationID is not serialized because it is used as the key in the
// database, so it doesn't need to be stored in the value.
Expand All @@ -27,6 +38,14 @@ type SubnetOnlyValidator struct {
// guaranteed to be populated.
PublicKey []byte `serialize:"true"`

// RemainingBalanceOwner is the owner that will be used when returning the
// balance of the validator after removing accrued fees.
RemainingBalanceOwner []byte `serialize:"true"`

// DeactivationOwner is the owner that can manually deactivate the
// validator.
DeactivationOwner []byte `serialize:"true"`

// StartTime is the unix timestamp, in seconds, when this validator was
// added to the set.
StartTime uint64 `serialize:"true"`
Expand All @@ -46,44 +65,59 @@ type SubnetOnlyValidator struct {
// accrue before this validator must be deactivated. It is equal to the
// amount of fees this validator is willing to pay plus the amount of
// globally accumulated fees when this validator started validating.
//
// If this value is 0, the validator is inactive.
EndAccumulatedFee uint64 `serialize:"true"`
}

// Less determines a canonical ordering of *SubnetOnlyValidators based on their
// EndAccumulatedFees and ValidationIDs.
//
// Returns true if:
//
// 1. This validator has a lower EndAccumulatedFee than the other.
// 2. This validator has an equal EndAccumulatedFee to the other and has a
// lexicographically lower ValidationID.
func (v *SubnetOnlyValidator) Less(o *SubnetOnlyValidator) bool {
func (v SubnetOnlyValidator) Less(o SubnetOnlyValidator) bool {
return v.Compare(o) == -1
}

// Compare determines a canonical ordering of SubnetOnlyValidators based on
// their EndAccumulatedFees and ValidationIDs. Lower EndAccumulatedFees result
// in an earlier ordering.
func (v SubnetOnlyValidator) Compare(o SubnetOnlyValidator) int {
switch {
case v.EndAccumulatedFee < o.EndAccumulatedFee:
return true
return -1
case o.EndAccumulatedFee < v.EndAccumulatedFee:
return false
return 1
default:
return v.ValidationID.Compare(o.ValidationID) == -1
return v.ValidationID.Compare(o.ValidationID)
}
}

// validateConstants returns true if the constants of this validator have not
// been modified.
func (v SubnetOnlyValidator) validateConstants(o SubnetOnlyValidator) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

(No action required) Maybe rename to something more descriptive/a question answered by a boolean (e.g. constantsAreUnmodified)?

Also, is this method intended going to be used by non-test code in subsequent PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this helper is used is later PRs

if v.ValidationID != o.ValidationID {
return true
}
return v.SubnetID == o.SubnetID &&
v.NodeID == o.NodeID &&
bytes.Equal(v.PublicKey, o.PublicKey) &&
bytes.Equal(v.RemainingBalanceOwner, o.RemainingBalanceOwner) &&
bytes.Equal(v.DeactivationOwner, o.DeactivationOwner) &&
v.StartTime == o.StartTime
}

func getSubnetOnlyValidator(db database.KeyValueReader, validationID ids.ID) (*SubnetOnlyValidator, error) {
func getSubnetOnlyValidator(db database.KeyValueReader, validationID ids.ID) (SubnetOnlyValidator, error) {
bytes, err := db.Get(validationID[:])
if err != nil {
return nil, err
return SubnetOnlyValidator{}, err
}

vdr := &SubnetOnlyValidator{
vdr := SubnetOnlyValidator{
ValidationID: validationID,
}
if _, err = block.GenesisCodec.Unmarshal(bytes, vdr); err != nil {
return nil, fmt.Errorf("failed to unmarshal SubnetOnlyValidator: %w", err)
if _, err := block.GenesisCodec.Unmarshal(bytes, &vdr); err != nil {
return SubnetOnlyValidator{}, fmt.Errorf("failed to unmarshal SubnetOnlyValidator: %w", err)
}
return vdr, err
return vdr, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

(No action required) 😄

}

func putSubnetOnlyValidator(db database.KeyValueWriter, vdr *SubnetOnlyValidator) error {
func putSubnetOnlyValidator(db database.KeyValueWriter, vdr SubnetOnlyValidator) error {
bytes, err := block.GenesisCodec.Marshal(block.CodecVersion, vdr)
if err != nil {
return fmt.Errorf("failed to marshal SubnetOnlyValidator: %w", err)
Expand Down
202 changes: 173 additions & 29 deletions vms/platformvm/state/subnet_only_validator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,62 +12,184 @@ import (
"github.com/ava-labs/avalanchego/database"
"github.com/ava-labs/avalanchego/database/memdb"
"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/utils"
"github.com/ava-labs/avalanchego/utils/crypto/bls"
"github.com/ava-labs/avalanchego/vms/platformvm/block"
"github.com/ava-labs/avalanchego/vms/platformvm/fx"
"github.com/ava-labs/avalanchego/vms/secp256k1fx"
)

func TestSubnetOnlyValidator_Less(t *testing.T) {
func TestSubnetOnlyValidator_Compare(t *testing.T) {
tests := []struct {
name string
v *SubnetOnlyValidator
o *SubnetOnlyValidator
equal bool
name string
v SubnetOnlyValidator
o SubnetOnlyValidator
expected int
}{
{
name: "v.EndAccumulatedFee < o.EndAccumulatedFee",
v: &SubnetOnlyValidator{
v: SubnetOnlyValidator{
ValidationID: ids.GenerateTestID(),
EndAccumulatedFee: 1,
},
o: &SubnetOnlyValidator{
o: SubnetOnlyValidator{
ValidationID: ids.GenerateTestID(),
EndAccumulatedFee: 2,
},
equal: false,
expected: -1,
},
{
name: "v.EndAccumulatedFee = o.EndAccumulatedFee, v.ValidationID < o.ValidationID",
v: &SubnetOnlyValidator{
v: SubnetOnlyValidator{
ValidationID: ids.ID{0},
EndAccumulatedFee: 1,
},
o: &SubnetOnlyValidator{
o: SubnetOnlyValidator{
ValidationID: ids.ID{1},
EndAccumulatedFee: 1,
},
equal: false,
expected: -1,
},
{
name: "v.EndAccumulatedFee = o.EndAccumulatedFee, v.ValidationID = o.ValidationID",
v: &SubnetOnlyValidator{
v: SubnetOnlyValidator{
ValidationID: ids.ID{0},
EndAccumulatedFee: 1,
},
o: &SubnetOnlyValidator{
o: SubnetOnlyValidator{
ValidationID: ids.ID{0},
EndAccumulatedFee: 1,
},
equal: true,
expected: 0,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
require := require.New(t)

less := test.v.Less(test.o)
require.Equal(!test.equal, less)
require.Equal(test.expected, test.v.Compare(test.o))
require.Equal(-test.expected, test.o.Compare(test.v))
require.Equal(test.expected == -1, test.v.Less(test.o))
require.False(test.o.Less(test.v))
})
}
}

func TestSubnetOnlyValidator_validateConstants(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

(No action required) As much as I like test tables, maybe it would be easier to maintain a procedural approach that modified the parameter to validateConstants field-by-field?

sov := SubnetOnlyValidator{
ValidationID: ids.GenerateTestID(),
SubnetID: ids.GenerateTestID(),
NodeID: ids.GenerateTestNodeID(),
PublicKey: utils.RandomBytes(bls.PublicKeyLen),
RemainingBalanceOwner: utils.RandomBytes(32),
DeactivationOwner: utils.RandomBytes(32),
StartTime: rand.Uint64(), // #nosec G404
}

tests := []struct {
name string
v SubnetOnlyValidator
expected bool
}{
{
name: "equal",
v: sov,
expected: true,
},
{
name: "everything is different",
v: SubnetOnlyValidator{
ValidationID: ids.GenerateTestID(),
SubnetID: ids.GenerateTestID(),
NodeID: ids.GenerateTestNodeID(),
PublicKey: utils.RandomBytes(bls.PublicKeyLen),
RemainingBalanceOwner: utils.RandomBytes(32),
DeactivationOwner: utils.RandomBytes(32),
StartTime: rand.Uint64(), // #nosec G404
},
expected: true,
},
{
name: "different subnetID",
v: SubnetOnlyValidator{
ValidationID: sov.ValidationID,
SubnetID: ids.GenerateTestID(),
NodeID: sov.NodeID,
PublicKey: sov.PublicKey,
RemainingBalanceOwner: sov.RemainingBalanceOwner,
DeactivationOwner: sov.DeactivationOwner,
StartTime: sov.StartTime,
},
},
{
name: "different nodeID",
v: SubnetOnlyValidator{
ValidationID: sov.ValidationID,
SubnetID: sov.SubnetID,
NodeID: ids.GenerateTestNodeID(),
PublicKey: sov.PublicKey,
RemainingBalanceOwner: sov.RemainingBalanceOwner,
DeactivationOwner: sov.DeactivationOwner,
StartTime: sov.StartTime,
},
},
{
name: "different publicKey",
v: SubnetOnlyValidator{
ValidationID: sov.ValidationID,
SubnetID: sov.SubnetID,
NodeID: sov.NodeID,
PublicKey: utils.RandomBytes(bls.PublicKeyLen),
RemainingBalanceOwner: sov.RemainingBalanceOwner,
DeactivationOwner: sov.DeactivationOwner,
StartTime: sov.StartTime,
},
},
{
name: "different remainingBalanceOwner",
v: SubnetOnlyValidator{
ValidationID: sov.ValidationID,
SubnetID: sov.SubnetID,
NodeID: sov.NodeID,
PublicKey: sov.PublicKey,
RemainingBalanceOwner: utils.RandomBytes(32),
DeactivationOwner: sov.DeactivationOwner,
StartTime: sov.StartTime,
},
},
{
name: "different deactivationOwner",
v: SubnetOnlyValidator{
ValidationID: sov.ValidationID,
SubnetID: sov.SubnetID,
NodeID: sov.NodeID,
PublicKey: sov.PublicKey,
RemainingBalanceOwner: sov.RemainingBalanceOwner,
DeactivationOwner: utils.RandomBytes(32),
StartTime: sov.StartTime,
},
},
{
name: "different startTime",
v: SubnetOnlyValidator{
ValidationID: sov.ValidationID,
SubnetID: sov.SubnetID,
NodeID: sov.NodeID,
PublicKey: sov.PublicKey,
RemainingBalanceOwner: sov.RemainingBalanceOwner,
DeactivationOwner: sov.DeactivationOwner,
StartTime: rand.Uint64(), // #nosec G404
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// Randomize the unrelated fields
test.v.Weight = rand.Uint64() // #nosec G404
test.v.MinNonce = rand.Uint64() // #nosec G404
test.v.EndAccumulatedFee = rand.Uint64() // #nosec G404

greater := test.o.Less(test.v)
require.False(greater)
require.Equal(t, test.expected, sov.validateConstants(test.v))
})
}
}
Expand All @@ -78,22 +200,44 @@ func TestSubnetOnlyValidator_DatabaseHelpers(t *testing.T) {

sk, err := bls.NewSecretKey()
require.NoError(err)
pk := bls.PublicFromSecretKey(sk)
pkBytes := bls.PublicKeyToUncompressedBytes(pk)

var remainingBalanceOwner fx.Owner = &secp256k1fx.OutputOwners{
Threshold: 1,
Addrs: []ids.ShortID{
ids.GenerateTestShortID(),
},
}
remainingBalanceOwnerBytes, err := block.GenesisCodec.Marshal(block.CodecVersion, &remainingBalanceOwner)
require.NoError(err)

var deactivationOwner fx.Owner = &secp256k1fx.OutputOwners{
Threshold: 1,
Addrs: []ids.ShortID{
ids.GenerateTestShortID(),
},
}
deactivationOwnerBytes, err := block.GenesisCodec.Marshal(block.CodecVersion, &deactivationOwner)
require.NoError(err)

vdr := &SubnetOnlyValidator{
ValidationID: ids.GenerateTestID(),
SubnetID: ids.GenerateTestID(),
NodeID: ids.GenerateTestNodeID(),
PublicKey: bls.PublicKeyToUncompressedBytes(bls.PublicFromSecretKey(sk)),
StartTime: rand.Uint64(), // #nosec G404
Weight: rand.Uint64(), // #nosec G404
MinNonce: rand.Uint64(), // #nosec G404
EndAccumulatedFee: rand.Uint64(), // #nosec G404
vdr := SubnetOnlyValidator{
ValidationID: ids.GenerateTestID(),
SubnetID: ids.GenerateTestID(),
NodeID: ids.GenerateTestNodeID(),
PublicKey: pkBytes,
RemainingBalanceOwner: remainingBalanceOwnerBytes,
DeactivationOwner: deactivationOwnerBytes,
StartTime: rand.Uint64(), // #nosec G404
Weight: rand.Uint64(), // #nosec G404
MinNonce: rand.Uint64(), // #nosec G404
EndAccumulatedFee: rand.Uint64(), // #nosec G404
}

// Validator hasn't been put on disk yet
gotVdr, err := getSubnetOnlyValidator(db, vdr.ValidationID)
require.ErrorIs(err, database.ErrNotFound)
require.Nil(gotVdr)
require.Zero(gotVdr)

// Place the validator on disk
require.NoError(putSubnetOnlyValidator(db, vdr))
Expand All @@ -109,5 +253,5 @@ func TestSubnetOnlyValidator_DatabaseHelpers(t *testing.T) {
// Verify that the validator has been removed from disk
gotVdr, err = getSubnetOnlyValidator(db, vdr.ValidationID)
require.ErrorIs(err, database.ErrNotFound)
require.Nil(gotVdr)
require.Zero(gotVdr)
}
Loading