From 0e4286a3c84b69e25f4896cffcc4279acee6f1f0 Mon Sep 17 00:00:00 2001 From: Taras Maliarchuk Date: Fri, 28 Nov 2025 21:12:41 +0100 Subject: [PATCH 1/3] test: achieve 100% test coverage for engine/common/rpc/convert package. Add comprehensive tests for all previously untested conversion and validation functions. --- engine/common/rpc/convert/blocks_test.go | 145 ++++++++ .../rpc/convert/compatible_range_test.go | 17 + engine/common/rpc/convert/convert_test.go | 159 +++++++++ engine/common/rpc/convert/events.go | 16 +- engine/common/rpc/convert/events_test.go | 319 ++++++++++++++++- .../rpc/convert/execution_state_query_test.go | 70 ++++ .../common/rpc/convert/transactions_test.go | 37 +- engine/common/rpc/convert/validate_test.go | 333 ++++++++++++++++++ 8 files changed, 1079 insertions(+), 17 deletions(-) create mode 100644 engine/common/rpc/convert/convert_test.go create mode 100644 engine/common/rpc/convert/execution_state_query_test.go create mode 100644 engine/common/rpc/convert/validate_test.go diff --git a/engine/common/rpc/convert/blocks_test.go b/engine/common/rpc/convert/blocks_test.go index 7a8580051d4..907215057c2 100644 --- a/engine/common/rpc/convert/blocks_test.go +++ b/engine/common/rpc/convert/blocks_test.go @@ -3,7 +3,9 @@ package convert_test import ( "bytes" "testing" + "time" + "github.com/onflow/flow/protobuf/go/flow/entities" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -76,3 +78,146 @@ func TestConvertRootBlock(t *testing.T) { assert.Equal(t, block.ID(), converted.ID()) } + +// TestConvertBlockStatus tests converting protobuf BlockStatus messages to flow.BlockStatus. +func TestConvertBlockStatus(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + pbStatus entities.BlockStatus + expected flow.BlockStatus + }{ + {"Unknown", entities.BlockStatus_BLOCK_UNKNOWN, flow.BlockStatusUnknown}, + {"Finalized", entities.BlockStatus_BLOCK_FINALIZED, flow.BlockStatusFinalized}, + {"Sealed", entities.BlockStatus_BLOCK_SEALED, flow.BlockStatusSealed}, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + converted := convert.MessageToBlockStatus(tc.pbStatus) + assert.Equal(t, tc.expected, converted) + }) + } +} + +// TestConvertBlockSeal tests converting a flow.Seal to and from a protobuf BlockSeal message. +func TestConvertBlockSeal(t *testing.T) { + t.Parallel() + + seal := unittest.Seal.Fixture() + + msg := convert.BlockSealToMessage(seal) + converted, err := convert.MessageToBlockSeal(msg) + require.NoError(t, err) + + assert.Equal(t, seal.BlockID, converted.BlockID) + assert.Equal(t, seal.ResultID, converted.ResultID) + assert.Equal(t, seal.FinalState, converted.FinalState) + assert.Equal(t, seal.AggregatedApprovalSigs, converted.AggregatedApprovalSigs) +} + +// TestConvertBlockSeals tests converting multiple flow.Seal to and from protobuf BlockSeal messages. +func TestConvertBlockSeals(t *testing.T) { + t.Parallel() + + seals := []*flow.Seal{ + unittest.Seal.Fixture(), + unittest.Seal.Fixture(), + unittest.Seal.Fixture(), + } + + msgs := convert.BlockSealsToMessages(seals) + require.Len(t, msgs, len(seals)) + + converted, err := convert.MessagesToBlockSeals(msgs) + require.NoError(t, err) + require.Len(t, converted, len(seals)) + + for i, seal := range seals { + assert.Equal(t, seal.BlockID, converted[i].BlockID) + assert.Equal(t, seal.ResultID, converted[i].ResultID) + assert.Equal(t, seal.FinalState, converted[i].FinalState) + assert.Equal(t, seal.AggregatedApprovalSigs, converted[i].AggregatedApprovalSigs) + } +} + +// TestConvertPayloadFromMessage tests converting a protobuf Block message to a flow.Payload. +func TestConvertPayloadFromMessage(t *testing.T) { + t.Parallel() + + block := unittest.FullBlockFixture() + signerIDs := unittest.IdentifierListFixture(5) + + msg, err := convert.BlockToMessage(block, signerIDs) + require.NoError(t, err) + + payload, err := convert.PayloadFromMessage(msg) + require.NoError(t, err) + + assert.Equal(t, block.Payload.Guarantees, payload.Guarantees) + assert.Equal(t, block.Payload.Seals, payload.Seals) + assert.Equal(t, block.Payload.Receipts, payload.Receipts) + assert.Equal(t, block.Payload.Results, payload.Results) + assert.Equal(t, block.Payload.ProtocolStateID, payload.ProtocolStateID) +} + +// TestConvertBlockTimestamp2ProtobufTime tests converting block timestamps to protobuf Timestamp format. +func TestConvertBlockTimestamp2ProtobufTime(t *testing.T) { + t.Parallel() + + t.Run("convert current timestamp", func(t *testing.T) { + t.Parallel() + + // Use current time in unix milliseconds + now := time.Now() + timestampMillis := uint64(now.UnixMilli()) + + pbTime := convert.BlockTimestamp2ProtobufTime(timestampMillis) + require.NotNil(t, pbTime) + + // Convert back and verify + convertedTime := pbTime.AsTime() + assert.Equal(t, timestampMillis, uint64(convertedTime.UnixMilli())) + }) + + t.Run("convert zero timestamp", func(t *testing.T) { + t.Parallel() + + pbTime := convert.BlockTimestamp2ProtobufTime(0) + require.NotNil(t, pbTime) + + convertedTime := pbTime.AsTime() + assert.Equal(t, uint64(0), uint64(convertedTime.UnixMilli())) + }) + + t.Run("convert various timestamps", func(t *testing.T) { + t.Parallel() + + testTimestamps := []uint64{ + 1609459200000, // 2021-01-01 00:00:00 UTC + 1640995200000, // 2022-01-01 00:00:00 UTC + 1672531200000, // 2023-01-01 00:00:00 UTC + } + + for _, ts := range testTimestamps { + pbTime := convert.BlockTimestamp2ProtobufTime(ts) + require.NotNil(t, pbTime) + + convertedTime := pbTime.AsTime() + assert.Equal(t, ts, uint64(convertedTime.UnixMilli())) + } + }) + + t.Run("roundtrip conversion with block", func(t *testing.T) { + t.Parallel() + + block := unittest.FullBlockFixture() + msg := convert.BlockToMessageLight(block) + + assert.Equal(t, block.Timestamp, uint64(msg.Timestamp.AsTime().UnixMilli())) + }) +} diff --git a/engine/common/rpc/convert/compatible_range_test.go b/engine/common/rpc/convert/compatible_range_test.go index eb27dfa858b..187deb8e3e3 100644 --- a/engine/common/rpc/convert/compatible_range_test.go +++ b/engine/common/rpc/convert/compatible_range_test.go @@ -38,4 +38,21 @@ func TestConvertCompatibleRange(t *testing.T) { msg := convert.CompatibleRangeToMessage(comparableRange) assert.Equal(t, msg, expected) }) + + t.Run("roundtrip conversion", func(t *testing.T) { + t.Parallel() + + startHeight := uint64(rand.Uint32()) + endHeight := uint64(rand.Uint32()) + + original := &accessmodel.CompatibleRange{ + StartHeight: startHeight, + EndHeight: endHeight, + } + + msg := convert.CompatibleRangeToMessage(original) + converted := convert.MessageToCompatibleRange(msg) + + assert.Equal(t, original, converted) + }) } diff --git a/engine/common/rpc/convert/convert_test.go b/engine/common/rpc/convert/convert_test.go new file mode 100644 index 00000000000..2ffd69304b5 --- /dev/null +++ b/engine/common/rpc/convert/convert_test.go @@ -0,0 +1,159 @@ +package convert_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/onflow/flow-go/engine/common/rpc/convert" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" +) + +// TestConvertIdentifier tests converting a flow.Identifier to and from a protobuf message. +func TestConvertIdentifier(t *testing.T) { + t.Parallel() + + id := unittest.IdentifierFixture() + + msg := convert.IdentifierToMessage(id) + converted := convert.MessageToIdentifier(msg) + + assert.Equal(t, id, converted) +} + +// TestConvertIdentifiers tests converting a slice of flow.Identifiers to and from protobuf messages. +func TestConvertIdentifiers(t *testing.T) { + t.Parallel() + + ids := unittest.IdentifierListFixture(10) + + msgs := convert.IdentifiersToMessages(ids) + converted := convert.MessagesToIdentifiers(msgs) + + assert.Equal(t, ids, flow.IdentifierList(converted)) +} + +// TestConvertSignature tests converting a crypto.Signature to and from a protobuf message. +func TestConvertSignature(t *testing.T) { + t.Parallel() + + sig := unittest.SignatureFixture() + + msg := convert.SignatureToMessage(sig) + converted := convert.MessageToSignature(msg) + + assert.Equal(t, sig, converted) +} + +// TestConvertSignatures tests converting a slice of crypto.Signatures to and from protobuf messages. +func TestConvertSignatures(t *testing.T) { + t.Parallel() + + sigs := unittest.SignaturesFixture(5) + + msgs := convert.SignaturesToMessages(sigs) + converted := convert.MessagesToSignatures(msgs) + + assert.Equal(t, sigs, converted) +} + +// TestConvertStateCommitment tests converting a flow.StateCommitment to and from a protobuf message. +func TestConvertStateCommitment(t *testing.T) { + t.Parallel() + + sc := unittest.StateCommitmentFixture() + + msg := convert.StateCommitmentToMessage(sc) + converted, err := convert.MessageToStateCommitment(msg) + require.NoError(t, err) + + assert.Equal(t, sc, converted) +} + +// TestConvertStateCommitmentInvalidLength tests that MessageToStateCommitment returns an error +// for invalid length byte slices. +func TestConvertStateCommitmentInvalidLength(t *testing.T) { + t.Parallel() + + invalidMsg := []byte{0x01, 0x02, 0x03} // Too short + + _, err := convert.MessageToStateCommitment(invalidMsg) + assert.Error(t, err) +} + +// TestConvertAggregatedSignatures tests converting a slice of flow.AggregatedSignatures to and from +// protobuf messages. +func TestConvertAggregatedSignatures(t *testing.T) { + t.Parallel() + + aggSigs := []flow.AggregatedSignature{ + { + SignerIDs: unittest.IdentifierListFixture(3), + VerifierSignatures: unittest.SignaturesFixture(3), + }, + { + SignerIDs: unittest.IdentifierListFixture(5), + VerifierSignatures: unittest.SignaturesFixture(5), + }, + } + + msgs := convert.AggregatedSignaturesToMessages(aggSigs) + converted := convert.MessagesToAggregatedSignatures(msgs) + + assert.Equal(t, aggSigs, converted) +} + +// TestConvertAggregatedSignaturesEmpty tests converting an empty slice of flow.AggregatedSignatures. +func TestConvertAggregatedSignaturesEmpty(t *testing.T) { + t.Parallel() + + aggSigs := []flow.AggregatedSignature{} + + msgs := convert.AggregatedSignaturesToMessages(aggSigs) + converted := convert.MessagesToAggregatedSignatures(msgs) + + assert.Empty(t, converted) +} + +// TestConvertChainId tests converting a valid chainId string to flow.ChainID. +func TestConvertChainId(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + chainID string + valid bool + }{ + {"Mainnet", flow.Mainnet.String(), true}, + {"Testnet", flow.Testnet.String(), true}, + {"Emulator", flow.Emulator.String(), true}, + {"Localnet", flow.Localnet.String(), true}, + {"Sandboxnet", flow.Sandboxnet.String(), true}, + {"Previewnet", flow.Previewnet.String(), true}, + {"Benchnet", flow.Benchnet.String(), true}, + {"BftTestnet", flow.BftTestnet.String(), true}, + {"MonotonicEmulator", flow.MonotonicEmulator.String(), true}, + {"Invalid", "invalid-chain", false}, + {"Empty", "", false}, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + result, err := convert.MessageToChainId(tc.chainID) + + if tc.valid { + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, tc.chainID, result.String()) + } else { + assert.Error(t, err) + assert.Nil(t, result) + } + }) + } +} diff --git a/engine/common/rpc/convert/events.go b/engine/common/rpc/convert/events.go index 2d78f3b62f2..65ed96b3857 100644 --- a/engine/common/rpc/convert/events.go +++ b/engine/common/rpc/convert/events.go @@ -280,21 +280,9 @@ func CcfEventToJsonEvent(e flow.Event) (*flow.Event, error) { func CcfEventsToJsonEvents(events []flow.Event) ([]flow.Event, error) { convertedEvents := make([]flow.Event, len(events)) for i, e := range events { - payload, err := CcfPayloadToJsonPayload(e.Payload) + convertedEvent, err := CcfEventToJsonEvent(e) if err != nil { - return nil, fmt.Errorf("failed to convert event payload for event %d: %w", i, err) - } - convertedEvent, err := flow.NewEvent( - flow.UntrustedEvent{ - Type: e.Type, - TransactionID: e.TransactionID, - TransactionIndex: e.TransactionIndex, - EventIndex: e.EventIndex, - Payload: payload, - }, - ) - if err != nil { - return nil, fmt.Errorf("could not construct event: %w", err) + return nil, err } convertedEvents[i] = *convertedEvent } diff --git a/engine/common/rpc/convert/events_test.go b/engine/common/rpc/convert/events_test.go index 7db322f80d4..a8cb2cb9b4f 100644 --- a/engine/common/rpc/convert/events_test.go +++ b/engine/common/rpc/convert/events_test.go @@ -27,7 +27,6 @@ func TestConvertEventWithoutPayloadConversion(t *testing.T) { require.NoError(t, err) event := unittest.EventFixture( - unittest.Event.WithEventType(flow.EventAccountCreated), unittest.Event.WithPayload(ccfPayload), ) @@ -43,7 +42,6 @@ func TestConvertEventWithoutPayloadConversion(t *testing.T) { require.NoError(t, err) event := unittest.EventFixture( - unittest.Event.WithEventType(flow.EventAccountCreated), unittest.Event.WithPayload(jsonPayload), ) @@ -233,3 +231,320 @@ func TestConvertMessagesToBlockEvents(t *testing.T) { require.Equal(t, blockEvents, converted) } + +// TestConvertBlockEventsToMessage tests converting a single flow.BlockEvents to a protobuf message. +func TestConvertBlockEventsToMessage(t *testing.T) { + t.Parallel() + + header := unittest.BlockHeaderFixture(unittest.WithHeaderHeight(42)) + blockEvents := unittest.BlockEventsFixture(header, 3) + + msg, err := convert.BlockEventsToMessage(blockEvents) + require.NoError(t, err) + require.NotNil(t, msg) + + require.Equal(t, blockEvents.BlockHeight, msg.BlockHeight) + require.Equal(t, blockEvents.BlockID[:], msg.BlockId) + require.Equal(t, blockEvents.BlockTimestamp, msg.BlockTimestamp.AsTime()) + require.Len(t, msg.Events, len(blockEvents.Events)) + + for i, eventMsg := range msg.Events { + require.Equal(t, blockEvents.Events[i].Type, flow.EventType(eventMsg.Type)) + require.Equal(t, blockEvents.Events[i].TransactionID[:], eventMsg.TransactionId) + } +} + +// TestConvertMessageToBlockEvents tests converting a single protobuf message to flow.BlockEvents. +func TestConvertMessageToBlockEvents(t *testing.T) { + t.Parallel() + + header := unittest.BlockHeaderFixture(unittest.WithHeaderHeight(42)) + blockEvents := unittest.BlockEventsFixture(header, 3) + + // Convert to message first + msg, err := convert.BlockEventsToMessage(blockEvents) + require.NoError(t, err) + + // Convert back to BlockEvents + converted, err := convert.MessageToBlockEvents(msg) + require.NoError(t, err) + require.NotNil(t, converted) + + require.Equal(t, blockEvents.BlockHeight, converted.BlockHeight) + require.Equal(t, blockEvents.BlockID, converted.BlockID) + require.Equal(t, blockEvents.BlockTimestamp.Unix(), converted.BlockTimestamp.Unix()) + require.Len(t, converted.Events, len(blockEvents.Events)) +} + +// TestConvertCcfEventToJsonEvent tests converting a single CCF event to JSON event. +func TestConvertCcfEventToJsonEvent(t *testing.T) { + t.Parallel() + + t.Run("converts CCF event to JSON event", func(t *testing.T) { + t.Parallel() + + // Prepare input CCF event and expected JSON output + cadenceValue := cadence.NewInt(42) + ccfPayload, err := ccf.Encode(cadenceValue) + require.NoError(t, err) + + expectedJsonPayload, err := jsoncdc.Encode(cadenceValue) + require.NoError(t, err) + + ccfEvent := unittest.EventFixture( + unittest.Event.WithPayload(ccfPayload), + ) + + jsonEvent, err := convert.CcfEventToJsonEvent(ccfEvent) + require.NoError(t, err) + require.NotNil(t, jsonEvent) + + require.Equal(t, ccfEvent.Type, jsonEvent.Type) + require.Equal(t, ccfEvent.TransactionID, jsonEvent.TransactionID) + require.Equal(t, ccfEvent.TransactionIndex, jsonEvent.TransactionIndex) + require.Equal(t, ccfEvent.EventIndex, jsonEvent.EventIndex) + + // Verify payload matches expected JSON-CDC encoding + require.Equal(t, expectedJsonPayload, jsonEvent.Payload) + }) + + t.Run("returns error on invalid CCF payload", func(t *testing.T) { + t.Parallel() + + invalidEvent := unittest.EventFixture( + unittest.Event.WithPayload([]byte{0x01, 0x02, 0x03}), // Invalid CCF + ) + + _, err := convert.CcfEventToJsonEvent(invalidEvent) + require.Error(t, err) + }) + + t.Run("returns error on empty payload", func(t *testing.T) { + t.Parallel() + + emptyEvent := unittest.EventFixture( + unittest.Event.WithPayload([]byte{}), + ) + + _, err := convert.CcfEventToJsonEvent(emptyEvent) + require.Error(t, err, "empty payload should result in error from flow.NewEvent") + }) +} + +// TestConvertCcfPayloadToJsonPayload tests converting CCF-encoded payloads to JSON-encoded payloads. +func TestConvertCcfPayloadToJsonPayload(t *testing.T) { + t.Parallel() + + t.Run("convert ccf payload to json payload", func(t *testing.T) { + t.Parallel() + + cadenceValue := cadence.NewInt(42) + ccfPayload, err := ccf.Encode(cadenceValue) + require.NoError(t, err) + + jsonPayload, err := convert.CcfPayloadToJsonPayload(ccfPayload) + require.NoError(t, err) + + decoded, err := jsoncdc.Decode(nil, jsonPayload) + require.NoError(t, err) + require.Equal(t, cadenceValue, decoded) + }) + + t.Run("empty payload returns empty", func(t *testing.T) { + t.Parallel() + + result, err := convert.CcfPayloadToJsonPayload([]byte{}) + require.NoError(t, err) + require.Empty(t, result) + }) + + t.Run("invalid ccf payload returns error", func(t *testing.T) { + t.Parallel() + + invalidPayload := []byte{0x01, 0x02, 0x03} + _, err := convert.CcfPayloadToJsonPayload(invalidPayload) + require.Error(t, err) + }) +} + +// TestConvertCcfEventsToJsonEvents tests converting multiple CCF events to JSON events. +func TestConvertCcfEventsToJsonEvents(t *testing.T) { + t.Parallel() + + t.Run("converts multiple events correctly", func(t *testing.T) { + t.Parallel() + + eventCount := 5 + ccfEvents := make([]flow.Event, eventCount) + + // Create test events with varying payloads + for i := 0; i < eventCount; i++ { + cadenceValue := cadence.NewInt(i) + ccfPayload, err := ccf.Encode(cadenceValue) + require.NoError(t, err) + + ccfEvents[i] = unittest.EventFixture( + unittest.Event.WithPayload(ccfPayload), + ) + } + + converted, err := convert.CcfEventsToJsonEvents(ccfEvents) + require.NoError(t, err) + require.Len(t, converted, len(ccfEvents)) + + for i, convertedEvent := range converted { + require.Equal(t, ccfEvents[i].Type, convertedEvent.Type) + require.Equal(t, ccfEvents[i].TransactionID, convertedEvent.TransactionID) + require.Equal(t, ccfEvents[i].TransactionIndex, convertedEvent.TransactionIndex) + require.Equal(t, ccfEvents[i].EventIndex, convertedEvent.EventIndex) + + decoded, err := jsoncdc.Decode(nil, convertedEvent.Payload) + require.NoError(t, err, "payload should be valid JSON-CDC") + require.Equal(t, cadence.NewInt(i), decoded, "decoded value should match original") + } + }) + + t.Run("returns error on invalid CCF payload", func(t *testing.T) { + t.Parallel() + + invalidEvents := []flow.Event{ + unittest.EventFixture( + unittest.Event.WithPayload([]byte{0x01, 0x02, 0x03}), // Invalid CCF + ), + } + + _, err := convert.CcfEventsToJsonEvents(invalidEvents) + require.Error(t, err) + }) +} + +// TestConvertEventToMessageFromVersion tests converting events to messages with version-specific encoding. +func TestConvertEventToMessageFromVersion(t *testing.T) { + t.Parallel() + + cadenceValue := cadence.NewInt(123) + + t.Run("convert ccf event to json message", func(t *testing.T) { + t.Parallel() + + ccfPayload, err := ccf.Encode(cadenceValue) + require.NoError(t, err) + + event := unittest.EventFixture( + unittest.Event.WithPayload(ccfPayload), + ) + + message, err := convert.EventToMessageFromVersion(event, entities.EventEncodingVersion_CCF_V0) + require.NoError(t, err) + + decoded, err := jsoncdc.Decode(nil, message.Payload) + require.NoError(t, err) + require.Equal(t, cadenceValue, decoded) + }) + + t.Run("convert json event to json message", func(t *testing.T) { + t.Parallel() + + jsonPayload, err := jsoncdc.Encode(cadenceValue) + require.NoError(t, err) + + event := unittest.EventFixture( + unittest.Event.WithPayload(jsonPayload), + ) + + message, err := convert.EventToMessageFromVersion(event, entities.EventEncodingVersion_JSON_CDC_V0) + require.NoError(t, err) + + require.Equal(t, jsonPayload, message.Payload) + }) + + t.Run("empty payload is handled correctly", func(t *testing.T) { + t.Parallel() + + event := unittest.EventFixture( + unittest.Event.WithEventType(flow.EventAccountCreated), + unittest.Event.WithPayload([]byte{}), + ) + + message, err := convert.EventToMessageFromVersion(event, entities.EventEncodingVersion_CCF_V0) + require.NoError(t, err) + require.Empty(t, message.Payload) + }) + + t.Run("invalid version returns error", func(t *testing.T) { + t.Parallel() + + event := unittest.EventFixture() + + _, err := convert.EventToMessageFromVersion(event, entities.EventEncodingVersion(999)) + require.Error(t, err) + }) +} + +// TestConvertEventsToMessagesWithEncodingConversion tests batch conversion with encoding changes. +func TestConvertEventsToMessagesWithEncodingConversion(t *testing.T) { + t.Parallel() + + eventCount := 3 + ccfEvents := make([]flow.Event, eventCount) + + for i := 0; i < eventCount; i++ { + cadenceValue := cadence.NewInt(i) + ccfPayload, err := ccf.Encode(cadenceValue) + require.NoError(t, err) + + ccfEvents[i] = unittest.EventFixture( + unittest.Event.WithEventType(flow.EventAccountCreated), + unittest.Event.WithPayload(ccfPayload), + ) + } + + t.Run("convert from ccf to json", func(t *testing.T) { + t.Parallel() + + messages, err := convert.EventsToMessagesWithEncodingConversion( + ccfEvents, + entities.EventEncodingVersion_CCF_V0, + entities.EventEncodingVersion_JSON_CDC_V0, + ) + require.NoError(t, err) + require.Len(t, messages, len(ccfEvents)) + + for i, msg := range messages { + decoded, err := jsoncdc.Decode(nil, msg.Payload) + require.NoError(t, err) + require.Equal(t, cadence.NewInt(i), decoded) + } + }) + + t.Run("same version uses passthrough", func(t *testing.T) { + t.Parallel() + + messages, err := convert.EventsToMessagesWithEncodingConversion( + ccfEvents, + entities.EventEncodingVersion_CCF_V0, + entities.EventEncodingVersion_CCF_V0, + ) + require.NoError(t, err) + require.Len(t, messages, len(ccfEvents)) + }) + + t.Run("json to ccf conversion is not supported", func(t *testing.T) { + t.Parallel() + + jsonEvents := make([]flow.Event, 1) + jsonPayload, err := jsoncdc.Encode(cadence.NewInt(1)) + require.NoError(t, err) + + jsonEvents[0] = unittest.EventFixture( + unittest.Event.WithPayload(jsonPayload), + ) + + _, err = convert.EventsToMessagesWithEncodingConversion( + jsonEvents, + entities.EventEncodingVersion_JSON_CDC_V0, + entities.EventEncodingVersion_CCF_V0, + ) + require.Error(t, err) + }) +} diff --git a/engine/common/rpc/convert/execution_state_query_test.go b/engine/common/rpc/convert/execution_state_query_test.go new file mode 100644 index 00000000000..e639510ad21 --- /dev/null +++ b/engine/common/rpc/convert/execution_state_query_test.go @@ -0,0 +1,70 @@ +package convert_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/onflow/flow/protobuf/go/flow/entities" + + "github.com/onflow/flow-go/engine/common/rpc/convert" + "github.com/onflow/flow-go/module/executiondatasync/optimistic_sync" + "github.com/onflow/flow-go/utils/unittest" +) + +// TestConvertExecutionStateQuery tests converting a protobuf ExecutionStateQuery to Criteria. +func TestConvertExecutionStateQuery(t *testing.T) { + t.Parallel() + + t.Run("non-nil query", func(t *testing.T) { + executorIDs := unittest.IdentifierListFixture(5) + agreeingCount := uint64(3) + + query := &entities.ExecutionStateQuery{ + AgreeingExecutorsCount: agreeingCount, + RequiredExecutorIds: convert.IdentifiersToMessages(executorIDs), + } + + criteria := convert.NewCriteria(query) + + assert.Equal(t, uint(agreeingCount), criteria.AgreeingExecutorsCount) + require.Equal(t, len(executorIDs), len(criteria.RequiredExecutors)) + for i, id := range executorIDs { + assert.Equal(t, id, criteria.RequiredExecutors[i]) + } + }) + + t.Run("nil query", func(t *testing.T) { + criteria := convert.NewCriteria(nil) + + expected := optimistic_sync.Criteria{} + assert.Equal(t, expected, criteria) + }) + + t.Run("empty required executors", func(t *testing.T) { + query := &entities.ExecutionStateQuery{ + AgreeingExecutorsCount: 5, + RequiredExecutorIds: [][]byte{}, + } + + criteria := convert.NewCriteria(query) + + assert.Equal(t, uint(5), criteria.AgreeingExecutorsCount) + assert.Empty(t, criteria.RequiredExecutors) + }) + + t.Run("zero agreeing executors count", func(t *testing.T) { + executorIDs := unittest.IdentifierListFixture(3) + + query := &entities.ExecutionStateQuery{ + AgreeingExecutorsCount: 0, + RequiredExecutorIds: convert.IdentifiersToMessages(executorIDs), + } + + criteria := convert.NewCriteria(query) + + assert.Equal(t, uint(0), criteria.AgreeingExecutorsCount) + assert.Equal(t, len(executorIDs), len(criteria.RequiredExecutors)) + }) +} diff --git a/engine/common/rpc/convert/transactions_test.go b/engine/common/rpc/convert/transactions_test.go index c9c5141f9a8..715eca7b51e 100644 --- a/engine/common/rpc/convert/transactions_test.go +++ b/engine/common/rpc/convert/transactions_test.go @@ -17,7 +17,7 @@ import ( func TestConvertTransaction(t *testing.T) { t.Parallel() - tx := unittest.TransactionBodyFixture() + tx := unittest.TransactionFixture() arg, err := jsoncdc.Encode(cadence.NewAddress(unittest.AddressFixture())) require.NoError(t, err) @@ -32,3 +32,38 @@ func TestConvertTransaction(t *testing.T) { assert.Equal(t, tx, converted) assert.Equal(t, tx.ID(), converted.ID()) } + +// TestConvertTransactionsToMessages tests converting multiple flow.TransactionBody to protobuf messages. +func TestConvertTransactionsToMessages(t *testing.T) { + t.Parallel() + + // Create multiple transactions with varying fields + transactions := make([]*flow.TransactionBody, 6) + for i := 0; i < len(transactions); i++ { + tx := unittest.TransactionFixture() + + // Add some variation to exercise different code paths + if i%2 == 0 { + arg, err := jsoncdc.Encode(cadence.NewAddress(unittest.AddressFixture())) + require.NoError(t, err) + tx.Arguments = append(tx.Arguments, arg) + } + + if i%3 == 0 { + tx.EnvelopeSignatures = append(tx.EnvelopeSignatures, unittest.TransactionSignatureFixture()) + } + + transactions[i] = &tx + } + + messages := convert.TransactionsToMessages(transactions) + require.Len(t, messages, len(transactions)) + + chain := flow.Testnet.Chain() + for i, msg := range messages { + converted, err := convert.MessageToTransaction(msg, chain) + require.NoError(t, err) + assert.Equal(t, *transactions[i], converted) + assert.Equal(t, transactions[i].ID(), converted.ID()) + } +} diff --git a/engine/common/rpc/convert/validate_test.go b/engine/common/rpc/convert/validate_test.go new file mode 100644 index 00000000000..98f52e3a07f --- /dev/null +++ b/engine/common/rpc/convert/validate_test.go @@ -0,0 +1,333 @@ +package convert_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/onflow/flow-go/engine/common/rpc/convert" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/utils/unittest" +) + +// TestValidateAddress tests the Address validation function. +func TestValidateAddress(t *testing.T) { + t.Parallel() + + // Use Testnet chain which is what AddressFixture uses + chain := flow.Testnet.Chain() + + t.Run("valid address", func(t *testing.T) { + t.Parallel() + + validAddr := unittest.AddressFixture() + rawAddr := validAddr.Bytes() + + addr, err := convert.Address(rawAddr, chain) + require.NoError(t, err) + assert.Equal(t, validAddr, addr) + }) + + t.Run("valid random addresses", func(t *testing.T) { + t.Parallel() + + // Test multiple random addresses to exercise conversion pathways + for i := 0; i < 10; i++ { + validAddr := unittest.RandomAddressFixture() + rawAddr := validAddr.Bytes() + + addr, err := convert.Address(rawAddr, chain) + require.NoError(t, err) + assert.Equal(t, validAddr, addr) + } + }) + + t.Run("empty address", func(t *testing.T) { + t.Parallel() + + _, err := convert.Address([]byte{}, chain) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "address cannot be empty") + }) + + t.Run("invalid address for chain", func(t *testing.T) { + t.Parallel() + + // Use an address from a different chain + mainnetAddr := unittest.RandomAddressFixtureForChain(flow.Mainnet) + rawAddr := mainnetAddr.Bytes() + + // This address should be invalid for Testnet chain + _, err := convert.Address(rawAddr, chain) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "is invalid for chain") + }) +} + +// TestValidateHexToAddress tests the HexToAddress validation function. +func TestValidateHexToAddress(t *testing.T) { + t.Parallel() + + chain := flow.Testnet.Chain() + + t.Run("valid hex address", func(t *testing.T) { + t.Parallel() + + validAddr := unittest.AddressFixture() + hexAddr := validAddr.Hex() + + addr, err := convert.HexToAddress(hexAddr, chain) + require.NoError(t, err) + assert.Equal(t, validAddr, addr) + }) + + t.Run("valid random hex addresses", func(t *testing.T) { + t.Parallel() + + // Test multiple random addresses to exercise conversion pathways + for i := 0; i < 10; i++ { + validAddr := unittest.RandomAddressFixture() + hexAddr := validAddr.Hex() + + addr, err := convert.HexToAddress(hexAddr, chain) + require.NoError(t, err) + assert.Equal(t, validAddr, addr) + } + }) + + t.Run("empty hex address", func(t *testing.T) { + t.Parallel() + + _, err := convert.HexToAddress("", chain) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "address cannot be empty") + }) + + t.Run("invalid hex address for chain", func(t *testing.T) { + t.Parallel() + + // Use an address from a different chain + mainnetAddr := unittest.RandomAddressFixtureForChain(flow.Mainnet) + hexAddr := mainnetAddr.Hex() + + // This address should be invalid for Testnet chain + _, err := convert.HexToAddress(hexAddr, chain) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "is invalid for chain") + }) +} + +// TestValidateBlockID tests the BlockID validation function. +func TestValidateBlockID(t *testing.T) { + t.Parallel() + + t.Run("valid block ID", func(t *testing.T) { + t.Parallel() + + expectedID := unittest.IdentifierFixture() + rawID := expectedID[:] + + blockID, err := convert.BlockID(rawID) + require.NoError(t, err) + assert.Equal(t, expectedID, blockID) + }) + + t.Run("invalid block ID length - too short", func(t *testing.T) { + t.Parallel() + + invalidID := []byte{0x01, 0x02, 0x03} + + _, err := convert.BlockID(invalidID) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "invalid block id") + }) + + t.Run("invalid block ID length - too long", func(t *testing.T) { + t.Parallel() + + invalidID := make([]byte, flow.IdentifierLen+1) + + _, err := convert.BlockID(invalidID) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "invalid block id") + }) + + t.Run("empty block ID", func(t *testing.T) { + t.Parallel() + + _, err := convert.BlockID([]byte{}) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + }) +} + +// TestValidateBlockIDs tests the BlockIDs validation function. +func TestValidateBlockIDs(t *testing.T) { + t.Parallel() + + t.Run("valid random block IDs lists", func(t *testing.T) { + t.Parallel() + + // Test multiple random lists to exercise conversion pathways + for count := 3; count < 8; count++ { + expectedIDs := unittest.IdentifierListFixture(count) + rawIDs := make([][]byte, len(expectedIDs)) + for j, id := range expectedIDs { + rawIDs[j] = id[:] + } + + blockIDs, err := convert.BlockIDs(rawIDs) + require.NoError(t, err) + assert.Equal(t, []flow.Identifier(expectedIDs), blockIDs) + } + }) + + t.Run("empty block IDs list", func(t *testing.T) { + t.Parallel() + + _, err := convert.BlockIDs([][]byte{}) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "empty block ids") + }) + + t.Run("one invalid block ID in list", func(t *testing.T) { + t.Parallel() + + validID := unittest.IdentifierFixture() + invalidID := []byte{0x01, 0x02} + + rawIDs := [][]byte{validID[:], invalidID} + + _, err := convert.BlockIDs(rawIDs) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + }) +} + +// TestValidateCollectionID tests the CollectionID validation function. +func TestValidateCollectionID(t *testing.T) { + t.Parallel() + + t.Run("valid collection ID", func(t *testing.T) { + t.Parallel() + + expectedID := unittest.IdentifierFixture() + rawID := expectedID[:] + + collectionID, err := convert.CollectionID(rawID) + require.NoError(t, err) + assert.Equal(t, expectedID, collectionID) + }) + + t.Run("empty collection ID", func(t *testing.T) { + t.Parallel() + + _, err := convert.CollectionID([]byte{}) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "invalid collection id") + }) + + t.Run("collection ID with different length", func(t *testing.T) { + t.Parallel() + + // CollectionID doesn't validate length, only that it's not empty + // so this should succeed + shortID := []byte{0x01, 0x02} + + collectionID, err := convert.CollectionID(shortID) + require.NoError(t, err) + assert.NotEqual(t, flow.ZeroID, collectionID) + }) +} + +// TestValidateEventType tests the EventType validation function. +func TestValidateEventType(t *testing.T) { + t.Parallel() + + t.Run("valid event type", func(t *testing.T) { + t.Parallel() + + eventType := "A.0x1.MyContract.MyEvent" + + result, err := convert.EventType(eventType) + require.NoError(t, err) + assert.Equal(t, eventType, result) + }) + + t.Run("empty event type", func(t *testing.T) { + t.Parallel() + + _, err := convert.EventType("") + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "invalid event type") + }) + + t.Run("whitespace only event type", func(t *testing.T) { + t.Parallel() + + _, err := convert.EventType(" ") + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "invalid event type") + }) + + t.Run("event type with leading/trailing whitespace", func(t *testing.T) { + t.Parallel() + + eventType := " A.0x1.MyContract.MyEvent " + + result, err := convert.EventType(eventType) + require.NoError(t, err) + // The function returns the original string, not trimmed + assert.Equal(t, eventType, result) + }) +} + +// TestValidateTransactionID tests the TransactionID validation function. +func TestValidateTransactionID(t *testing.T) { + t.Parallel() + + t.Run("valid transaction ID", func(t *testing.T) { + t.Parallel() + + expectedID := unittest.IdentifierFixture() + rawID := expectedID[:] + + txID, err := convert.TransactionID(rawID) + require.NoError(t, err) + assert.Equal(t, expectedID, txID) + }) + + t.Run("empty transaction ID", func(t *testing.T) { + t.Parallel() + + _, err := convert.TransactionID([]byte{}) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "invalid transaction id") + }) + + t.Run("transaction ID with different length", func(t *testing.T) { + t.Parallel() + + // TransactionID doesn't validate length, only that it's not empty + // so this should succeed + shortID := []byte{0x01, 0x02} + + txID, err := convert.TransactionID(shortID) + require.NoError(t, err) + assert.NotEqual(t, flow.ZeroID, txID) + }) +} From ae8c1d593535ae4a50b17f49a555c28c19e246ad Mon Sep 17 00:00:00 2001 From: Taras Maliarchuk Date: Mon, 8 Dec 2025 16:24:20 +0100 Subject: [PATCH 2/3] Test fixes after review; fix for TransactionID and CollectionID conversion --- engine/access/testutil/fixture.go | 2 +- engine/common/rpc/convert/blocks_test.go | 49 +---------- .../rpc/convert/compatible_range_test.go | 18 ---- engine/common/rpc/convert/convert_test.go | 83 ++++++++++--------- engine/common/rpc/convert/events_test.go | 56 ++++++------- .../common/rpc/convert/transactions_test.go | 19 +---- engine/common/rpc/convert/validate.go | 4 +- engine/common/rpc/convert/validate_test.go | 44 ++-------- module/mempool/herocache/transactions_test.go | 4 +- utils/unittest/fixtures.go | 4 +- 10 files changed, 86 insertions(+), 197 deletions(-) diff --git a/engine/access/testutil/fixture.go b/engine/access/testutil/fixture.go index aa728a28ec4..1fe5dafa486 100644 --- a/engine/access/testutil/fixture.go +++ b/engine/access/testutil/fixture.go @@ -197,7 +197,7 @@ func (tb *ProtocolDataFixtureBuilder) buildBlocks() ([]*flow.Block, []*flow.Head guarantees := make([]*flow.CollectionGuarantee, tb.colPerBlock) blockCollections := make([]*flow.Collection, tb.colPerBlock) for j := range tb.colPerBlock { - colTxs := unittest.TransactionBodyListFixture(tb.txPerCol) + colTxs := unittest.TransactionFixtures(tb.txPerCol) col := unittest.CompleteCollectionFromTransactions(colTxs) guarantees[j] = col.Guarantee diff --git a/engine/common/rpc/convert/blocks_test.go b/engine/common/rpc/convert/blocks_test.go index 907215057c2..d6f0c57b00a 100644 --- a/engine/common/rpc/convert/blocks_test.go +++ b/engine/common/rpc/convert/blocks_test.go @@ -114,21 +114,14 @@ func TestConvertBlockSeal(t *testing.T) { converted, err := convert.MessageToBlockSeal(msg) require.NoError(t, err) - assert.Equal(t, seal.BlockID, converted.BlockID) - assert.Equal(t, seal.ResultID, converted.ResultID) - assert.Equal(t, seal.FinalState, converted.FinalState) - assert.Equal(t, seal.AggregatedApprovalSigs, converted.AggregatedApprovalSigs) + assert.Equal(t, seal.ID(), converted.ID()) } // TestConvertBlockSeals tests converting multiple flow.Seal to and from protobuf BlockSeal messages. func TestConvertBlockSeals(t *testing.T) { t.Parallel() - seals := []*flow.Seal{ - unittest.Seal.Fixture(), - unittest.Seal.Fixture(), - unittest.Seal.Fixture(), - } + seals := unittest.Seal.Fixtures(3) msgs := convert.BlockSealsToMessages(seals) require.Len(t, msgs, len(seals)) @@ -138,10 +131,7 @@ func TestConvertBlockSeals(t *testing.T) { require.Len(t, converted, len(seals)) for i, seal := range seals { - assert.Equal(t, seal.BlockID, converted[i].BlockID) - assert.Equal(t, seal.ResultID, converted[i].ResultID) - assert.Equal(t, seal.FinalState, converted[i].FinalState) - assert.Equal(t, seal.AggregatedApprovalSigs, converted[i].AggregatedApprovalSigs) + assert.Equal(t, seal.ID(), converted[i].ID()) } } @@ -158,11 +148,7 @@ func TestConvertPayloadFromMessage(t *testing.T) { payload, err := convert.PayloadFromMessage(msg) require.NoError(t, err) - assert.Equal(t, block.Payload.Guarantees, payload.Guarantees) - assert.Equal(t, block.Payload.Seals, payload.Seals) - assert.Equal(t, block.Payload.Receipts, payload.Receipts) - assert.Equal(t, block.Payload.Results, payload.Results) - assert.Equal(t, block.Payload.ProtocolStateID, payload.ProtocolStateID) + assert.Equal(t, block.Payload.Hash(), payload.Hash()) } // TestConvertBlockTimestamp2ProtobufTime tests converting block timestamps to protobuf Timestamp format. @@ -193,31 +179,4 @@ func TestConvertBlockTimestamp2ProtobufTime(t *testing.T) { convertedTime := pbTime.AsTime() assert.Equal(t, uint64(0), uint64(convertedTime.UnixMilli())) }) - - t.Run("convert various timestamps", func(t *testing.T) { - t.Parallel() - - testTimestamps := []uint64{ - 1609459200000, // 2021-01-01 00:00:00 UTC - 1640995200000, // 2022-01-01 00:00:00 UTC - 1672531200000, // 2023-01-01 00:00:00 UTC - } - - for _, ts := range testTimestamps { - pbTime := convert.BlockTimestamp2ProtobufTime(ts) - require.NotNil(t, pbTime) - - convertedTime := pbTime.AsTime() - assert.Equal(t, ts, uint64(convertedTime.UnixMilli())) - } - }) - - t.Run("roundtrip conversion with block", func(t *testing.T) { - t.Parallel() - - block := unittest.FullBlockFixture() - msg := convert.BlockToMessageLight(block) - - assert.Equal(t, block.Timestamp, uint64(msg.Timestamp.AsTime().UnixMilli())) - }) } diff --git a/engine/common/rpc/convert/compatible_range_test.go b/engine/common/rpc/convert/compatible_range_test.go index 187deb8e3e3..cfa305eba67 100644 --- a/engine/common/rpc/convert/compatible_range_test.go +++ b/engine/common/rpc/convert/compatible_range_test.go @@ -4,7 +4,6 @@ import ( "math/rand" "testing" - "github.com/onflow/flow/protobuf/go/flow/entities" "github.com/stretchr/testify/assert" "github.com/onflow/flow-go/engine/common/rpc/convert" @@ -22,23 +21,6 @@ func TestConvertCompatibleRange(t *testing.T) { assert.Nil(t, convert.MessageToCompatibleRange(nil)) }) - t.Run("convert range to message", func(t *testing.T) { - startHeight := uint64(rand.Uint32()) - endHeight := uint64(rand.Uint32()) - - comparableRange := &accessmodel.CompatibleRange{ - StartHeight: startHeight, - EndHeight: endHeight, - } - expected := &entities.CompatibleRange{ - StartHeight: startHeight, - EndHeight: endHeight, - } - - msg := convert.CompatibleRangeToMessage(comparableRange) - assert.Equal(t, msg, expected) - }) - t.Run("roundtrip conversion", func(t *testing.T) { t.Parallel() diff --git a/engine/common/rpc/convert/convert_test.go b/engine/common/rpc/convert/convert_test.go index 2ffd69304b5..76f12f514aa 100644 --- a/engine/common/rpc/convert/convert_test.go +++ b/engine/common/rpc/convert/convert_test.go @@ -1,6 +1,7 @@ package convert_test import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -81,6 +82,7 @@ func TestConvertStateCommitmentInvalidLength(t *testing.T) { _, err := convert.MessageToStateCommitment(invalidMsg) assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid state commitment length") } // TestConvertAggregatedSignatures tests converting a slice of flow.AggregatedSignatures to and from @@ -88,16 +90,7 @@ func TestConvertStateCommitmentInvalidLength(t *testing.T) { func TestConvertAggregatedSignatures(t *testing.T) { t.Parallel() - aggSigs := []flow.AggregatedSignature{ - { - SignerIDs: unittest.IdentifierListFixture(3), - VerifierSignatures: unittest.SignaturesFixture(3), - }, - { - SignerIDs: unittest.IdentifierListFixture(5), - VerifierSignatures: unittest.SignaturesFixture(5), - }, - } + aggSigs := unittest.Seal.AggregatedSignatureFixtures(2) msgs := convert.AggregatedSignaturesToMessages(aggSigs) converted := convert.MessagesToAggregatedSignatures(msgs) @@ -121,39 +114,47 @@ func TestConvertAggregatedSignaturesEmpty(t *testing.T) { func TestConvertChainId(t *testing.T) { t.Parallel() - testCases := []struct { - name string - chainID string - valid bool - }{ - {"Mainnet", flow.Mainnet.String(), true}, - {"Testnet", flow.Testnet.String(), true}, - {"Emulator", flow.Emulator.String(), true}, - {"Localnet", flow.Localnet.String(), true}, - {"Sandboxnet", flow.Sandboxnet.String(), true}, - {"Previewnet", flow.Previewnet.String(), true}, - {"Benchnet", flow.Benchnet.String(), true}, - {"BftTestnet", flow.BftTestnet.String(), true}, - {"MonotonicEmulator", flow.MonotonicEmulator.String(), true}, - {"Invalid", "invalid-chain", false}, - {"Empty", "", false}, - } - - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - result, err := convert.MessageToChainId(tc.chainID) - - if tc.valid { + t.Run("valid chain IDs", func(t *testing.T) { + t.Parallel() + + validChainIDs := []flow.ChainID{ + flow.Mainnet, + flow.Testnet, + flow.Emulator, + flow.Localnet, + flow.Sandboxnet, + flow.Previewnet, + flow.Benchnet, + flow.BftTestnet, + flow.MonotonicEmulator, + } + + for _, chainID := range validChainIDs { + t.Run(chainID.String(), func(t *testing.T) { + t.Parallel() + + result, err := convert.MessageToChainId(chainID.String()) require.NoError(t, err) require.NotNil(t, result) - assert.Equal(t, tc.chainID, result.String()) - } else { + assert.Equal(t, chainID, *result) + }) + } + }) + + t.Run("invalid chain IDs", func(t *testing.T) { + t.Parallel() + + invalid := []string{"invalid-chain", ""} + + for _, chainID := range invalid { + t.Run(fmt.Sprintf("invalid_%q", chainID), func(t *testing.T) { + t.Parallel() + + result, err := convert.MessageToChainId(chainID) assert.Error(t, err) assert.Nil(t, result) - } - }) - } + assert.Contains(t, err.Error(), "invalid chainId") + }) + } + }) } diff --git a/engine/common/rpc/convert/events_test.go b/engine/common/rpc/convert/events_test.go index a8cb2cb9b4f..60a57bf9a4a 100644 --- a/engine/common/rpc/convert/events_test.go +++ b/engine/common/rpc/convert/events_test.go @@ -3,6 +3,7 @@ package convert_test import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/onflow/cadence" @@ -232,30 +233,8 @@ func TestConvertMessagesToBlockEvents(t *testing.T) { require.Equal(t, blockEvents, converted) } -// TestConvertBlockEventsToMessage tests converting a single flow.BlockEvents to a protobuf message. -func TestConvertBlockEventsToMessage(t *testing.T) { - t.Parallel() - - header := unittest.BlockHeaderFixture(unittest.WithHeaderHeight(42)) - blockEvents := unittest.BlockEventsFixture(header, 3) - - msg, err := convert.BlockEventsToMessage(blockEvents) - require.NoError(t, err) - require.NotNil(t, msg) - - require.Equal(t, blockEvents.BlockHeight, msg.BlockHeight) - require.Equal(t, blockEvents.BlockID[:], msg.BlockId) - require.Equal(t, blockEvents.BlockTimestamp, msg.BlockTimestamp.AsTime()) - require.Len(t, msg.Events, len(blockEvents.Events)) - - for i, eventMsg := range msg.Events { - require.Equal(t, blockEvents.Events[i].Type, flow.EventType(eventMsg.Type)) - require.Equal(t, blockEvents.Events[i].TransactionID[:], eventMsg.TransactionId) - } -} - -// TestConvertMessageToBlockEvents tests converting a single protobuf message to flow.BlockEvents. -func TestConvertMessageToBlockEvents(t *testing.T) { +// TestConvertBlockEvent tests round-trip converting a single protobuf message. +func TestConvertBlockEvent(t *testing.T) { t.Parallel() header := unittest.BlockHeaderFixture(unittest.WithHeaderHeight(42)) @@ -272,8 +251,11 @@ func TestConvertMessageToBlockEvents(t *testing.T) { require.Equal(t, blockEvents.BlockHeight, converted.BlockHeight) require.Equal(t, blockEvents.BlockID, converted.BlockID) - require.Equal(t, blockEvents.BlockTimestamp.Unix(), converted.BlockTimestamp.Unix()) + require.Equal(t, blockEvents.BlockTimestamp, converted.BlockTimestamp) require.Len(t, converted.Events, len(blockEvents.Events)) + for i, event := range blockEvents.Events { + require.Equal(t, event.ID(), converted.Events[i].ID()) + } } // TestConvertCcfEventToJsonEvent tests converting a single CCF event to JSON event. @@ -315,8 +297,10 @@ func TestConvertCcfEventToJsonEvent(t *testing.T) { unittest.Event.WithPayload([]byte{0x01, 0x02, 0x03}), // Invalid CCF ) - _, err := convert.CcfEventToJsonEvent(invalidEvent) + jsonEvent, err := convert.CcfEventToJsonEvent(invalidEvent) + require.Nil(t, jsonEvent) require.Error(t, err) + assert.Contains(t, err.Error(), "unable to decode from ccf format") }) t.Run("returns error on empty payload", func(t *testing.T) { @@ -326,8 +310,10 @@ func TestConvertCcfEventToJsonEvent(t *testing.T) { unittest.Event.WithPayload([]byte{}), ) - _, err := convert.CcfEventToJsonEvent(emptyEvent) + jsonEvent, err := convert.CcfEventToJsonEvent(emptyEvent) + require.Nil(t, jsonEvent) require.Error(t, err, "empty payload should result in error from flow.NewEvent") + assert.Contains(t, err.Error(), "payload must not be empty") }) } @@ -362,8 +348,10 @@ func TestConvertCcfPayloadToJsonPayload(t *testing.T) { t.Parallel() invalidPayload := []byte{0x01, 0x02, 0x03} - _, err := convert.CcfPayloadToJsonPayload(invalidPayload) + jsonPayload, err := convert.CcfPayloadToJsonPayload(invalidPayload) + require.Nil(t, jsonPayload) require.Error(t, err) + assert.Contains(t, err.Error(), "unable to decode from ccf format") }) } @@ -413,8 +401,10 @@ func TestConvertCcfEventsToJsonEvents(t *testing.T) { ), } - _, err := convert.CcfEventsToJsonEvents(invalidEvents) + jsonEvents, err := convert.CcfEventsToJsonEvents(invalidEvents) + require.Nil(t, jsonEvents) require.Error(t, err) + assert.Contains(t, err.Error(), "unable to decode from ccf format") }) } @@ -476,8 +466,10 @@ func TestConvertEventToMessageFromVersion(t *testing.T) { event := unittest.EventFixture() - _, err := convert.EventToMessageFromVersion(event, entities.EventEncodingVersion(999)) + message, err := convert.EventToMessageFromVersion(event, entities.EventEncodingVersion(999)) + require.Nil(t, message) require.Error(t, err) + assert.Contains(t, err.Error(), "invalid encoding format") }) } @@ -540,11 +532,13 @@ func TestConvertEventsToMessagesWithEncodingConversion(t *testing.T) { unittest.Event.WithPayload(jsonPayload), ) - _, err = convert.EventsToMessagesWithEncodingConversion( + message, err := convert.EventsToMessagesWithEncodingConversion( jsonEvents, entities.EventEncodingVersion_JSON_CDC_V0, entities.EventEncodingVersion_CCF_V0, ) + require.Nil(t, message) require.Error(t, err) + assert.Contains(t, err.Error(), "conversion from format") }) } diff --git a/engine/common/rpc/convert/transactions_test.go b/engine/common/rpc/convert/transactions_test.go index 715eca7b51e..19c6bcc8aaf 100644 --- a/engine/common/rpc/convert/transactions_test.go +++ b/engine/common/rpc/convert/transactions_test.go @@ -37,24 +37,7 @@ func TestConvertTransaction(t *testing.T) { func TestConvertTransactionsToMessages(t *testing.T) { t.Parallel() - // Create multiple transactions with varying fields - transactions := make([]*flow.TransactionBody, 6) - for i := 0; i < len(transactions); i++ { - tx := unittest.TransactionFixture() - - // Add some variation to exercise different code paths - if i%2 == 0 { - arg, err := jsoncdc.Encode(cadence.NewAddress(unittest.AddressFixture())) - require.NoError(t, err) - tx.Arguments = append(tx.Arguments, arg) - } - - if i%3 == 0 { - tx.EnvelopeSignatures = append(tx.EnvelopeSignatures, unittest.TransactionSignatureFixture()) - } - - transactions[i] = &tx - } + transactions := unittest.TransactionFixtures(6) messages := convert.TransactionsToMessages(transactions) require.Len(t, messages, len(transactions)) diff --git a/engine/common/rpc/convert/validate.go b/engine/common/rpc/convert/validate.go index 35b93851198..faecc1e9a4f 100644 --- a/engine/common/rpc/convert/validate.go +++ b/engine/common/rpc/convert/validate.go @@ -63,7 +63,7 @@ func BlockIDs(blockIDs [][]byte) ([]flow.Identifier, error) { } func CollectionID(collectionID []byte) (flow.Identifier, error) { - if len(collectionID) == 0 { + if len(collectionID) != flow.IdentifierLen { return flow.ZeroID, status.Error(codes.InvalidArgument, "invalid collection id") } return flow.HashToID(collectionID), nil @@ -77,7 +77,7 @@ func EventType(eventType string) (string, error) { } func TransactionID(txID []byte) (flow.Identifier, error) { - if len(txID) == 0 { + if len(txID) != flow.IdentifierLen { return flow.ZeroID, status.Error(codes.InvalidArgument, "invalid transaction id") } return flow.HashToID(txID), nil diff --git a/engine/common/rpc/convert/validate_test.go b/engine/common/rpc/convert/validate_test.go index 98f52e3a07f..697a5e06244 100644 --- a/engine/common/rpc/convert/validate_test.go +++ b/engine/common/rpc/convert/validate_test.go @@ -31,20 +31,6 @@ func TestValidateAddress(t *testing.T) { assert.Equal(t, validAddr, addr) }) - t.Run("valid random addresses", func(t *testing.T) { - t.Parallel() - - // Test multiple random addresses to exercise conversion pathways - for i := 0; i < 10; i++ { - validAddr := unittest.RandomAddressFixture() - rawAddr := validAddr.Bytes() - - addr, err := convert.Address(rawAddr, chain) - require.NoError(t, err) - assert.Equal(t, validAddr, addr) - } - }) - t.Run("empty address", func(t *testing.T) { t.Parallel() @@ -86,20 +72,6 @@ func TestValidateHexToAddress(t *testing.T) { assert.Equal(t, validAddr, addr) }) - t.Run("valid random hex addresses", func(t *testing.T) { - t.Parallel() - - // Test multiple random addresses to exercise conversion pathways - for i := 0; i < 10; i++ { - validAddr := unittest.RandomAddressFixture() - hexAddr := validAddr.Hex() - - addr, err := convert.HexToAddress(hexAddr, chain) - require.NoError(t, err) - assert.Equal(t, validAddr, addr) - } - }) - t.Run("empty hex address", func(t *testing.T) { t.Parallel() @@ -241,13 +213,12 @@ func TestValidateCollectionID(t *testing.T) { t.Run("collection ID with different length", func(t *testing.T) { t.Parallel() - // CollectionID doesn't validate length, only that it's not empty // so this should succeed shortID := []byte{0x01, 0x02} - collectionID, err := convert.CollectionID(shortID) - require.NoError(t, err) - assert.NotEqual(t, flow.ZeroID, collectionID) + _, err := convert.CollectionID(shortID) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "invalid collection id") }) } @@ -322,12 +293,11 @@ func TestValidateTransactionID(t *testing.T) { t.Run("transaction ID with different length", func(t *testing.T) { t.Parallel() - // TransactionID doesn't validate length, only that it's not empty - // so this should succeed shortID := []byte{0x01, 0x02} - txID, err := convert.TransactionID(shortID) - require.NoError(t, err) - assert.NotEqual(t, flow.ZeroID, txID) + _, err := convert.TransactionID(shortID) + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err)) + assert.Contains(t, err.Error(), "invalid transaction id") }) } diff --git a/module/mempool/herocache/transactions_test.go b/module/mempool/herocache/transactions_test.go index b3b3841360d..90eb311ac2b 100644 --- a/module/mempool/herocache/transactions_test.go +++ b/module/mempool/herocache/transactions_test.go @@ -62,7 +62,7 @@ func TestTransactionPool(t *testing.T) { // TestConcurrentWriteAndRead checks correctness of transactions mempool under concurrent read and write. func TestConcurrentWriteAndRead(t *testing.T) { total := 100 - txs := unittest.TransactionBodyListFixture(total) + txs := unittest.TransactionFixtures(total) transactions := herocache.NewTransactions(uint32(total), unittest.Logger(), metrics.NewNoopCollector()) wg := sync.WaitGroup{} @@ -98,7 +98,7 @@ func TestConcurrentWriteAndRead(t *testing.T) { // transactions in the same order as they are returned. func TestValuesReturnsInOrder(t *testing.T) { total := 100 - txs := unittest.TransactionBodyListFixture(total) + txs := unittest.TransactionFixtures(total) transactions := herocache.NewTransactions(uint32(total), unittest.Logger(), metrics.NewNoopCollector()) // storing all transactions diff --git a/utils/unittest/fixtures.go b/utils/unittest/fixtures.go index 5410c7e036f..482877ac6da 100644 --- a/utils/unittest/fixtures.go +++ b/utils/unittest/fixtures.go @@ -1624,10 +1624,10 @@ func TransactionBodyFixture(opts ...func(*flow.TransactionBody)) flow.Transactio return tb } -func TransactionBodyListFixture(n int) []*flow.TransactionBody { +func TransactionFixtures(n int) []*flow.TransactionBody { l := make([]*flow.TransactionBody, n) for i := 0; i < n; i++ { - tx := TransactionBodyFixture() + tx := TransactionFixture() l[i] = &tx } From 44bffb8c141b5c7676775da2fdae922e15fcf243 Mon Sep 17 00:00:00 2001 From: Taras Maliarchuk Date: Thu, 11 Dec 2025 13:40:45 +0100 Subject: [PATCH 3/3] tests fix: compare blocks by ID, require result not Nil --- engine/common/rpc/convert/blocks_test.go | 2 +- engine/common/rpc/convert/execution_data_test.go | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/engine/common/rpc/convert/blocks_test.go b/engine/common/rpc/convert/blocks_test.go index d6f0c57b00a..b1d560e9af4 100644 --- a/engine/common/rpc/convert/blocks_test.go +++ b/engine/common/rpc/convert/blocks_test.go @@ -28,7 +28,7 @@ func TestConvertBlock(t *testing.T) { converted, err := convert.MessageToBlock(msg) require.NoError(t, err) - assert.Equal(t, block, converted) + assert.Equal(t, block.ID(), converted.ID()) } // TestConvertBlockLight tests that converting a block to its light form results in only the correct diff --git a/engine/common/rpc/convert/execution_data_test.go b/engine/common/rpc/convert/execution_data_test.go index 2d8ec9d5eb1..e093b0e475c 100644 --- a/engine/common/rpc/convert/execution_data_test.go +++ b/engine/common/rpc/convert/execution_data_test.go @@ -47,7 +47,8 @@ func TestConvertBlockExecutionDataEventPayloads(t *testing.T) { for i, e := range events { require.Equal(t, ccfEvents[i], e) - _, err := ccf.Decode(nil, e.Payload) + res, err := ccf.Decode(nil, e.Payload) + require.NotNil(t, res) require.NoError(t, err) } } @@ -64,7 +65,8 @@ func TestConvertBlockExecutionDataEventPayloads(t *testing.T) { for i, e := range events { require.Equal(t, jsonEvents[i], e) - _, err := jsoncdc.Decode(nil, e.Payload) + res, err := jsoncdc.Decode(nil, e.Payload) + require.NotNil(t, res) require.NoError(t, err) } }