Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions engine/common/rpc/convert/blocks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -76,3 +78,146 @@ func TestConvertRootBlock(t *testing.T) {

assert.Equal(t, block.ID(), converted.ID())
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please also update TestConvertBlockto use the ID() method to confirm the blocks are cryptographically identical, rather than comparing the full struct.

// 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)
Comment on lines +117 to +120
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could use the ID() method here and in other cases to ensure that the objects are cryptographically identical.

Suggested change
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(),
}
Comment on lines +127 to +131
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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))

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)
Comment on lines +141 to +144
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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())

}
}

// 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)
Comment on lines +161 to +165
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think the convert current timestamp subtest already covers this, so the convert various timestamps subtest may not be necessary.

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()))
})
Comment on lines +215 to +222
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

roundtrip conversion with block subtest duplicates the logic already tested in TestConvertBlockLight.”

Suggested change
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()))
})

}
17 changes: 17 additions & 0 deletions engine/common/rpc/convert/compatible_range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,21 @@ func TestConvertCompatibleRange(t *testing.T) {
msg := convert.CompatibleRangeToMessage(comparableRange)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove the "convert range to message" subtest, since the "roundtrip conversion" subtest now covers the same behavior.

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)
})
}
159 changes: 159 additions & 0 deletions engine/common/rpc/convert/convert_test.go
Original file line number Diff line number Diff line change
@@ -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)
Comment on lines +82 to +83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please also check that the returned error? How about adding:

Suggested change
_, err := convert.MessageToStateCommitment(invalidMsg)
assert.Error(t, err)
result, err := convert.MessageToStateCommitment(invalidMsg)
assert.Error(t, err)
assert.Nil(t, result)
assert.Contains(t, err.Error(), "invalid state commitment length")

}

// 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),
},
}
Comment on lines +91 to +100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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)

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about restructuring this test into two subtests — one for valid chain IDs and one for invalid ones? This avoids the boolean valid flag in the table and makes the test easier to read.

func TestConvertChainId(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,
	}

	t.Run("valid chain IDs", func(t *testing.T) {
		t.Parallel()

		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, 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")
			})
		}
	})
}

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)
}
})
}
}
16 changes: 2 additions & 14 deletions engine/common/rpc/convert/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading
Loading