From 14252bcbb4304403c5fe9f8b4e59a0a3edf86ef6 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Fri, 20 Dec 2024 17:04:40 +0200 Subject: [PATCH 1/3] no panic on new request id --- commit/merkleroot/rmn/controller.go | 24 +++++++++++++++++++----- commit/merkleroot/rmn/controller_test.go | 10 ++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/commit/merkleroot/rmn/controller.go b/commit/merkleroot/rmn/controller.go index 525ca3c12..19554b3f7 100644 --- a/commit/merkleroot/rmn/controller.go +++ b/commit/merkleroot/rmn/controller.go @@ -12,6 +12,7 @@ import ( "fmt" "math/rand" "sort" + "sync/atomic" "time" mapset "github.com/deckarep/golang-set/v2" @@ -364,7 +365,7 @@ func (c *controller) sendObservationRequests( } req := &rmnpb.Request{ - RequestId: newRequestID(), + RequestId: newRequestID(c.lggr), Request: &rmnpb.Request_ObservationRequest{ ObservationRequest: &rmnpb.ObservationRequest{ LaneDest: destChain, @@ -810,7 +811,7 @@ func (c *controller) sendReportSignatureRequest( } req := &rmnpb.Request{ - RequestId: newRequestID(), + RequestId: newRequestID(c.lggr), Request: &rmnpb.Request_ReportSignatureRequest{ ReportSignatureRequest: reportSigReq, }, @@ -908,7 +909,7 @@ func (c *controller) listenForRmnReportSignatures( continue } req := &rmnpb.Request{ - RequestId: newRequestID(), + RequestId: newRequestID(c.lggr), Request: &rmnpb.Request_ReportSignatureRequest{ ReportSignatureRequest: reportSigReq, }, @@ -1072,11 +1073,24 @@ func randomShuffle[T any](s []T) []T { return ret } -func newRequestID() uint64 { +var ( + // atomicCounter is used to generate unique request IDs. + atomicCounter = &atomic.Int64{} +) + +// newRequestID generates a new unique request ID. +func newRequestID(lggr logger.Logger) uint64 { b := make([]byte, 8) _, err := crand.Read(b) if err != nil { - panic(err) + // fallback to time-based id in the very rare scenario that the random number generator fails + lggr.Warnw("failed to generate random request id, falling back to time based", + "err", err, + "atomicCounter", atomicCounter.Load(), + ) + t := time.Now().UTC().UnixNano() + c := atomicCounter.Add(1) + return uint64(t + c) } randomUint64 := binary.LittleEndian.Uint64(b) return randomUint64 diff --git a/commit/merkleroot/rmn/controller_test.go b/commit/merkleroot/rmn/controller_test.go index dc7c44807..c9fd9c5a1 100644 --- a/commit/merkleroot/rmn/controller_test.go +++ b/commit/merkleroot/rmn/controller_test.go @@ -733,6 +733,16 @@ func Test_controller_validateSignedObservationResponse(t *testing.T) { } } +func Test_newRequestID(t *testing.T) { + ids := map[uint64]struct{}{} + for i := 0; i < 1000; i++ { + id := newRequestID(logger.Test(t)) + _, ok := ids[id] + assert.False(t, ok) + ids[id] = struct{}{} + } +} + func (ts *testSetup) waitForObservationRequestsToBeSent( rmnClient *mockPeerClient, homeF int, From 6975e4625bcfffecbdf44cf2ae0ed962ade30b45 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Fri, 20 Dec 2024 17:35:38 +0200 Subject: [PATCH 2/3] fallback to x/exp/rand --- commit/merkleroot/rmn/controller.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/commit/merkleroot/rmn/controller.go b/commit/merkleroot/rmn/controller.go index 19554b3f7..df9f39b9e 100644 --- a/commit/merkleroot/rmn/controller.go +++ b/commit/merkleroot/rmn/controller.go @@ -12,11 +12,11 @@ import ( "fmt" "math/rand" "sort" - "sync/atomic" "time" mapset "github.com/deckarep/golang-set/v2" "golang.org/x/exp/maps" + rand2 "golang.org/x/exp/rand" "google.golang.org/protobuf/proto" chainsel "github.com/smartcontractkit/chain-selectors" @@ -1073,24 +1073,17 @@ func randomShuffle[T any](s []T) []T { return ret } -var ( - // atomicCounter is used to generate unique request IDs. - atomicCounter = &atomic.Int64{} -) - // newRequestID generates a new unique request ID. func newRequestID(lggr logger.Logger) uint64 { b := make([]byte, 8) _, err := crand.Read(b) - if err != nil { + if err == nil { // fallback to time-based id in the very rare scenario that the random number generator fails - lggr.Warnw("failed to generate random request id, falling back to time based", + lggr.Warnw("failed to generate random request id, falling back to golang.org/x/exp/rand", "err", err, - "atomicCounter", atomicCounter.Load(), ) - t := time.Now().UTC().UnixNano() - c := atomicCounter.Add(1) - return uint64(t + c) + rand2.Seed(uint64(time.Now().UnixNano())) + return rand2.Uint64() } randomUint64 := binary.LittleEndian.Uint64(b) return randomUint64 From ab8fe26f1feecb111e890623aec35d5771c68eb0 Mon Sep 17 00:00:00 2001 From: dimkouv Date: Fri, 20 Dec 2024 17:36:36 +0200 Subject: [PATCH 3/3] fix --- commit/merkleroot/rmn/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commit/merkleroot/rmn/controller.go b/commit/merkleroot/rmn/controller.go index df9f39b9e..61d0f5b90 100644 --- a/commit/merkleroot/rmn/controller.go +++ b/commit/merkleroot/rmn/controller.go @@ -1077,7 +1077,7 @@ func randomShuffle[T any](s []T) []T { func newRequestID(lggr logger.Logger) uint64 { b := make([]byte, 8) _, err := crand.Read(b) - if err == nil { + if err != nil { // fallback to time-based id in the very rare scenario that the random number generator fails lggr.Warnw("failed to generate random request id, falling back to golang.org/x/exp/rand", "err", err,