Merge pull request #5138 from onflow/petera/5137-fix-getregistervalues

[Access] Fix GetRegisterValues input types

engine/access/state_stream/backend/backend.go

@@ -228,12 +228,17 @@ func (b *StateStreamBackend) GetRegisterValues(ids flow.RegisterIDs, height uint
 	if len(ids) > b.registerRequestLimit {
 		return nil, status.Errorf(codes.InvalidArgument, "number of register IDs exceeds limit of %d", b.registerRequestLimit)
 	}
+
 	values, err := b.registers.RegisterValues(ids, height)
-	if errors.Is(err, storage.ErrHeightNotIndexed) {
-		return nil, status.Errorf(codes.OutOfRange, "register values for block %d is not available", height)
-	}
-	if errors.Is(err, storage.ErrNotFound) {
-		return nil, status.Errorf(codes.NotFound, "register values for block %d is not available", height)
+	if err != nil {
+		if errors.Is(err, storage.ErrHeightNotIndexed) {
+			return nil, status.Errorf(codes.OutOfRange, "register values for block %d is not available", height)
+		}
+		if errors.Is(err, storage.ErrNotFound) {
+			return nil, status.Errorf(codes.NotFound, "register values for block %d not found", height)
+		}
+		return nil, err
 	}
-	return values, err
+
+	return values, nil
 }

engine/access/state_stream/backend/backend_executiondata_test.go

@@ -441,7 +441,7 @@ func (s *BackendExecutionDataSuite) TestSubscribeExecutionDataHandlesErrors() {
 	})
 }
 
-func (s *BackendExecutionDataSuite) TestGetRegisterValuesErrors() {
+func (s *BackendExecutionDataSuite) TestGetRegisterValues() {
 	s.Run("normal case", func() {
 		res, err := s.backend.GetRegisterValues(flow.RegisterIDs{s.registerID}, s.backend.rootBlockHeight)
 		require.NoError(s.T(), err)

engine/access/state_stream/backend/handler.go

@@ -202,14 +202,16 @@ func (h *Handler) SubscribeEvents(request *executiondata.SubscribeEventsRequest,
 
 func (h *Handler) GetRegisterValues(_ context.Context, request *executiondata.GetRegisterValuesRequest) (*executiondata.GetRegisterValuesResponse, error) {
 	// Convert data
-	registerIDs, err := convert.MessagesToRegisterIDs(request.GetRegisterIds())
+	registerIDs, err := convert.MessagesToRegisterIDs(request.GetRegisterIds(), h.chain)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "could not convert register IDs: %v", err)
 	}
+
 	// get payload from store
 	values, err := h.api.GetRegisterValues(registerIDs, request.GetBlockHeight())
 	if err != nil {
 		return nil, rpc.ConvertError(err, "could not get register values", codes.Internal)
 	}
+
 	return &executiondata.GetRegisterValuesResponse{Values: values}, nil
 }

engine/access/state_stream/backend/handler_test.go

@@ -514,6 +514,10 @@ func TestEventStream(t *testing.T) {
 // TestGetRegisterValues tests the register values.
 func TestGetRegisterValues(t *testing.T) {
 	t.Parallel()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	testHeight := uint64(1)
 
 	// test register IDs + values
@@ -531,73 +535,73 @@ func TestGetRegisterValues(t *testing.T) {
 
 	t.Run("invalid message", func(t *testing.T) {
 		api := ssmock.NewAPI(t)
-		h := NewHandler(api, flow.Localnet.Chain(), makeConfig(1))
-		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
+		h := NewHandler(api, flow.Testnet.Chain(), makeConfig(1))
+
 		invalidMessage := &executiondata.GetRegisterValuesRequest{
 			RegisterIds: nil,
 		}
 		_, err := h.GetRegisterValues(ctx, invalidMessage)
-		require.Equal(t, status.Code(err), codes.InvalidArgument)
+		require.Equal(t, codes.InvalidArgument, status.Code(err))
 	})
 
 	t.Run("valid registers", func(t *testing.T) {
 		api := ssmock.NewAPI(t)
 		api.On("GetRegisterValues", testIds, testHeight).Return(testValues, nil)
-		h := NewHandler(api, flow.Localnet.Chain(), makeConfig(1))
+		h := NewHandler(api, flow.Testnet.Chain(), makeConfig(1))
+
 		validRegisters := make([]*entities.RegisterID, len(testIds))
 		for i, id := range testIds {
 			validRegisters[i] = convert.RegisterIDToMessage(id)
 		}
+
 		req := &executiondata.GetRegisterValuesRequest{
 			RegisterIds: validRegisters,
 			BlockHeight: testHeight,
 		}
-		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
+
 		resp, err := h.GetRegisterValues(ctx, req)
 		require.NoError(t, err)
-		require.Equal(t, resp.GetValues(), testValues)
-
+		require.Equal(t, testValues, resp.GetValues())
 	})
 
 	t.Run("unavailable registers", func(t *testing.T) {
 		api := ssmock.NewAPI(t)
 		expectedErr := status.Errorf(codes.NotFound, "could not get register values: %v", storage.ErrNotFound)
 		api.On("GetRegisterValues", invalidIDs, testHeight).Return(nil, expectedErr)
-		h := NewHandler(api, flow.Localnet.Chain(), makeConfig(1))
+		h := NewHandler(api, flow.Testnet.Chain(), makeConfig(1))
+
 		unavailableRegisters := make([]*entities.RegisterID, len(invalidIDs))
 		for i, id := range invalidIDs {
 			unavailableRegisters[i] = convert.RegisterIDToMessage(id)
 		}
+
 		req := &executiondata.GetRegisterValuesRequest{
 			RegisterIds: unavailableRegisters,
 			BlockHeight: testHeight,
 		}
-		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
-		_, err := h.GetRegisterValues(ctx, req)
-		require.Equal(t, status.Code(err), codes.NotFound)
 
+		_, err := h.GetRegisterValues(ctx, req)
+		require.Equal(t, codes.NotFound, status.Code(err))
 	})
 
 	t.Run("wrong height", func(t *testing.T) {
 		api := ssmock.NewAPI(t)
 		expectedErr := status.Errorf(codes.OutOfRange, "could not get register values: %v", storage.ErrHeightNotIndexed)
 		api.On("GetRegisterValues", testIds, testHeight+1).Return(nil, expectedErr)
-		h := NewHandler(api, flow.Localnet.Chain(), makeConfig(1))
+		h := NewHandler(api, flow.Testnet.Chain(), makeConfig(1))
+
 		validRegisters := make([]*entities.RegisterID, len(testIds))
 		for i, id := range testIds {
 			validRegisters[i] = convert.RegisterIDToMessage(id)
 		}
+
 		req := &executiondata.GetRegisterValuesRequest{
 			RegisterIds: validRegisters,
 			BlockHeight: testHeight + 1,
 		}
-		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
+
 		_, err := h.GetRegisterValues(ctx, req)
-		require.Equal(t, status.Code(err), codes.OutOfRange)
+		require.Equal(t, codes.OutOfRange, status.Code(err))
 	})
 }
 

engine/common/rpc/convert/execution_data.go

@@ -268,7 +268,7 @@ func messageToTrustedTransaction(
 
 	proposalKey := m.GetProposalKey()
 	if proposalKey != nil {
-		proposalAddress, err := insecureAddress(proposalKey.GetAddress(), chain)
+		proposalAddress, err := insecureAddress(proposalKey.GetAddress())
 		if err != nil {
 			return *t, fmt.Errorf("could not convert proposer address: %w", err)
 		}
@@ -277,7 +277,7 @@ func messageToTrustedTransaction(
 
 	payer := m.GetPayer()
 	if payer != nil {
-		payerAddress, err := insecureAddress(payer, chain)
+		payerAddress, err := insecureAddress(payer)
 		if err != nil {
 			return *t, fmt.Errorf("could not convert payer address: %w", err)
 		}
@@ -316,43 +316,52 @@ func messageToTrustedTransaction(
 	return *t, nil
 }
 
-func MessageToRegisterID(m *entities.RegisterID) (flow.RegisterID, error) {
+func MessageToRegisterID(m *entities.RegisterID, chain flow.Chain) (flow.RegisterID, error) {
 	if m == nil {
 		return flow.RegisterID{}, ErrEmptyMessage
 	}
-	return flow.RegisterID{
-		Owner: m.GetOwner(),
-		Key:   m.GetKey(),
-	}, nil
+
+	owner := flow.EmptyAddress
+	if len(m.GetOwner()) > 0 {
+		var err error
+		owner, err = Address(m.GetOwner(), chain)
+		if err != nil {
+			return flow.RegisterID{}, fmt.Errorf("could not convert owner address: %w", err)
+		}
+	}
+
+	key := string(m.GetKey())
+
+	return flow.NewRegisterID(owner, key), nil
 }
 
 // MessagesToRegisterIDs converts a protobuf message to RegisterIDs
-func MessagesToRegisterIDs(m []*entities.RegisterID) (flow.RegisterIDs, error) {
+func MessagesToRegisterIDs(m []*entities.RegisterID, chain flow.Chain) (flow.RegisterIDs, error) {
 	if m == nil {
 		return nil, ErrEmptyMessage
 	}
 	result := make(flow.RegisterIDs, len(m))
 	for i, entry := range m {
-		regId, err := MessageToRegisterID(entry)
+		regID, err := MessageToRegisterID(entry, chain)
 		if err != nil {
 			return nil, fmt.Errorf("failed to convert register id %d: %w", i, err)
 		}
-		result[i] = regId
+		result[i] = regID
 	}
 	return result, nil
 }
 
 func RegisterIDToMessage(id flow.RegisterID) *entities.RegisterID {
 	return &entities.RegisterID{
-		Owner: id.Owner,
-		Key:   id.Key,
+		Owner: []byte(id.Owner),
+		Key:   []byte(id.Key),
 	}
 }
 
 // insecureAddress converts a raw address to a flow.Address, skipping validation
 // This is useful when converting transactions from trusted state like BlockExecutionData.
 // This should only be used for trusted inputs
-func insecureAddress(rawAddress []byte, chain flow.Chain) (flow.Address, error) {
+func insecureAddress(rawAddress []byte) (flow.Address, error) {
 	if len(rawAddress) == 0 {
 		return flow.EmptyAddress, status.Error(codes.InvalidArgument, "address cannot be empty")
 	}

engine/common/rpc/convert/execution_data_test.go

@@ -172,7 +172,8 @@ func TestConvertChunkExecutionData(t *testing.T) {
 	}
 }
 
-func TestMessageToRegisterIDs(t *testing.T) {
+func TestMessageToRegisterID(t *testing.T) {
+	chain := flow.Testnet.Chain()
 	tests := []struct {
 		name  string
 		regID flow.RegisterID
@@ -193,12 +194,58 @@ func TestMessageToRegisterIDs(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			registerIDMessage := convert.RegisterIDToMessage(test.regID)
-			reConvertedRegisterID, err := convert.MessageToRegisterID(registerIDMessage)
+			msg := convert.RegisterIDToMessage(test.regID)
+			converted, err := convert.MessageToRegisterID(msg, chain)
 			require.NoError(t, err)
-			assert.Equal(t, test.regID, reConvertedRegisterID)
+			assert.Equal(t, test.regID, converted)
 		})
 	}
-	_, err := convert.MessageToRegisterID(nil)
-	require.ErrorIs(t, err, convert.ErrEmptyMessage)
+
+	t.Run("nil owner converts to empty string", func(t *testing.T) {
+		msg := &entities.RegisterID{
+			Owner: nil,
+			Key:   []byte("key"),
+		}
+		converted, err := convert.MessageToRegisterID(msg, chain)
+		require.NoError(t, err)
+		assert.Equal(t, "", converted.Owner)
+		assert.Equal(t, "key", converted.Key)
+	})
+
+	t.Run("nil message returns error", func(t *testing.T) {
+		_, err := convert.MessageToRegisterID(nil, chain)
+		require.ErrorIs(t, err, convert.ErrEmptyMessage)
+	})
+
+	t.Run("invalid address returns error", func(t *testing.T) {
+		// addresses for other chains are invalid
+		registerID := flow.NewRegisterID(
+			unittest.RandomAddressFixtureForChain(flow.Mainnet),
+			"key",
+		)
+
+		msg := convert.RegisterIDToMessage(registerID)
+		_, err := convert.MessageToRegisterID(msg, chain)
+		require.Error(t, err)
+	})
+
+	t.Run("multiple registerIDs", func(t *testing.T) {
+		expected := flow.RegisterIDs{
+			flow.UUIDRegisterID(0),
+			flow.AccountStatusRegisterID(unittest.AddressFixture()),
+			unittest.RegisterIDFixture(),
+		}
+
+		messages := make([]*entities.RegisterID, len(expected))
+		for i, regID := range expected {
+			regID := regID
+			messages[i] = convert.RegisterIDToMessage(regID)
+			require.Equal(t, regID.Owner, string(messages[i].Owner))
+			require.Equal(t, regID.Key, string(messages[i].Key))
+		}
+
+		actual, err := convert.MessagesToRegisterIDs(messages, chain)
+		require.NoError(t, err)
+		assert.Equal(t, expected, actual)
+	})
 }

go.mod

@@ -57,7 +57,7 @@ require (
 	github.com/onflow/flow-core-contracts/lib/go/templates v0.14.0
 	github.com/onflow/flow-go-sdk v0.41.17
 	github.com/onflow/flow-go/crypto v0.25.0
-	github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6
+	github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2
 	github.com/onflow/go-bitswap v0.0.0-20230703214630-6d3db958c73d
 	github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58
 	github.com/pierrec/lz4 v2.6.1+incompatible

go.sum

@@ -1344,8 +1344,8 @@ github.com/onflow/flow-go/crypto v0.25.0/go.mod h1:OOb2vYcS8AOCajBClhHTJ0NKftFl1
 github.com/onflow/flow-nft/lib/go/contracts v1.1.0 h1:rhUDeD27jhLwOqQKI/23008CYfnqXErrJvc4EFRP2a0=
 github.com/onflow/flow-nft/lib/go/contracts v1.1.0/go.mod h1:YsvzYng4htDgRB9sa9jxdwoTuuhjK8WYWXTyLkIigZY=
 github.com/onflow/flow/protobuf/go/flow v0.2.2/go.mod h1:gQxYqCfkI8lpnKsmIjwtN2mV/N2PIwc1I+RUK4HPIc8=
-github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6 h1:KMN+OEVaw7KAgxL3p8ux7CMuyTvacAlYTbasOqowh4M=
-github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
+github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2 h1:+rT+UsfTR39JZO8ht2+4fkaWfHw74SCj1fyz1lWuX8A=
+github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
 github.com/onflow/go-bitswap v0.0.0-20230703214630-6d3db958c73d h1:QcOAeEyF3iAUHv21LQ12sdcsr0yFrJGoGLyCAzYYtvI=
 github.com/onflow/go-bitswap v0.0.0-20230703214630-6d3db958c73d/go.mod h1:GCPpiyRoHncdqPj++zPr9ZOYBX4hpJ0pYZRYqSE8VKk=
 github.com/onflow/sdks v0.5.0 h1:2HCRibwqDaQ1c9oUApnkZtEAhWiNY2GTpRD5+ftdkN8=

insecure/go.mod

@@ -204,7 +204,7 @@ require (
 	github.com/onflow/flow-ft/lib/go/contracts v0.7.1-0.20230711213910-baad011d2b13 // indirect
 	github.com/onflow/flow-go-sdk v0.41.17 // indirect
 	github.com/onflow/flow-nft/lib/go/contracts v1.1.0 // indirect
-	github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6 // indirect
+	github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2 // indirect
 	github.com/onflow/go-bitswap v0.0.0-20230703214630-6d3db958c73d // indirect
 	github.com/onflow/sdks v0.5.0 // indirect
 	github.com/onflow/wal v0.0.0-20230529184820-bc9f8244608d // indirect

insecure/go.sum

@@ -1319,8 +1319,8 @@ github.com/onflow/flow-go/crypto v0.25.0/go.mod h1:OOb2vYcS8AOCajBClhHTJ0NKftFl1
 github.com/onflow/flow-nft/lib/go/contracts v1.1.0 h1:rhUDeD27jhLwOqQKI/23008CYfnqXErrJvc4EFRP2a0=
 github.com/onflow/flow-nft/lib/go/contracts v1.1.0/go.mod h1:YsvzYng4htDgRB9sa9jxdwoTuuhjK8WYWXTyLkIigZY=
 github.com/onflow/flow/protobuf/go/flow v0.2.2/go.mod h1:gQxYqCfkI8lpnKsmIjwtN2mV/N2PIwc1I+RUK4HPIc8=
-github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6 h1:KMN+OEVaw7KAgxL3p8ux7CMuyTvacAlYTbasOqowh4M=
-github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
+github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2 h1:+rT+UsfTR39JZO8ht2+4fkaWfHw74SCj1fyz1lWuX8A=
+github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
 github.com/onflow/go-bitswap v0.0.0-20230703214630-6d3db958c73d h1:QcOAeEyF3iAUHv21LQ12sdcsr0yFrJGoGLyCAzYYtvI=
 github.com/onflow/go-bitswap v0.0.0-20230703214630-6d3db958c73d/go.mod h1:GCPpiyRoHncdqPj++zPr9ZOYBX4hpJ0pYZRYqSE8VKk=
 github.com/onflow/sdks v0.5.0 h1:2HCRibwqDaQ1c9oUApnkZtEAhWiNY2GTpRD5+ftdkN8=

integration/go.mod

@@ -27,7 +27,7 @@ require (
 	github.com/onflow/flow-go-sdk v0.41.17
 	github.com/onflow/flow-go/crypto v0.25.0
 	github.com/onflow/flow-go/insecure v0.0.0-00010101000000-000000000000
-	github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6
+	github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2
 	github.com/plus3it/gorecurcopy v0.0.1
 	github.com/prometheus/client_golang v1.16.0
 	github.com/prometheus/client_model v0.5.0

integration/go.sum

@@ -1413,8 +1413,8 @@ github.com/onflow/flow-go/crypto v0.25.0/go.mod h1:OOb2vYcS8AOCajBClhHTJ0NKftFl1
 github.com/onflow/flow-nft/lib/go/contracts v1.1.0 h1:rhUDeD27jhLwOqQKI/23008CYfnqXErrJvc4EFRP2a0=
 github.com/onflow/flow-nft/lib/go/contracts v1.1.0/go.mod h1:YsvzYng4htDgRB9sa9jxdwoTuuhjK8WYWXTyLkIigZY=
 github.com/onflow/flow/protobuf/go/flow v0.2.2/go.mod h1:gQxYqCfkI8lpnKsmIjwtN2mV/N2PIwc1I+RUK4HPIc8=
-github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6 h1:KMN+OEVaw7KAgxL3p8ux7CMuyTvacAlYTbasOqowh4M=
-github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231124194313-106cc495def6/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
+github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2 h1:+rT+UsfTR39JZO8ht2+4fkaWfHw74SCj1fyz1lWuX8A=
+github.com/onflow/flow/protobuf/go/flow v0.3.2-0.20231213135419-ae911cc351a2/go.mod h1:NA2pX2nw8zuaxfKphhKsk00kWLwfd+tv8mS23YXO4Sk=
 github.com/onflow/go-bitswap v0.0.0-20230703214630-6d3db958c73d h1:QcOAeEyF3iAUHv21LQ12sdcsr0yFrJGoGLyCAzYYtvI=
 github.com/onflow/go-bitswap v0.0.0-20230703214630-6d3db958c73d/go.mod h1:GCPpiyRoHncdqPj++zPr9ZOYBX4hpJ0pYZRYqSE8VKk=
 github.com/onflow/nft-storefront/lib/go/contracts v0.0.0-20221222181731-14b90207cead h1:2j1Unqs76Z1b95Gu4C3Y28hzNUHBix7wL490e61SMSw=