test(conway): Create tests for creating block CBOR #1055

arepala-uml · 2025-07-07T03:33:47Z

Fetched Conway block CBOR data using gouroboros starter kit and converted it to hex using xxd for testing. (https://cexplorer.io/block/27807a70215e3e018eec9be8c619c692e06a78ebcb63daf90d7abe823f3bbf47)
Added two test cases where one using inbuilt cbor.Marshal() and another using cbor.Encode() to ensure round-trip serialization of ConwayBlock.
Compared original and re-encoded CBOR bytes in both tests, logged mismatch details, and saved outputs to respective hex files for inspection.

agaffney

Our cbor.Encode() function is nothing but a wrapper for the upstream cbor.Marshal() function with the proper encoder mode set. We only need/want to test with our custom cbor.Encode(), since the plain upstream function will produce different results in some cases.

agaffney · 2025-07-07T12:05:40Z

ledger/conway/testdata/conway_block.cbor.hex

Is there a good reason to store this block as hex? It requires an extra step to decode, and it takes up 2x the disk space.

I told him to store it as hex.

agaffney · 2025-07-07T12:06:34Z

ledger/conway/testdata/conway_block.cbor.hex

We should name this file in a way that tells us where it came from. We at least want the network name, block hash, and block slot, so that we can look it up again later

I prefer we put it into a variable within the Go code rather than as a separate file we have to load, and we simply comment this information.

I have removed the hex from the file and stored it inside the code itself. Added the information about block hash, block slot in the comments. Please review it once

wolf31o2 · 2025-07-07T13:07:25Z

ledger/conway/pparams_test.go

 	}
 }
+
+func isValidHexString(s string) error {


You won't need this. You're providing the string. You know it's good.

I removed this extra validation check and please review it once.

wolf31o2 · 2025-07-07T13:08:04Z

ledger/conway/pparams_test.go

These are block tests. Create a block_test.go file rather than putting tests not related to protocol parameters in this file.

I have created a new file block_test.go and moved the tests to this file. Please review it once

wolf31o2 · 2025-07-07T13:10:14Z

ledger/conway/pparams_test.go

+
+func TestConwayBlock_CborRoundTrip_UsingCustomEncode(t *testing.T) {
+	// Read the hex-encoded CBOR string for a Conway block
+	filePath := filepath.Join("testdata", "conway_block.cbor.hex")


Put this into a string var instead of reading from disk. We can reuse it across the package without doing disk seeks in every test.

Something like:

// taken from https://cexplorer.io/block/abcd... var conwayBlockHex string = "..."

I have made the changes for this like stored in a variable

wolf31o2 · 2025-07-07T13:11:47Z

ledger/conway/pparams_test.go

+		t.Errorf("Custom CBOR round-trip mismatch for Conway block\nOriginal CBOR (hex): %x\nCustom Encoded CBOR (hex): %x", dataBytes, encoded)
+	}
+
+	// Save the re-encoded CBOR for comparison


Skip this. The text/hex output is good enough.

Removed this part like storing back again the rendered CBOR. Please review it once.

arepala-uml · 2025-07-07T16:14:28Z

Our cbor.Encode() function is nothing but a wrapper for the upstream cbor.Marshal() function with the proper encoder mode set. We only need/want to test with our custom cbor.Encode(), since the plain upstream function will produce different results in some cases.

I have removed the test function with inbuilt cbor.Marshal() function

…ock to validate against original bytes Signed-off-by: Akhil Repala <[email protected]>

…s to block_test.go Signed-off-by: Akhil Repala <[email protected]>

…unction Signed-off-by: Akhil Repala <[email protected]>

Requested changes have been made.

arepala-uml marked this pull request as ready for review July 7, 2025 03:35

arepala-uml requested a review from a team as a code owner July 7, 2025 03:35

arepala-uml requested review from wolf31o2 and agaffney July 7, 2025 03:36

agaffney previously requested changes Jul 7, 2025

View reviewed changes

agaffney reviewed Jul 7, 2025

View reviewed changes

wolf31o2 requested changes Jul 7, 2025

View reviewed changes

arepala-uml requested review from agaffney and wolf31o2 July 7, 2025 16:23

agaffney force-pushed the cbor_block_test branch 3 times, most recently from b4364fd to 5a84909 Compare July 12, 2025 16:07

arepala-uml added 3 commits July 12, 2025 16:13

test(conway): Added CBOR round-trip serialization tests for Conway bl…

fad62a0

…ock to validate against original bytes Signed-off-by: Akhil Repala <[email protected]>

test(conway): Addressed all the comments of the PR and moved the test…

2e4ee12

…s to block_test.go Signed-off-by: Akhil Repala <[email protected]>

test(conway): Removed the test function with inbuilt Cbor.Marshal() f…

a8067f2

…unction Signed-off-by: Akhil Repala <[email protected]>

agaffney force-pushed the cbor_block_test branch from 5a84909 to a8067f2 Compare July 12, 2025 20:13

wolf31o2 approved these changes Jul 12, 2025

View reviewed changes

wolf31o2 merged commit 968b34e into main Jul 12, 2025
9 checks passed

wolf31o2 deleted the cbor_block_test branch July 12, 2025 22:38

test(conway): Create tests for creating block CBOR #1055

test(conway): Create tests for creating block CBOR #1055

Uh oh!

Conversation

arepala-uml commented Jul 7, 2025

Uh oh!

agaffney left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arepala-uml commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arepala-uml commented Jul 7, 2025 •

edited

Loading