From 56dc93ba4a283d8e3d27743768f7e7e54dc4317b Mon Sep 17 00:00:00 2001 From: aaronbuchwald Date: Mon, 6 Feb 2023 16:54:01 -0800 Subject: [PATCH] Signature handler refactor (#495) * Refactor signature handler and stats * Remove unnecessary lock --- handlers/handler.go | 5 +- handlers/stats/mock_stats.go | 152 +--------- handlers/stats/stats.go | 269 +----------------- handlers/{ => warp}/signature_request.go | 4 +- handlers/{ => warp}/signature_request_test.go | 2 +- handlers/warp/stats/stats.go | 95 +++++++ plugin/evm/warp/backend.go | 8 +- 7 files changed, 119 insertions(+), 416 deletions(-) rename handlers/{ => warp}/signature_request.go (97%) rename handlers/{ => warp}/signature_request_test.go (99%) create mode 100644 handlers/warp/stats/stats.go diff --git a/handlers/handler.go b/handlers/handler.go index 79eff82d70..0ab0a626fb 100644 --- a/handlers/handler.go +++ b/handlers/handler.go @@ -9,6 +9,7 @@ import ( "github.com/ava-labs/avalanchego/codec" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/subnet-evm/handlers/stats" + warpHandlers "github.com/ava-labs/subnet-evm/handlers/warp" "github.com/ava-labs/subnet-evm/metrics" "github.com/ava-labs/subnet-evm/plugin/evm/message" "github.com/ava-labs/subnet-evm/sync/handlers" @@ -22,7 +23,7 @@ type networkHandler struct { stateTrieLeafsRequestHandler *syncHandlers.LeafsRequestHandler blockRequestHandler *syncHandlers.BlockRequestHandler codeRequestHandler *syncHandlers.CodeRequestHandler - signatureRequestHandler SignatureRequestHandler + signatureRequestHandler warpHandlers.SignatureRequestHandler } // NewNetworkHandler constructs the handler for serving network requests. @@ -39,7 +40,7 @@ func NewNetworkHandler( codeRequestHandler: syncHandlers.NewCodeRequestHandler(evmTrieDB.DiskDB(), networkCodec, handlerStats), // TODO: initialize actual signature request handler when warp is ready - signatureRequestHandler: &NoopSignatureRequestHandler{}, + signatureRequestHandler: &warpHandlers.NoopSignatureRequestHandler{}, } } diff --git a/handlers/stats/mock_stats.go b/handlers/stats/mock_stats.go index ab9b4e340d..45ffb7aaa2 100644 --- a/handlers/stats/mock_stats.go +++ b/handlers/stats/mock_stats.go @@ -4,9 +4,7 @@ package stats import ( - "sync" - "time" - + warpStats "github.com/ava-labs/subnet-evm/handlers/warp/stats" syncStats "github.com/ava-labs/subnet-evm/sync/handlers/stats" ) @@ -14,151 +12,11 @@ var _ HandlerStats = &MockHandlerStats{} // MockHandlerStats is mock for capturing and asserting on handler metrics in test type MockHandlerStats struct { - lock sync.Mutex - - syncHandlerStats syncStats.MockHandlerStats - - SignatureRequestCount, - SignatureRequestHit, - SignatureRequestMiss uint32 - SignatureRequestDuration time.Duration + syncStats.MockHandlerStats + warpStats.MockSignatureRequestHandlerStats } func (m *MockHandlerStats) Reset() { - m.lock.Lock() - defer m.lock.Unlock() - m.syncHandlerStats.Reset() - - m.SignatureRequestCount = 0 - m.SignatureRequestHit = 0 - m.SignatureRequestMiss = 0 - m.SignatureRequestDuration = 0 -} - -func (m *MockHandlerStats) IncBlockRequest() { - m.syncHandlerStats.IncBlockRequest() -} - -func (m *MockHandlerStats) IncMissingBlockHash() { - m.syncHandlerStats.IncMissingBlockHash() -} - -func (m *MockHandlerStats) UpdateBlocksReturned(num uint16) { - m.syncHandlerStats.UpdateBlocksReturned(num) -} - -func (m *MockHandlerStats) UpdateBlockRequestProcessingTime(duration time.Duration) { - m.syncHandlerStats.UpdateBlockRequestProcessingTime(duration) -} - -func (m *MockHandlerStats) IncCodeRequest() { - m.syncHandlerStats.IncCodeRequest() -} - -func (m *MockHandlerStats) IncMissingCodeHash() { - m.syncHandlerStats.IncMissingCodeHash() -} - -func (m *MockHandlerStats) IncTooManyHashesRequested() { - m.syncHandlerStats.IncTooManyHashesRequested() -} - -func (m *MockHandlerStats) IncDuplicateHashesRequested() { - m.syncHandlerStats.IncDuplicateHashesRequested() -} - -func (m *MockHandlerStats) UpdateCodeReadTime(duration time.Duration) { - m.syncHandlerStats.UpdateCodeReadTime(duration) -} - -func (m *MockHandlerStats) UpdateCodeBytesReturned(bytes uint32) { - m.syncHandlerStats.UpdateCodeBytesReturned(bytes) -} - -func (m *MockHandlerStats) IncLeafsRequest() { - m.syncHandlerStats.IncLeafsRequest() -} - -func (m *MockHandlerStats) IncInvalidLeafsRequest() { - m.syncHandlerStats.IncInvalidLeafsRequest() -} - -func (m *MockHandlerStats) UpdateLeafsReturned(numLeafs uint16) { - m.syncHandlerStats.UpdateLeafsReturned(numLeafs) -} - -func (m *MockHandlerStats) UpdateLeafsRequestProcessingTime(duration time.Duration) { - m.syncHandlerStats.UpdateLeafsRequestProcessingTime(duration) -} - -func (m *MockHandlerStats) UpdateReadLeafsTime(duration time.Duration) { - m.syncHandlerStats.UpdateReadLeafsTime(duration) -} - -func (m *MockHandlerStats) UpdateSnapshotReadTime(duration time.Duration) { - m.syncHandlerStats.UpdateSnapshotReadTime(duration) -} - -func (m *MockHandlerStats) UpdateGenerateRangeProofTime(duration time.Duration) { - m.syncHandlerStats.UpdateGenerateRangeProofTime(duration) -} - -func (m *MockHandlerStats) UpdateRangeProofValsReturned(numProofVals int64) { - m.syncHandlerStats.UpdateRangeProofValsReturned(numProofVals) -} - -func (m *MockHandlerStats) IncMissingRoot() { - m.syncHandlerStats.IncMissingRoot() -} - -func (m *MockHandlerStats) IncTrieError() { - m.syncHandlerStats.IncTrieError() -} - -func (m *MockHandlerStats) IncProofError() { - m.syncHandlerStats.IncProofError() -} - -func (m *MockHandlerStats) IncSnapshotReadError() { - m.syncHandlerStats.IncSnapshotReadError() -} - -func (m *MockHandlerStats) IncSnapshotReadAttempt() { - m.syncHandlerStats.IncSnapshotReadAttempt() -} - -func (m *MockHandlerStats) IncSnapshotReadSuccess() { - m.syncHandlerStats.IncSnapshotReadSuccess() -} - -func (m *MockHandlerStats) IncSnapshotSegmentValid() { - m.syncHandlerStats.IncSnapshotSegmentValid() -} - -func (m *MockHandlerStats) IncSnapshotSegmentInvalid() { - m.syncHandlerStats.IncSnapshotSegmentInvalid() -} - -func (m *MockHandlerStats) IncSignatureRequest() { - m.lock.Lock() - defer m.lock.Unlock() - m.SignatureRequestCount++ -} - -func (m *MockHandlerStats) IncSignatureHit() { - m.lock.Lock() - defer m.lock.Unlock() - m.SignatureRequestHit++ -} - -func (m *MockHandlerStats) IncSignatureMiss() { - m.lock.Lock() - defer m.lock.Unlock() - m.SignatureRequestMiss++ -} - -func (m *MockHandlerStats) UpdateSignatureRequestTime(duration time.Duration) { - m.lock.Lock() - defer m.lock.Unlock() - m.SignatureRequestDuration += duration + m.MockHandlerStats.Reset() + m.MockSignatureRequestHandlerStats.Reset() } diff --git a/handlers/stats/stats.go b/handlers/stats/stats.go index 946027a910..357eb18f0b 100644 --- a/handlers/stats/stats.go +++ b/handlers/stats/stats.go @@ -4,284 +4,35 @@ package stats import ( - "time" - - "github.com/ava-labs/subnet-evm/metrics" + warpStats "github.com/ava-labs/subnet-evm/handlers/warp/stats" syncStats "github.com/ava-labs/subnet-evm/sync/handlers/stats" ) var ( _ HandlerStats = &handlerStats{} - _ HandlerStats = &noopHandlerStats{} + _ HandlerStats = &MockHandlerStats{} ) // HandlerStats reports prometheus metrics for the network handlers type HandlerStats interface { - SignatureRequestHandlerStats + warpStats.SignatureRequestHandlerStats syncStats.HandlerStats } -type SignatureRequestHandlerStats interface { - IncSignatureRequest() - IncSignatureHit() - IncSignatureMiss() - UpdateSignatureRequestTime(duration time.Duration) -} - type handlerStats struct { - // State sync metrics - syncHandlerStats syncStats.HandlerStats - - // SignatureRequestHandler metrics - signatureRequest metrics.Counter - signatureHit metrics.Counter - signatureMiss metrics.Counter - signatureProcessingTime metrics.Timer -} - -func (h *handlerStats) IncBlockRequest() { - h.syncHandlerStats.IncBlockRequest() -} - -func (h *handlerStats) IncMissingBlockHash() { - h.syncHandlerStats.IncMissingBlockHash() -} - -func (h *handlerStats) UpdateBlocksReturned(num uint16) { - h.syncHandlerStats.UpdateBlocksReturned(num) -} - -func (h *handlerStats) UpdateBlockRequestProcessingTime(duration time.Duration) { - h.syncHandlerStats.UpdateBlockRequestProcessingTime(duration) -} - -func (h *handlerStats) IncCodeRequest() { - h.syncHandlerStats.IncCodeRequest() -} - -func (h *handlerStats) IncMissingCodeHash() { - h.syncHandlerStats.IncMissingCodeHash() -} - -func (h *handlerStats) IncTooManyHashesRequested() { - h.syncHandlerStats.IncTooManyHashesRequested() -} - -func (h *handlerStats) IncDuplicateHashesRequested() { - h.syncHandlerStats.IncDuplicateHashesRequested() -} - -func (h *handlerStats) UpdateCodeReadTime(duration time.Duration) { - h.syncHandlerStats.UpdateCodeReadTime(duration) -} - -func (h *handlerStats) UpdateCodeBytesReturned(bytes uint32) { - h.syncHandlerStats.UpdateCodeBytesReturned(bytes) -} - -func (h *handlerStats) IncLeafsRequest() { - h.syncHandlerStats.IncLeafsRequest() -} - -func (h *handlerStats) IncInvalidLeafsRequest() { - h.syncHandlerStats.IncInvalidLeafsRequest() -} - -func (h *handlerStats) UpdateLeafsReturned(numLeafs uint16) { - h.syncHandlerStats.UpdateLeafsReturned(numLeafs) -} - -func (h *handlerStats) UpdateLeafsRequestProcessingTime(duration time.Duration) { - h.syncHandlerStats.UpdateLeafsRequestProcessingTime(duration) -} - -func (h *handlerStats) UpdateReadLeafsTime(duration time.Duration) { - h.syncHandlerStats.UpdateReadLeafsTime(duration) -} - -func (h *handlerStats) UpdateSnapshotReadTime(duration time.Duration) { - h.syncHandlerStats.UpdateSnapshotReadTime(duration) -} - -func (h *handlerStats) UpdateGenerateRangeProofTime(duration time.Duration) { - h.syncHandlerStats.UpdateGenerateRangeProofTime(duration) -} - -func (h *handlerStats) UpdateRangeProofValsReturned(numProofVals int64) { - h.syncHandlerStats.UpdateRangeProofValsReturned(numProofVals) -} - -func (h *handlerStats) IncMissingRoot() { - h.syncHandlerStats.IncMissingRoot() -} - -func (h *handlerStats) IncTrieError() { - h.syncHandlerStats.IncTrieError() -} - -func (h *handlerStats) IncProofError() { - h.syncHandlerStats.IncProofError() -} - -func (h *handlerStats) IncSnapshotReadError() { - h.syncHandlerStats.IncSnapshotReadError() -} - -func (h *handlerStats) IncSnapshotReadAttempt() { - h.syncHandlerStats.IncSnapshotReadAttempt() -} - -func (h *handlerStats) IncSnapshotReadSuccess() { - h.syncHandlerStats.IncSnapshotReadSuccess() -} - -func (h *handlerStats) IncSnapshotSegmentValid() { - h.syncHandlerStats.IncSnapshotSegmentValid() -} - -func (h *handlerStats) IncSnapshotSegmentInvalid() { - h.syncHandlerStats.IncSnapshotSegmentInvalid() -} + // State sync handler metrics + syncStats.HandlerStats -func (h *handlerStats) IncSignatureRequest() { h.signatureRequest.Inc(1) } -func (h *handlerStats) IncSignatureHit() { h.signatureHit.Inc(1) } -func (h *handlerStats) IncSignatureMiss() { h.signatureMiss.Inc(1) } -func (h *handlerStats) UpdateSignatureRequestTime(duration time.Duration) { - h.signatureProcessingTime.Update(duration) + // Warp handler metrics + warpStats.SignatureRequestHandlerStats } func NewHandlerStats(enabled bool) HandlerStats { if !enabled { - return NewNoopHandlerStats() + return &MockHandlerStats{} } return &handlerStats{ - // initialize state sync handler stats - syncHandlerStats: syncStats.NewHandlerStats(enabled), - - // initialize signature request stats - signatureRequest: metrics.GetOrRegisterCounter("signature_request_count", nil), - signatureHit: metrics.GetOrRegisterCounter("signature_request_hit", nil), - signatureMiss: metrics.GetOrRegisterCounter("signature_request_miss", nil), - signatureProcessingTime: metrics.GetOrRegisterTimer("signature_request_duration", nil), + HandlerStats: syncStats.NewHandlerStats(enabled), + SignatureRequestHandlerStats: warpStats.NewStats(enabled), } } - -// no op implementation -type noopHandlerStats struct { - syncHandlerStats syncStats.HandlerStats -} - -func NewNoopHandlerStats() HandlerStats { - return &noopHandlerStats{ - syncHandlerStats: syncStats.NewNoopHandlerStats(), - } -} - -func (m *noopHandlerStats) IncBlockRequest() { - m.syncHandlerStats.IncBlockRequest() -} - -func (m *noopHandlerStats) IncMissingBlockHash() { - m.syncHandlerStats.IncMissingBlockHash() -} - -func (m *noopHandlerStats) UpdateBlocksReturned(num uint16) { - m.syncHandlerStats.UpdateBlocksReturned(num) -} - -func (m *noopHandlerStats) UpdateBlockRequestProcessingTime(duration time.Duration) { - m.syncHandlerStats.UpdateBlockRequestProcessingTime(duration) -} - -func (m *noopHandlerStats) IncCodeRequest() { - m.syncHandlerStats.IncCodeRequest() -} - -func (m *noopHandlerStats) IncMissingCodeHash() { - m.syncHandlerStats.IncMissingCodeHash() -} - -func (m *noopHandlerStats) IncTooManyHashesRequested() { - m.syncHandlerStats.IncTooManyHashesRequested() -} - -func (m *noopHandlerStats) IncDuplicateHashesRequested() { - m.syncHandlerStats.IncDuplicateHashesRequested() -} - -func (m *noopHandlerStats) UpdateCodeReadTime(duration time.Duration) { - m.syncHandlerStats.UpdateCodeReadTime(duration) -} - -func (m *noopHandlerStats) UpdateCodeBytesReturned(bytes uint32) { - m.syncHandlerStats.UpdateCodeBytesReturned(bytes) -} - -func (m *noopHandlerStats) IncLeafsRequest() { - m.syncHandlerStats.IncLeafsRequest() -} - -func (m *noopHandlerStats) IncInvalidLeafsRequest() { - m.syncHandlerStats.IncInvalidLeafsRequest() -} - -func (m *noopHandlerStats) UpdateLeafsReturned(numLeafs uint16) { - m.syncHandlerStats.UpdateLeafsReturned(numLeafs) -} - -func (m *noopHandlerStats) UpdateLeafsRequestProcessingTime(duration time.Duration) { - m.syncHandlerStats.UpdateLeafsRequestProcessingTime(duration) -} - -func (m *noopHandlerStats) UpdateReadLeafsTime(duration time.Duration) { - m.syncHandlerStats.UpdateReadLeafsTime(duration) -} - -func (m *noopHandlerStats) UpdateSnapshotReadTime(duration time.Duration) { - m.syncHandlerStats.UpdateSnapshotReadTime(duration) -} - -func (m *noopHandlerStats) UpdateGenerateRangeProofTime(duration time.Duration) { - m.syncHandlerStats.UpdateGenerateRangeProofTime(duration) -} - -func (m *noopHandlerStats) UpdateRangeProofValsReturned(numProofVals int64) { - m.syncHandlerStats.UpdateRangeProofValsReturned(numProofVals) -} - -func (m *noopHandlerStats) IncMissingRoot() { - m.syncHandlerStats.IncMissingRoot() -} - -func (m *noopHandlerStats) IncTrieError() { - m.syncHandlerStats.IncTrieError() -} - -func (m *noopHandlerStats) IncProofError() { - m.syncHandlerStats.IncProofError() -} - -func (m *noopHandlerStats) IncSnapshotReadError() { - m.syncHandlerStats.IncSnapshotReadError() -} - -func (m *noopHandlerStats) IncSnapshotReadAttempt() { - m.syncHandlerStats.IncSnapshotReadAttempt() -} - -func (m *noopHandlerStats) IncSnapshotReadSuccess() { - m.syncHandlerStats.IncSnapshotReadSuccess() -} - -func (m *noopHandlerStats) IncSnapshotSegmentValid() { - m.syncHandlerStats.IncSnapshotSegmentValid() -} - -func (m *noopHandlerStats) IncSnapshotSegmentInvalid() { - m.syncHandlerStats.IncSnapshotSegmentInvalid() -} - -func (m *noopHandlerStats) IncSignatureRequest() {} -func (m *noopHandlerStats) IncSignatureHit() {} -func (m *noopHandlerStats) IncSignatureMiss() {} -func (m *noopHandlerStats) UpdateSignatureRequestTime(duration time.Duration) {} diff --git a/handlers/signature_request.go b/handlers/warp/signature_request.go similarity index 97% rename from handlers/signature_request.go rename to handlers/warp/signature_request.go index 097fad41a5..13b6c7e66b 100644 --- a/handlers/signature_request.go +++ b/handlers/warp/signature_request.go @@ -1,7 +1,7 @@ // (c) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. -package handlers +package warp import ( "context" @@ -9,7 +9,7 @@ import ( "github.com/ava-labs/avalanchego/codec" "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/subnet-evm/handlers/stats" + "github.com/ava-labs/subnet-evm/handlers/warp/stats" "github.com/ava-labs/subnet-evm/plugin/evm/message" "github.com/ava-labs/subnet-evm/plugin/evm/warp" "github.com/ethereum/go-ethereum/log" diff --git a/handlers/signature_request_test.go b/handlers/warp/signature_request_test.go similarity index 99% rename from handlers/signature_request_test.go rename to handlers/warp/signature_request_test.go index 0a2d5c75ca..d5e3103063 100644 --- a/handlers/signature_request_test.go +++ b/handlers/warp/signature_request_test.go @@ -1,7 +1,7 @@ // (c) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. -package handlers +package warp import ( "context" diff --git a/handlers/warp/stats/stats.go b/handlers/warp/stats/stats.go new file mode 100644 index 0000000000..2cc593667e --- /dev/null +++ b/handlers/warp/stats/stats.go @@ -0,0 +1,95 @@ +// (c) 2023, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package stats + +import ( + "sync" + "time" + + "github.com/ava-labs/subnet-evm/metrics" +) + +var ( + _ SignatureRequestHandlerStats = (*handlerStats)(nil) + _ SignatureRequestHandlerStats = (*MockSignatureRequestHandlerStats)(nil) +) + +type SignatureRequestHandlerStats interface { + IncSignatureRequest() + IncSignatureHit() + IncSignatureMiss() + UpdateSignatureRequestTime(duration time.Duration) +} + +type handlerStats struct { + // SignatureRequestHandler metrics + signatureRequest metrics.Counter + signatureHit metrics.Counter + signatureMiss metrics.Counter + signatureProcessingTime metrics.Timer +} + +func NewStats(enabled bool) SignatureRequestHandlerStats { + if !enabled { + return &MockSignatureRequestHandlerStats{} + } + + return &handlerStats{ + signatureRequest: metrics.GetOrRegisterCounter("signature_request_count", nil), + signatureHit: metrics.GetOrRegisterCounter("signature_request_hit", nil), + signatureMiss: metrics.GetOrRegisterCounter("signature_request_miss", nil), + signatureProcessingTime: metrics.GetOrRegisterTimer("signature_request_duration", nil), + } +} + +func (h *handlerStats) IncSignatureRequest() { h.signatureRequest.Inc(1) } +func (h *handlerStats) IncSignatureHit() { h.signatureHit.Inc(1) } +func (h *handlerStats) IncSignatureMiss() { h.signatureMiss.Inc(1) } +func (h *handlerStats) UpdateSignatureRequestTime(duration time.Duration) { + h.signatureProcessingTime.Update(duration) +} + +// MockSignatureRequestHandlerStats is mock for capturing and asserting on handler metrics in test +type MockSignatureRequestHandlerStats struct { + lock sync.Mutex + + SignatureRequestCount, + SignatureRequestHit, + SignatureRequestMiss uint32 + SignatureRequestDuration time.Duration +} + +func (m *MockSignatureRequestHandlerStats) Reset() { + m.lock.Lock() + defer m.lock.Unlock() + + m.SignatureRequestCount = 0 + m.SignatureRequestHit = 0 + m.SignatureRequestMiss = 0 + m.SignatureRequestDuration = 0 +} + +func (m *MockSignatureRequestHandlerStats) IncSignatureRequest() { + m.lock.Lock() + defer m.lock.Unlock() + m.SignatureRequestCount++ +} + +func (m *MockSignatureRequestHandlerStats) IncSignatureHit() { + m.lock.Lock() + defer m.lock.Unlock() + m.SignatureRequestHit++ +} + +func (m *MockSignatureRequestHandlerStats) IncSignatureMiss() { + m.lock.Lock() + defer m.lock.Unlock() + m.SignatureRequestMiss++ +} + +func (m *MockSignatureRequestHandlerStats) UpdateSignatureRequestTime(duration time.Duration) { + m.lock.Lock() + defer m.lock.Unlock() + m.SignatureRequestDuration += duration +} diff --git a/plugin/evm/warp/backend.go b/plugin/evm/warp/backend.go index 7aade1514d..004745fdaa 100644 --- a/plugin/evm/warp/backend.go +++ b/plugin/evm/warp/backend.go @@ -18,8 +18,6 @@ import ( var ( _ WarpBackend = &warpBackend{} - - emptySignature = [bls.SignatureLen]byte{} ) // WarpBackend tracks signature eligible warp messages and provides an interface to fetch them. @@ -76,18 +74,18 @@ func (w *warpBackend) GetSignature(ctx context.Context, messageID ids.ID) ([bls. unsignedMessageBytes, err := w.db.Get(messageID[:]) if err != nil { - return emptySignature, fmt.Errorf("failed to get warp message %s from db: %w", messageID.String(), err) + return [bls.SignatureLen]byte{}, fmt.Errorf("failed to get warp message %s from db: %w", messageID.String(), err) } unsignedMessage, err := teleporter.ParseUnsignedMessage(unsignedMessageBytes) if err != nil { - return emptySignature, fmt.Errorf("failed to parse unsigned message %s: %w", messageID.String(), err) + return [bls.SignatureLen]byte{}, fmt.Errorf("failed to parse unsigned message %s: %w", messageID.String(), err) } var signature [bls.SignatureLen]byte sig, err := w.snowCtx.TeleporterSigner.Sign(unsignedMessage) if err != nil { - return emptySignature, fmt.Errorf("failed to sign warp message: %w", err) + return [bls.SignatureLen]byte{}, fmt.Errorf("failed to sign warp message: %w", err) } copy(signature[:], sig)