From ae5f19a1bf58ca11c4adedf10dfa9262fcc7becb Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Sat, 2 Nov 2024 11:31:19 -0400 Subject: [PATCH 1/2] Add caching to SoV DB helpers --- vms/platformvm/state/subnet_only_validator.go | 45 ++++++- .../state/subnet_only_validator_test.go | 111 ++++++++++++++---- 2 files changed, 128 insertions(+), 28 deletions(-) diff --git a/vms/platformvm/state/subnet_only_validator.go b/vms/platformvm/state/subnet_only_validator.go index 79033297e18b..5aa1f08295cf 100644 --- a/vms/platformvm/state/subnet_only_validator.go +++ b/vms/platformvm/state/subnet_only_validator.go @@ -10,9 +10,11 @@ import ( "github.com/google/btree" + "github.com/ava-labs/avalanchego/cache" "github.com/ava-labs/avalanchego/database" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils" + "github.com/ava-labs/avalanchego/utils/maybe" "github.com/ava-labs/avalanchego/vms/platformvm/block" ) @@ -21,6 +23,8 @@ var ( _ utils.Sortable[SubnetOnlyValidator] = SubnetOnlyValidator{} ErrMutatedSubnetOnlyValidator = errors.New("subnet only validator contains mutated constant fields") + + emptySoVCache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]] = &cache.Empty[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{} ) // SubnetOnlyValidator defines an ACP-77 validator. For a given ValidationID, it @@ -103,7 +107,18 @@ func (v SubnetOnlyValidator) immutableFieldsAreUnmodified(o SubnetOnlyValidator) v.StartTime == o.StartTime } -func getSubnetOnlyValidator(db database.KeyValueReader, validationID ids.ID) (SubnetOnlyValidator, error) { +func getSubnetOnlyValidator( + cache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]], + db database.KeyValueReader, + validationID ids.ID, +) (SubnetOnlyValidator, error) { + if maybeSOV, ok := cache.Get(validationID); ok { + if maybeSOV.IsNothing() { + return SubnetOnlyValidator{}, database.ErrNotFound + } + return maybeSOV.Value(), nil + } + bytes, err := db.Get(validationID[:]) if err != nil { return SubnetOnlyValidator{}, err @@ -118,14 +133,32 @@ func getSubnetOnlyValidator(db database.KeyValueReader, validationID ids.ID) (Su return vdr, nil } -func putSubnetOnlyValidator(db database.KeyValueWriter, vdr SubnetOnlyValidator) error { - bytes, err := block.GenesisCodec.Marshal(block.CodecVersion, vdr) +func putSubnetOnlyValidator( + db database.KeyValueWriter, + cache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]], + sov SubnetOnlyValidator, +) error { + bytes, err := block.GenesisCodec.Marshal(block.CodecVersion, sov) if err != nil { return fmt.Errorf("failed to marshal SubnetOnlyValidator: %w", err) } - return db.Put(vdr.ValidationID[:], bytes) + if err := db.Put(sov.ValidationID[:], bytes); err != nil { + return err + } + + cache.Put(sov.ValidationID, maybe.Some(sov)) + return nil } -func deleteSubnetOnlyValidator(db database.KeyValueDeleter, validationID ids.ID) error { - return db.Delete(validationID[:]) +func deleteSubnetOnlyValidator( + db database.KeyValueDeleter, + cache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]], + validationID ids.ID, +) error { + if err := db.Delete(validationID[:]); err != nil { + return err + } + + cache.Put(validationID, maybe.Nothing[SubnetOnlyValidator]()) + return nil } diff --git a/vms/platformvm/state/subnet_only_validator_test.go b/vms/platformvm/state/subnet_only_validator_test.go index 4f73cc287194..20f278654107 100644 --- a/vms/platformvm/state/subnet_only_validator_test.go +++ b/vms/platformvm/state/subnet_only_validator_test.go @@ -9,11 +9,13 @@ import ( "github.com/stretchr/testify/require" + "github.com/ava-labs/avalanchego/cache" "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/utils/maybe" "github.com/ava-labs/avalanchego/vms/platformvm/block" "github.com/ava-labs/avalanchego/vms/platformvm/fx" "github.com/ava-labs/avalanchego/vms/secp256k1fx" @@ -139,11 +141,8 @@ func TestSubnetOnlyValidator_immutableFieldsAreUnmodified(t *testing.T) { } func TestSubnetOnlyValidator_DatabaseHelpers(t *testing.T) { - require := require.New(t) - db := memdb.New() - sk, err := bls.NewSecretKey() - require.NoError(err) + require.NoError(t, err) pk := bls.PublicFromSecretKey(sk) pkBytes := bls.PublicKeyToUncompressedBytes(pk) @@ -154,7 +153,7 @@ func TestSubnetOnlyValidator_DatabaseHelpers(t *testing.T) { }, } remainingBalanceOwnerBytes, err := block.GenesisCodec.Marshal(block.CodecVersion, &remainingBalanceOwner) - require.NoError(err) + require.NoError(t, err) var deactivationOwner fx.Owner = &secp256k1fx.OutputOwners{ Threshold: 1, @@ -163,9 +162,9 @@ func TestSubnetOnlyValidator_DatabaseHelpers(t *testing.T) { }, } deactivationOwnerBytes, err := block.GenesisCodec.Marshal(block.CodecVersion, &deactivationOwner) - require.NoError(err) + require.NoError(t, err) - vdr := SubnetOnlyValidator{ + sov := SubnetOnlyValidator{ ValidationID: ids.GenerateTestID(), SubnetID: ids.GenerateTestID(), NodeID: ids.GenerateTestNodeID(), @@ -178,24 +177,92 @@ func TestSubnetOnlyValidator_DatabaseHelpers(t *testing.T) { 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.Zero(gotVdr) + var ( + addedDB = memdb.New() + removedDB = memdb.New() + addedAndRemovedDB = memdb.New() + addedAndRemovedAndAddedDB = memdb.New() + + addedCache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} + removedCache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} + addedAndRemovedCache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} + addedAndRemovedAndAddedCache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} + ) // Place the validator on disk - require.NoError(putSubnetOnlyValidator(db, vdr)) + require.NoError(t, putSubnetOnlyValidator(addedDB, addedCache, sov)) + require.NoError(t, putSubnetOnlyValidator(addedAndRemovedDB, addedAndRemovedCache, sov)) + require.NoError(t, putSubnetOnlyValidator(addedAndRemovedAndAddedDB, addedAndRemovedAndAddedCache, sov)) - // Verify that the validator can be fetched from disk - gotVdr, err = getSubnetOnlyValidator(db, vdr.ValidationID) - require.NoError(err) - require.Equal(vdr, gotVdr) + // Remove the validator on disk + require.NoError(t, deleteSubnetOnlyValidator(removedDB, removedCache, sov.ValidationID)) + require.NoError(t, deleteSubnetOnlyValidator(addedAndRemovedDB, addedAndRemovedCache, sov.ValidationID)) + require.NoError(t, deleteSubnetOnlyValidator(addedAndRemovedAndAddedDB, addedAndRemovedAndAddedCache, sov.ValidationID)) - // Remove the validator from disk - require.NoError(deleteSubnetOnlyValidator(db, vdr.ValidationID)) + // Reintroduce the validator to disk + require.NoError(t, putSubnetOnlyValidator(addedAndRemovedAndAddedDB, addedAndRemovedAndAddedCache, sov)) - // Verify that the validator has been removed from disk - gotVdr, err = getSubnetOnlyValidator(db, vdr.ValidationID) - require.ErrorIs(err, database.ErrNotFound) - require.Zero(gotVdr) + addedTests := []struct { + name string + db database.Database + cache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]] + }{ + { + name: "added in cache", + db: memdb.New(), + cache: addedCache, + }, + { + name: "added on disk", + db: addedDB, + cache: emptySoVCache, + }, + { + name: "added and removed and added in cache", + db: memdb.New(), + cache: addedAndRemovedAndAddedCache, + }, + { + name: "added and removed and added on disk", + db: addedAndRemovedAndAddedDB, + cache: emptySoVCache, + }, + } + for _, test := range addedTests { + t.Run(test.name, func(t *testing.T) { + require := require.New(t) + + gotSOV, err := getSubnetOnlyValidator(test.cache, test.db, sov.ValidationID) + require.NoError(err) + require.Equal(sov, gotSOV) + }) + } + + removedTests := []struct { + name string + db database.Database + cache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]] + }{ + { + name: "empty", + db: memdb.New(), + cache: emptySoVCache, + }, + { + name: "removed from cache", + db: addedDB, + cache: removedCache, + }, + { + name: "removed from disk", + db: removedDB, + cache: emptySoVCache, + }, + } + for _, test := range removedTests { + t.Run(test.name, func(t *testing.T) { + _, err := getSubnetOnlyValidator(test.cache, test.db, sov.ValidationID) + require.ErrorIs(t, err, database.ErrNotFound) + }) + } } From 405695147820d8ce2292cf0a0167524d9d65b702 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Mon, 4 Nov 2024 12:47:28 -0500 Subject: [PATCH 2/2] Improve caching and split out tests --- vms/platformvm/state/subnet_only_validator.go | 14 +- .../state/subnet_only_validator_test.go | 216 ++++++++---------- 2 files changed, 106 insertions(+), 124 deletions(-) diff --git a/vms/platformvm/state/subnet_only_validator.go b/vms/platformvm/state/subnet_only_validator.go index 5aa1f08295cf..1e078e028ce2 100644 --- a/vms/platformvm/state/subnet_only_validator.go +++ b/vms/platformvm/state/subnet_only_validator.go @@ -23,8 +23,6 @@ var ( _ utils.Sortable[SubnetOnlyValidator] = SubnetOnlyValidator{} ErrMutatedSubnetOnlyValidator = errors.New("subnet only validator contains mutated constant fields") - - emptySoVCache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]] = &cache.Empty[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{} ) // SubnetOnlyValidator defines an ACP-77 validator. For a given ValidationID, it @@ -120,17 +118,23 @@ func getSubnetOnlyValidator( } bytes, err := db.Get(validationID[:]) + if err == database.ErrNotFound { + cache.Put(validationID, maybe.Nothing[SubnetOnlyValidator]()) + return SubnetOnlyValidator{}, database.ErrNotFound + } if err != nil { return SubnetOnlyValidator{}, err } - vdr := SubnetOnlyValidator{ + sov := SubnetOnlyValidator{ ValidationID: validationID, } - if _, err := block.GenesisCodec.Unmarshal(bytes, &vdr); err != nil { + if _, err := block.GenesisCodec.Unmarshal(bytes, &sov); err != nil { return SubnetOnlyValidator{}, fmt.Errorf("failed to unmarshal SubnetOnlyValidator: %w", err) } - return vdr, nil + + cache.Put(validationID, maybe.Some(sov)) + return sov, nil } func putSubnetOnlyValidator( diff --git a/vms/platformvm/state/subnet_only_validator_test.go b/vms/platformvm/state/subnet_only_validator_test.go index 20f278654107..3ffe080e0723 100644 --- a/vms/platformvm/state/subnet_only_validator_test.go +++ b/vms/platformvm/state/subnet_only_validator_test.go @@ -17,8 +17,6 @@ import ( "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/maybe" "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_Compare(t *testing.T) { @@ -79,17 +77,6 @@ func TestSubnetOnlyValidator_Compare(t *testing.T) { func TestSubnetOnlyValidator_immutableFieldsAreUnmodified(t *testing.T) { var ( - randomSOV = func() SubnetOnlyValidator { - return 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 - } - } randomizeSOV = func(sov SubnetOnlyValidator) SubnetOnlyValidator { // Randomize unrelated fields sov.Weight = rand.Uint64() // #nosec G404 @@ -97,7 +84,7 @@ func TestSubnetOnlyValidator_immutableFieldsAreUnmodified(t *testing.T) { sov.EndAccumulatedFee = rand.Uint64() // #nosec G404 return sov } - sov = randomSOV() + sov = newSoV() ) t.Run("equal", func(t *testing.T) { @@ -105,7 +92,7 @@ func TestSubnetOnlyValidator_immutableFieldsAreUnmodified(t *testing.T) { require.True(t, sov.immutableFieldsAreUnmodified(v)) }) t.Run("everything is different", func(t *testing.T) { - v := randomizeSOV(randomSOV()) + v := randomizeSOV(newSoV()) require.True(t, sov.immutableFieldsAreUnmodified(v)) }) t.Run("different subnetID", func(t *testing.T) { @@ -140,129 +127,120 @@ func TestSubnetOnlyValidator_immutableFieldsAreUnmodified(t *testing.T) { }) } -func TestSubnetOnlyValidator_DatabaseHelpers(t *testing.T) { - sk, err := bls.NewSecretKey() - require.NoError(t, 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(t, err) - - var deactivationOwner fx.Owner = &secp256k1fx.OutputOwners{ - Threshold: 1, - Addrs: []ids.ShortID{ - ids.GenerateTestShortID(), - }, - } - deactivationOwnerBytes, err := block.GenesisCodec.Marshal(block.CodecVersion, &deactivationOwner) - require.NoError(t, err) - - sov := 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 - } - +func TestGetSubnetOnlyValidator(t *testing.T) { var ( - addedDB = memdb.New() - removedDB = memdb.New() - addedAndRemovedDB = memdb.New() - addedAndRemovedAndAddedDB = memdb.New() - - addedCache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} - removedCache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} - addedAndRemovedCache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} - addedAndRemovedAndAddedCache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} + sov = newSoV() + dbWithSoV = memdb.New() + dbWithoutSoV = memdb.New() + cacheWithSoV = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} + cacheWithoutSoV = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} ) - // Place the validator on disk - require.NoError(t, putSubnetOnlyValidator(addedDB, addedCache, sov)) - require.NoError(t, putSubnetOnlyValidator(addedAndRemovedDB, addedAndRemovedCache, sov)) - require.NoError(t, putSubnetOnlyValidator(addedAndRemovedAndAddedDB, addedAndRemovedAndAddedCache, sov)) + require.NoError(t, putSubnetOnlyValidator(dbWithSoV, cacheWithSoV, sov)) + require.NoError(t, deleteSubnetOnlyValidator(dbWithoutSoV, cacheWithoutSoV, sov.ValidationID)) - // Remove the validator on disk - require.NoError(t, deleteSubnetOnlyValidator(removedDB, removedCache, sov.ValidationID)) - require.NoError(t, deleteSubnetOnlyValidator(addedAndRemovedDB, addedAndRemovedCache, sov.ValidationID)) - require.NoError(t, deleteSubnetOnlyValidator(addedAndRemovedAndAddedDB, addedAndRemovedAndAddedCache, sov.ValidationID)) - - // Reintroduce the validator to disk - require.NoError(t, putSubnetOnlyValidator(addedAndRemovedAndAddedDB, addedAndRemovedAndAddedCache, sov)) - - addedTests := []struct { - name string - db database.Database - cache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]] + tests := []struct { + name string + cache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]] + db database.KeyValueReader + expectedSoV SubnetOnlyValidator + expectedErr error + expectedEntry maybe.Maybe[SubnetOnlyValidator] }{ { - name: "added in cache", - db: memdb.New(), - cache: addedCache, + name: "cached with validator", + cache: cacheWithSoV, + db: dbWithoutSoV, + expectedSoV: sov, + expectedEntry: maybe.Some(sov), }, { - name: "added on disk", - db: addedDB, - cache: emptySoVCache, + name: "from disk with validator", + cache: &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10}, + db: dbWithSoV, + expectedSoV: sov, + expectedEntry: maybe.Some(sov), }, { - name: "added and removed and added in cache", - db: memdb.New(), - cache: addedAndRemovedAndAddedCache, + name: "cached without validator", + cache: cacheWithoutSoV, + db: dbWithSoV, + expectedErr: database.ErrNotFound, }, { - name: "added and removed and added on disk", - db: addedAndRemovedAndAddedDB, - cache: emptySoVCache, + name: "from disk without validator", + cache: &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10}, + db: dbWithoutSoV, + expectedErr: database.ErrNotFound, }, } - for _, test := range addedTests { + for _, test := range tests { t.Run(test.name, func(t *testing.T) { require := require.New(t) - gotSOV, err := getSubnetOnlyValidator(test.cache, test.db, sov.ValidationID) - require.NoError(err) - require.Equal(sov, gotSOV) + gotSoV, err := getSubnetOnlyValidator(test.cache, test.db, sov.ValidationID) + require.ErrorIs(err, test.expectedErr) + require.Equal(test.expectedSoV, gotSoV) + + cachedSoV, ok := test.cache.Get(sov.ValidationID) + require.True(ok) + require.Equal(test.expectedEntry, cachedSoV) }) } +} - removedTests := []struct { - name string - db database.Database - cache cache.Cacher[ids.ID, maybe.Maybe[SubnetOnlyValidator]] - }{ - { - name: "empty", - db: memdb.New(), - cache: emptySoVCache, - }, - { - name: "removed from cache", - db: addedDB, - cache: removedCache, - }, - { - name: "removed from disk", - db: removedDB, - cache: emptySoVCache, - }, - } - for _, test := range removedTests { - t.Run(test.name, func(t *testing.T) { - _, err := getSubnetOnlyValidator(test.cache, test.db, sov.ValidationID) - require.ErrorIs(t, err, database.ErrNotFound) - }) +func TestPutSubnetOnlyValidator(t *testing.T) { + var ( + require = require.New(t) + sov = newSoV() + db = memdb.New() + cache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} + ) + expectedSoVBytes, err := block.GenesisCodec.Marshal(block.CodecVersion, sov) + require.NoError(err) + + require.NoError(putSubnetOnlyValidator(db, cache, sov)) + + sovBytes, err := db.Get(sov.ValidationID[:]) + require.NoError(err) + require.Equal(expectedSoVBytes, sovBytes) + + sovFromCache, ok := cache.Get(sov.ValidationID) + require.True(ok) + require.Equal(maybe.Some(sov), sovFromCache) +} + +func TestDeleteSubnetOnlyValidator(t *testing.T) { + var ( + require = require.New(t) + validationID = ids.GenerateTestID() + db = memdb.New() + cache = &cache.LRU[ids.ID, maybe.Maybe[SubnetOnlyValidator]]{Size: 10} + ) + require.NoError(db.Put(validationID[:], nil)) + + require.NoError(deleteSubnetOnlyValidator(db, cache, validationID)) + + hasSoV, err := db.Has(validationID[:]) + require.NoError(err) + require.False(hasSoV) + + sovFromCache, ok := cache.Get(validationID) + require.True(ok) + require.Equal(maybe.Nothing[SubnetOnlyValidator](), sovFromCache) +} + +func newSoV() SubnetOnlyValidator { + return 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 + Weight: rand.Uint64(), // #nosec G404 + MinNonce: rand.Uint64(), // #nosec G404 + EndAccumulatedFee: rand.Uint64(), // #nosec G404 } }