From 64600c47e82cdba2d9a8668661fe42b172cec86a Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Thu, 30 Mar 2023 12:44:21 -0400 Subject: [PATCH 01/43] Add Proof size limit to client --- x/merkledb/history.go | 41 ++--- x/sync/README.md | 4 + x/sync/client.go | 22 ++- x/sync/client_test.go | 93 +++++++---- x/sync/network_server.go | 215 ++++++++++++++++++++----- x/sync/network_server_test.go | 289 ++++++++++++++++++++++++++++++++++ x/sync/request.go | 22 +-- x/sync/sync_test.go | 26 ++- x/sync/syncmanager.go | 20 +-- 9 files changed, 609 insertions(+), 123 deletions(-) create mode 100644 x/sync/README.md create mode 100644 x/sync/network_server_test.go diff --git a/x/merkledb/history.go b/x/merkledb/history.go index fbaeca3bd8e3..cb985873e129 100644 --- a/x/merkledb/history.go +++ b/x/merkledb/history.go @@ -97,27 +97,32 @@ func (th *trieHistory) getValueChanges(startRoot, endRoot ids.ID, start, end []b } // [lastStartRootChange] is the latest appearance of [startRoot] - // which came before [lastEndRootChange]. - var lastStartRootChange *changeSummaryAndIndex - th.history.DescendLessOrEqual( - lastEndRootChange, - func(item *changeSummaryAndIndex) bool { - if item == lastEndRootChange { - return true // Skip first iteration - } - if item.rootID == startRoot { - lastStartRootChange = item - return false - } - return true - }, - ) - - // There's no change resulting in [startRoot] before the latest change resulting in [endRoot]. - if lastStartRootChange == nil { + lastStartRootChange, ok := th.lastChanges[startRoot] + if !ok { return nil, ErrStartRootNotFound } + // if lastStartRootChange is after the lastEndRootChange, then attempt to find an entry that comes before lastEndRootChange + if lastStartRootChange.index > lastEndRootChange.index { + th.history.DescendLessOrEqual( + lastEndRootChange, + func(item *changeSummaryAndIndex) bool { + if item == lastEndRootChange { + return true // Skip first iteration + } + if item.rootID == startRoot { + lastStartRootChange = item + return false + } + return true + }, + ) + // There's no change resulting in [startRoot] before the latest change resulting in [endRoot]. + if lastStartRootChange == nil { + return nil, ErrStartRootNotFound + } + } + // Keep changes sorted so the largest can be removed in order to stay within the maxLength limit. sortedKeys := btree.NewG( 2, diff --git a/x/sync/README.md b/x/sync/README.md new file mode 100644 index 000000000000..ba44e8b8088c --- /dev/null +++ b/x/sync/README.md @@ -0,0 +1,4 @@ +## TODOs + +- [ ] Handle errors on proof requests. Currently, any errors that occur server side are not sent back to the client. +- [ ] Handle missing roots in change proofs by returning a range proof instead of failing diff --git a/x/sync/client.go b/x/sync/client.go index 99416f0dc3f6..903d4f66ea51 100644 --- a/x/sync/client.go +++ b/x/sync/client.go @@ -10,12 +10,13 @@ import ( "sync/atomic" "time" + "github.com/ava-labs/avalanchego/x/merkledb" + "go.uber.org/zap" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/version" - "github.com/ava-labs/avalanchego/x/merkledb" ) const ( @@ -28,7 +29,8 @@ var ( _ Client = &client{} errInvalidRangeProof = errors.New("failed to verify range proof") - errTooManyLeaves = errors.New("response contains more than requested leaves") + errTooManyKeys = errors.New("response contains more than requested keys") + errTooManyBytes = errors.New("response contains more than requested bytes") ) // Client synchronously fetches data from the network to fulfill state sync requests. @@ -76,6 +78,10 @@ func NewClient(config *ClientConfig) Client { // The returned change proof is verified. func (c *client) GetChangeProof(ctx context.Context, req *ChangeProofRequest, db *merkledb.Database) (*merkledb.ChangeProof, error) { parseFn := func(ctx context.Context, responseBytes []byte) (*merkledb.ChangeProof, error) { + if len(responseBytes) > int(req.BytesLimit) { + return nil, fmt.Errorf("%w: (%d) > %d)", errTooManyBytes, len(responseBytes), req.BytesLimit) + } + changeProof := &merkledb.ChangeProof{} if _, err := merkledb.Codec.DecodeChangeProof(responseBytes, changeProof); err != nil { return nil, err @@ -83,8 +89,8 @@ func (c *client) GetChangeProof(ctx context.Context, req *ChangeProofRequest, db // Ensure the response does not contain more than the requested number of leaves // and the start and end roots match the requested roots. - if len(changeProof.KeyValues)+len(changeProof.DeletedKeys) > int(req.Limit) { - return nil, fmt.Errorf("%w: (%d) > %d)", errTooManyLeaves, len(changeProof.KeyValues), req.Limit) + if len(changeProof.KeyValues)+len(changeProof.DeletedKeys) > int(req.KeyLimit) { + return nil, fmt.Errorf("%w: (%d) > %d)", errTooManyKeys, len(changeProof.KeyValues), req.KeyLimit) } if err := changeProof.Verify(ctx, db, req.Start, req.End, req.EndingRoot); err != nil { @@ -100,14 +106,18 @@ func (c *client) GetChangeProof(ctx context.Context, req *ChangeProofRequest, db // The returned range proof is verified. func (c *client) GetRangeProof(ctx context.Context, req *RangeProofRequest) (*merkledb.RangeProof, error) { parseFn := func(ctx context.Context, responseBytes []byte) (*merkledb.RangeProof, error) { + if len(responseBytes) > int(req.BytesLimit) { + return nil, fmt.Errorf("%w: (%d) > %d)", errTooManyBytes, len(responseBytes), req.BytesLimit) + } + rangeProof := &merkledb.RangeProof{} if _, err := merkledb.Codec.DecodeRangeProof(responseBytes, rangeProof); err != nil { return nil, err } // Ensure the response does not contain more than the maximum requested number of leaves. - if len(rangeProof.KeyValues) > int(req.Limit) { - return nil, fmt.Errorf("%w: (%d) > %d)", errTooManyLeaves, len(rangeProof.KeyValues), req.Limit) + if len(rangeProof.KeyValues) > int(req.KeyLimit) { + return nil, fmt.Errorf("%w: (%d) > %d)", errTooManyKeys, len(rangeProof.KeyValues), req.KeyLimit) } if err := rangeProof.Verify( diff --git a/x/sync/client_test.go b/x/sync/client_test.go index ac72e312a953..c8f9ada8e1a1 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -10,6 +10,10 @@ import ( "testing" "time" + "github.com/ava-labs/avalanchego/x/merkledb" + + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -19,7 +23,6 @@ import ( "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/set" "github.com/ava-labs/avalanchego/version" - "github.com/ava-labs/avalanchego/x/merkledb" ) func sendRequest( @@ -110,7 +113,7 @@ func sendRequest( func TestGetRangeProof(t *testing.T) { r := rand.New(rand.NewSource(1)) // #nosec G404 - smallTrieKeyCount := defaultLeafRequestLimit + smallTrieKeyCount := defaultRequestKeyLimit smallTrieDB, _, err := generateTrieWithMinKeyLen(t, r, smallTrieKeyCount, 1) require.NoError(t, err) smallTrieRoot, err := smallTrieDB.GetMerkleRoot(context.Background()) @@ -129,57 +132,71 @@ func TestGetRangeProof(t *testing.T) { expectedErr error expectedResponseLen int }{ + "proof restricted by BytesLimit": { + db: smallTrieDB, + request: &RangeProofRequest{ + Root: smallTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 10000, + }, + }, "full response for small (single request) trie": { db: smallTrieDB, request: &RangeProofRequest{ - Root: smallTrieRoot, - Limit: defaultLeafRequestLimit, + Root: smallTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, - expectedResponseLen: defaultLeafRequestLimit, + expectedResponseLen: defaultRequestKeyLimit, }, "too many leaves in response": { db: smallTrieDB, request: &RangeProofRequest{ - Root: smallTrieRoot, - Limit: defaultLeafRequestLimit, + Root: smallTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, modifyResponse: func(response *merkledb.RangeProof) { response.KeyValues = append(response.KeyValues, merkledb.KeyValue{}) }, - expectedErr: errTooManyLeaves, + expectedErr: errTooManyKeys, }, "partial response to request for entire trie (full leaf limit)": { db: largeTrieDB, request: &RangeProofRequest{ - Root: largeTrieRoot, - Limit: defaultLeafRequestLimit, + Root: largeTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, - expectedResponseLen: defaultLeafRequestLimit, + expectedResponseLen: defaultRequestKeyLimit, }, "full response from near end of trie to end of trie (less than leaf limit)": { db: largeTrieDB, request: &RangeProofRequest{ - Root: largeTrieRoot, - Start: largeTrieKeys[len(largeTrieKeys)-30], // Set start 30 keys from the end of the large trie - Limit: defaultLeafRequestLimit, + Root: largeTrieRoot, + Start: largeTrieKeys[len(largeTrieKeys)-30], // Set start 30 keys from the end of the large trie + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, expectedResponseLen: 30, }, "full response for intermediate range of trie (less than leaf limit)": { db: largeTrieDB, request: &RangeProofRequest{ - Root: largeTrieRoot, - Start: largeTrieKeys[1000], // Set the range for 1000 leafs in an intermediate range of the trie - End: largeTrieKeys[1099], // (inclusive range) - Limit: defaultLeafRequestLimit, + Root: largeTrieRoot, + Start: largeTrieKeys[1000], // Set the range for 1000 leafs in an intermediate range of the trie + End: largeTrieKeys[1099], // (inclusive range) + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, expectedResponseLen: 100, }, "removed first key in response": { db: largeTrieDB, request: &RangeProofRequest{ - Root: largeTrieRoot, - Limit: defaultLeafRequestLimit, + Root: largeTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, modifyResponse: func(response *merkledb.RangeProof) { response.KeyValues = response.KeyValues[1:] @@ -189,12 +206,13 @@ func TestGetRangeProof(t *testing.T) { "removed first key in response and replaced proof": { db: largeTrieDB, request: &RangeProofRequest{ - Root: largeTrieRoot, - Limit: defaultLeafRequestLimit, + Root: largeTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, modifyResponse: func(response *merkledb.RangeProof) { start := response.KeyValues[1].Key - proof, err := largeTrieDB.GetRangeProof(context.Background(), start, nil, defaultLeafRequestLimit) + proof, err := largeTrieDB.GetRangeProof(context.Background(), start, nil, defaultRequestKeyLimit) if err != nil { panic(err) } @@ -207,8 +225,9 @@ func TestGetRangeProof(t *testing.T) { "removed last key in response": { db: largeTrieDB, request: &RangeProofRequest{ - Root: largeTrieRoot, - Limit: defaultLeafRequestLimit, + Root: largeTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, modifyResponse: func(response *merkledb.RangeProof) { response.KeyValues = response.KeyValues[:len(response.KeyValues)-2] @@ -218,8 +237,9 @@ func TestGetRangeProof(t *testing.T) { "removed key from middle of response": { db: largeTrieDB, request: &RangeProofRequest{ - Root: largeTrieRoot, - Limit: defaultLeafRequestLimit, + Root: largeTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, modifyResponse: func(response *merkledb.RangeProof) { response.KeyValues = append(response.KeyValues[:100], response.KeyValues[101:]...) @@ -229,8 +249,9 @@ func TestGetRangeProof(t *testing.T) { "all proof keys removed from response": { db: largeTrieDB, request: &RangeProofRequest{ - Root: largeTrieRoot, - Limit: defaultLeafRequestLimit, + Root: largeTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, modifyResponse: func(response *merkledb.RangeProof) { response.StartProof = nil @@ -249,7 +270,12 @@ func TestGetRangeProof(t *testing.T) { return } require.NoError(err) - require.Len(proof.KeyValues, test.expectedResponseLen) + if test.expectedResponseLen > 0 { + require.Len(proof.KeyValues, test.expectedResponseLen) + } + bytes, err := merkledb.Codec.EncodeRangeProof(Version, proof) + require.NoError(err) + require.Less(len(bytes), int(test.request.BytesLimit)) }) } } @@ -258,7 +284,7 @@ func TestRetries(t *testing.T) { r := rand.New(rand.NewSource(1)) // #nosec G404 require := require.New(t) - keyCount := defaultLeafRequestLimit + keyCount := defaultRequestKeyLimit db, _, err := generateTrieWithMinKeyLen(t, r, keyCount, 1) require.NoError(err) root, err := db.GetMerkleRoot(context.Background()) @@ -266,8 +292,9 @@ func TestRetries(t *testing.T) { maxRequests := 4 request := &RangeProofRequest{ - Root: root, - Limit: uint16(keyCount), + Root: root, + KeyLimit: uint16(keyCount), + BytesLimit: constants.DefaultMaxMessageSize, } responseCount := 0 diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 9109a3bbcdb1..5853eed1acb0 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -6,9 +6,14 @@ package sync import ( "bytes" "context" + "encoding/binary" "errors" "time" + "github.com/ava-labs/avalanchego/x/merkledb" + + "github.com/ava-labs/avalanchego/utils/constants" + "go.uber.org/zap" "google.golang.org/grpc/codes" @@ -17,7 +22,6 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" "github.com/ava-labs/avalanchego/utils/logging" - "github.com/ava-labs/avalanchego/x/merkledb" ) // Maximum number of key-value pairs to return in a proof. @@ -25,7 +29,10 @@ import ( // or ChangeProofRequest if the given Limit is greater. const maxKeyValuesLimit = 1024 -var _ Handler = (*NetworkServer)(nil) +var ( + _ Handler = (*NetworkServer)(nil) + ErrMinProofSizeIsTooLarge = errors.New("cannot generate any proof within the requested limit") +) type NetworkServer struct { appSender common.AppSender // Used to respond to peer requests via AppResponse. @@ -45,7 +52,7 @@ func NewNetworkServer(appSender common.AppSender, db *merkledb.Database, log log // Never returns errors as they are considered fatal. // Sends a response back to the sender if length of response returned by the handler > 0. func (s *NetworkServer) AppRequest( - _ context.Context, + ctx context.Context, nodeID ids.NodeID, requestID uint32, deadline time.Time, @@ -87,8 +94,7 @@ func (s *NetworkServer) AppRequest( return nil } - // TODO danlaine: Why don't we use the passed in context instead of [context.Background()]? - handleCtx, cancel := context.WithDeadline(context.Background(), bufferedDeadline) + handleCtx, cancel := context.WithDeadline(ctx, bufferedDeadline) defer cancel() err := req.Handle(handleCtx, nodeID, requestID, s) @@ -125,7 +131,7 @@ func (s *NetworkServer) HandleChangeProofRequest( requestID uint32, req *ChangeProofRequest, ) error { - if req.Limit == 0 || req.EndingRoot == ids.Empty || (len(req.End) > 0 && bytes.Compare(req.Start, req.End) > 0) { + if req.BytesLimit == 0 || req.KeyLimit == 0 || req.EndingRoot == ids.Empty || (len(req.End) > 0 && bytes.Compare(req.Start, req.End) > 0) { s.log.Debug( "dropping invalid change proof request", zap.Stringer("nodeID", nodeID), @@ -136,32 +142,100 @@ func (s *NetworkServer) HandleChangeProofRequest( } // override limit if it is greater than maxKeyValuesLimit - limit := req.Limit - if limit > maxKeyValuesLimit { - limit = maxKeyValuesLimit + keyLimit := req.KeyLimit + if keyLimit > maxKeyValuesLimit { + keyLimit = maxKeyValuesLimit + } + bytesLimit := int(req.BytesLimit) + if bytesLimit > constants.DefaultMaxMessageSize { + bytesLimit = constants.DefaultMaxMessageSize } - changeProof, err := s.db.GetChangeProof(ctx, req.StartingRoot, req.EndingRoot, req.Start, req.End, int(limit)) - if err != nil { - // handle expected errors so clients cannot cause servers to spam warning logs. - if errors.Is(err, merkledb.ErrRootIDNotPresent) || errors.Is(err, merkledb.ErrStartRootNotFound) { - s.log.Debug( - "dropping invalid change proof request", + // attempt to get a proof within the bytes limit + for keyLimit > 0 { + changeProof, err := s.db.GetChangeProof(ctx, req.StartingRoot, req.EndingRoot, req.Start, req.End, int(keyLimit)) + if err != nil { + // handle expected errors so clients cannot cause servers to spam warning logs. + if errors.Is(err, merkledb.ErrRootIDNotPresent) || errors.Is(err, merkledb.ErrStartRootNotFound) { + s.log.Debug( + "dropping invalid change proof request", + zap.Stringer("nodeID", nodeID), + zap.Uint32("requestID", requestID), + zap.Stringer("req", req), + zap.Error(err), + ) + return nil // dropping request + } + return err + } + + proofBytes, err := merkledb.Codec.EncodeChangeProof(Version, changeProof) + if err != nil { + return err + } + if len(proofBytes) < bytesLimit { + return s.appSender.SendAppResponse(ctx, nodeID, requestID, proofBytes) + } + // the proof size was too large, try to shrink it + + // ensure that the new limit is always smaller + keyLimit = uint16((len(changeProof.KeyValues) + len(changeProof.DeletedKeys)) / 2) + + // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit + bytesEstimate := getBytesEstimateOfProofNodes(changeProof.StartProof) + + // just the start proof is too large, so a proof is impossible + if bytesEstimate > int(req.BytesLimit) { + // errors are fatal, so log for the moment + s.log.Warn( + "cannot generate a proof within bytes limit", zap.Stringer("nodeID", nodeID), zap.Uint32("requestID", requestID), zap.Stringer("req", req), - zap.Error(err), + zap.Error(ErrMinProofSizeIsTooLarge), ) - return nil // dropping request + return nil } - return err - } - proofBytes, err := merkledb.Codec.EncodeChangeProof(Version, changeProof) - if err != nil { - return err + bytesEstimate += getBytesEstimateOfProofNodes(changeProof.EndProof) + deleteKeyIndex := 0 + changeKeyIndex := 0 + + // shrink more if the early keys are extremely large + for keyIndex := uint16(1); keyIndex < keyLimit; keyIndex++ { + // determine if the deleted key or changed key is the next smallest key + currentBytes := 0 + + // if there is a deleted key at deleteKeyIndex and + // (there are no more change keys or the changed key is larger than the deleted key) + if deleteKeyIndex < len(changeProof.DeletedKeys) && + (changeKeyIndex >= len(changeProof.KeyValues) || + bytes.Compare(changeProof.KeyValues[changeKeyIndex].Key, changeProof.DeletedKeys[deleteKeyIndex]) > 0) { + currentBytes = len(changeProof.DeletedKeys[deleteKeyIndex]) + binary.MaxVarintLen64 + deleteKeyIndex++ + } else if changeKeyIndex < len(changeProof.KeyValues) { + currentBytes = len(changeProof.KeyValues[changeKeyIndex].Key) + len(changeProof.KeyValues[changeKeyIndex].Value) + binary.MaxVarintLen64 + changeKeyIndex++ + } + + if bytesEstimate+currentBytes > bytesLimit { + // adding the current KV would put the size over the limit + // so only return up to the keyIndex number of keys + keyLimit = keyIndex + break + } + bytesEstimate += currentBytes + } } - return s.appSender.SendAppResponse(ctx, nodeID, requestID, proofBytes) + // errors are fatal, so log for the moment + s.log.Warn( + "cannot generate a proof within bytes limit", + zap.Stringer("nodeID", nodeID), + zap.Uint32("requestID", requestID), + zap.Stringer("req", req), + zap.Error(ErrMinProofSizeIsTooLarge), + ) + return nil } // Generates a range proof and sends it to [nodeID]. @@ -172,7 +246,7 @@ func (s *NetworkServer) HandleRangeProofRequest( requestID uint32, req *RangeProofRequest, ) error { - if req.Limit == 0 || req.Root == ids.Empty || (len(req.End) > 0 && bytes.Compare(req.Start, req.End) > 0) { + if req.BytesLimit == 0 || req.KeyLimit == 0 || req.Root == ids.Empty || (len(req.End) > 0 && bytes.Compare(req.Start, req.End) > 0) { s.log.Debug( "dropping invalid range proof request", zap.Stringer("nodeID", nodeID), @@ -183,30 +257,91 @@ func (s *NetworkServer) HandleRangeProofRequest( } // override limit if it is greater than maxKeyValuesLimit - limit := req.Limit - if limit > maxKeyValuesLimit { - limit = maxKeyValuesLimit + keyLimit := req.KeyLimit + if keyLimit > maxKeyValuesLimit { + keyLimit = maxKeyValuesLimit } + bytesLimit := int(req.BytesLimit) + if bytesLimit > constants.DefaultMaxMessageSize { + bytesLimit = constants.DefaultMaxMessageSize + } + for keyLimit > 0 { + rangeProof, err := s.db.GetRangeProofAtRoot(ctx, req.Root, req.Start, req.End, int(keyLimit)) + if err != nil { + // handle expected errors so clients cannot cause servers to spam warning logs. + if errors.Is(err, merkledb.ErrRootIDNotPresent) { + s.log.Debug( + "dropping invalid range proof request", + zap.Stringer("nodeID", nodeID), + zap.Uint32("requestID", requestID), + zap.Stringer("req", req), + zap.Error(err), + ) + return nil // dropping request + } + return err + } + + proofBytes, err := merkledb.Codec.EncodeRangeProof(Version, rangeProof) + if err != nil { + return err + } + if len(proofBytes) < bytesLimit { + return s.appSender.SendAppResponse(ctx, nodeID, requestID, proofBytes) + } + // the proof size was too large, try to shrink it + + // ensure that the new limit is always smaller + keyLimit = uint16(len(rangeProof.KeyValues) / 2) + + // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit + bytesEstimate := getBytesEstimateOfProofNodes(rangeProof.StartProof) - rangeProof, err := s.db.GetRangeProofAtRoot(ctx, req.Root, req.Start, req.End, int(limit)) - if err != nil { - // handle expected errors so clients cannot cause servers to spam warning logs. - if errors.Is(err, merkledb.ErrRootIDNotPresent) { - s.log.Debug( - "dropping invalid range proof request", + // just the start proof is too large, so a proof is impossible + if bytesEstimate > int(req.BytesLimit) { + // errors are fatal, so log for the moment + s.log.Warn( + "cannot generate a proof within bytes limit", zap.Stringer("nodeID", nodeID), zap.Uint32("requestID", requestID), zap.Stringer("req", req), - zap.Error(err), + zap.Error(ErrMinProofSizeIsTooLarge), ) - return nil // dropping request + return nil + } + + bytesEstimate += getBytesEstimateOfProofNodes(rangeProof.EndProof) + + // shrink more if the early keys are extremely large + for keyIndex := uint16(1); keyIndex < keyLimit; keyIndex++ { + nextKV := rangeProof.KeyValues[keyIndex] + kvEstBytes := len(nextKV.Key) + len(nextKV.Value) + binary.MaxVarintLen64 + + if bytesEstimate+kvEstBytes > bytesLimit { + // adding the current KV would put the size over the limit + // so only return up to the keyIndex number of keys + keyLimit = keyIndex + break + } + bytesEstimate += kvEstBytes } - return err } + // errors are fatal, so log for the moment + s.log.Warn( + "cannot generate a proof within bytes limit", + zap.Stringer("nodeID", nodeID), + zap.Uint32("requestID", requestID), + zap.Stringer("req", req), + zap.Error(ErrMinProofSizeIsTooLarge), + ) + return nil +} - proofBytes, err := merkledb.Codec.EncodeRangeProof(Version, rangeProof) - if err != nil { - return err +func getBytesEstimateOfProofNodes(proofNodes []merkledb.ProofNode) int { + total := 0 + for _, proofNode := range proofNodes { + // size of a node is the bytes in the key, the value, and the children hashes (plus 1 byte for each children map index) + total += binary.MaxVarintLen64 + len(proofNode.KeyPath.Value) + len(proofNode.ValueOrHash.Value()) + len(proofNode.Children)*(len(ids.Empty)+1) } - return s.appSender.SendAppResponse(ctx, nodeID, requestID, proofBytes) + return total } diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go new file mode 100644 index 000000000000..b8b765f1564f --- /dev/null +++ b/x/sync/network_server_test.go @@ -0,0 +1,289 @@ +// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package sync + +import ( + "context" + "math/rand" + "testing" + + merkledb "github.com/avalanchego/x/merkledb" + + "github.com/ava-labs/avalanchego/ids" + "github.com/ava-labs/avalanchego/snow/engine/common" + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/utils/logging" + "github.com/golang/mock/gomock" + + "github.com/stretchr/testify/require" +) + +func Test_Server_GetRangeProof(t *testing.T) { + r := rand.New(rand.NewSource(1)) // #nosec G404 + + smallTrieDB, _, err := generateTrieWithMinKeyLen(t, r, defaultRequestKeyLimit, 1) + require.NoError(t, err) + smallTrieRoot, err := smallTrieDB.GetMerkleRoot(context.Background()) + require.NoError(t, err) + + tests := map[string]struct { + request *RangeProofRequest + modifyResponse func(*merkledb.RangeProof) + expectedErr error + expectedResponseLen int + nodeID ids.NodeID + proofNil bool + }{ + "proof too small": { + request: &RangeProofRequest{ + Root: smallTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 1000, + }, + proofNil: true, + }, + "byteslimit is 0": { + request: &RangeProofRequest{ + Root: smallTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 0, + }, + proofNil: true, + }, + "keylimit is 0": { + request: &RangeProofRequest{ + Root: smallTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 0, + }, + proofNil: true, + }, + "keys out of order": { + request: &RangeProofRequest{ + Root: smallTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + Start: []byte{1}, + End: []byte{0}, + }, + proofNil: true, + }, + "key limit too large": { + request: &RangeProofRequest{ + Root: smallTrieRoot, + KeyLimit: 2 * defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + expectedResponseLen: defaultRequestKeyLimit, + }, + "bytes limit too large": { + request: &RangeProofRequest{ + Root: smallTrieRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 2 * constants.DefaultMaxMessageSize, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + require := require.New(t) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + sender := common.NewMockSender(ctrl) + var proofResult *merkledb.RangeProof + sender.EXPECT().SendAppResponse( + gomock.Any(), // ctx + gomock.Any(), + gomock.Any(), // requestID + gomock.Any(), // responseBytes + ).DoAndReturn( + func(_ context.Context, _ ids.NodeID, requestID uint32, responseBytes []byte) error { + // deserialize the response so we can modify it if needed. + if !test.proofNil { + var err error + proofResult = &merkledb.RangeProof{} + _, err = merkledb.Codec.DecodeRangeProof(responseBytes, proofResult) + require.NoError(err) + } + return nil + }, + ).AnyTimes() + handler := NewNetworkServer(sender, smallTrieDB, logging.NoLog{}) + err := handler.HandleRangeProofRequest(context.Background(), test.nodeID, 0, test.request) + if test.expectedErr != nil { + require.ErrorIs(err, test.expectedErr) + return + } + require.NoError(err) + if test.proofNil { + require.Nil(proofResult) + return + } + require.NotNil(proofResult) + if test.expectedResponseLen > 0 { + require.LessOrEqual(len(proofResult.KeyValues), test.expectedResponseLen) + } + + bytes, err := merkledb.Codec.EncodeRangeProof(Version, proofResult) + require.NoError(err) + require.Less(len(bytes), int(test.request.BytesLimit)) + }) + } +} + +func Test_Server_GetChangeProof(t *testing.T) { + require := require.New(t) + + r := rand.New(rand.NewSource(1)) // #nosec G404 + trieDB, _, err := generateTrieWithMinKeyLen(t, r, defaultRequestKeyLimit, 1) + require.NoError(err) + + startRoot, err := trieDB.GetMerkleRoot(context.Background()) + require.NoError(err) + + // create changes + for x := 0; x < 600; x++ { + view, err := trieDB.NewView() + require.NoError(err) + + key := make([]byte, r.Intn(100)) + _, err = r.Read(key) + require.NoError(err) + + val := make([]byte, r.Intn(100)) + _, err = r.Read(val) + require.NoError(err) + + err = view.Insert(context.Background(), key, val) + require.NoError(err) + + deleteKeyStart := make([]byte, r.Intn(10)) + _, err = r.Read(deleteKeyStart) + require.NoError(err) + + it := trieDB.NewIteratorWithStart(deleteKeyStart) + if it.Next() { + err = view.Remove(context.Background(), it.Key()) + require.NoError(err) + } + require.NoError(it.Error()) + it.Release() + + view.CommitToDB(context.Background()) + } + + endRoot, err := trieDB.GetMerkleRoot(context.Background()) + require.NoError(err) + + tests := map[string]struct { + request *ChangeProofRequest + modifyResponse func(proof *merkledb.ChangeProof) + expectedErr error + expectedResponseLen int + nodeID ids.NodeID + proofNil bool + }{ + "proof too small": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 1000, + }, + proofNil: true, + }, + "byteslimit is 0": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 0, + }, + proofNil: true, + }, + "keylimit is 0": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 0, + }, + proofNil: true, + }, + "keys out of order": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + Start: []byte{1}, + End: []byte{0}, + }, + proofNil: true, + }, + "key limit too large": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: 2 * defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + expectedResponseLen: defaultRequestKeyLimit, + }, + "bytes limit too large": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 2 * constants.DefaultMaxMessageSize, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + sender := common.NewMockSender(ctrl) + var proofResult *merkledb.ChangeProof + sender.EXPECT().SendAppResponse( + gomock.Any(), // ctx + gomock.Any(), + gomock.Any(), // requestID + gomock.Any(), // responseBytes + ).DoAndReturn( + func(_ context.Context, _ ids.NodeID, requestID uint32, responseBytes []byte) error { + // deserialize the response so we can modify it if needed. + if !test.proofNil { + var err error + proofResult = &merkledb.ChangeProof{} + _, err = merkledb.Codec.DecodeChangeProof(responseBytes, proofResult) + require.NoError(err) + } + return nil + }, + ).AnyTimes() + handler := NewNetworkServer(sender, trieDB, logging.NoLog{}) + err := handler.HandleChangeProofRequest(context.Background(), test.nodeID, 0, test.request) + if test.expectedErr != nil { + require.ErrorIs(err, test.expectedErr) + return + } + require.NoError(err) + if test.proofNil { + require.Nil(proofResult) + return + } + require.NotNil(proofResult) + if test.expectedResponseLen > 0 { + require.LessOrEqual(len(proofResult.KeyValues)+len(proofResult.DeletedKeys), test.expectedResponseLen) + } + + bytes, err := merkledb.Codec.EncodeChangeProof(Version, proofResult) + require.NoError(err) + require.Less(len(bytes), int(test.request.BytesLimit)) + }) + } +} diff --git a/x/sync/request.go b/x/sync/request.go index ca5168886c09..27de97535dbe 100644 --- a/x/sync/request.go +++ b/x/sync/request.go @@ -50,10 +50,11 @@ type Handler interface { } type RangeProofRequest struct { - Root ids.ID `serialize:"true"` - Start []byte `serialize:"true"` - End []byte `serialize:"true"` - Limit uint16 `serialize:"true"` + Root ids.ID `serialize:"true"` + Start []byte `serialize:"true"` + End []byte `serialize:"true"` + KeyLimit uint16 `serialize:"true"` + BytesLimit uint32 `serialize:"true"` } func (r *RangeProofRequest) Handle(ctx context.Context, nodeID ids.NodeID, requestID uint32, h Handler) error { @@ -62,11 +63,12 @@ func (r *RangeProofRequest) Handle(ctx context.Context, nodeID ids.NodeID, reque func (r RangeProofRequest) String() string { return fmt.Sprintf( - "RangeProofRequest(Root=%s, Start=%s, End=%s, Limit=%d)", + "RangeProofRequest(Root=%s, Start=%s, End=%s, KeyLimit=%d, BytesLimit=%d)", r.Root, hex.EncodeToString(r.Start), hex.EncodeToString(r.End), - r.Limit, + r.KeyLimit, + r.BytesLimit, ) } @@ -77,7 +79,8 @@ type ChangeProofRequest struct { EndingRoot ids.ID `serialize:"true"` Start []byte `serialize:"true"` End []byte `serialize:"true"` - Limit uint16 `serialize:"true"` + KeyLimit uint16 `serialize:"true"` + BytesLimit uint32 `serialize:"true"` } func (r *ChangeProofRequest) Handle(ctx context.Context, nodeID ids.NodeID, requestID uint32, h Handler) error { @@ -86,11 +89,12 @@ func (r *ChangeProofRequest) Handle(ctx context.Context, nodeID ids.NodeID, requ func (r ChangeProofRequest) String() string { return fmt.Sprintf( - "ChangeProofRequest(StartRoot=%s, EndRoot=%s, Start=%s, End=%s, Limit=%d)", + "ChangeProofRequest(StartRoot=%s, EndRoot=%s, Start=%s, End=%s, KeyLimit=%d, BytesLimit=%d)", r.StartingRoot, r.EndingRoot, hex.EncodeToString(r.Start), hex.EncodeToString(r.End), - r.Limit, + r.KeyLimit, + r.BytesLimit, ) } diff --git a/x/sync/sync_test.go b/x/sync/sync_test.go index b130416ce68c..9c7b610bbacb 100644 --- a/x/sync/sync_test.go +++ b/x/sync/sync_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/ava-labs/avalanchego/x/merkledb" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -20,7 +21,6 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/trace" "github.com/ava-labs/avalanchego/utils/logging" - "github.com/ava-labs/avalanchego/x/merkledb" ) var _ Client = &mockClient{} @@ -35,11 +35,11 @@ type mockClient struct { } func (client *mockClient) GetChangeProof(ctx context.Context, request *ChangeProofRequest, _ *merkledb.Database) (*merkledb.ChangeProof, error) { - return client.db.GetChangeProof(ctx, request.StartingRoot, request.EndingRoot, request.Start, request.End, int(request.Limit)) + return client.db.GetChangeProof(ctx, request.StartingRoot, request.EndingRoot, request.Start, request.End, int(request.KeyLimit)) } func (client *mockClient) GetRangeProof(ctx context.Context, request *RangeProofRequest) (*merkledb.RangeProof, error) { - return client.db.GetRangeProofAtRoot(ctx, request.Root, request.Start, request.End, int(request.Limit)) + return client.db.GetRangeProofAtRoot(ctx, request.Root, request.Start, request.End, int(request.KeyLimit)) } func Test_Creation(t *testing.T) { @@ -507,7 +507,19 @@ func Test_Sync_Result_Correct_Root_With_Sync_Restart(t *testing.T) { err = syncer.StartSyncing(context.Background()) require.NoError(t, err) - time.Sleep(15 * time.Millisecond) + // Wait until we've processed some work + // before updating the sync target. + require.Eventually( + t, + func() bool { + syncer.workLock.Lock() + defer syncer.workLock.Unlock() + + return syncer.processedWork.Len() > 0 + }, + 500*time.Millisecond, + 5*time.Millisecond, + ) syncer.Close() newSyncer, err := NewStateSyncManager(StateSyncConfig{ @@ -561,7 +573,7 @@ func Test_Sync_Error_During_Sync(t *testing.T) { ).AnyTimes() client.EXPECT().GetChangeProof(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, request *ChangeProofRequest, _ *merkledb.Database) (*merkledb.ChangeProof, error) { - return dbToSync.GetChangeProof(ctx, request.StartingRoot, request.EndingRoot, request.Start, request.End, int(request.Limit)) + return dbToSync.GetChangeProof(ctx, request.StartingRoot, request.EndingRoot, request.Start, request.End, int(request.KeyLimit)) }, ).AnyTimes() @@ -614,13 +626,13 @@ func Test_Sync_Result_Correct_Root_Update_Root_During(t *testing.T) { client.EXPECT().GetRangeProof(gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, request *RangeProofRequest) (*merkledb.RangeProof, error) { <-updatedRootChan - return dbToSync.GetRangeProofAtRoot(ctx, request.Root, request.Start, request.End, int(request.Limit)) + return dbToSync.GetRangeProofAtRoot(ctx, request.Root, request.Start, request.End, int(request.KeyLimit)) }, ).AnyTimes() client.EXPECT().GetChangeProof(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, request *ChangeProofRequest, _ *merkledb.Database) (*merkledb.ChangeProof, error) { <-updatedRootChan - return dbToSync.GetChangeProof(ctx, request.StartingRoot, request.EndingRoot, request.Start, request.End, int(request.Limit)) + return dbToSync.GetChangeProof(ctx, request.StartingRoot, request.EndingRoot, request.Start, request.End, int(request.KeyLimit)) }, ).AnyTimes() diff --git a/x/sync/syncmanager.go b/x/sync/syncmanager.go index deb6870aac83..3e9783e62ba4 100644 --- a/x/sync/syncmanager.go +++ b/x/sync/syncmanager.go @@ -9,25 +9,23 @@ import ( "errors" "fmt" "sync" - "time" + "github.com/ava-labs/avalanchego/utils/constants" + "github.com/ava-labs/avalanchego/x/merkledb" "go.uber.org/zap" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/logging" - "github.com/ava-labs/avalanchego/x/merkledb" ) const ( - defaultLeafRequestLimit = 1024 - maxTokenWaitTime = 5 * time.Second + defaultRequestKeyLimit = 1024 ) var ( token = struct{}{} ErrAlreadyStarted = errors.New("cannot start a StateSyncManager that has already been started") ErrAlreadyClosed = errors.New("StateSyncManager is closed") - ErrNotEnoughBytes = errors.New("less bytes read than the specified length") ErrNoClientProvided = errors.New("client is a required field of the sync config") ErrNoDatabaseProvided = errors.New("sync database is a required field of the sync config") ErrNoLogProvided = errors.New("log is a required field of the sync config") @@ -276,7 +274,8 @@ func (m *StateSyncManager) getAndApplyChangeProof(ctx context.Context, workItem EndingRoot: rootID, Start: workItem.start, End: workItem.end, - Limit: defaultLeafRequestLimit, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, m.config.SyncDB, ) @@ -329,10 +328,11 @@ func (m *StateSyncManager) getAndApplyRangeProof(ctx context.Context, workItem * rootID := m.getTargetRoot() proof, err := m.config.Client.GetRangeProof(ctx, &RangeProofRequest{ - Root: rootID, - Start: workItem.start, - End: workItem.end, - Limit: defaultLeafRequestLimit, + Root: rootID, + Start: workItem.start, + End: workItem.end, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, }, ) if err != nil { From 60438cb46db206a459db4ccdef8bfb144b4b4a8a Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Thu, 30 Mar 2023 12:55:37 -0400 Subject: [PATCH 02/43] Update network_server_test.go --- x/sync/network_server_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index b8b765f1564f..9a43a7c2110d 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -8,12 +8,11 @@ import ( "math/rand" "testing" - merkledb "github.com/avalanchego/x/merkledb" - "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/avalanchego/x/merkledb" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" From 1bfce3b9d2ad50ee9ae2e2b90b8f599978444c88 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Thu, 30 Mar 2023 13:41:59 -0400 Subject: [PATCH 03/43] Address comments --- x/sync/network_server.go | 4 ++-- x/sync/network_server_test.go | 15 +++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 5853eed1acb0..3b7fe1f4b282 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -214,7 +214,7 @@ func (s *NetworkServer) HandleChangeProofRequest( currentBytes = len(changeProof.DeletedKeys[deleteKeyIndex]) + binary.MaxVarintLen64 deleteKeyIndex++ } else if changeKeyIndex < len(changeProof.KeyValues) { - currentBytes = len(changeProof.KeyValues[changeKeyIndex].Key) + len(changeProof.KeyValues[changeKeyIndex].Value) + binary.MaxVarintLen64 + currentBytes = len(changeProof.KeyValues[changeKeyIndex].Key) + len(changeProof.KeyValues[changeKeyIndex].Value) + 2*binary.MaxVarintLen64 changeKeyIndex++ } @@ -315,7 +315,7 @@ func (s *NetworkServer) HandleRangeProofRequest( // shrink more if the early keys are extremely large for keyIndex := uint16(1); keyIndex < keyLimit; keyIndex++ { nextKV := rangeProof.KeyValues[keyIndex] - kvEstBytes := len(nextKV.Key) + len(nextKV.Value) + binary.MaxVarintLen64 + kvEstBytes := len(nextKV.Key) + len(nextKV.Value) + 2*binary.MaxVarintLen64 if bytesEstimate+kvEstBytes > bytesLimit { // adding the current KV would put the size over the limit diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index 9a43a7c2110d..b32d35bcd216 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -28,7 +28,6 @@ func Test_Server_GetRangeProof(t *testing.T) { tests := map[string]struct { request *RangeProofRequest - modifyResponse func(*merkledb.RangeProof) expectedErr error expectedResponseLen int nodeID ids.NodeID @@ -94,12 +93,12 @@ func Test_Server_GetRangeProof(t *testing.T) { var proofResult *merkledb.RangeProof sender.EXPECT().SendAppResponse( gomock.Any(), // ctx - gomock.Any(), + gomock.Any(), // nodeID gomock.Any(), // requestID gomock.Any(), // responseBytes ).DoAndReturn( func(_ context.Context, _ ids.NodeID, requestID uint32, responseBytes []byte) error { - // deserialize the response so we can modify it if needed. + // grab a copy of the proof so we can inspect it later if !test.proofNil { var err error proofResult = &merkledb.RangeProof{} @@ -127,7 +126,7 @@ func Test_Server_GetRangeProof(t *testing.T) { bytes, err := merkledb.Codec.EncodeRangeProof(Version, proofResult) require.NoError(err) - require.Less(len(bytes), int(test.request.BytesLimit)) + require.LessOrEqual(len(bytes), int(test.request.BytesLimit)) }) } } @@ -170,7 +169,7 @@ func Test_Server_GetChangeProof(t *testing.T) { require.NoError(it.Error()) it.Release() - view.CommitToDB(context.Background()) + require.NoError(view.CommitToDB(context.Background())) } endRoot, err := trieDB.GetMerkleRoot(context.Background()) @@ -249,12 +248,12 @@ func Test_Server_GetChangeProof(t *testing.T) { var proofResult *merkledb.ChangeProof sender.EXPECT().SendAppResponse( gomock.Any(), // ctx - gomock.Any(), + gomock.Any(), // nodeID gomock.Any(), // requestID gomock.Any(), // responseBytes ).DoAndReturn( func(_ context.Context, _ ids.NodeID, requestID uint32, responseBytes []byte) error { - // deserialize the response so we can modify it if needed. + // grab a copy of the proof so we can inspect it later if !test.proofNil { var err error proofResult = &merkledb.ChangeProof{} @@ -282,7 +281,7 @@ func Test_Server_GetChangeProof(t *testing.T) { bytes, err := merkledb.Codec.EncodeChangeProof(Version, proofResult) require.NoError(err) - require.Less(len(bytes), int(test.request.BytesLimit)) + require.LessOrEqual(len(bytes), int(test.request.BytesLimit)) }) } } From 79775ad82bc041aeed71bd063d24429f49aca98f Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Thu, 30 Mar 2023 13:50:48 -0400 Subject: [PATCH 04/43] Fix error case condition for missing start root --- x/merkledb/history.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/merkledb/history.go b/x/merkledb/history.go index cb985873e129..2383e0f9ae61 100644 --- a/x/merkledb/history.go +++ b/x/merkledb/history.go @@ -118,7 +118,7 @@ func (th *trieHistory) getValueChanges(startRoot, endRoot ids.ID, start, end []b }, ) // There's no change resulting in [startRoot] before the latest change resulting in [endRoot]. - if lastStartRootChange == nil { + if lastStartRootChange.index > lastEndRootChange.index { return nil, ErrStartRootNotFound } } From bb3b25b3bd14cd5bcfd327e6817c203af671f6ad Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Fri, 31 Mar 2023 09:38:31 -0400 Subject: [PATCH 05/43] improve tests --- x/sync/client_test.go | 275 +++++++++++++++++++++++++++++++++- x/sync/network_server_test.go | 1 - 2 files changed, 271 insertions(+), 5 deletions(-) diff --git a/x/sync/client_test.go b/x/sync/client_test.go index c8f9ada8e1a1..87aba8a6425e 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -5,6 +5,7 @@ package sync import ( "context" + "github.com/ava-labs/avalanchego/database/memdb" "math/rand" "sync" "testing" @@ -25,7 +26,7 @@ import ( "github.com/ava-labs/avalanchego/version" ) -func sendRequest( +func sendRangeRequest( t *testing.T, db *merkledb.Database, request *RangeProofRequest, @@ -264,7 +265,7 @@ func TestGetRangeProof(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { require := require.New(t) - proof, err := sendRequest(t, test.db, test.request, 1, test.modifyResponse) + proof, err := sendRangeRequest(t, test.db, test.request, 1, test.modifyResponse) if test.expectedErr != nil { require.ErrorIs(err, test.expectedErr) return @@ -280,7 +281,273 @@ func TestGetRangeProof(t *testing.T) { } } -func TestRetries(t *testing.T) { +func sendChangeRequest( + t *testing.T, + db *merkledb.Database, + verificationDB *merkledb.Database, + request *ChangeProofRequest, + maxAttempts uint32, + modifyResponse func(*merkledb.ChangeProof), +) (*merkledb.ChangeProof, error) { + t.Helper() + + var wg sync.WaitGroup + defer wg.Wait() // wait for goroutines spawned + + require := require.New(t) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + sender := common.NewMockSender(ctrl) + handler := NewNetworkServer(sender, db, logging.NoLog{}) + clientNodeID, serverNodeID := ids.GenerateTestNodeID(), ids.GenerateTestNodeID() + networkClient := NewNetworkClient(sender, clientNodeID, 1, logging.NoLog{}) + err := networkClient.Connected(context.Background(), serverNodeID, version.CurrentApp) + require.NoError(err) + client := NewClient(&ClientConfig{ + NetworkClient: networkClient, + Metrics: &mockMetrics{}, + Log: logging.NoLog{}, + }) + + ctx, cancel := context.WithCancel(context.Background()) + deadline := time.Now().Add(1 * time.Hour) // enough time to complete a request + defer cancel() // avoid leaking a goroutine + + expectedSendNodeIDs := set.NewSet[ids.NodeID](1) + expectedSendNodeIDs.Add(serverNodeID) + sender.EXPECT().SendAppRequest( + gomock.Any(), // ctx + expectedSendNodeIDs, // {serverNodeID} + gomock.Any(), // requestID + gomock.Any(), // requestBytes + ).DoAndReturn( + func(ctx context.Context, _ set.Set[ids.NodeID], requestID uint32, requestBytes []byte) error { + // limit the number of attempts to [maxAttempts] by cancelling the context if needed. + if requestID >= maxAttempts { + cancel() + return ctx.Err() + } + + wg.Add(1) + go func() { + defer wg.Done() + err := handler.AppRequest(ctx, clientNodeID, requestID, deadline, requestBytes) + require.NoError(err) + }() // should be on a goroutine so the test can make progress. + return nil + }, + ).AnyTimes() + sender.EXPECT().SendAppResponse( + gomock.Any(), // ctx + clientNodeID, + gomock.Any(), // requestID + gomock.Any(), // responseBytes + ).DoAndReturn( + func(_ context.Context, _ ids.NodeID, requestID uint32, responseBytes []byte) error { + // deserialize the response so we can modify it if needed. + response := &merkledb.ChangeProof{} + _, err := merkledb.Codec.DecodeChangeProof(responseBytes, response) + require.NoError(err) + + // modify if needed + if modifyResponse != nil { + modifyResponse(response) + } + + // reserialize the response and pass it to the client to complete the handling. + responseBytes, err = merkledb.Codec.EncodeChangeProof(merkledb.Version, response) + require.NoError(err) + err = networkClient.AppResponse(context.Background(), serverNodeID, requestID, responseBytes) + require.NoError(err) + return nil + }, + ).AnyTimes() + + return client.GetChangeProof(ctx, request, verificationDB) +} + +func TestGetChangeProof(t *testing.T) { + r := rand.New(rand.NewSource(1)) // #nosec G404 + + require := require.New(t) + + trieDB, err := merkledb.New( + context.Background(), + memdb.New(), + merkledb.Config{ + Tracer: newNoopTracer(), + HistoryLength: 1000, + NodeCacheSize: 1000, + }, + ) + + verificationDB, err := merkledb.New( + context.Background(), + memdb.New(), + merkledb.Config{ + Tracer: newNoopTracer(), + HistoryLength: 1000, + NodeCacheSize: 1000, + }, + ) + + startRoot, err := trieDB.GetMerkleRoot(context.Background()) + require.NoError(err) + + // create changes + for x := 0; x < 200; x++ { + view, err := trieDB.NewView() + require.NoError(err) + + // add some key/values + for i := 0; i < 10; i++ { + key := make([]byte, r.Intn(100)) + _, err = r.Read(key) + require.NoError(err) + + val := make([]byte, r.Intn(100)) + _, err = r.Read(val) + require.NoError(err) + + err = view.Insert(context.Background(), key, val) + require.NoError(err) + } + + // delete a key + deleteKeyStart := make([]byte, r.Intn(10)) + _, err = r.Read(deleteKeyStart) + require.NoError(err) + + it := trieDB.NewIteratorWithStart(deleteKeyStart) + if it.Next() { + err = view.Remove(context.Background(), it.Key()) + require.NoError(err) + } + require.NoError(it.Error()) + it.Release() + + require.NoError(view.CommitToDB(context.Background())) + } + + endRoot, err := trieDB.GetMerkleRoot(context.Background()) + require.NoError(err) + + tests := map[string]struct { + db *merkledb.Database + request *ChangeProofRequest + modifyResponse func(*merkledb.ChangeProof) + expectedErr error + expectedResponseLen int + }{ + "proof restricted by BytesLimit": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: 10000, + }, + }, + "full response for small (single request) trie": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + expectedResponseLen: defaultRequestKeyLimit, + }, + "too many keys in response": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + modifyResponse: func(response *merkledb.ChangeProof) { + response.KeyValues = append(response.KeyValues, merkledb.KeyValue{}) + }, + expectedErr: errTooManyKeys, + }, + "partial response to request for entire trie (full leaf limit)": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + expectedResponseLen: defaultRequestKeyLimit, + }, + "removed first key in response": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + modifyResponse: func(response *merkledb.ChangeProof) { + response.KeyValues = response.KeyValues[1:] + }, + expectedErr: merkledb.ErrProofValueDoesntMatch, + }, + "removed last key in response": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + modifyResponse: func(response *merkledb.ChangeProof) { + response.KeyValues = response.KeyValues[:len(response.KeyValues)-2] + }, + expectedErr: merkledb.ErrProofNodeNotForKey, + }, + "removed key from middle of response": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + modifyResponse: func(response *merkledb.ChangeProof) { + response.KeyValues = append(response.KeyValues[:100], response.KeyValues[101:]...) + }, + expectedErr: merkledb.ErrInvalidProof, + }, + "all proof keys removed from response": { + request: &ChangeProofRequest{ + StartingRoot: startRoot, + EndingRoot: endRoot, + KeyLimit: defaultRequestKeyLimit, + BytesLimit: constants.DefaultMaxMessageSize, + }, + modifyResponse: func(response *merkledb.ChangeProof) { + response.StartProof = nil + response.EndProof = nil + }, + expectedErr: merkledb.ErrInvalidProof, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + proof, err := sendChangeRequest(t, trieDB, verificationDB, test.request, 1, test.modifyResponse) + if test.expectedErr != nil { + require.ErrorIs(err, test.expectedErr) + return + } + require.NoError(err) + if test.expectedResponseLen > 0 { + require.LessOrEqual(len(proof.KeyValues)+len(proof.DeletedKeys), test.expectedResponseLen) + } + bytes, err := merkledb.Codec.EncodeChangeProof(Version, proof) + require.NoError(err) + require.LessOrEqual(len(bytes), int(test.request.BytesLimit)) + }) + } +} + +func TestRangeProofRetries(t *testing.T) { r := rand.New(rand.NewSource(1)) // #nosec G404 require := require.New(t) @@ -305,7 +572,7 @@ func TestRetries(t *testing.T) { response.KeyValues = nil } } - proof, err := sendRequest(t, db, request, uint32(maxRequests), modifyResponse) + proof, err := sendRangeRequest(t, db, request, uint32(maxRequests), modifyResponse) require.NoError(err) require.Len(proof.KeyValues, keyCount) diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index b32d35bcd216..dea9062893c4 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -177,7 +177,6 @@ func Test_Server_GetChangeProof(t *testing.T) { tests := map[string]struct { request *ChangeProofRequest - modifyResponse func(proof *merkledb.ChangeProof) expectedErr error expectedResponseLen int nodeID ids.NodeID From 22218e56860ac50d786a5b3c6cc6d9653a5aa525 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Fri, 31 Mar 2023 10:52:32 -0400 Subject: [PATCH 06/43] Update client_test.go --- x/sync/client_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x/sync/client_test.go b/x/sync/client_test.go index 87aba8a6425e..bfd532b70ee2 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -5,12 +5,13 @@ package sync import ( "context" - "github.com/ava-labs/avalanchego/database/memdb" "math/rand" "sync" "testing" "time" + "github.com/ava-labs/avalanchego/database/memdb" + "github.com/ava-labs/avalanchego/x/merkledb" "github.com/ava-labs/avalanchego/utils/constants" @@ -381,7 +382,7 @@ func TestGetChangeProof(t *testing.T) { NodeCacheSize: 1000, }, ) - + require.NoError(err) verificationDB, err := merkledb.New( context.Background(), memdb.New(), @@ -391,7 +392,7 @@ func TestGetChangeProof(t *testing.T) { NodeCacheSize: 1000, }, ) - + require.NoError(err) startRoot, err := trieDB.GetMerkleRoot(context.Background()) require.NoError(err) From 8f134d7d42767c22386bf81cdd18486401d1fa7e Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 3 Apr 2023 11:44:44 -0400 Subject: [PATCH 07/43] import nit --- x/sync/client.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/sync/client.go b/x/sync/client.go index 903d4f66ea51..ee9070c0b51c 100644 --- a/x/sync/client.go +++ b/x/sync/client.go @@ -10,13 +10,12 @@ import ( "sync/atomic" "time" - "github.com/ava-labs/avalanchego/x/merkledb" - "go.uber.org/zap" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/version" + "github.com/ava-labs/avalanchego/x/merkledb" ) const ( From bea04934f656e480f83dee3406bbdaaabfa6a649 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 3 Apr 2023 11:46:12 -0400 Subject: [PATCH 08/43] import nits --- x/sync/network_server.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 3b7fe1f4b282..67a4aa258528 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -10,10 +10,6 @@ import ( "errors" "time" - "github.com/ava-labs/avalanchego/x/merkledb" - - "github.com/ava-labs/avalanchego/utils/constants" - "go.uber.org/zap" "google.golang.org/grpc/codes" @@ -21,7 +17,9 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" + "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/avalanchego/x/merkledb" ) // Maximum number of key-value pairs to return in a proof. @@ -30,8 +28,9 @@ import ( const maxKeyValuesLimit = 1024 var ( - _ Handler = (*NetworkServer)(nil) - ErrMinProofSizeIsTooLarge = errors.New("cannot generate any proof within the requested limit") + _ Handler = (*NetworkServer)(nil) + + ErrMinProofSizeIsTooLarge = errors.New("cannot generate any proof within the requested limit") ) type NetworkServer struct { From b7a033b8914b1b7fd939cfd0f6d74f24a45d150a Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 3 Apr 2023 12:03:22 -0400 Subject: [PATCH 09/43] naming nit --- x/sync/network_server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 67a4aa258528..ae0b8752f371 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -93,10 +93,10 @@ func (s *NetworkServer) AppRequest( return nil } - handleCtx, cancel := context.WithDeadline(ctx, bufferedDeadline) + ctx, cancel := context.WithDeadline(ctx, bufferedDeadline) defer cancel() - err := req.Handle(handleCtx, nodeID, requestID, s) + err := req.Handle(ctx, nodeID, requestID, s) if err != nil && !isTimeout(err) { // log unexpected errors instead of returning them, since they are fatal. s.log.Warn( From 9dbf918b63e8317cecc6bf2a06f365738e4841d7 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 3 Apr 2023 14:16:28 -0400 Subject: [PATCH 10/43] import nit --- x/sync/sync_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/sync/sync_test.go b/x/sync/sync_test.go index 9c7b610bbacb..85a8490f0d05 100644 --- a/x/sync/sync_test.go +++ b/x/sync/sync_test.go @@ -10,7 +10,6 @@ import ( "testing" "time" - "github.com/ava-labs/avalanchego/x/merkledb" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -21,6 +20,7 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/trace" "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/avalanchego/x/merkledb" ) var _ Client = &mockClient{} From c5a01028555355f0fc1efb69a07e26fac4f877fa Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 3 Apr 2023 14:18:40 -0400 Subject: [PATCH 11/43] lengthen eventually check to reduce flakiness --- x/sync/sync_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/sync/sync_test.go b/x/sync/sync_test.go index 85a8490f0d05..ffba8cdf6f9b 100644 --- a/x/sync/sync_test.go +++ b/x/sync/sync_test.go @@ -517,7 +517,7 @@ func Test_Sync_Result_Correct_Root_With_Sync_Restart(t *testing.T) { return syncer.processedWork.Len() > 0 }, - 500*time.Millisecond, + 5*time.Second, 5*time.Millisecond, ) syncer.Close() From edc68f02646a0e7dcd8e94d003357945753dfcaf Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 3 Apr 2023 14:23:29 -0400 Subject: [PATCH 12/43] nits --- x/sync/syncmanager.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x/sync/syncmanager.go b/x/sync/syncmanager.go index 3e9783e62ba4..049b7b44d453 100644 --- a/x/sync/syncmanager.go +++ b/x/sync/syncmanager.go @@ -10,17 +10,15 @@ import ( "fmt" "sync" - "github.com/ava-labs/avalanchego/utils/constants" - "github.com/ava-labs/avalanchego/x/merkledb" "go.uber.org/zap" "github.com/ava-labs/avalanchego/ids" + "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/avalanchego/x/merkledb" ) -const ( - defaultRequestKeyLimit = 1024 -) +const defaultRequestKeyLimit = 1024 var ( token = struct{}{} From be0f5c039f3e49a415ba7931495c8d9dd28ad26c Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Mon, 3 Apr 2023 14:32:43 -0400 Subject: [PATCH 13/43] import nit --- x/sync/client_test.go | 9 +++------ x/sync/network_server_test.go | 7 ++++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/x/sync/client_test.go b/x/sync/client_test.go index bfd532b70ee2..c2727d128ca6 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -10,21 +10,18 @@ import ( "testing" "time" - "github.com/ava-labs/avalanchego/database/memdb" - - "github.com/ava-labs/avalanchego/x/merkledb" - - "github.com/ava-labs/avalanchego/utils/constants" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "github.com/ava-labs/avalanchego/database/memdb" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" + "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/set" "github.com/ava-labs/avalanchego/version" + "github.com/ava-labs/avalanchego/x/merkledb" ) func sendRangeRequest( diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index dea9062893c4..7a7b791a9da3 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -8,14 +8,15 @@ import ( "math/rand" "testing" + "github.com/golang/mock/gomock" + + "github.com/stretchr/testify/require" + "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/x/merkledb" - "github.com/golang/mock/gomock" - - "github.com/stretchr/testify/require" ) func Test_Server_GetRangeProof(t *testing.T) { From d74d9bf81d3d759738b7fe571af2045112f54a4e Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Mon, 10 Apr 2023 09:59:42 -0400 Subject: [PATCH 14/43] Update history.go --- x/merkledb/history.go | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/x/merkledb/history.go b/x/merkledb/history.go index 2383e0f9ae61..fbaeca3bd8e3 100644 --- a/x/merkledb/history.go +++ b/x/merkledb/history.go @@ -97,30 +97,25 @@ func (th *trieHistory) getValueChanges(startRoot, endRoot ids.ID, start, end []b } // [lastStartRootChange] is the latest appearance of [startRoot] - lastStartRootChange, ok := th.lastChanges[startRoot] - if !ok { - return nil, ErrStartRootNotFound - } + // which came before [lastEndRootChange]. + var lastStartRootChange *changeSummaryAndIndex + th.history.DescendLessOrEqual( + lastEndRootChange, + func(item *changeSummaryAndIndex) bool { + if item == lastEndRootChange { + return true // Skip first iteration + } + if item.rootID == startRoot { + lastStartRootChange = item + return false + } + return true + }, + ) - // if lastStartRootChange is after the lastEndRootChange, then attempt to find an entry that comes before lastEndRootChange - if lastStartRootChange.index > lastEndRootChange.index { - th.history.DescendLessOrEqual( - lastEndRootChange, - func(item *changeSummaryAndIndex) bool { - if item == lastEndRootChange { - return true // Skip first iteration - } - if item.rootID == startRoot { - lastStartRootChange = item - return false - } - return true - }, - ) - // There's no change resulting in [startRoot] before the latest change resulting in [endRoot]. - if lastStartRootChange.index > lastEndRootChange.index { - return nil, ErrStartRootNotFound - } + // There's no change resulting in [startRoot] before the latest change resulting in [endRoot]. + if lastStartRootChange == nil { + return nil, ErrStartRootNotFound } // Keep changes sorted so the largest can be removed in order to stay within the maxLength limit. From ba94b474e43a463ba1202bc87de976920a3e53e9 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Mon, 10 Apr 2023 10:16:20 -0400 Subject: [PATCH 15/43] remove dependency on binary.varint --- x/merkledb/codec.go | 8 ++++++++ x/sync/network_server.go | 18 ++++++++++-------- x/sync/network_server_test.go | 24 ++++++++++++------------ 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/x/merkledb/codec.go b/x/merkledb/codec.go index 51984d141a1d..8ef1587044ba 100644 --- a/x/merkledb/codec.go +++ b/x/merkledb/codec.go @@ -71,6 +71,7 @@ type EncoderDecoder interface { // TODO actually encode the version and remove version from the interface type Encoder interface { + ByteSliceSize(value []byte) int EncodeProof(version uint16, p *Proof) ([]byte, error) EncodeChangeProof(version uint16, p *ChangeProof) ([]byte, error) EncodeRangeProof(version uint16, p *RangeProof) ([]byte, error) @@ -619,6 +620,13 @@ func (c *codecImpl) decodeByteSlice(src *bytes.Reader) ([]byte, error) { return result, nil } +func (c *codecImpl) ByteSliceSize(value []byte) int { + buf := c.varIntPool.Get().([]byte) + size := binary.PutVarint(buf, int64(len(value))) + c.varIntPool.Put(buf) + return size + len(value) +} + func (c *codecImpl) encodeByteSlice(dst io.Writer, value []byte) error { if err := c.encodeInt(dst, len(value)); err != nil { return err diff --git a/x/sync/network_server.go b/x/sync/network_server.go index ae0b8752f371..b6ab4ad4d293 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -6,7 +6,6 @@ package sync import ( "bytes" "context" - "encoding/binary" "errors" "time" @@ -203,27 +202,30 @@ func (s *NetworkServer) HandleChangeProofRequest( // shrink more if the early keys are extremely large for keyIndex := uint16(1); keyIndex < keyLimit; keyIndex++ { // determine if the deleted key or changed key is the next smallest key - currentBytes := 0 + keyBytesCount := 0 // if there is a deleted key at deleteKeyIndex and // (there are no more change keys or the changed key is larger than the deleted key) if deleteKeyIndex < len(changeProof.DeletedKeys) && (changeKeyIndex >= len(changeProof.KeyValues) || bytes.Compare(changeProof.KeyValues[changeKeyIndex].Key, changeProof.DeletedKeys[deleteKeyIndex]) > 0) { - currentBytes = len(changeProof.DeletedKeys[deleteKeyIndex]) + binary.MaxVarintLen64 + keyBytesCount = merkledb.Codec.ByteSliceSize(changeProof.DeletedKeys[deleteKeyIndex]) + if err != nil { + return err + } deleteKeyIndex++ } else if changeKeyIndex < len(changeProof.KeyValues) { - currentBytes = len(changeProof.KeyValues[changeKeyIndex].Key) + len(changeProof.KeyValues[changeKeyIndex].Value) + 2*binary.MaxVarintLen64 + keyBytesCount = merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Value) changeKeyIndex++ } - if bytesEstimate+currentBytes > bytesLimit { + if bytesEstimate+keyBytesCount > bytesLimit { // adding the current KV would put the size over the limit // so only return up to the keyIndex number of keys keyLimit = keyIndex break } - bytesEstimate += currentBytes + bytesEstimate += keyBytesCount } } // errors are fatal, so log for the moment @@ -314,7 +316,7 @@ func (s *NetworkServer) HandleRangeProofRequest( // shrink more if the early keys are extremely large for keyIndex := uint16(1); keyIndex < keyLimit; keyIndex++ { nextKV := rangeProof.KeyValues[keyIndex] - kvEstBytes := len(nextKV.Key) + len(nextKV.Value) + 2*binary.MaxVarintLen64 + kvEstBytes := merkledb.Codec.ByteSliceSize(nextKV.Key) + merkledb.Codec.ByteSliceSize(nextKV.Value) if bytesEstimate+kvEstBytes > bytesLimit { // adding the current KV would put the size over the limit @@ -340,7 +342,7 @@ func getBytesEstimateOfProofNodes(proofNodes []merkledb.ProofNode) int { total := 0 for _, proofNode := range proofNodes { // size of a node is the bytes in the key, the value, and the children hashes (plus 1 byte for each children map index) - total += binary.MaxVarintLen64 + len(proofNode.KeyPath.Value) + len(proofNode.ValueOrHash.Value()) + len(proofNode.Children)*(len(ids.Empty)+1) + total += merkledb.Codec.ByteSliceSize(proofNode.KeyPath.Value) + merkledb.Codec.ByteSliceSize(proofNode.ValueOrHash.Value()) + len(proofNode.Children)*(len(ids.Empty)+1) } return total } diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index 7a7b791a9da3..0d4ab5738beb 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -133,48 +133,47 @@ func Test_Server_GetRangeProof(t *testing.T) { } func Test_Server_GetChangeProof(t *testing.T) { - require := require.New(t) r := rand.New(rand.NewSource(1)) // #nosec G404 trieDB, _, err := generateTrieWithMinKeyLen(t, r, defaultRequestKeyLimit, 1) - require.NoError(err) + require.NoError(t, err) startRoot, err := trieDB.GetMerkleRoot(context.Background()) - require.NoError(err) + require.NoError(t, err) // create changes for x := 0; x < 600; x++ { view, err := trieDB.NewView() - require.NoError(err) + require.NoError(t, err) key := make([]byte, r.Intn(100)) _, err = r.Read(key) - require.NoError(err) + require.NoError(t, err) val := make([]byte, r.Intn(100)) _, err = r.Read(val) - require.NoError(err) + require.NoError(t, err) err = view.Insert(context.Background(), key, val) - require.NoError(err) + require.NoError(t, err) deleteKeyStart := make([]byte, r.Intn(10)) _, err = r.Read(deleteKeyStart) - require.NoError(err) + require.NoError(t, err) it := trieDB.NewIteratorWithStart(deleteKeyStart) if it.Next() { err = view.Remove(context.Background(), it.Key()) - require.NoError(err) + require.NoError(t, err) } - require.NoError(it.Error()) + require.NoError(t, it.Error()) it.Release() - require.NoError(view.CommitToDB(context.Background())) + require.NoError(t, view.CommitToDB(context.Background())) } endRoot, err := trieDB.GetMerkleRoot(context.Background()) - require.NoError(err) + require.NoError(t, err) tests := map[string]struct { request *ChangeProofRequest @@ -242,6 +241,7 @@ func Test_Server_GetChangeProof(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { + require := require.New(t) ctrl := gomock.NewController(t) defer ctrl.Finish() sender := common.NewMockSender(ctrl) From 088d6d842c23371b84edc98cf99690b4cd2ef756 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Mon, 10 Apr 2023 10:27:00 -0400 Subject: [PATCH 16/43] dd tests to enforce connection between byteslicesize and encodebyteslice --- x/merkledb/codec_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/x/merkledb/codec_test.go b/x/merkledb/codec_test.go index f3561c815e5b..b262ac53946a 100644 --- a/x/merkledb/codec_test.go +++ b/x/merkledb/codec_test.go @@ -555,6 +555,46 @@ func FuzzCodecDBNodeDeterministic(f *testing.F) { ) } +func FuzzCodecImpl_ByteSliceSize_Matches_EncodeByteSlice(f *testing.F) { + f.Fuzz( + func( + t *testing.T, + b []byte, + ) { + require := require.New(t) + + codec := Codec.(*codecImpl) + buf := &bytes.Buffer{} + err := codec.encodeByteSlice(buf, b) + require.NoError(err) + require.Equal(buf.Len(), codec.ByteSliceSize(b)) + }, + ) +} + +func TestCodecImpl_ByteSliceSize_Matches_EncodeByteSlice(t *testing.T) { + require := require.New(t) + codec := Codec.(*codecImpl) + + var currentValue []byte + buf := &bytes.Buffer{} + err := codec.encodeByteSlice(buf, currentValue) + require.NoError(err) + require.Equal(buf.Len(), codec.ByteSliceSize(currentValue)) + + for i := 0; i < 100; i++ { + r := rand.New(rand.NewSource(int64(i))) + currentValue = make([]byte, r.Intn(500)) + _, err := r.Read(currentValue) + require.NoError(err) + + buf := &bytes.Buffer{} + err = codec.encodeByteSlice(buf, currentValue) + require.NoError(err) + require.Equal(buf.Len(), codec.ByteSliceSize(currentValue)) + } +} + func TestCodec_DecodeProof(t *testing.T) { require := require.New(t) From a866a65bdaac470efacc89bc37dcaffda6030311 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Mon, 10 Apr 2023 10:41:03 -0400 Subject: [PATCH 17/43] move byte estimation to codec --- x/merkledb/codec.go | 14 ++++++++++++++ x/merkledb/codec_test.go | 15 +++++++++++++++ x/sync/network_server.go | 6 +++--- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/x/merkledb/codec.go b/x/merkledb/codec.go index 8ef1587044ba..d27a8e896d71 100644 --- a/x/merkledb/codec.go +++ b/x/merkledb/codec.go @@ -72,6 +72,7 @@ type EncoderDecoder interface { // TODO actually encode the version and remove version from the interface type Encoder interface { ByteSliceSize(value []byte) int + ProofNodeSize(value ProofNode) int EncodeProof(version uint16, p *Proof) ([]byte, error) EncodeChangeProof(version uint16, p *ChangeProof) ([]byte, error) EncodeRangeProof(version uint16, p *RangeProof) ([]byte, error) @@ -627,6 +628,19 @@ func (c *codecImpl) ByteSliceSize(value []byte) int { return size + len(value) } +func (c *codecImpl) ProofNodeSize(proofNode ProofNode) int { + sizeOfKeyPath := c.ByteSliceSize(proofNode.KeyPath.Value) + sizeOfMaybeValue := c.ByteSliceSize(proofNode.ValueOrHash.Value()) + 1 + + buf := c.varIntPool.Get().([]byte) + sizeOfChildrenCount := binary.PutVarint(buf, int64(len(proofNode.Children))) + c.varIntPool.Put(buf) + + sizeOfChildren := sizeOfChildrenCount + len(proofNode.Children)*(len(ids.Empty)+1) + + return sizeOfKeyPath + sizeOfMaybeValue + sizeOfChildren +} + func (c *codecImpl) encodeByteSlice(dst io.Writer, value []byte) error { if err := c.encodeInt(dst, len(value)); err != nil { return err diff --git a/x/merkledb/codec_test.go b/x/merkledb/codec_test.go index b262ac53946a..e46ef3adae84 100644 --- a/x/merkledb/codec_test.go +++ b/x/merkledb/codec_test.go @@ -572,6 +572,21 @@ func FuzzCodecImpl_ByteSliceSize_Matches_EncodeByteSlice(f *testing.F) { ) } +func TestCodecImpl_ProofNodeSize_Matches_EncodeProofNode(t *testing.T) { + require := require.New(t) + codec := Codec.(*codecImpl) + for i := 0; i < 100; i++ { + r := rand.New(rand.NewSource(int64(i))) // #nosec G404 + + proofNode := newRandomProofNode(r) + + buf := &bytes.Buffer{} + err := codec.encodeProofNode(proofNode, buf) + require.NoError(err) + require.Equal(buf.Len(), codec.ProofNodeSize(proofNode)) + } +} + func TestCodecImpl_ByteSliceSize_Matches_EncodeByteSlice(t *testing.T) { require := require.New(t) codec := Codec.(*codecImpl) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index b6ab4ad4d293..2222e6ca7a26 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -215,7 +215,8 @@ func (s *NetworkServer) HandleChangeProofRequest( } deleteKeyIndex++ } else if changeKeyIndex < len(changeProof.KeyValues) { - keyBytesCount = merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Value) + keyBytesCount = merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + + merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Value) changeKeyIndex++ } @@ -341,8 +342,7 @@ func (s *NetworkServer) HandleRangeProofRequest( func getBytesEstimateOfProofNodes(proofNodes []merkledb.ProofNode) int { total := 0 for _, proofNode := range proofNodes { - // size of a node is the bytes in the key, the value, and the children hashes (plus 1 byte for each children map index) - total += merkledb.Codec.ByteSliceSize(proofNode.KeyPath.Value) + merkledb.Codec.ByteSliceSize(proofNode.ValueOrHash.Value()) + len(proofNode.Children)*(len(ids.Empty)+1) + total += merkledb.Codec.ProofNodeSize(proofNode) } return total } From 604ba92f6aa370c67d9355d978b0f3b53dfcb95a Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Mon, 10 Apr 2023 10:50:48 -0400 Subject: [PATCH 18/43] lint --- x/merkledb/codec_test.go | 4 ++-- x/sync/network_server_test.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/x/merkledb/codec_test.go b/x/merkledb/codec_test.go index e46ef3adae84..4e65ada6cbde 100644 --- a/x/merkledb/codec_test.go +++ b/x/merkledb/codec_test.go @@ -555,7 +555,7 @@ func FuzzCodecDBNodeDeterministic(f *testing.F) { ) } -func FuzzCodecImpl_ByteSliceSize_Matches_EncodeByteSlice(f *testing.F) { +func FuzzCodecImplByteSliceSizeMatchesEncodeByteSlice(f *testing.F) { f.Fuzz( func( t *testing.T, @@ -598,7 +598,7 @@ func TestCodecImpl_ByteSliceSize_Matches_EncodeByteSlice(t *testing.T) { require.Equal(buf.Len(), codec.ByteSliceSize(currentValue)) for i := 0; i < 100; i++ { - r := rand.New(rand.NewSource(int64(i))) + r := rand.New(rand.NewSource(int64(i))) // #nosec G404 currentValue = make([]byte, r.Intn(500)) _, err := r.Read(currentValue) require.NoError(err) diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index 0d4ab5738beb..44aea4d9b76e 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -133,7 +133,6 @@ func Test_Server_GetRangeProof(t *testing.T) { } func Test_Server_GetChangeProof(t *testing.T) { - r := rand.New(rand.NewSource(1)) // #nosec G404 trieDB, _, err := generateTrieWithMinKeyLen(t, r, defaultRequestKeyLimit, 1) require.NoError(t, err) From 39b9cd58df9c7d915b34ec1ef47c81b2ec6ab38d Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Tue, 11 Apr 2023 06:41:49 -0400 Subject: [PATCH 19/43] use smaller max bytes size --- x/sync/client_test.go | 37 +++++++++++++++++------------------ x/sync/network_server.go | 9 ++++----- x/sync/network_server_test.go | 13 ++++++------ x/sync/syncmanager.go | 11 +++++++---- 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/x/sync/client_test.go b/x/sync/client_test.go index c2727d128ca6..50f40d355e9d 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -17,7 +17,6 @@ import ( "github.com/ava-labs/avalanchego/database/memdb" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/set" "github.com/ava-labs/avalanchego/version" @@ -144,7 +143,7 @@ func TestGetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: smallTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, expectedResponseLen: defaultRequestKeyLimit, }, @@ -153,7 +152,7 @@ func TestGetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: smallTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.RangeProof) { response.KeyValues = append(response.KeyValues, merkledb.KeyValue{}) @@ -165,7 +164,7 @@ func TestGetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: largeTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, expectedResponseLen: defaultRequestKeyLimit, }, @@ -175,7 +174,7 @@ func TestGetRangeProof(t *testing.T) { Root: largeTrieRoot, Start: largeTrieKeys[len(largeTrieKeys)-30], // Set start 30 keys from the end of the large trie KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, expectedResponseLen: 30, }, @@ -186,7 +185,7 @@ func TestGetRangeProof(t *testing.T) { Start: largeTrieKeys[1000], // Set the range for 1000 leafs in an intermediate range of the trie End: largeTrieKeys[1099], // (inclusive range) KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, expectedResponseLen: 100, }, @@ -195,7 +194,7 @@ func TestGetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: largeTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.RangeProof) { response.KeyValues = response.KeyValues[1:] @@ -207,7 +206,7 @@ func TestGetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: largeTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.RangeProof) { start := response.KeyValues[1].Key @@ -226,7 +225,7 @@ func TestGetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: largeTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.RangeProof) { response.KeyValues = response.KeyValues[:len(response.KeyValues)-2] @@ -238,7 +237,7 @@ func TestGetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: largeTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.RangeProof) { response.KeyValues = append(response.KeyValues[:100], response.KeyValues[101:]...) @@ -250,7 +249,7 @@ func TestGetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: largeTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.RangeProof) { response.StartProof = nil @@ -451,7 +450,7 @@ func TestGetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, expectedResponseLen: defaultRequestKeyLimit, }, @@ -460,7 +459,7 @@ func TestGetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.ChangeProof) { response.KeyValues = append(response.KeyValues, merkledb.KeyValue{}) @@ -472,7 +471,7 @@ func TestGetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, expectedResponseLen: defaultRequestKeyLimit, }, @@ -481,7 +480,7 @@ func TestGetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.ChangeProof) { response.KeyValues = response.KeyValues[1:] @@ -493,7 +492,7 @@ func TestGetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.ChangeProof) { response.KeyValues = response.KeyValues[:len(response.KeyValues)-2] @@ -505,7 +504,7 @@ func TestGetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.ChangeProof) { response.KeyValues = append(response.KeyValues[:100], response.KeyValues[101:]...) @@ -517,7 +516,7 @@ func TestGetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.ChangeProof) { response.StartProof = nil @@ -559,7 +558,7 @@ func TestRangeProofRetries(t *testing.T) { request := &RangeProofRequest{ Root: root, KeyLimit: uint16(keyCount), - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, } responseCount := 0 diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 2222e6ca7a26..cc737c074ee9 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -16,7 +16,6 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/x/merkledb" ) @@ -145,8 +144,8 @@ func (s *NetworkServer) HandleChangeProofRequest( keyLimit = maxKeyValuesLimit } bytesLimit := int(req.BytesLimit) - if bytesLimit > constants.DefaultMaxMessageSize { - bytesLimit = constants.DefaultMaxMessageSize + if bytesLimit > defaultRequestByteSizeLimit { + bytesLimit = defaultRequestByteSizeLimit } // attempt to get a proof within the bytes limit @@ -264,8 +263,8 @@ func (s *NetworkServer) HandleRangeProofRequest( keyLimit = maxKeyValuesLimit } bytesLimit := int(req.BytesLimit) - if bytesLimit > constants.DefaultMaxMessageSize { - bytesLimit = constants.DefaultMaxMessageSize + if bytesLimit > defaultRequestByteSizeLimit { + bytesLimit = defaultRequestByteSizeLimit } for keyLimit > 0 { rangeProof, err := s.db.GetRangeProofAtRoot(ctx, req.Root, req.Start, req.End, int(keyLimit)) diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index 44aea4d9b76e..952052d36f92 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -14,7 +14,6 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/x/merkledb" ) @@ -62,7 +61,7 @@ func Test_Server_GetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: smallTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, Start: []byte{1}, End: []byte{0}, }, @@ -72,7 +71,7 @@ func Test_Server_GetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: smallTrieRoot, KeyLimit: 2 * defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, expectedResponseLen: defaultRequestKeyLimit, }, @@ -80,7 +79,7 @@ func Test_Server_GetRangeProof(t *testing.T) { request: &RangeProofRequest{ Root: smallTrieRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: 2 * constants.DefaultMaxMessageSize, + BytesLimit: 2 * defaultRequestByteSizeLimit, }, }, } @@ -213,7 +212,7 @@ func Test_Server_GetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, Start: []byte{1}, End: []byte{0}, }, @@ -224,7 +223,7 @@ func Test_Server_GetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: 2 * defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, expectedResponseLen: defaultRequestKeyLimit, }, @@ -233,7 +232,7 @@ func Test_Server_GetChangeProof(t *testing.T) { StartingRoot: startRoot, EndingRoot: endRoot, KeyLimit: defaultRequestKeyLimit, - BytesLimit: 2 * constants.DefaultMaxMessageSize, + BytesLimit: 2 * defaultRequestByteSizeLimit, }, }, } diff --git a/x/sync/syncmanager.go b/x/sync/syncmanager.go index 049b7b44d453..7badfc457168 100644 --- a/x/sync/syncmanager.go +++ b/x/sync/syncmanager.go @@ -8,17 +8,20 @@ import ( "context" "errors" "fmt" + "github.com/ava-labs/avalanchego/utils/units" "sync" "go.uber.org/zap" "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/x/merkledb" ) -const defaultRequestKeyLimit = 1024 +const ( + defaultRequestKeyLimit = 1024 + defaultRequestByteSizeLimit = units.MiB +) var ( token = struct{}{} @@ -273,7 +276,7 @@ func (m *StateSyncManager) getAndApplyChangeProof(ctx context.Context, workItem Start: workItem.start, End: workItem.end, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, m.config.SyncDB, ) @@ -330,7 +333,7 @@ func (m *StateSyncManager) getAndApplyRangeProof(ctx context.Context, workItem * Start: workItem.start, End: workItem.end, KeyLimit: defaultRequestKeyLimit, - BytesLimit: constants.DefaultMaxMessageSize, + BytesLimit: defaultRequestByteSizeLimit, }, ) if err != nil { From 90e3468edeebf595c72366168b402c3e6d2f4289 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Tue, 11 Apr 2023 06:46:36 -0400 Subject: [PATCH 20/43] Update codec.go --- x/merkledb/codec.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/merkledb/codec.go b/x/merkledb/codec.go index d27a8e896d71..3ff058049ea8 100644 --- a/x/merkledb/codec.go +++ b/x/merkledb/codec.go @@ -71,8 +71,11 @@ type EncoderDecoder interface { // TODO actually encode the version and remove version from the interface type Encoder interface { + // ByteSliceSize provides the number of bytes in the serialized form of byte slice ByteSliceSize(value []byte) int + // ProofNodeSize provides the number of bytes in the serialized form of a ProofNode ProofNodeSize(value ProofNode) int + EncodeProof(version uint16, p *Proof) ([]byte, error) EncodeChangeProof(version uint16, p *ChangeProof) ([]byte, error) EncodeRangeProof(version uint16, p *RangeProof) ([]byte, error) From 71cbbb209190740e9025987654a5760da25daf09 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Tue, 11 Apr 2023 06:55:10 -0400 Subject: [PATCH 21/43] Update syncmanager.go --- x/sync/syncmanager.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/sync/syncmanager.go b/x/sync/syncmanager.go index 7badfc457168..d18573c0ae9f 100644 --- a/x/sync/syncmanager.go +++ b/x/sync/syncmanager.go @@ -8,9 +8,10 @@ import ( "context" "errors" "fmt" - "github.com/ava-labs/avalanchego/utils/units" "sync" + "github.com/ava-labs/avalanchego/utils/units" + "go.uber.org/zap" "github.com/ava-labs/avalanchego/ids" From d479c98372d1ab5bac0594d43f56ca48b3dd7cf0 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Tue, 11 Apr 2023 10:48:48 -0400 Subject: [PATCH 22/43] Update network_server.go --- x/sync/network_server.go | 40 ++++------------------------------------ 1 file changed, 4 insertions(+), 36 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index cc737c074ee9..041df42897be 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -183,15 +183,7 @@ func (s *NetworkServer) HandleChangeProofRequest( // just the start proof is too large, so a proof is impossible if bytesEstimate > int(req.BytesLimit) { - // errors are fatal, so log for the moment - s.log.Warn( - "cannot generate a proof within bytes limit", - zap.Stringer("nodeID", nodeID), - zap.Uint32("requestID", requestID), - zap.Stringer("req", req), - zap.Error(ErrMinProofSizeIsTooLarge), - ) - return nil + return ErrMinProofSizeIsTooLarge } bytesEstimate += getBytesEstimateOfProofNodes(changeProof.EndProof) @@ -228,15 +220,7 @@ func (s *NetworkServer) HandleChangeProofRequest( bytesEstimate += keyBytesCount } } - // errors are fatal, so log for the moment - s.log.Warn( - "cannot generate a proof within bytes limit", - zap.Stringer("nodeID", nodeID), - zap.Uint32("requestID", requestID), - zap.Stringer("req", req), - zap.Error(ErrMinProofSizeIsTooLarge), - ) - return nil + return ErrMinProofSizeIsTooLarge } // Generates a range proof and sends it to [nodeID]. @@ -300,15 +284,7 @@ func (s *NetworkServer) HandleRangeProofRequest( // just the start proof is too large, so a proof is impossible if bytesEstimate > int(req.BytesLimit) { - // errors are fatal, so log for the moment - s.log.Warn( - "cannot generate a proof within bytes limit", - zap.Stringer("nodeID", nodeID), - zap.Uint32("requestID", requestID), - zap.Stringer("req", req), - zap.Error(ErrMinProofSizeIsTooLarge), - ) - return nil + return ErrMinProofSizeIsTooLarge } bytesEstimate += getBytesEstimateOfProofNodes(rangeProof.EndProof) @@ -327,15 +303,7 @@ func (s *NetworkServer) HandleRangeProofRequest( bytesEstimate += kvEstBytes } } - // errors are fatal, so log for the moment - s.log.Warn( - "cannot generate a proof within bytes limit", - zap.Stringer("nodeID", nodeID), - zap.Uint32("requestID", requestID), - zap.Stringer("req", req), - zap.Error(ErrMinProofSizeIsTooLarge), - ) - return nil + return ErrMinProofSizeIsTooLarge } func getBytesEstimateOfProofNodes(proofNodes []merkledb.ProofNode) int { From 98720ef4a0fea33e22a52fe955e6d8ed3f46961a Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Tue, 11 Apr 2023 10:54:11 -0400 Subject: [PATCH 23/43] create max size --- x/sync/network_server.go | 30 +++++++++++++++++++----------- x/sync/syncmanager.go | 2 +- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 041df42897be..6df6747f85fd 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "errors" + "github.com/ava-labs/avalanchego/utils/units" "time" "go.uber.org/zap" @@ -20,10 +21,14 @@ import ( "github.com/ava-labs/avalanchego/x/merkledb" ) -// Maximum number of key-value pairs to return in a proof. -// This overrides any other Limit specified in a RangeProofRequest -// or ChangeProofRequest if the given Limit is greater. -const maxKeyValuesLimit = 1024 +const ( + // Maximum number of key-value pairs to return in a proof. + // This overrides any other Limit specified in a RangeProofRequest + // or ChangeProofRequest if the given Limit is greater. + maxKeyValuesLimit = 2048 + maxByteSizeLimit = units.MiB + endProofSizeBufferAmount = 2 * units.KiB +) var ( _ Handler = (*NetworkServer)(nil) @@ -144,8 +149,8 @@ func (s *NetworkServer) HandleChangeProofRequest( keyLimit = maxKeyValuesLimit } bytesLimit := int(req.BytesLimit) - if bytesLimit > defaultRequestByteSizeLimit { - bytesLimit = defaultRequestByteSizeLimit + if bytesLimit > maxByteSizeLimit { + bytesLimit = maxByteSizeLimit } // attempt to get a proof within the bytes limit @@ -185,8 +190,9 @@ func (s *NetworkServer) HandleChangeProofRequest( if bytesEstimate > int(req.BytesLimit) { return ErrMinProofSizeIsTooLarge } - - bytesEstimate += getBytesEstimateOfProofNodes(changeProof.EndProof) + // since the number of keys is decreasing, the new endproof will be a different size than the current one + // use the current endproof plus a small buffer to account for potential size differences + bytesEstimate += getBytesEstimateOfProofNodes(changeProof.EndProof) + endProofSizeBufferAmount deleteKeyIndex := 0 changeKeyIndex := 0 @@ -247,8 +253,8 @@ func (s *NetworkServer) HandleRangeProofRequest( keyLimit = maxKeyValuesLimit } bytesLimit := int(req.BytesLimit) - if bytesLimit > defaultRequestByteSizeLimit { - bytesLimit = defaultRequestByteSizeLimit + if bytesLimit > maxByteSizeLimit { + bytesLimit = maxByteSizeLimit } for keyLimit > 0 { rangeProof, err := s.db.GetRangeProofAtRoot(ctx, req.Root, req.Start, req.End, int(keyLimit)) @@ -287,7 +293,9 @@ func (s *NetworkServer) HandleRangeProofRequest( return ErrMinProofSizeIsTooLarge } - bytesEstimate += getBytesEstimateOfProofNodes(rangeProof.EndProof) + // since the number of keys is decreasing, the new endproof will be a different size than the current one + // use the current endproof plus a small buffer to account for potential size differences + bytesEstimate += getBytesEstimateOfProofNodes(rangeProof.EndProof) + endProofSizeBufferAmount // shrink more if the early keys are extremely large for keyIndex := uint16(1); keyIndex < keyLimit; keyIndex++ { diff --git a/x/sync/syncmanager.go b/x/sync/syncmanager.go index d18573c0ae9f..c4cd7e2f1f4a 100644 --- a/x/sync/syncmanager.go +++ b/x/sync/syncmanager.go @@ -21,7 +21,7 @@ import ( const ( defaultRequestKeyLimit = 1024 - defaultRequestByteSizeLimit = units.MiB + defaultRequestByteSizeLimit = 512 * units.KiB ) var ( From fa91204d432c00624ed89593f4a685abd2b0c007 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Tue, 11 Apr 2023 14:33:10 -0400 Subject: [PATCH 24/43] nits --- x/sync/client_test.go | 28 ++++++++++++++-------------- x/sync/network_server.go | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/x/sync/client_test.go b/x/sync/client_test.go index 50f40d355e9d..6b1fdfc0ab84 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -367,8 +367,6 @@ func sendChangeRequest( func TestGetChangeProof(t *testing.T) { r := rand.New(rand.NewSource(1)) // #nosec G404 - require := require.New(t) - trieDB, err := merkledb.New( context.Background(), memdb.New(), @@ -378,7 +376,7 @@ func TestGetChangeProof(t *testing.T) { NodeCacheSize: 1000, }, ) - require.NoError(err) + require.NoError(t, err) verificationDB, err := merkledb.New( context.Background(), memdb.New(), @@ -388,47 +386,47 @@ func TestGetChangeProof(t *testing.T) { NodeCacheSize: 1000, }, ) - require.NoError(err) + require.NoError(t, err) startRoot, err := trieDB.GetMerkleRoot(context.Background()) - require.NoError(err) + require.NoError(t, err) // create changes for x := 0; x < 200; x++ { view, err := trieDB.NewView() - require.NoError(err) + require.NoError(t, err) // add some key/values for i := 0; i < 10; i++ { key := make([]byte, r.Intn(100)) _, err = r.Read(key) - require.NoError(err) + require.NoError(t, err) val := make([]byte, r.Intn(100)) _, err = r.Read(val) - require.NoError(err) + require.NoError(t, err) err = view.Insert(context.Background(), key, val) - require.NoError(err) + require.NoError(t, err) } // delete a key deleteKeyStart := make([]byte, r.Intn(10)) _, err = r.Read(deleteKeyStart) - require.NoError(err) + require.NoError(t, err) it := trieDB.NewIteratorWithStart(deleteKeyStart) if it.Next() { err = view.Remove(context.Background(), it.Key()) - require.NoError(err) + require.NoError(t, err) } - require.NoError(it.Error()) + require.NoError(t, it.Error()) it.Release() - require.NoError(view.CommitToDB(context.Background())) + require.NoError(t, view.CommitToDB(context.Background())) } endRoot, err := trieDB.GetMerkleRoot(context.Background()) - require.NoError(err) + require.NoError(t, err) tests := map[string]struct { db *merkledb.Database @@ -528,6 +526,8 @@ func TestGetChangeProof(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { + require := require.New(t) + proof, err := sendChangeRequest(t, trieDB, verificationDB, test.request, 1, test.modifyResponse) if test.expectedErr != nil { require.ErrorIs(err, test.expectedErr) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 6df6747f85fd..2bc5ad42d714 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -7,7 +7,6 @@ import ( "bytes" "context" "errors" - "github.com/ava-labs/avalanchego/utils/units" "time" "go.uber.org/zap" @@ -18,6 +17,7 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/avalanchego/utils/units" "github.com/ava-labs/avalanchego/x/merkledb" ) From b63ebaf5878be6ff6187b8e2563b27820e041715 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Tue, 11 Apr 2023 14:41:56 -0400 Subject: [PATCH 25/43] Update network_server.go --- x/sync/network_server.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 2bc5ad42d714..852e8caa1ec4 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "errors" + "github.com/ava-labs/avalanchego/utils/constants" "time" "go.uber.org/zap" @@ -26,7 +27,7 @@ const ( // This overrides any other Limit specified in a RangeProofRequest // or ChangeProofRequest if the given Limit is greater. maxKeyValuesLimit = 2048 - maxByteSizeLimit = units.MiB + maxByteSizeLimit = constants.DefaultMaxMessageSize - 4*units.KiB endProofSizeBufferAmount = 2 * units.KiB ) From 008c8c423df9bcebe3ddd27c7be2fdeb9534629e Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Tue, 11 Apr 2023 14:42:41 -0400 Subject: [PATCH 26/43] Update network_server_test.go --- x/sync/network_server_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index 952052d36f92..426268dda348 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -39,7 +39,8 @@ func Test_Server_GetRangeProof(t *testing.T) { KeyLimit: defaultRequestKeyLimit, BytesLimit: 1000, }, - proofNil: true, + proofNil: true, + expectedErr: ErrMinProofSizeIsTooLarge, }, "byteslimit is 0": { request: &RangeProofRequest{ From 89be9d42969fb5e8f9e9590b47c2d3208bed10e6 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Tue, 11 Apr 2023 14:49:55 -0400 Subject: [PATCH 27/43] Update network_server.go --- x/sync/network_server.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 852e8caa1ec4..43756f96798b 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -7,9 +7,10 @@ import ( "bytes" "context" "errors" - "github.com/ava-labs/avalanchego/utils/constants" "time" + "github.com/ava-labs/avalanchego/utils/constants" + "go.uber.org/zap" "google.golang.org/grpc/codes" From 5db6c9ce30b79de441aa58b955190dfa5096755d Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Tue, 11 Apr 2023 15:44:58 -0400 Subject: [PATCH 28/43] import nit --- x/sync/network_server.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 43756f96798b..635b931d4017 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -9,8 +9,6 @@ import ( "errors" "time" - "github.com/ava-labs/avalanchego/utils/constants" - "go.uber.org/zap" "google.golang.org/grpc/codes" @@ -18,6 +16,7 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/common" + "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/units" "github.com/ava-labs/avalanchego/x/merkledb" From 113a028536c33772dcc0e8753aba2d5070175688 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Tue, 11 Apr 2023 15:46:48 -0400 Subject: [PATCH 29/43] add comment and constant --- x/sync/network_server.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 635b931d4017..52e4b03a9400 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -26,8 +26,12 @@ const ( // Maximum number of key-value pairs to return in a proof. // This overrides any other Limit specified in a RangeProofRequest // or ChangeProofRequest if the given Limit is greater. - maxKeyValuesLimit = 2048 - maxByteSizeLimit = constants.DefaultMaxMessageSize - 4*units.KiB + maxKeyValuesLimit = 2048 + // Estimated max overhead, in bytes, of putting a proof into a message. + // We use this to ensure that the proof we generate is not too large to fit in a message. + // TODO: refine this estimate. This is almost certainly a large overestimate. + estimatedMessageOverhead = 4 * units.KiB + maxByteSizeLimit = constants.DefaultMaxMessageSize - estimatedMessageOverhead endProofSizeBufferAmount = 2 * units.KiB ) From 467c482ee2018c72a329c4eeaa88c370e75e789b Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Tue, 11 Apr 2023 15:47:40 -0400 Subject: [PATCH 30/43] import nit --- x/sync/syncmanager.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/sync/syncmanager.go b/x/sync/syncmanager.go index c4cd7e2f1f4a..5fc6579349b4 100644 --- a/x/sync/syncmanager.go +++ b/x/sync/syncmanager.go @@ -10,12 +10,11 @@ import ( "fmt" "sync" - "github.com/ava-labs/avalanchego/utils/units" - "go.uber.org/zap" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/avalanchego/utils/units" "github.com/ava-labs/avalanchego/x/merkledb" ) From 0ca36e5ffc8bbb0a7d6dd329768f607a7bff4c5a Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Wed, 12 Apr 2023 11:20:32 -0400 Subject: [PATCH 31/43] Update network_server.go --- x/sync/network_server.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 635b931d4017..fcb7152e688c 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -182,7 +182,7 @@ func (s *NetworkServer) HandleChangeProofRequest( // the proof size was too large, try to shrink it // ensure that the new limit is always smaller - keyLimit = uint16((len(changeProof.KeyValues) + len(changeProof.DeletedKeys)) / 2) + keyLimit = uint16(len(changeProof.KeyValues) + len(changeProof.DeletedKeys)) // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit bytesEstimate := getBytesEstimateOfProofNodes(changeProof.StartProof) @@ -198,7 +198,7 @@ func (s *NetworkServer) HandleChangeProofRequest( changeKeyIndex := 0 // shrink more if the early keys are extremely large - for keyIndex := uint16(1); keyIndex < keyLimit; keyIndex++ { + for keyIndex := uint16(0); keyIndex < keyLimit; keyIndex++ { // determine if the deleted key or changed key is the next smallest key keyBytesCount := 0 @@ -208,9 +208,6 @@ func (s *NetworkServer) HandleChangeProofRequest( (changeKeyIndex >= len(changeProof.KeyValues) || bytes.Compare(changeProof.KeyValues[changeKeyIndex].Key, changeProof.DeletedKeys[deleteKeyIndex]) > 0) { keyBytesCount = merkledb.Codec.ByteSliceSize(changeProof.DeletedKeys[deleteKeyIndex]) - if err != nil { - return err - } deleteKeyIndex++ } else if changeKeyIndex < len(changeProof.KeyValues) { keyBytesCount = merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + @@ -284,7 +281,7 @@ func (s *NetworkServer) HandleRangeProofRequest( // the proof size was too large, try to shrink it // ensure that the new limit is always smaller - keyLimit = uint16(len(rangeProof.KeyValues) / 2) + keyLimit = uint16(len(rangeProof.KeyValues)) // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit bytesEstimate := getBytesEstimateOfProofNodes(rangeProof.StartProof) @@ -299,7 +296,7 @@ func (s *NetworkServer) HandleRangeProofRequest( bytesEstimate += getBytesEstimateOfProofNodes(rangeProof.EndProof) + endProofSizeBufferAmount // shrink more if the early keys are extremely large - for keyIndex := uint16(1); keyIndex < keyLimit; keyIndex++ { + for keyIndex := uint16(0); keyIndex < keyLimit; keyIndex++ { nextKV := rangeProof.KeyValues[keyIndex] kvEstBytes := merkledb.Codec.ByteSliceSize(nextKV.Key) + merkledb.Codec.ByteSliceSize(nextKV.Value) From 6bfa30fde28af4b26b2091b8d52910bcde6b1e09 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Wed, 12 Apr 2023 11:36:00 -0400 Subject: [PATCH 32/43] switch order --- x/sync/network_server.go | 45 +++++++++++++++++------------------ x/sync/network_server_test.go | 9 ------- 2 files changed, 22 insertions(+), 32 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index fcb7152e688c..1b1a87f7c8f7 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -182,46 +182,48 @@ func (s *NetworkServer) HandleChangeProofRequest( // the proof size was too large, try to shrink it // ensure that the new limit is always smaller - keyLimit = uint16(len(changeProof.KeyValues) + len(changeProof.DeletedKeys)) + keyLimit = uint16(len(changeProof.KeyValues)+len(changeProof.DeletedKeys)) - 1 // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit - bytesEstimate := getBytesEstimateOfProofNodes(changeProof.StartProof) + sizeOfStartProof := getBytesEstimateOfProofNodes(changeProof.StartProof) // just the start proof is too large, so a proof is impossible - if bytesEstimate > int(req.BytesLimit) { + if sizeOfStartProof > int(req.BytesLimit) { return ErrMinProofSizeIsTooLarge } - // since the number of keys is decreasing, the new endproof will be a different size than the current one - // use the current endproof plus a small buffer to account for potential size differences - bytesEstimate += getBytesEstimateOfProofNodes(changeProof.EndProof) + endProofSizeBufferAmount + + keyValueBytes := len(proofBytes) - (sizeOfStartProof + getBytesEstimateOfProofNodes(changeProof.EndProof)) + + // since the number of keys is changing, the new endproof will be a different size than the current one + // add some small buffer to account for potential size differences + keyValueBytes += endProofSizeBufferAmount + deleteKeyIndex := 0 changeKeyIndex := 0 - // shrink more if the early keys are extremely large - for keyIndex := uint16(0); keyIndex < keyLimit; keyIndex++ { + // shrink the number of keys until it fits within the limit + for keyIndex := keyLimit - 1; keyIndex >= 0; keyIndex-- { // determine if the deleted key or changed key is the next smallest key - keyBytesCount := 0 // if there is a deleted key at deleteKeyIndex and // (there are no more change keys or the changed key is larger than the deleted key) if deleteKeyIndex < len(changeProof.DeletedKeys) && (changeKeyIndex >= len(changeProof.KeyValues) || bytes.Compare(changeProof.KeyValues[changeKeyIndex].Key, changeProof.DeletedKeys[deleteKeyIndex]) > 0) { - keyBytesCount = merkledb.Codec.ByteSliceSize(changeProof.DeletedKeys[deleteKeyIndex]) + keyValueBytes -= merkledb.Codec.ByteSliceSize(changeProof.DeletedKeys[deleteKeyIndex]) deleteKeyIndex++ } else if changeKeyIndex < len(changeProof.KeyValues) { - keyBytesCount = merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + + keyValueBytes -= merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Value) changeKeyIndex++ } - if bytesEstimate+keyBytesCount > bytesLimit { + if keyValueBytes < bytesLimit { // adding the current KV would put the size over the limit // so only return up to the keyIndex number of keys keyLimit = keyIndex break } - bytesEstimate += keyBytesCount } } return ErrMinProofSizeIsTooLarge @@ -281,32 +283,29 @@ func (s *NetworkServer) HandleRangeProofRequest( // the proof size was too large, try to shrink it // ensure that the new limit is always smaller - keyLimit = uint16(len(rangeProof.KeyValues)) + keyLimit = uint16(len(rangeProof.KeyValues)) - 1 // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit - bytesEstimate := getBytesEstimateOfProofNodes(rangeProof.StartProof) + sizeOfStartProof := getBytesEstimateOfProofNodes(rangeProof.StartProof) // just the start proof is too large, so a proof is impossible - if bytesEstimate > int(req.BytesLimit) { + if sizeOfStartProof > int(req.BytesLimit) { return ErrMinProofSizeIsTooLarge } - // since the number of keys is decreasing, the new endproof will be a different size than the current one - // use the current endproof plus a small buffer to account for potential size differences - bytesEstimate += getBytesEstimateOfProofNodes(rangeProof.EndProof) + endProofSizeBufferAmount + keyValueBytes := len(proofBytes) - (sizeOfStartProof + getBytesEstimateOfProofNodes(rangeProof.EndProof)) // shrink more if the early keys are extremely large - for keyIndex := uint16(0); keyIndex < keyLimit; keyIndex++ { + for keyIndex := keyLimit - 1; keyIndex >= 0; keyIndex-- { nextKV := rangeProof.KeyValues[keyIndex] - kvEstBytes := merkledb.Codec.ByteSliceSize(nextKV.Key) + merkledb.Codec.ByteSliceSize(nextKV.Value) + keyValueBytes -= merkledb.Codec.ByteSliceSize(nextKV.Key) + merkledb.Codec.ByteSliceSize(nextKV.Value) - if bytesEstimate+kvEstBytes > bytesLimit { + if keyValueBytes < bytesLimit { // adding the current KV would put the size over the limit // so only return up to the keyIndex number of keys keyLimit = keyIndex break } - bytesEstimate += kvEstBytes } } return ErrMinProofSizeIsTooLarge diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index 426268dda348..dfe968e7396a 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -181,15 +181,6 @@ func Test_Server_GetChangeProof(t *testing.T) { nodeID ids.NodeID proofNil bool }{ - "proof too small": { - request: &ChangeProofRequest{ - StartingRoot: startRoot, - EndingRoot: endRoot, - KeyLimit: defaultRequestKeyLimit, - BytesLimit: 1000, - }, - proofNil: true, - }, "byteslimit is 0": { request: &ChangeProofRequest{ StartingRoot: startRoot, From 75784944bf61002d83e435d5d6be9247f37cef71 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Wed, 12 Apr 2023 11:38:19 -0400 Subject: [PATCH 33/43] Update network_server.go --- x/sync/network_server.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 1b1a87f7c8f7..93c4a6a41e94 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -181,8 +181,7 @@ func (s *NetworkServer) HandleChangeProofRequest( } // the proof size was too large, try to shrink it - // ensure that the new limit is always smaller - keyLimit = uint16(len(changeProof.KeyValues)+len(changeProof.DeletedKeys)) - 1 + keyLimit = uint16(len(changeProof.KeyValues) + len(changeProof.DeletedKeys)) // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit sizeOfStartProof := getBytesEstimateOfProofNodes(changeProof.StartProof) @@ -283,7 +282,7 @@ func (s *NetworkServer) HandleRangeProofRequest( // the proof size was too large, try to shrink it // ensure that the new limit is always smaller - keyLimit = uint16(len(rangeProof.KeyValues)) - 1 + keyLimit = uint16(len(rangeProof.KeyValues)) // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit sizeOfStartProof := getBytesEstimateOfProofNodes(rangeProof.StartProof) From 87d6266e757e4521fb2b79fc5b86bf94021415e1 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Wed, 12 Apr 2023 11:51:00 -0400 Subject: [PATCH 34/43] Update network_server.go --- x/sync/network_server.go | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 93c4a6a41e94..1f4616ef2825 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -197,30 +197,28 @@ func (s *NetworkServer) HandleChangeProofRequest( // add some small buffer to account for potential size differences keyValueBytes += endProofSizeBufferAmount - deleteKeyIndex := 0 - changeKeyIndex := 0 + deleteKeyIndex := len(changeProof.DeletedKeys) - 1 + changeKeyIndex := len(changeProof.KeyValues) - 1 // shrink the number of keys until it fits within the limit - for keyIndex := keyLimit - 1; keyIndex >= 0; keyIndex-- { + for ; keyLimit > 0; keyLimit-- { // determine if the deleted key or changed key is the next smallest key // if there is a deleted key at deleteKeyIndex and // (there are no more change keys or the changed key is larger than the deleted key) - if deleteKeyIndex < len(changeProof.DeletedKeys) && - (changeKeyIndex >= len(changeProof.KeyValues) || + if deleteKeyIndex >= 0 && + (changeKeyIndex >= 0 || bytes.Compare(changeProof.KeyValues[changeKeyIndex].Key, changeProof.DeletedKeys[deleteKeyIndex]) > 0) { keyValueBytes -= merkledb.Codec.ByteSliceSize(changeProof.DeletedKeys[deleteKeyIndex]) - deleteKeyIndex++ - } else if changeKeyIndex < len(changeProof.KeyValues) { + deleteKeyIndex-- + } else if changeKeyIndex >= 0 { keyValueBytes -= merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Value) - changeKeyIndex++ + changeKeyIndex-- } + // we have eliminated enough keys to be under the limit if keyValueBytes < bytesLimit { - // adding the current KV would put the size over the limit - // so only return up to the keyIndex number of keys - keyLimit = keyIndex break } } @@ -295,14 +293,12 @@ func (s *NetworkServer) HandleRangeProofRequest( keyValueBytes := len(proofBytes) - (sizeOfStartProof + getBytesEstimateOfProofNodes(rangeProof.EndProof)) // shrink more if the early keys are extremely large - for keyIndex := keyLimit - 1; keyIndex >= 0; keyIndex-- { - nextKV := rangeProof.KeyValues[keyIndex] + for ; keyLimit > 0; keyLimit-- { + nextKV := rangeProof.KeyValues[keyLimit-1] keyValueBytes -= merkledb.Codec.ByteSliceSize(nextKV.Key) + merkledb.Codec.ByteSliceSize(nextKV.Value) + // we have eliminated enough keys to be under the limit if keyValueBytes < bytesLimit { - // adding the current KV would put the size over the limit - // so only return up to the keyIndex number of keys - keyLimit = keyIndex break } } From fd382fd11bab2d21574e1753bd8a6a938b573bca Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Wed, 12 Apr 2023 12:05:16 -0400 Subject: [PATCH 35/43] Update network_server.go --- x/sync/network_server.go | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index 0ef8b821c001..a3e36f209627 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -195,36 +195,27 @@ func (s *NetworkServer) HandleChangeProofRequest( return ErrMinProofSizeIsTooLarge } - keyValueBytes := len(proofBytes) - (sizeOfStartProof + getBytesEstimateOfProofNodes(changeProof.EndProof)) - // since the number of keys is changing, the new endproof will be a different size than the current one // add some small buffer to account for potential size differences - keyValueBytes += endProofSizeBufferAmount + totalBytes := len(proofBytes) + endProofSizeBufferAmount deleteKeyIndex := len(changeProof.DeletedKeys) - 1 changeKeyIndex := len(changeProof.KeyValues) - 1 // shrink the number of keys until it fits within the limit - for ; keyLimit > 0; keyLimit-- { - // determine if the deleted key or changed key is the next smallest key - + for ; keyLimit > 0 && totalBytes >= bytesLimit; keyLimit-- { // if there is a deleted key at deleteKeyIndex and // (there are no more change keys or the changed key is larger than the deleted key) if deleteKeyIndex >= 0 && (changeKeyIndex >= 0 || bytes.Compare(changeProof.KeyValues[changeKeyIndex].Key, changeProof.DeletedKeys[deleteKeyIndex]) > 0) { - keyValueBytes -= merkledb.Codec.ByteSliceSize(changeProof.DeletedKeys[deleteKeyIndex]) + totalBytes -= merkledb.Codec.ByteSliceSize(changeProof.DeletedKeys[deleteKeyIndex]) deleteKeyIndex-- } else if changeKeyIndex >= 0 { - keyValueBytes -= merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + + totalBytes -= merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Value) changeKeyIndex-- } - - // we have eliminated enough keys to be under the limit - if keyValueBytes < bytesLimit { - break - } } } return ErrMinProofSizeIsTooLarge @@ -294,17 +285,14 @@ func (s *NetworkServer) HandleRangeProofRequest( return ErrMinProofSizeIsTooLarge } - keyValueBytes := len(proofBytes) - (sizeOfStartProof + getBytesEstimateOfProofNodes(rangeProof.EndProof)) + // since the number of keys is changing, the new endproof will be a different size than the current one + // add some small buffer to account for potential size differences + totalBytes := len(proofBytes) + endProofSizeBufferAmount // shrink more if the early keys are extremely large - for ; keyLimit > 0; keyLimit-- { + for ; keyLimit > 0 && totalBytes >= bytesLimit; keyLimit-- { nextKV := rangeProof.KeyValues[keyLimit-1] - keyValueBytes -= merkledb.Codec.ByteSliceSize(nextKV.Key) + merkledb.Codec.ByteSliceSize(nextKV.Value) - - // we have eliminated enough keys to be under the limit - if keyValueBytes < bytesLimit { - break - } + totalBytes -= merkledb.Codec.ByteSliceSize(nextKV.Key) + merkledb.Codec.ByteSliceSize(nextKV.Value) } } return ErrMinProofSizeIsTooLarge From aca8108474c0483fd83257d93a5e25f2e7f73255 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Wed, 12 Apr 2023 14:01:32 -0400 Subject: [PATCH 36/43] Update network_server.go --- x/sync/network_server.go | 56 ++-------------------------------------- 1 file changed, 2 insertions(+), 54 deletions(-) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index a3e36f209627..bbb05d9b3aac 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -184,39 +184,7 @@ func (s *NetworkServer) HandleChangeProofRequest( return s.appSender.SendAppResponse(ctx, nodeID, requestID, proofBytes) } // the proof size was too large, try to shrink it - - keyLimit = uint16(len(changeProof.KeyValues) + len(changeProof.DeletedKeys)) - - // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit - sizeOfStartProof := getBytesEstimateOfProofNodes(changeProof.StartProof) - - // just the start proof is too large, so a proof is impossible - if sizeOfStartProof > int(req.BytesLimit) { - return ErrMinProofSizeIsTooLarge - } - - // since the number of keys is changing, the new endproof will be a different size than the current one - // add some small buffer to account for potential size differences - totalBytes := len(proofBytes) + endProofSizeBufferAmount - - deleteKeyIndex := len(changeProof.DeletedKeys) - 1 - changeKeyIndex := len(changeProof.KeyValues) - 1 - - // shrink the number of keys until it fits within the limit - for ; keyLimit > 0 && totalBytes >= bytesLimit; keyLimit-- { - // if there is a deleted key at deleteKeyIndex and - // (there are no more change keys or the changed key is larger than the deleted key) - if deleteKeyIndex >= 0 && - (changeKeyIndex >= 0 || - bytes.Compare(changeProof.KeyValues[changeKeyIndex].Key, changeProof.DeletedKeys[deleteKeyIndex]) > 0) { - totalBytes -= merkledb.Codec.ByteSliceSize(changeProof.DeletedKeys[deleteKeyIndex]) - deleteKeyIndex-- - } else if changeKeyIndex >= 0 { - totalBytes -= merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Key) + - merkledb.Codec.ByteSliceSize(changeProof.KeyValues[changeKeyIndex].Value) - changeKeyIndex-- - } - } + keyLimit = uint16(len(changeProof.KeyValues)+len(changeProof.DeletedKeys)) / 2 } return ErrMinProofSizeIsTooLarge } @@ -273,27 +241,7 @@ func (s *NetworkServer) HandleRangeProofRequest( return s.appSender.SendAppResponse(ctx, nodeID, requestID, proofBytes) } // the proof size was too large, try to shrink it - - // ensure that the new limit is always smaller - keyLimit = uint16(len(rangeProof.KeyValues)) - - // estimate the bytes of the start and end proof to ensure that everything will fit into the bytesLimit - sizeOfStartProof := getBytesEstimateOfProofNodes(rangeProof.StartProof) - - // just the start proof is too large, so a proof is impossible - if sizeOfStartProof > int(req.BytesLimit) { - return ErrMinProofSizeIsTooLarge - } - - // since the number of keys is changing, the new endproof will be a different size than the current one - // add some small buffer to account for potential size differences - totalBytes := len(proofBytes) + endProofSizeBufferAmount - - // shrink more if the early keys are extremely large - for ; keyLimit > 0 && totalBytes >= bytesLimit; keyLimit-- { - nextKV := rangeProof.KeyValues[keyLimit-1] - totalBytes -= merkledb.Codec.ByteSliceSize(nextKV.Key) + merkledb.Codec.ByteSliceSize(nextKV.Value) - } + keyLimit = uint16(len(rangeProof.KeyValues)) / 2 } return ErrMinProofSizeIsTooLarge } From a9d8596b86bdec7ca040bbf209330df747addbc0 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Wed, 12 Apr 2023 14:04:21 -0400 Subject: [PATCH 37/43] remove unused codec functions --- x/merkledb/codec.go | 25 ------------------ x/merkledb/codec_test.go | 55 ---------------------------------------- x/sync/network_server.go | 8 ------ 3 files changed, 88 deletions(-) diff --git a/x/merkledb/codec.go b/x/merkledb/codec.go index e5ece433b33b..7baa37159981 100644 --- a/x/merkledb/codec.go +++ b/x/merkledb/codec.go @@ -72,11 +72,6 @@ type EncoderDecoder interface { } type Encoder interface { - // ByteSliceSize provides the number of bytes in the serialized form of byte slice - ByteSliceSize(value []byte) int - // ProofNodeSize provides the number of bytes in the serialized form of a ProofNode - ProofNodeSize(value ProofNode) int - EncodeProof(version uint16, p *Proof) ([]byte, error) EncodeChangeProof(version uint16, p *ChangeProof) ([]byte, error) EncodeRangeProof(version uint16, p *RangeProof) ([]byte, error) @@ -669,26 +664,6 @@ func (c *codecImpl) decodeByteSlice(src *bytes.Reader) ([]byte, error) { return result, nil } -func (c *codecImpl) ByteSliceSize(value []byte) int { - buf := c.varIntPool.Get().([]byte) - size := binary.PutVarint(buf, int64(len(value))) - c.varIntPool.Put(buf) - return size + len(value) -} - -func (c *codecImpl) ProofNodeSize(proofNode ProofNode) int { - sizeOfKeyPath := c.ByteSliceSize(proofNode.KeyPath.Value) - sizeOfMaybeValue := c.ByteSliceSize(proofNode.ValueOrHash.Value()) + 1 - - buf := c.varIntPool.Get().([]byte) - sizeOfChildrenCount := binary.PutVarint(buf, int64(len(proofNode.Children))) - c.varIntPool.Put(buf) - - sizeOfChildren := sizeOfChildrenCount + len(proofNode.Children)*(len(ids.Empty)+1) - - return sizeOfKeyPath + sizeOfMaybeValue + sizeOfChildren -} - func (c *codecImpl) encodeByteSlice(dst io.Writer, value []byte) error { if err := c.encodeInt(dst, len(value)); err != nil { return err diff --git a/x/merkledb/codec_test.go b/x/merkledb/codec_test.go index 4e65ada6cbde..f3561c815e5b 100644 --- a/x/merkledb/codec_test.go +++ b/x/merkledb/codec_test.go @@ -555,61 +555,6 @@ func FuzzCodecDBNodeDeterministic(f *testing.F) { ) } -func FuzzCodecImplByteSliceSizeMatchesEncodeByteSlice(f *testing.F) { - f.Fuzz( - func( - t *testing.T, - b []byte, - ) { - require := require.New(t) - - codec := Codec.(*codecImpl) - buf := &bytes.Buffer{} - err := codec.encodeByteSlice(buf, b) - require.NoError(err) - require.Equal(buf.Len(), codec.ByteSliceSize(b)) - }, - ) -} - -func TestCodecImpl_ProofNodeSize_Matches_EncodeProofNode(t *testing.T) { - require := require.New(t) - codec := Codec.(*codecImpl) - for i := 0; i < 100; i++ { - r := rand.New(rand.NewSource(int64(i))) // #nosec G404 - - proofNode := newRandomProofNode(r) - - buf := &bytes.Buffer{} - err := codec.encodeProofNode(proofNode, buf) - require.NoError(err) - require.Equal(buf.Len(), codec.ProofNodeSize(proofNode)) - } -} - -func TestCodecImpl_ByteSliceSize_Matches_EncodeByteSlice(t *testing.T) { - require := require.New(t) - codec := Codec.(*codecImpl) - - var currentValue []byte - buf := &bytes.Buffer{} - err := codec.encodeByteSlice(buf, currentValue) - require.NoError(err) - require.Equal(buf.Len(), codec.ByteSliceSize(currentValue)) - - for i := 0; i < 100; i++ { - r := rand.New(rand.NewSource(int64(i))) // #nosec G404 - currentValue = make([]byte, r.Intn(500)) - _, err := r.Read(currentValue) - require.NoError(err) - - buf := &bytes.Buffer{} - err = codec.encodeByteSlice(buf, currentValue) - require.NoError(err) - require.Equal(buf.Len(), codec.ByteSliceSize(currentValue)) - } -} - func TestCodec_DecodeProof(t *testing.T) { require := require.New(t) diff --git a/x/sync/network_server.go b/x/sync/network_server.go index bbb05d9b3aac..fb38879787ff 100644 --- a/x/sync/network_server.go +++ b/x/sync/network_server.go @@ -245,11 +245,3 @@ func (s *NetworkServer) HandleRangeProofRequest( } return ErrMinProofSizeIsTooLarge } - -func getBytesEstimateOfProofNodes(proofNodes []merkledb.ProofNode) int { - total := 0 - for _, proofNode := range proofNodes { - total += merkledb.Codec.ProofNodeSize(proofNode) - } - return total -} From 46f7326325908481fbfe57317311587250dcf6a5 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Wed, 12 Apr 2023 14:07:08 -0400 Subject: [PATCH 38/43] update constants --- x/sync/syncmanager.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x/sync/syncmanager.go b/x/sync/syncmanager.go index 5fc6579349b4..7682a6ff3301 100644 --- a/x/sync/syncmanager.go +++ b/x/sync/syncmanager.go @@ -14,13 +14,12 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/utils/logging" - "github.com/ava-labs/avalanchego/utils/units" "github.com/ava-labs/avalanchego/x/merkledb" ) const ( - defaultRequestKeyLimit = 1024 - defaultRequestByteSizeLimit = 512 * units.KiB + defaultRequestKeyLimit = maxKeyValuesLimit + defaultRequestByteSizeLimit = maxByteSizeLimit ) var ( From 4b44b873d7880a433960391ace09ef09e543c392 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Wed, 12 Apr 2023 14:53:31 -0400 Subject: [PATCH 39/43] fix test --- x/sync/client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/sync/client_test.go b/x/sync/client_test.go index 6b1fdfc0ab84..02c8f0801e56 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -460,7 +460,7 @@ func TestGetChangeProof(t *testing.T) { BytesLimit: defaultRequestByteSizeLimit, }, modifyResponse: func(response *merkledb.ChangeProof) { - response.KeyValues = append(response.KeyValues, merkledb.KeyValue{}) + response.KeyValues = append(response.KeyValues, make([]merkledb.KeyValue, defaultRequestKeyLimit)...) }, expectedErr: errTooManyKeys, }, From 3ab91a671eef95b7c8081ebe772d726400d1492e Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Wed, 12 Apr 2023 15:15:01 -0400 Subject: [PATCH 40/43] Update client_test.go --- x/sync/client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/sync/client_test.go b/x/sync/client_test.go index 02c8f0801e56..baaec01056ac 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -495,7 +495,7 @@ func TestGetChangeProof(t *testing.T) { modifyResponse: func(response *merkledb.ChangeProof) { response.KeyValues = response.KeyValues[:len(response.KeyValues)-2] }, - expectedErr: merkledb.ErrProofNodeNotForKey, + expectedErr: merkledb.ErrInvalidProof, }, "removed key from middle of response": { request: &ChangeProofRequest{ From 90e4209f7ca658e3468638e5140b01032cffc67f Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Wed, 12 Apr 2023 15:27:15 -0400 Subject: [PATCH 41/43] Update client_test.go --- x/sync/client_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x/sync/client_test.go b/x/sync/client_test.go index baaec01056ac..23debd08b527 100644 --- a/x/sync/client_test.go +++ b/x/sync/client_test.go @@ -372,8 +372,8 @@ func TestGetChangeProof(t *testing.T) { memdb.New(), merkledb.Config{ Tracer: newNoopTracer(), - HistoryLength: 1000, - NodeCacheSize: 1000, + HistoryLength: defaultRequestKeyLimit, + NodeCacheSize: defaultRequestKeyLimit, }, ) require.NoError(t, err) @@ -382,8 +382,8 @@ func TestGetChangeProof(t *testing.T) { memdb.New(), merkledb.Config{ Tracer: newNoopTracer(), - HistoryLength: 1000, - NodeCacheSize: 1000, + HistoryLength: defaultRequestKeyLimit, + NodeCacheSize: defaultRequestKeyLimit, }, ) require.NoError(t, err) @@ -391,7 +391,7 @@ func TestGetChangeProof(t *testing.T) { require.NoError(t, err) // create changes - for x := 0; x < 200; x++ { + for x := 0; x < defaultRequestKeyLimit/2; x++ { view, err := trieDB.NewView() require.NoError(t, err) @@ -483,7 +483,7 @@ func TestGetChangeProof(t *testing.T) { modifyResponse: func(response *merkledb.ChangeProof) { response.KeyValues = response.KeyValues[1:] }, - expectedErr: merkledb.ErrProofValueDoesntMatch, + expectedErr: merkledb.ErrInvalidProof, }, "removed last key in response": { request: &ChangeProofRequest{ @@ -495,7 +495,7 @@ func TestGetChangeProof(t *testing.T) { modifyResponse: func(response *merkledb.ChangeProof) { response.KeyValues = response.KeyValues[:len(response.KeyValues)-2] }, - expectedErr: merkledb.ErrInvalidProof, + expectedErr: merkledb.ErrProofNodeNotForKey, }, "removed key from middle of response": { request: &ChangeProofRequest{ From 51bdf7f974a192ac420429b4478794eb5c634bcc Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Thu, 13 Apr 2023 16:19:06 -0400 Subject: [PATCH 42/43] Update network_server_test.go --- x/sync/network_server_test.go | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index dfe968e7396a..2199bce4361c 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -27,11 +27,12 @@ func Test_Server_GetRangeProof(t *testing.T) { require.NoError(t, err) tests := map[string]struct { - request *RangeProofRequest - expectedErr error - expectedResponseLen int - nodeID ids.NodeID - proofNil bool + request *RangeProofRequest + expectedErr error + expectedResponseLen int + expectedMaxResponseBytes int + nodeID ids.NodeID + proofNil bool }{ "proof too small": { request: &RangeProofRequest{ @@ -82,6 +83,7 @@ func Test_Server_GetRangeProof(t *testing.T) { KeyLimit: defaultRequestKeyLimit, BytesLimit: 2 * defaultRequestByteSizeLimit, }, + expectedMaxResponseBytes: defaultRequestByteSizeLimit, }, } @@ -128,6 +130,9 @@ func Test_Server_GetRangeProof(t *testing.T) { bytes, err := merkledb.Codec.EncodeRangeProof(Version, proofResult) require.NoError(err) require.LessOrEqual(len(bytes), int(test.request.BytesLimit)) + if test.expectedMaxResponseBytes > 0 { + require.LessOrEqual(len(bytes), test.expectedMaxResponseBytes) + } }) } } @@ -175,11 +180,12 @@ func Test_Server_GetChangeProof(t *testing.T) { require.NoError(t, err) tests := map[string]struct { - request *ChangeProofRequest - expectedErr error - expectedResponseLen int - nodeID ids.NodeID - proofNil bool + request *ChangeProofRequest + expectedErr error + expectedResponseLen int + expectedMaxResponseBytes int + nodeID ids.NodeID + proofNil bool }{ "byteslimit is 0": { request: &ChangeProofRequest{ @@ -226,6 +232,7 @@ func Test_Server_GetChangeProof(t *testing.T) { KeyLimit: defaultRequestKeyLimit, BytesLimit: 2 * defaultRequestByteSizeLimit, }, + expectedMaxResponseBytes: defaultRequestByteSizeLimit, }, } @@ -272,6 +279,9 @@ func Test_Server_GetChangeProof(t *testing.T) { bytes, err := merkledb.Codec.EncodeChangeProof(Version, proofResult) require.NoError(err) require.LessOrEqual(len(bytes), int(test.request.BytesLimit)) + if test.expectedMaxResponseBytes > 0 { + require.LessOrEqual(len(bytes), test.expectedMaxResponseBytes) + } }) } } From a6cceb1db2e5738ef2906ed6a67615532f1cb447 Mon Sep 17 00:00:00 2001 From: dboehm-avalabs Date: Thu, 13 Apr 2023 16:33:53 -0400 Subject: [PATCH 43/43] Update network_server_test.go --- x/sync/network_server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/sync/network_server_test.go b/x/sync/network_server_test.go index 2199bce4361c..0c373cacca1d 100644 --- a/x/sync/network_server_test.go +++ b/x/sync/network_server_test.go @@ -34,7 +34,7 @@ func Test_Server_GetRangeProof(t *testing.T) { nodeID ids.NodeID proofNil bool }{ - "proof too small": { + "proof too large": { request: &RangeProofRequest{ Root: smallTrieRoot, KeyLimit: defaultRequestKeyLimit,