Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
e11c476
move subnet owner lookup to platformvm state
dhrubabasu Sep 14, 2023
2d66725
fix tests
dhrubabasu Sep 14, 2023
e83458d
moar
dhrubabasu Sep 14, 2023
8f63ba4
Merge branch 'dev' into simplify-subnet-auth-verification
dhrubabasu Sep 15, 2023
a6364db
helper
dhrubabasu Sep 15, 2023
895f0c7
Merge branch 'dev' into simplify-subnet-auth-verification
dhrubabasu Sep 15, 2023
5faf60b
alt impl without helper (#2025)
dhrubabasu Sep 15, 2023
d65e483
add `SetSubnetOwner` to `Chain` interface
dhrubabasu Sep 15, 2023
9b5ad9e
dedupe some code
dhrubabasu Sep 15, 2023
32ce3b8
wip
dhrubabasu Sep 15, 2023
192e2f1
merged
dhrubabasu Sep 15, 2023
2a94bdf
fix merge
dhrubabasu Sep 15, 2023
7d59881
merged
dhrubabasu Sep 21, 2023
f43dc90
`blocks` -> `block`
dhrubabasu Sep 21, 2023
5e8c152
fix
dhrubabasu Sep 21, 2023
07b74c6
nit
dhrubabasu Sep 21, 2023
645013f
Merge branch 'dev' into add-support-for-setting-subnet-owner
StephenButtolph Sep 21, 2023
e0890d4
pr review
dhrubabasu Sep 21, 2023
897ac65
nit
dhrubabasu Sep 21, 2023
c374609
Merge branch 'dev' into add-support-for-setting-subnet-owner
dhrubabasu Sep 21, 2023
54e6387
nits
dhrubabasu Sep 22, 2023
b3edbeb
nit
dhrubabasu Sep 22, 2023
60836b7
Merge branch 'dev' into add-support-for-setting-subnet-owner
dhrubabasu Sep 22, 2023
f1c86ca
Merge branch 'dev' into add-support-for-setting-subnet-owner
dhrubabasu Sep 22, 2023
bef73ab
nits
dhrubabasu Sep 28, 2023
28803b6
nit
dhrubabasu Sep 28, 2023
3a9bfaf
nit
dhrubabasu Sep 28, 2023
da17042
reduce diff
dhrubabasu Sep 28, 2023
2154de3
add a UT"
dhrubabasu Sep 28, 2023
4f0124a
UT on diff
dhrubabasu Sep 28, 2023
4436795
remove `ErrCantFindSubnet`
dhrubabasu Sep 28, 2023
85afb53
nits
dhrubabasu Sep 28, 2023
c6d03d2
Merge branch 'dev' into add-support-for-setting-subnet-owner
dhrubabasu Sep 28, 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
2 changes: 2 additions & 0 deletions vms/platformvm/config/execution_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var DefaultExecutionConfig = ExecutionConfig{
ChainCacheSize: 2048,
ChainDBCacheSize: 2048,
BlockIDCacheSize: 8192,
FxOwnerCacheSize: 4 * units.MiB,
ChecksumsEnabled: false,
}

Expand All @@ -29,6 +30,7 @@ type ExecutionConfig struct {
ChainCacheSize int `json:"chain-cache-size"`
ChainDBCacheSize int `json:"chain-db-cache-size"`
BlockIDCacheSize int `json:"block-id-cache-size"`
FxOwnerCacheSize int `json:"fx-owner-cache-size"`
ChecksumsEnabled bool `json:"checksums-enabled"`
}

Expand Down
2 changes: 2 additions & 0 deletions vms/platformvm/config/execution_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func TestExecutionConfigUnmarshal(t *testing.T) {
"chain-cache-size": 6,
"chain-db-cache-size": 7,
"block-id-cache-size": 8,
"fx-owner-cache-size": 9,
"checksums-enabled": true
}`)
ec, err := GetExecutionConfig(b)
Expand All @@ -58,6 +59,7 @@ func TestExecutionConfigUnmarshal(t *testing.T) {
ChainCacheSize: 6,
ChainDBCacheSize: 7,
BlockIDCacheSize: 8,
FxOwnerCacheSize: 9,
ChecksumsEnabled: true,
}
require.Equal(expected, ec)
Expand Down
18 changes: 8 additions & 10 deletions vms/platformvm/state/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func NewDiff(
parentID: parentID,
stateVersions: stateVersions,
timestamp: parentState.GetTimestamp(),
subnetOwners: make(map[ids.ID]fx.Owner),
}, nil
}

Expand Down Expand Up @@ -293,16 +294,6 @@ func (d *diff) AddSubnet(createSubnetTx *txs.Tx) {
if d.cachedSubnets != nil {
d.cachedSubnets = append(d.cachedSubnets, createSubnetTx)
}

castTx := createSubnetTx.Unsigned.(*txs.CreateSubnetTx)
subnetID := createSubnetTx.ID()
if d.subnetOwners == nil {
d.subnetOwners = map[ids.ID]fx.Owner{
subnetID: castTx.Owner,
}
} else {
d.subnetOwners[subnetID] = castTx.Owner
}
}

func (d *diff) GetSubnetOwner(subnetID ids.ID) (fx.Owner, error) {
Expand All @@ -319,6 +310,10 @@ func (d *diff) GetSubnetOwner(subnetID ids.ID) (fx.Owner, error) {
return parentState.GetSubnetOwner(subnetID)
}

func (d *diff) SetSubnetOwner(subnetID ids.ID, owner fx.Owner) {
d.subnetOwners[subnetID] = owner
}

func (d *diff) GetSubnetTransformation(subnetID ids.ID) (*txs.Tx, error) {
tx, exists := d.transformedSubnets[subnetID]
if exists {
Expand Down Expand Up @@ -562,5 +557,8 @@ func (d *diff) Apply(baseState State) error {
baseState.DeleteUTXO(utxoID)
}
}
for subnetID, owner := range d.subnetOwners {
baseState.SetSubnetOwner(subnetID, owner)
}
return nil
}
62 changes: 62 additions & 0 deletions vms/platformvm/state/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,3 +516,65 @@ func assertChainsEqual(t *testing.T, expected, actual Chain) {
}
}
}

func TestDiffSubnetOwner(t *testing.T) {
require := require.New(t)
ctrl := gomock.NewController(t)

state, _ := newInitializedState(require)

states := NewMockVersions(ctrl)
lastAcceptedID := ids.GenerateTestID()
states.EXPECT().GetState(lastAcceptedID).Return(state, true).AnyTimes()

var (
owner1 = fx.NewMockOwner(ctrl)
owner2 = fx.NewMockOwner(ctrl)

createSubnetTx = &txs.Tx{
Unsigned: &txs.CreateSubnetTx{
BaseTx: txs.BaseTx{},
Owner: owner1,
},
}

subnetID = createSubnetTx.ID()
)

// Create subnet on base state
owner, err := state.GetSubnetOwner(subnetID)
require.ErrorIs(err, database.ErrNotFound)
require.Nil(owner)

state.AddSubnet(createSubnetTx)
state.SetSubnetOwner(subnetID, owner1)

owner, err = state.GetSubnetOwner(subnetID)
require.NoError(err)
require.Equal(owner1, owner)

// Create diff and verify that subnet owner returns correctly
d, err := NewDiff(lastAcceptedID, states)
require.NoError(err)

owner, err = d.GetSubnetOwner(subnetID)
require.NoError(err)
require.Equal(owner1, owner)

// Transferring subnet ownership on diff should be reflected on diff not state
d.SetSubnetOwner(subnetID, owner2)
owner, err = d.GetSubnetOwner(subnetID)
require.NoError(err)
require.Equal(owner2, owner)

owner, err = state.GetSubnetOwner(subnetID)
require.NoError(err)
require.Equal(owner1, owner)

// State should reflect new subnet owner after diff is applied.
require.NoError(d.Apply(state))

owner, err = state.GetSubnetOwner(subnetID)
require.NoError(err)
require.Equal(owner2, owner)
}
12 changes: 12 additions & 0 deletions vms/platformvm/state/mock_chain.go

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

12 changes: 12 additions & 0 deletions vms/platformvm/state/mock_diff.go

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

12 changes: 12 additions & 0 deletions vms/platformvm/state/mock_state.go

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

97 changes: 90 additions & 7 deletions vms/platformvm/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ const (
var (
_ State = (*state)(nil)

ErrCantFindSubnet = errors.New("couldn't find subnet")
errMissingValidatorSet = errors.New("missing validator set")
errValidatorSetAlreadyPopulated = errors.New("validator set already populated")
errDuplicateValidatorSet = errors.New("duplicate validator set")
Expand All @@ -81,6 +80,7 @@ var (
rewardUTXOsPrefix = []byte("rewardUTXOs")
utxoPrefix = []byte("utxo")
subnetPrefix = []byte("subnet")
subnetOwnerPrefix = []byte("subnetOwner")
transformedSubnetPrefix = []byte("transformedSubnet")
supplyPrefix = []byte("supply")
chainPrefix = []byte("chain")
Expand Down Expand Up @@ -115,6 +115,7 @@ type Chain interface {
AddSubnet(createSubnetTx *txs.Tx)

GetSubnetOwner(subnetID ids.ID) (fx.Owner, error)
SetSubnetOwner(subnetID ids.ID, owner fx.Owner)

GetSubnetTransformation(subnetID ids.ID) (*txs.Tx, error)
AddSubnetTransformation(transformSubnetTx *txs.Tx)
Expand Down Expand Up @@ -274,6 +275,8 @@ type stateBlk struct {
* |-. subnets
* | '-. list
* | '-- txID -> nil
* |-. subnetOwners
* | '-. subnetID -> owner
* |-. chains
* | '-. subnetID
* | '-. list
Expand Down Expand Up @@ -352,6 +355,11 @@ type state struct {
subnetBaseDB database.Database
subnetDB linkeddb.LinkedDB

// Subnet ID --> Owner of the subnet
subnetOwners map[ids.ID]fx.Owner
subnetOwnerCache cache.Cacher[ids.ID, fxOwnerAndSize] // cache of subnetID -> owner if the entry is nil, it is not in the database
subnetOwnerDB database.Database

transformedSubnets map[ids.ID]*txs.Tx // map of subnetID -> transformSubnetTx
transformedSubnetCache cache.Cacher[ids.ID, *txs.Tx] // cache of subnetID -> transformSubnetTx if the entry is nil, it is not in the database
transformedSubnetDB database.Database
Expand Down Expand Up @@ -420,6 +428,11 @@ type txAndStatus struct {
status status.Status
}

type fxOwnerAndSize struct {
owner fx.Owner
size int
}

func txSize(_ ids.ID, tx *txs.Tx) int {
if tx == nil {
return ids.IDLen + constants.PointerOverhead
Expand Down Expand Up @@ -573,6 +586,18 @@ func newState(

subnetBaseDB := prefixdb.New(subnetPrefix, baseDB)

subnetOwnerDB := prefixdb.New(subnetOwnerPrefix, baseDB)
subnetOwnerCache, err := metercacher.New[ids.ID, fxOwnerAndSize](
"subnet_owner_cache",
metricsReg,
cache.NewSizedLRU[ids.ID, fxOwnerAndSize](execCfg.FxOwnerCacheSize, func(_ ids.ID, f fxOwnerAndSize) int {
return ids.IDLen + f.size
}),
)
if err != nil {
return nil, err
}

transformedSubnetCache, err := metercacher.New(
"transformed_subnet_cache",
metricsReg,
Expand Down Expand Up @@ -669,6 +694,10 @@ func newState(
subnetBaseDB: subnetBaseDB,
subnetDB: linkeddb.NewDefault(subnetBaseDB),

subnetOwners: make(map[ids.ID]fx.Owner),
subnetOwnerDB: subnetOwnerDB,
subnetOwnerCache: subnetOwnerCache,

transformedSubnets: make(map[ids.ID]*txs.Tx),
transformedSubnetCache: transformedSubnetCache,
transformedSubnetDB: prefixdb.New(transformedSubnetPrefix, baseDB),
Expand Down Expand Up @@ -819,24 +848,54 @@ func (s *state) AddSubnet(createSubnetTx *txs.Tx) {
}

func (s *state) GetSubnetOwner(subnetID ids.ID) (fx.Owner, error) {
if owner, exists := s.subnetOwners[subnetID]; exists {
return owner, nil
}

if ownerAndSize, cached := s.subnetOwnerCache.Get(subnetID); cached {
if ownerAndSize.owner == nil {
return nil, database.ErrNotFound
}
return ownerAndSize.owner, nil
}

ownerBytes, err := s.subnetOwnerDB.Get(subnetID[:])
if err == nil {
var owner fx.Owner
if _, err := block.GenesisCodec.Unmarshal(ownerBytes, &owner); err != nil {
return nil, err
}
s.subnetOwnerCache.Put(subnetID, fxOwnerAndSize{
owner: owner,
size: len(ownerBytes),
})
return owner, nil
}
if err != database.ErrNotFound {
return nil, err
}

subnetIntf, _, err := s.GetTx(subnetID)
if err != nil {
return nil, fmt.Errorf(
"%w %q: %w",
ErrCantFindSubnet,
subnetID,
err,
)
if err == database.ErrNotFound {
s.subnetOwnerCache.Put(subnetID, fxOwnerAndSize{})
}
return nil, err
}

subnet, ok := subnetIntf.Unsigned.(*txs.CreateSubnetTx)
if !ok {
return nil, fmt.Errorf("%q %w", subnetID, errIsNotSubnet)
}

s.SetSubnetOwner(subnetID, subnet.Owner)
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels a bit weird (to be performing a write on the read)... But I think it's ok

return subnet.Owner, nil
}

func (s *state) SetSubnetOwner(subnetID ids.ID, owner fx.Owner) {
s.subnetOwners[subnetID] = owner
}

func (s *state) GetSubnetTransformation(subnetID ids.ID) (*txs.Tx, error) {
if tx, exists := s.transformedSubnets[subnetID]; exists {
return tx, nil
Expand Down Expand Up @@ -1683,6 +1742,7 @@ func (s *state) write(updateValidators bool, height uint64) error {
s.writeRewardUTXOs(),
s.writeUTXOs(),
s.writeSubnets(),
s.writeSubnetOwners(),
s.writeTransformedSubnets(),
s.writeSubnetSupplies(),
s.writeChains(),
Expand Down Expand Up @@ -2273,6 +2333,29 @@ func (s *state) writeSubnets() error {
return nil
}

func (s *state) writeSubnetOwners() error {
for subnetID, owner := range s.subnetOwners {
subnetID := subnetID
owner := owner
Comment on lines +2338 to +2339
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought loop variables in go were created once and the value was updated with each iteration, so I thought we wouldn't need these copies here. I think it's safe to remove these lines but i could be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, it is safe to remove but I'd like to keep them since it's easier to understand that this is correct

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 the linter is going to yell at us because we are passing references to these values into functions. While it is true that those functions don't hold a reference to them... it feels like this is more clearly correct (until go 1.22)

delete(s.subnetOwners, subnetID)

ownerBytes, err := block.GenesisCodec.Marshal(block.Version, &owner)
if err != nil {
return fmt.Errorf("failed to marshal subnet owner: %w", err)
}

s.subnetOwnerCache.Put(subnetID, fxOwnerAndSize{
owner: owner,
size: len(ownerBytes),
})

if err := s.subnetOwnerDB.Put(subnetID[:], ownerBytes); err != nil {
return fmt.Errorf("failed to write subnet owner: %w", err)
}
}
return nil
}

func (s *state) writeTransformedSubnets() error {
for subnetID, tx := range s.transformedSubnets {
txID := tx.ID()
Expand Down
Loading