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

[vms/platformvm] Replace GetSubnets with GetSubnetIDs in State #3055

Merged
merged 9 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
35 changes: 22 additions & 13 deletions vms/platformvm/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -527,14 +527,13 @@ func (s *Service) GetSubnets(_ *http.Request, args *GetSubnetsArgs, response *Ge

getAll := len(args.IDs) == 0
if getAll {
subnets, err := s.vm.state.GetSubnets() // all subnets
subnetIDs, err := s.vm.state.GetSubnetIDs() // all subnets
if err != nil {
return fmt.Errorf("error getting subnets from database: %w", err)
}

response.Subnets = make([]APISubnet, len(subnets)+1)
for i, subnet := range subnets {
subnetID := subnet.ID()
response.Subnets = make([]APISubnet, len(subnetIDs)+1)
for i, subnetID := range subnetIDs {
if _, err := s.vm.state.GetSubnetTransformation(subnetID); err == nil {
response.Subnets[i] = APISubnet{
ID: subnetID,
Expand All @@ -544,15 +543,26 @@ func (s *Service) GetSubnets(_ *http.Request, args *GetSubnetsArgs, response *Ge
continue
}

unsignedTx := subnet.Unsigned.(*txs.CreateSubnetTx)
owner := unsignedTx.Owner.(*secp256k1fx.OutputOwners)
controlAddrs := []string{}
for _, controlKeyID := range owner.Addrs {
subnetOwner, err := s.vm.state.GetSubnetOwner(subnetID)
if err == database.ErrNotFound {
continue
}
if err != nil {
return err
}

owner, ok := subnetOwner.(*secp256k1fx.OutputOwners)
if !ok {
return fmt.Errorf("expected *secp256k1fx.OutputOwners but got %T", subnetOwner)
}

controlAddrs := make([]string, len(owner.Addrs))
for i, controlKeyID := range owner.Addrs {
addr, err := s.addrManager.FormatLocalAddress(controlKeyID)
if err != nil {
return fmt.Errorf("problem formatting address: %w", err)
}
controlAddrs = append(controlAddrs, addr)
controlAddrs[i] = addr
}
response.Subnets[i] = APISubnet{
ID: subnetID,
Expand All @@ -561,7 +571,7 @@ func (s *Service) GetSubnets(_ *http.Request, args *GetSubnetsArgs, response *Ge
}
}
// Include primary network
response.Subnets[len(subnets)] = APISubnet{
response.Subnets[len(subnetIDs)] = APISubnet{
ID: constants.PrimaryNetworkID,
ControlKeys: []string{},
Threshold: avajson.Uint32(0),
Expand Down Expand Up @@ -1226,14 +1236,13 @@ func (s *Service) GetBlockchains(_ *http.Request, _ *struct{}, response *GetBloc
s.vm.ctx.Lock.Lock()
defer s.vm.ctx.Lock.Unlock()

subnets, err := s.vm.state.GetSubnets()
subnetIDs, err := s.vm.state.GetSubnetIDs()
if err != nil {
return fmt.Errorf("couldn't retrieve subnets: %w", err)
}

response.Blockchains = []APIBlockchain{}
for _, subnet := range subnets {
subnetID := subnet.ID()
for _, subnetID := range subnetIDs {
chains, err := s.vm.state.GetChains(subnetID)
if err != nil {
return fmt.Errorf(
Expand Down
50 changes: 50 additions & 0 deletions vms/platformvm/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1041,3 +1041,53 @@ func TestServiceGetBlockByHeight(t *testing.T) {
})
}
}

func TestServiceGetSubnets(t *testing.T) {
require := require.New(t)
service, _, _ := defaultService(t)

testSubnet1ID := testSubnet1.ID()

var response GetSubnetsResponse
require.NoError(service.GetSubnets(nil, &GetSubnetsArgs{}, &response))
require.Equal([]APISubnet{
{
ID: testSubnet1ID,
ControlKeys: []string{
"P-testing1d6kkj0qh4wcmus3tk59npwt3rluc6en72ngurd",
"P-testing17fpqs358de5lgu7a5ftpw2t8axf0pm33983krk",
"P-testing1lnk637g0edwnqc2tn8tel39652fswa3xk4r65e",
},
Threshold: 2,
},
{
ID: constants.PrimaryNetworkID,
ControlKeys: []string{},
Threshold: 0,
},
}, response.Subnets)

newOwnerIDStr := "P-testing1t73fa4p4dypa4s3kgufuvr6hmprjclw66mgqgm"
newOwnerID, err := service.addrManager.ParseLocalAddress(newOwnerIDStr)
require.NoError(err)
service.vm.state.SetSubnetOwner(testSubnet1ID, &secp256k1fx.OutputOwners{
Addrs: []ids.ShortID{newOwnerID},
Threshold: 1,
})

require.NoError(service.GetSubnets(nil, &GetSubnetsArgs{}, &response))
require.Equal([]APISubnet{
{
ID: testSubnet1ID,
ControlKeys: []string{
newOwnerIDStr,
},
Threshold: 1,
},
{
ID: constants.PrimaryNetworkID,
ControlKeys: []string{},
Threshold: 0,
},
}, response.Subnets)
}
18 changes: 9 additions & 9 deletions vms/platformvm/state/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,11 @@ func TestDiffSubnet(t *testing.T) {
state.AddSubnet(parentStateCreateSubnetTx)

// Verify parent returns one subnet
subnets, err := state.GetSubnets()
subnetIDs, err := state.GetSubnetIDs()
require.NoError(err)
require.Equal([]*txs.Tx{
parentStateCreateSubnetTx,
}, subnets)
require.Equal([]ids.ID{
parentStateCreateSubnetTx.ID(),
}, subnetIDs)

states := NewMockVersions(ctrl)
lastAcceptedID := ids.GenerateTestID()
Expand All @@ -285,12 +285,12 @@ func TestDiffSubnet(t *testing.T) {
require.NoError(diff.Apply(state))

// Verify parent now returns two subnets
subnets, err = state.GetSubnets()
subnetIDs, err = state.GetSubnetIDs()
require.NoError(err)
require.Equal([]*txs.Tx{
parentStateCreateSubnetTx,
createSubnetTx,
}, subnets)
require.Equal([]ids.ID{
parentStateCreateSubnetTx.ID(),
createSubnetTx.ID(),
}, subnetIDs)
}

func TestDiffChain(t *testing.T) {
Expand Down
30 changes: 15 additions & 15 deletions vms/platformvm/state/mock_state.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 20 additions & 24 deletions vms/platformvm/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ type State interface {
GetBlockIDAtHeight(height uint64) (ids.ID, error)

GetRewardUTXOs(txID ids.ID) ([]*avax.UTXO, error)
GetSubnets() ([]*txs.Tx, error)
GetSubnetIDs() ([]ids.ID, error)
GetChains(subnetID ids.ID) ([]*txs.Tx, error)

// ApplyValidatorWeightDiffs iterates from [startHeight] towards the genesis
Expand Down Expand Up @@ -330,10 +330,10 @@ type state struct {
utxoDB database.Database
utxoState avax.UTXOState

cachedSubnets []*txs.Tx // nil if the subnets haven't been loaded
addedSubnets []*txs.Tx
subnetBaseDB database.Database
subnetDB linkeddb.LinkedDB
cachedSubnetIDs []ids.ID // nil if the subnets haven't been loaded
addedSubnetIDs []ids.ID
subnetBaseDB database.Database
subnetDB linkeddb.LinkedDB

// Subnet ID --> Owner of the subnet
subnetOwners map[ids.ID]fx.Owner
Expand Down Expand Up @@ -728,39 +728,37 @@ func (s *state) doneInit() error {
return s.singletonDB.Put(InitializedKey, nil)
}

func (s *state) GetSubnets() ([]*txs.Tx, error) {
if s.cachedSubnets != nil {
return s.cachedSubnets, nil
func (s *state) GetSubnetIDs() ([]ids.ID, error) {
if s.cachedSubnetIDs != nil {
return s.cachedSubnetIDs, nil
}

subnetDBIt := s.subnetDB.NewIterator()
defer subnetDBIt.Release()

txs := []*txs.Tx(nil)
subnetIDs := []ids.ID{}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a super small change in behavior, but I think it's a good change. In the case that there are no subnets (which isn't realistic in most cases outside of testing), previously we wouldn't cache the lookup. Now we will cache the lookup as there being no subnets.

for subnetDBIt.Next() {
subnetIDBytes := subnetDBIt.Key()
subnetID, err := ids.ToID(subnetIDBytes)
if err != nil {
return nil, err
}
subnetTx, _, err := s.GetTx(subnetID)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this GetTx call from every invocation is the main benefit of this PR

if err != nil {
return nil, err
}
txs = append(txs, subnetTx)
subnetIDs = append(subnetIDs, subnetID)
}
if err := subnetDBIt.Error(); err != nil {
return nil, err
}
txs = append(txs, s.addedSubnets...)
s.cachedSubnets = txs
return txs, nil
subnetIDs = append(subnetIDs, s.addedSubnetIDs...)
s.cachedSubnetIDs = subnetIDs
return subnetIDs, nil
}

func (s *state) AddSubnet(createSubnetTx *txs.Tx) {
dhrubabasu marked this conversation as resolved.
Show resolved Hide resolved
s.addedSubnets = append(s.addedSubnets, createSubnetTx)
if s.cachedSubnets != nil {
s.cachedSubnets = append(s.cachedSubnets, createSubnetTx)
createSubnetTxID := createSubnetTx.ID()

s.addedSubnetIDs = append(s.addedSubnetIDs, createSubnetTxID)
if s.cachedSubnetIDs != nil {
s.cachedSubnetIDs = append(s.cachedSubnetIDs, createSubnetTxID)
}
}

Expand Down Expand Up @@ -2185,14 +2183,12 @@ func (s *state) writeUTXOs() error {
}

func (s *state) writeSubnets() error {
for _, subnet := range s.addedSubnets {
subnetID := subnet.ID()

for _, subnetID := range s.addedSubnetIDs {
if err := s.subnetDB.Put(subnetID[:], nil); err != nil {
return fmt.Errorf("failed to write subnet: %w", err)
}
}
s.addedSubnets = nil
s.addedSubnetIDs = nil
return nil
}

Expand Down
6 changes: 3 additions & 3 deletions vms/platformvm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,12 @@ func (vm *VM) initBlockchains() error {
}
}
} else {
subnets, err := vm.state.GetSubnets()
subnetIDs, err := vm.state.GetSubnetIDs()
if err != nil {
return err
}
for _, subnet := range subnets {
if err := vm.createSubnet(subnet.ID()); err != nil {
for _, subnetID := range subnetIDs {
if err := vm.createSubnet(subnetID); err != nil {
return err
}
}
Expand Down
6 changes: 3 additions & 3 deletions vms/platformvm/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -958,12 +958,12 @@ func TestCreateSubnet(t *testing.T) {
require.NoError(err)
require.Equal(status.Committed, txStatus)

subnets, err := vm.state.GetSubnets()
subnetIDs, err := vm.state.GetSubnetIDs()
require.NoError(err)

found := false
for _, subnet := range subnets {
if subnet.ID() == createSubnetTx.ID() {
for _, subnetID := range subnetIDs {
if subnetID == createSubnetTx.ID() {
dhrubabasu marked this conversation as resolved.
Show resolved Hide resolved
found = true
break
}
Expand Down
Loading